[PATCH] Bluetooth: BT Driver: mediatek: add gpio pin to reset bt

[Paul Menzel]

Dear Zhangchao,


Thank you for your patch. For the summary, I’d just use:

Bluetooth: btmtk: Add gpio pin to reset bt


Am 28.05.25 um 09:03 schrieb Zhangchao Zhang:
> This patch provides some methods btmtk_reset_by_gpio,

some → the two

> btmtk_reset_by_gpio_work_for mediatek controller.
> 
> The pin is configured in dts files, bluetooth is reset by pulling
> the pin, when an exception or coredump occurs, the above method will
> be used to reset the bluetooth.

How did you test this? Could you please add that to the commit message.

> Co-developed-by Hao qin <hao.qin at mediatek.com>

qin → Qin

> Co-developed-by Chirs lu <chris.lu at mediatek.com>

Chirs → Chris
lu → Lu

> Co-developed-by Jiande lu <jiande.lu at mediatek.com>

lu → Lu

> Signed-off-by: Zhangchao Zhang <ot_zhangchao.zhang at mediatek.com>
> ---
>   drivers/bluetooth/btmtk.c | 59 +++++++++++++++++++++++++++++++++++++++
>   drivers/bluetooth/btmtk.h |  5 ++++
>   2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 4390fd571dbd..16542c724b40 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,59 @@ 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 SW reset");

gpio → GPIO
SW → software
reet → reset?

Please also log the GPIO number. Also, a hint, what the user should do 
in this situation would be great.

> +			return -EINVAL;
> +		}
> +	} else {
> +		bt_dev_warn(hdev, "no reset gpio, use SW reet");

gpio → GPIO
SW → software
reet → 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");

It’d be great, if this error message could be rephrased, so users know 
what is wrong, and what to do.

> +		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);
> +	schedule_delayed_work(&reset_gpio_data->reset_work, msecs_to_jiffies(200));

A 200 ms delay is quite long. Maybe add the datasheet section as a 
reference in a comment?

> +
> +	return 0;
> +}
> +
>   void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
>   			   u32 fw_flavor)
>   {
> @@ -364,6 +419,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*/

1.  Please add a space in the beginning and end.
2.  provides (A spell checker should have found this.)

> +	err = btmtk_reset_by_gpio(hdev);
> +	if (!err)
> +		return;
>   	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 *);
>   


Kind regards,

Paul