[PATCH] Bluetooth: Allow reset via sysfs

[PATCH] Bluetooth: Allow reset via sysfs

[Hsin-chen Chuang]

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.

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@chromium.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

RE: Bluetooth: Allow reset via sysfs

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=921922

---Test result---

Test Summary:
CheckPatch                    PENDING   0.26 seconds
GitLint                       PENDING   0.21 seconds
SubjectPrefix                 PASS      0.70 seconds
BuildKernel                   PASS      24.84 seconds
CheckAllWarning               PASS      27.05 seconds
CheckSparse                   PASS      30.62 seconds
BuildKernel32                 PASS      24.57 seconds
TestRunnerSetup               PASS      443.19 seconds
TestRunner_l2cap-tester       PASS      22.93 seconds
TestRunner_iso-tester         PASS      32.42 seconds
TestRunner_bnep-tester        PASS      4.97 seconds
TestRunner_mgmt-tester        PASS      118.65 seconds
TestRunner_rfcomm-tester      PASS      8.41 seconds
TestRunner_sco-tester         PASS      9.47 seconds
TestRunner_ioctl-tester       PASS      8.23 seconds
TestRunner_mesh-tester        FAIL      8.27 seconds
TestRunner_smp-tester         PASS      7.13 seconds
TestRunner_userchan-tester    PASS      5.10 seconds
IncrementalBuild              PENDING   0.61 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
BUG: KASAN: slab-use-after-free in run_timer_softirq+0x76c/0x7d0
WARNING: CPU: 0 PID: 64 at kernel/workqueue.c:2257 __queue_work+0x687/0xb40
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Failed       0.110 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Luiz Augusto von Dentz]

Hi Hsin-chen,

On Thu, Jan 2, 2025 at 10:21 PM Hsin-chen Chuang <chharry@chromium.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@chromium.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

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Hsin-chen Chuang]

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@gmail.com> wrote:
>
> Hi Hsin-chen,
>
> On Thu, Jan 2, 2025 at 10:21 PM Hsin-chen Chuang <chharry@chromium.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@chromium.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

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Luiz Augusto von Dentz]

Hi Hsin-chen,

On Tue, Jan 7, 2025 at 2:21 AM Hsin-chen Chuang <chharry@chromium.org> wrote:
>
> 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.

My bad, I didn't realize it was under hci_sysfs, which makes sense,
that said I do wonder if it wouldn't be better to name the callback as
reset rather than cmd_timeout and we should probably move them from
btusb if there are not usb specific, in fact there is already a reset
callback but it doesn't seem to be used which I think it is a mistake
and we should actually make the vendor reset the callback o
hdev->reset not have hdev->cmd_timeout -> vendor_cmd_timeout ->
btusb_reset -> hdev->reset but rather btusb_reset -> hdev->reset ->
vendor_reset then we can get rid of cmd_timeout.

> 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.

Ok, then please have these changes splitted since they have different purposes.

>
>
> On Tue, Jan 7, 2025 at 12:29 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Hsin-chen,
> >
> > On Thu, Jan 2, 2025 at 10:21 PM Hsin-chen Chuang <chharry@chromium.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@chromium.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



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Greg KH]

On Fri, Jan 03, 2025 at 11:20:20AM +0800, Hsin-chen Chuang 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.
> 
> 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@chromium.org>
> ---
> This commit has been tested on a Chromebook by running
> `echo 1 > /sys/class/bluetooth/hci0/reset`

You forgot the required Documentation/ABI/ update for your new sysfs
file :(

thanks,

greg k-h

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Luiz Augusto von Dentz]

Hi Greg,

On Wed, Jan 8, 2025 at 7:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jan 03, 2025 at 11:20:20AM +0800, Hsin-chen Chuang 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.
> >
> > 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@chromium.org>
> > ---
> > This commit has been tested on a Chromebook by running
> > `echo 1 > /sys/class/bluetooth/hci0/reset`
>
> You forgot the required Documentation/ABI/ update for your new sysfs
> file :(

Looks like I've missed that when reviewing these changes, anyway no
pull-request has been made, I assume we should follow what is
documentation on Documentation/ABI/README? Does it include debugfs
entries as well or only sysfs are required to be documented?

> thanks,
>
> greg k-h



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Greg KH]

