linux-kernel - Re: [PATCH v3 5/5] Bluetooth: btusb: Use the hw_reset method to allow resetting the BT chip

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Dec 2018 00:46:08 -0800
From:   Rajat Jain <rajatxjain@...il.com>
To:     Rajat Jain <rajatja@...gle.com>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Dmitry Torokhov <dtor@...omium.org>,
        Alex Hung <alex.hung@...onical.com>,
        linux-bluetooth@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-usb@...r.kernel.org, netdev <netdev@...r.kernel.org>,
        Dmitry Torokhov <dtor@...gle.com>, raghuram.hegde@...el.com,
        chethan.tumkur.narayan@...el.com, sukumar.ghorai@...el.com
Subject: Re: [PATCH v3 5/5] Bluetooth: btusb: Use the hw_reset method to allow
 resetting the BT chip

On Wed, Nov 21, 2018 at 3:50 PM Rajat Jain <rajatja@...gle.com> wrote:
>
> If the platform provides it, use the reset gpio to reset the BT
> chip (requested by the HCI core if needed). This has been found helpful
> on some of Intel bluetooth controllers where the firmware gets stuck and
> the only way out is a hard reset pin provided by the platform.

Hi Folks,

I was wondering if you got a change to look at this patchset, and if
there is any feedback.

Thanks,

Rajat


>
> Signed-off-by: Rajat Jain <rajatja@...gle.com>
> ---
> v3: Better error handling for gpiod_get_optional()
> v2: Handle the EPROBE_DEFER case.
>
>  drivers/bluetooth/btusb.c | 40 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e8e148480c91..e7631f770fae 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -29,6 +29,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
>  #include <linux/suspend.h>
> +#include <linux/gpio/consumer.h>
>  #include <asm/unaligned.h>
>
>  #include <net/bluetooth/bluetooth.h>
> @@ -475,6 +476,8 @@ struct btusb_data {
>         struct usb_endpoint_descriptor *diag_tx_ep;
>         struct usb_endpoint_descriptor *diag_rx_ep;
>
> +       struct gpio_desc *reset_gpio;
> +
>         __u8 cmdreq_type;
>         __u8 cmdreq;
>
> @@ -490,6 +493,26 @@ struct btusb_data {
>         int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>  };
>
> +
> +static void btusb_hw_reset(struct hci_dev *hdev)
> +{
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       struct gpio_desc *reset_gpio = data->reset_gpio;
> +
> +       /*
> +        * Toggle the hard reset line if the platform provides one. The reset
> +        * is going to yank the device off the USB and then replug. So doing
> +        * once is enough. 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.
> +        */
> +       bt_dev_dbg(hdev, "%s: Initiating HW reset via gpio", __func__);
> +       clear_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks);
> +       gpiod_set_value(reset_gpio, 1);
> +       mdelay(100);
> +       gpiod_set_value(reset_gpio, 0);
> +}
> +
>  static inline void btusb_free_frags(struct btusb_data *data)
>  {
>         unsigned long flags;
> @@ -2917,6 +2940,7 @@ static int btusb_probe(struct usb_interface *intf,
>                        const struct usb_device_id *id)
>  {
>         struct usb_endpoint_descriptor *ep_desc;
> +       struct gpio_desc *reset_gpio;
>         struct btusb_data *data;
>         struct hci_dev *hdev;
>         unsigned ifnum_base;
> @@ -3030,6 +3054,16 @@ static int btusb_probe(struct usb_interface *intf,
>
>         SET_HCIDEV_DEV(hdev, &intf->dev);
>
> +       reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> +                                       GPIOD_OUT_LOW);
> +       if (IS_ERR(reset_gpio)) {
> +               err = PTR_ERR(reset_gpio);
> +               goto out_free_dev;
> +       } else if (reset_gpio) {
> +               data->reset_gpio = reset_gpio;
> +               hdev->hw_reset = btusb_hw_reset;
> +       }
> +
>         hdev->open   = btusb_open;
>         hdev->close  = btusb_close;
>         hdev->flush  = btusb_flush;
> @@ -3085,6 +3119,7 @@ static int btusb_probe(struct usb_interface *intf,
>                 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);
> +               set_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks);
>
>                 if (id->driver_info & BTUSB_INTEL) {
>                         hdev->setup = btusb_setup_intel;
> @@ -3225,6 +3260,8 @@ static int btusb_probe(struct usb_interface *intf,
>         return 0;
>
>  out_free_dev:
> +       if (data->reset_gpio)
> +               gpiod_put(data->reset_gpio);
>         hci_free_dev(hdev);
>         return err;
>  }
> @@ -3268,6 +3305,9 @@ static void btusb_disconnect(struct usb_interface *intf)
>         if (data->oob_wake_irq)
>                 device_init_wakeup(&data->udev->dev, false);
>
> +       if (data->reset_gpio)
> +               gpiod_put(data->reset_gpio);
> +
>         hci_free_dev(hdev);
>  }
>
> --
> 2.19.1.1215.g8438c0b245-goog
>