linux-kernel - Re: [PATCH] Bluetooth: Allow reset via sysfs

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]

Message-ID: <CAG=_-8cMgr_NqtX_zEV_7K1hunmOzQB4mgGDAiBPaX-vd9E=4A@mail.gmail.com>
Date: Tue, 7 Jan 2025 15:21:42 +0800
From: Hsin-chen Chuang <chharry@...omium.org>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: linux-bluetooth@...r.kernel.org, 
	chromeos-bluetooth-upstreaming@...omium.org, 
	Johan Hedberg <johan.hedberg@...il.com>, Marcel Holtmann <marcel@...tmann.org>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: Allow reset via sysfs

Hi Luiz,

This is not btusb specific, and is not limited to USB reset.

The cmd_timeout handler can also be found in the QCA uart and the MTK
sdio controller drivers. It's up to the vendors to implement a reliable
reset mechanism, and we welcome - or prefer - an OOB solution such as
reset over GPIO.

This patch involves some btusb changes because the handlers in btusb
can only do something after being called 5 times, while the other
handlers don't.



On Tue, Jan 7, 2025 at 12:29 AM Luiz Augusto von Dentz
<luiz.dentz@...il.com> wrote:
>
> Hi Hsin-chen,
>
> On Thu, Jan 2, 2025 at 10:21 PM Hsin-chen Chuang <chharry@...omium.org> wrote:
> >
> > Allow sysfs to trigger reset via the cmd_timeout function in hci device.
> > This is required to recover devices that are not responsive from
> > userspace.
>
> Don't we have a similar control over USB to reset the device? I think
> that would be better than introducing something btusb specific.
>
> > Also remove the cmd timeout count in btusb since we only ever allow one
> > command in flight at a time. We should always reset after a single
> > command times out.
> >
> > Signed-off-by: Hsin-chen Chuang <chharry@...omium.org>
> > ---
> > This commit has been tested on a Chromebook by running
> > `echo 1 > /sys/class/bluetooth/hci0/reset`
> >
> >  drivers/bluetooth/btusb.c | 10 ----------
> >  net/bluetooth/hci_sysfs.c | 19 +++++++++++++++++++
> >  2 files changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 279fe6c115fac..a4810c77fa0da 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -879,7 +879,6 @@ struct btusb_data {
> >         int (*disconnect)(struct hci_dev *hdev);
> >
> >         int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
> > -       unsigned cmd_timeout_cnt;
> >
> >         struct qca_dump_info qca_dump;
> >  };
> > @@ -912,9 +911,6 @@ static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
> >         struct gpio_desc *reset_gpio = data->reset_gpio;
> >         struct btintel_data *intel_data = hci_get_priv(hdev);
> >
> > -       if (++data->cmd_timeout_cnt < 5)
> > -               return;
> > -
> >         if (intel_data->acpi_reset_method) {
> >                 if (test_and_set_bit(INTEL_ACPI_RESET_ACTIVE, intel_data->flags)) {
> >                         bt_dev_err(hdev, "acpi: last reset failed ? Not resetting again");
> > @@ -997,9 +993,6 @@ static void btusb_rtl_cmd_timeout(struct hci_dev *hdev)
> >
> >         btusb_rtl_alloc_devcoredump(hdev, &hdr, NULL, 0);
> >
> > -       if (++data->cmd_timeout_cnt < 5)
> > -               return;
> > -
> >         if (!reset_gpio) {
> >                 btusb_reset(hdev);
> >                 return;
> > @@ -1044,9 +1037,6 @@ static void btusb_qca_cmd_timeout(struct hci_dev *hdev)
> >                 return;
> >         }
> >
> > -       if (++data->cmd_timeout_cnt < 5)
> > -               return;
> > -
> >         if (reset_gpio) {
> >                 bt_dev_err(hdev, "Reset qca device via bt_en gpio");
> >
> > diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > index 4b54dbbf0729a..7bf2b10b0a7cf 100644
> > --- a/net/bluetooth/hci_sysfs.c
> > +++ b/net/bluetooth/hci_sysfs.c
> > @@ -90,9 +90,28 @@ static void bt_host_release(struct device *dev)
> >         module_put(THIS_MODULE);
> >  }
> >
> > +static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
> > +                          const char *buf, size_t count)
> > +{
> > +       struct hci_dev *hdev = to_hci_dev(dev);
> > +
> > +       if (hdev->cmd_timeout)
> > +               hdev->cmd_timeout(hdev);
> > +
> > +       return count;
> > +}
> > +static DEVICE_ATTR_WO(reset);
> > +
> > +static struct attribute *bt_host_attrs[] = {
> > +       &dev_attr_reset.attr,
> > +       NULL,
> > +};
> > +ATTRIBUTE_GROUPS(bt_host);
> > +
> >  static const struct device_type bt_host = {
> >         .name    = "host",
> >         .release = bt_host_release,
> > +       .groups = bt_host_groups,
> >  };
> >
> >  void hci_init_sysfs(struct hci_dev *hdev)
> > --
> > 2.47.1.613.gc27f4b7a9f-goog
> >
>
>
> --
> Luiz Augusto von Dentz

-- 
Best Regards,
Hsin-chen