| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGp9LzqkcH6KQq+TcaTGgK-4oK6XU8pzPB4j35en+df3beAhzQ@mail.gmail.com>
Date: Thu, 5 Jun 2025 18:06:55 -0500
From: Sean Wang <sean.wang@...nel.org>
To: Zhangchao Zhang <ot_zhangchao.zhang@...iatek.com>
Cc: Marcel Holtmann <marcel@...tmann.org>, Johan Hedberg <johan.hedberg@...il.com>,
Luiz Von Dentz <luiz.dentz@...il.com>, Sean Wang <sean.wang@...iatek.com>,
Deren Wu <deren.Wu@...iatek.com>, Aaron Hou <aaron.hou@...iatek.com>,
Chris Lu <chris.lu@...iatek.com>, Hao Qin <Hao.qin@...iatek.com>,
linux-bluetooth <linux-bluetooth@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-mediatek <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v2] Bluetooth: BT Driver: mediatek: add gpio pin to reset bt
Hi Zhangchao,
Thanks for your recent patch submission. Could you help address the
following points?
1) Add a revision history
each patch version should include a clear changelog under the --- line
to show what has changed since the previous version.
2) Remove the "BT driver" in the prefix
To stay consistent with the other patches we've already submitted, the
"BT driver" should be removed from this prefix.
3) Update the bt-bindings document
Please also add or update a corresponding entry to the bt-bindings
documentation in a separate patch to describe the "reset-gpios"
property and "mediatek,usb-bluetooth" and how it is used.
On Thu, Jun 5, 2025 at 4:54 AM Zhangchao Zhang
<ot_zhangchao.zhang@...iatek.com> wrote:
>
> This patch provides two methods btmtk_reset_by_gpio,
> btmtk_reset_by_gpio_work for mediatek controller,
> it has been tested locally many times and can reset normally.
>
> The pin is configured in dts files, bluetooth is reset by pulling
> the pin, when exception or coredump occurs, the above methods will
> be used to reset the bluetooth, if the pin is not found, it also can
> reset bluetooth successfully by software reset.
>
> Co-develop-by Hao Qin <hao.qin@...iatek.com>
> Co-develop-by Chris LU <chris.lu@...iatek.com>
> Co-develop-by Jiande Lu <jiande.lu@...iatek.com>
> Signed-off-by: Zhangchao Zhang <ot_zhangchao.zhang@...iatek.com>
> ---
> drivers/bluetooth/btmtk.c | 60 +++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btmtk.h | 5 ++++
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 4390fd571dbd..88e588f1b95b 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -6,6 +6,8 @@
> #include <linux/firmware.h>
> #include <linux/usb.h>
> #include <linux/iopoll.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> #include <linux/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -109,6 +111,60 @@ static void btmtk_coredump_notify(struct hci_dev *hdev, int state)
> }
> }
>
> +static void btmtk_reset_by_gpio_work(struct work_struct *work)
> +{
> + struct btmtk_reset_gpio *reset_gpio_data =
> + container_of(work, struct btmtk_reset_gpio, reset_work.work);
> +
> + gpio_direction_output(reset_gpio_data->gpio_number, 1);
> + kfree(reset_gpio_data);
> +}
> +
> +static int btmtk_reset_by_gpio(struct hci_dev *hdev)
> +{
> + struct btmtk_data *data = hci_get_priv(hdev);
> + struct btmtk_reset_gpio *reset_gpio_data;
> + struct device_node *node;
> + int reset_gpio_number;
> +
> + node = of_find_compatible_node(NULL, NULL, "mediatek,usb-bluetooth");
> + if (node) {
> + reset_gpio_number = of_get_named_gpio(node, "reset-gpios", 0);
> + if (!gpio_is_valid(reset_gpio_number)) {
> + bt_dev_warn(hdev, "invalid reset GPIO, use software reset");
> + return -EINVAL;
> + }
> + } else {
> + bt_dev_warn(hdev, "no reset GPIO, use software reset");
> + return -ENODEV;
> + }
> +
> + /* Toggle the hard reset line. The Mediatek device is going to
> + * yank itself off the USB and then replug. The cleanup is handled
> + * correctly on the way out (standard USB disconnect), and the new
> + * device is detected cleanly and bound to the driver again like
> + * it should be.
> + */
> +
> + if (test_and_set_bit(BTMTK_HW_RESET_ACTIVE, &data->flags)) {
> + bt_dev_err(hdev, "last reset failed? Not resetting again");
> + return 0;
> + }
> +
> + reset_gpio_data = kzalloc(sizeof(*reset_gpio_data), GFP_KERNEL);
> + if (!reset_gpio_data)
> + return -ENOMEM;
> +
> + INIT_DELAYED_WORK(&reset_gpio_data->reset_work, btmtk_reset_by_gpio_work);
> + reset_gpio_data->gpio_number = reset_gpio_number;
> +
> + gpio_direction_output(reset_gpio_number, 0);
> +
> + /* it requires 200ms for mtk bt chip to do reset */
> + schedule_delayed_work(&reset_gpio_data->reset_work, msecs_to_jiffies(200));
4) Just to clarify since schedule_delayed_work() is asynchronous, is
there a risk that the reset may not complete before subsequent logic
(like firmware loading or hci setup) begins? Would it be safer to wait
for the GPIO reset to complete explicitly?
> + return 0;
> +}
> +
> void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
> u32 fw_flavor)
> {
> @@ -364,6 +420,10 @@ void btmtk_reset_sync(struct hci_dev *hdev)
> struct btmtk_data *reset_work = hci_get_priv(hdev);
> int err;
>
> + /*Toggle reset gpio if the platform provieds one*/
> + err = btmtk_reset_by_gpio(hdev);
> + if (!err)
> + return;
5) We need this fallback to ensure that even if reset-gpios or
mediatek,usb-bluetooth aren't defined in the device tree, the platform
can still boot and operate normally. This helps maintain compatibility
with existing or older deployments.
Sean
> hci_dev_lock(hdev);
>
> err = hci_cmd_sync_queue(hdev, reset_work->reset_sync, NULL, NULL);
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index 5df7c3296624..8a265ce367d1 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -179,6 +179,11 @@ struct btmtk_data {
> spinlock_t isorxlock;
> };
>
> +struct btmtk_reset_gpio {
> + struct delayed_work reset_work;
> + int gpio_number;
> +};
> +
> typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
> struct btmtk_hci_wmt_params *);
>
> --
> 2.46.0
>
>
Powered by blists - more mailing lists