On Wed, Jan 08, 2025 at 10:06:34AM -0500, Luiz Augusto von Dentz wrote:
> Hi Greg,
> 
> On Wed, Jan 8, 2025 at 7:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Jan 03, 2025 at 11:20:20AM +0800, Hsin-chen Chuang 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.
> > >
> > > 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@chromium.org>
> > > ---
> > > This commit has been tested on a Chromebook by running
> > > `echo 1 > /sys/class/bluetooth/hci0/reset`
> >
> > You forgot the required Documentation/ABI/ update for your new sysfs
> > file :(
> 
> Looks like I've missed that when reviewing these changes, anyway no
> pull-request has been made, I assume we should follow what is
> documentation on Documentation/ABI/README?

Yes, all sysfs entries must be documented that way.  We have a script in
the tree that you can run to verify that all entries are documented, but
I don't think anyone runs it very often :(

> Does it include debugfs
> entries as well or only sysfs are required to be documented?

Only sysfs as that's the only stable api, debugfs can change all it
wants :)

thanks,

greg k-h

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Luiz Augusto von Dentz]

Hi Greg,

On Wed, Jan 8, 2025 at 10:11 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 08, 2025 at 10:06:34AM -0500, Luiz Augusto von Dentz wrote:
> > Hi Greg,
> >
> > On Wed, Jan 8, 2025 at 7:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Jan 03, 2025 at 11:20:20AM +0800, Hsin-chen Chuang 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.
> > > >
> > > > 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@chromium.org>
> > > > ---
> > > > This commit has been tested on a Chromebook by running
> > > > `echo 1 > /sys/class/bluetooth/hci0/reset`
> > >
> > > You forgot the required Documentation/ABI/ update for your new sysfs
> > > file :(
> >
> > Looks like I've missed that when reviewing these changes, anyway no
> > pull-request has been made, I assume we should follow what is
> > documentation on Documentation/ABI/README?
>
> Yes, all sysfs entries must be documented that way.  We have a script in
> the tree that you can run to verify that all entries are documented, but
> I don't think anyone runs it very often :(

Do you recall which script it is? I'd like to include that in my hooks
and in our CI so I don't miss it next time.

> > Does it include debugfs
> > entries as well or only sysfs are required to be documented?
>
> Only sysfs as that's the only stable api, debugfs can change all it
> wants :)
>
> thanks,
>
> greg k-h



-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: Allow reset via sysfs

[Greg KH]

On Wed, Jan 08, 2025 at 10:33:01AM -0500, Luiz Augusto von Dentz wrote:
> Hi Greg,
> 
> On Wed, Jan 8, 2025 at 10:11 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jan 08, 2025 at 10:06:34AM -0500, Luiz Augusto von Dentz wrote:
> > > Hi Greg,
> > >
> > > On Wed, Jan 8, 2025 at 7:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Jan 03, 2025 at 11:20:20AM +0800, Hsin-chen Chuang 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.
> > > > >
> > > > > 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@chromium.org>
> > > > > ---
> > > > > This commit has been tested on a Chromebook by running
> > > > > `echo 1 > /sys/class/bluetooth/hci0/reset`
> > > >
> > > > You forgot the required Documentation/ABI/ update for your new sysfs
> > > > file :(
> > >
> > > Looks like I've missed that when reviewing these changes, anyway no
> > > pull-request has been made, I assume we should follow what is
> > > documentation on Documentation/ABI/README?
> >
> > Yes, all sysfs entries must be documented that way.  We have a script in
> > the tree that you can run to verify that all entries are documented, but
> > I don't think anyone runs it very often :(
> 
> Do you recall which script it is? I'd like to include that in my hooks
> and in our CI so I don't miss it next time.

It's scripts/get_abi.pl