Hi Marcel,
This series updates the reset gpio based recovery from command
timeouts with the following changes:
* Refactor duplicate code to use a single btusb_gpio_cmd_timeout
* Reduce the number of command timeouts needed for reset to 3
* Expose the gpio based hardware reset to debugfs for testing
The last one is important to us so that we can confirm new chips support
hardware based reset (which is part of our requirements for BT chips).
We will probably also add the 'toggle_hw_reset' debugfs entry to other
drivers that support hardware based reset (i.e. hci_qca, hci_h5, etc) in
subsequent changes.
Thanks
Abhishek
Abhishek Pandit-Subedi (3):
Bluetooth: btusb: Refactor gpio reset
Bluetooth: btusb: Trigger gpio reset quicker
Bluetooth: btusb: Expose reset gpio to debugfs
drivers/bluetooth/btusb.c | 107 +++++++++++++++++++++++---------------
1 file changed, 66 insertions(+), 41 deletions(-)
--
2.30.0.284.gd98b1dd5eaa7-goog
Refactor gpio reset to use a common gpio reset function.
Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
Reviewed-by: Miao-chen Chou <[email protected]>
---
drivers/bluetooth/btusb.c | 59 +++++++++++++--------------------------
1 file changed, 19 insertions(+), 40 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index b14102fba6018..03341e6cbf3ed 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -547,6 +547,7 @@ struct btusb_data {
struct usb_endpoint_descriptor *diag_rx_ep;
struct gpio_desc *reset_gpio;
+ unsigned int reset_duration_ms;
__u8 cmdreq_type;
__u8 cmdreq;
@@ -566,15 +567,21 @@ struct btusb_data {
unsigned cmd_timeout_cnt;
};
-static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
+static void btusb_toggle_gpio(struct gpio_desc *desc, unsigned int duration)
+{
+ gpiod_set_value_cansleep(desc, 1);
+ msleep(duration);
+ gpiod_set_value_cansleep(desc, 0);
+}
+
+static void btusb_gpio_cmd_timeout(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
- struct gpio_desc *reset_gpio = data->reset_gpio;
if (++data->cmd_timeout_cnt < 5)
return;
- if (!reset_gpio) {
+ if (!data->reset_gpio) {
bt_dev_err(hdev, "No way to reset. Ignoring and continuing");
return;
}
@@ -592,39 +599,7 @@ static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
}
bt_dev_err(hdev, "Initiating HW reset via gpio");
- gpiod_set_value_cansleep(reset_gpio, 1);
- msleep(100);
- gpiod_set_value_cansleep(reset_gpio, 0);
-}
-
-static void btusb_rtl_cmd_timeout(struct hci_dev *hdev)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- struct gpio_desc *reset_gpio = data->reset_gpio;
-
- if (++data->cmd_timeout_cnt < 5)
- return;
-
- if (!reset_gpio) {
- bt_dev_err(hdev, "No gpio to reset Realtek device, ignoring");
- return;
- }
-
- /* Toggle the hard reset line. The Realtek 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(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
- bt_dev_err(hdev, "last reset failed? Not resetting again");
- return;
- }
-
- bt_dev_err(hdev, "Reset Realtek device via gpio");
- gpiod_set_value_cansleep(reset_gpio, 1);
- msleep(200);
- gpiod_set_value_cansleep(reset_gpio, 0);
+ btusb_toggle_gpio(data->reset_gpio, data->reset_duration_ms);
}
static void btusb_qca_cmd_timeout(struct hci_dev *hdev)
@@ -4462,7 +4437,8 @@ static int btusb_probe(struct usb_interface *intf,
hdev->shutdown = btusb_shutdown_intel;
hdev->set_diag = btintel_set_diag_mfg;
hdev->set_bdaddr = btintel_set_bdaddr;
- hdev->cmd_timeout = btusb_intel_cmd_timeout;
+ hdev->cmd_timeout = btusb_gpio_cmd_timeout;
+ data->reset_duration_ms = 100;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
@@ -4476,7 +4452,8 @@ static int btusb_probe(struct usb_interface *intf,
hdev->hw_error = btintel_hw_error;
hdev->set_diag = btintel_set_diag;
hdev->set_bdaddr = btintel_set_bdaddr;
- hdev->cmd_timeout = btusb_intel_cmd_timeout;
+ hdev->cmd_timeout = btusb_gpio_cmd_timeout;
+ data->reset_duration_ms = 100;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
@@ -4490,7 +4467,8 @@ static int btusb_probe(struct usb_interface *intf,
hdev->hw_error = btintel_hw_error;
hdev->set_diag = btintel_set_diag;
hdev->set_bdaddr = btintel_set_bdaddr;
- hdev->cmd_timeout = btusb_intel_cmd_timeout;
+ hdev->cmd_timeout = btusb_gpio_cmd_timeout;
+ data->reset_duration_ms = 100;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
@@ -4557,7 +4535,8 @@ static int btusb_probe(struct usb_interface *intf,
(id->driver_info & BTUSB_REALTEK)) {
hdev->setup = btrtl_setup_realtek;
hdev->shutdown = btrtl_shutdown_realtek;
- hdev->cmd_timeout = btusb_rtl_cmd_timeout;
+ hdev->cmd_timeout = btusb_gpio_cmd_timeout;
+ data->reset_duration_ms = 200;
/* Realtek devices lose their updated firmware over global
* suspend that means host doesn't send SET_FEATURE
--
2.30.0.284.gd98b1dd5eaa7-goog
Hi Abhishek,
> Refactor gpio reset to use a common gpio reset function.
>
> Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
> Reviewed-by: Miao-chen Chou <[email protected]>
> ---
>
> drivers/bluetooth/btusb.c | 59 +++++++++++++--------------------------
> 1 file changed, 19 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index b14102fba6018..03341e6cbf3ed 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -547,6 +547,7 @@ struct btusb_data {
> struct usb_endpoint_descriptor *diag_rx_ep;
>
> struct gpio_desc *reset_gpio;
> + unsigned int reset_duration_ms;
>
> __u8 cmdreq_type;
> __u8 cmdreq;
> @@ -566,15 +567,21 @@ struct btusb_data {
> unsigned cmd_timeout_cnt;
> };
>
> -static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
> +static void btusb_toggle_gpio(struct gpio_desc *desc, unsigned int duration)
> +{
> + gpiod_set_value_cansleep(desc, 1);
> + msleep(duration);
> + gpiod_set_value_cansleep(desc, 0);
> +}
> +
> +static void btusb_gpio_cmd_timeout(struct hci_dev *hdev)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> - struct gpio_desc *reset_gpio = data->reset_gpio;
>
> if (++data->cmd_timeout_cnt < 5)
> return;
>
> - if (!reset_gpio) {
> + if (!data->reset_gpio) {
> bt_dev_err(hdev, "No way to reset. Ignoring and continuing");
> return;
> }
> @@ -592,39 +599,7 @@ static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
> }
>
> bt_dev_err(hdev, "Initiating HW reset via gpio");
> - gpiod_set_value_cansleep(reset_gpio, 1);
> - msleep(100);
> - gpiod_set_value_cansleep(reset_gpio, 0);
> -}
> -
> -static void btusb_rtl_cmd_timeout(struct hci_dev *hdev)
> -{
> - struct btusb_data *data = hci_get_drvdata(hdev);
> - struct gpio_desc *reset_gpio = data->reset_gpio;
> -
> - if (++data->cmd_timeout_cnt < 5)
> - return;
> -
> - if (!reset_gpio) {
> - bt_dev_err(hdev, "No gpio to reset Realtek device, ignoring");
> - return;
> - }
> -
> - /* Toggle the hard reset line. The Realtek 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(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> - bt_dev_err(hdev, "last reset failed? Not resetting again");
> - return;
> - }
> -
> - bt_dev_err(hdev, "Reset Realtek device via gpio");
> - gpiod_set_value_cansleep(reset_gpio, 1);
> - msleep(200);
> - gpiod_set_value_cansleep(reset_gpio, 0);
> + btusb_toggle_gpio(data->reset_gpio, data->reset_duration_ms);
> }
You need to explain why this patch is correct. You are removing more code here. And there is an extra check in case of Realtek and a large comment.
Regards
Marcel