2021-04-06 09:26:49

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 5.12 regression fix resend 0/1] Bluetooth: btusb: Revert Fix the autosuspend enable and disable

Hi,

As discussed in the original submission of this patch, the patch this
reverts simply is wrong.

This revert was also Acked by the author of the original patch in the
thread (I've added his ack to this resend).

This fixes a regression where bluetooth autosuspend no longer works OOTB
with 5.12 leading to significantly increased power-consumption.

Can we get this regression fix on its way to Linus for 5.12-rc7 please ?

Regards,

Hans


Hans de Goede (1):
Bluetooth: btusb: Revert Fix the autosuspend enable and disable

drivers/bluetooth/btusb.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

--
2.30.2


2021-04-06 09:37:10

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 5.12 regression fix resend 1/1] Bluetooth: btusb: Revert Fix the autosuspend enable and disable

drivers/usb/core/hub.c: usb_new_device() contains the following:

/* By default, forbid autosuspend for all devices. It will be
* allowed for hubs during binding.
*/
usb_disable_autosuspend(udev);

So for anything which is not a hub, such as btusb devices, autosuspend is
disabled by default and we must call usb_enable_autosuspend(udev) to
enable it.

This means that the "Fix the autosuspend enable and disable" commit,
which drops the usb_enable_autosuspend() call when the enable_autosuspend
module option is true, is completely wrong, revert it.

This reverts commit 7bd9fb058d77213130e4b3e594115c028b708e7e.

Cc: Hui Wang <[email protected]>
Fixes: 7bd9fb058d77 ("Bluetooth: btusb: Fix the autosuspend enable and disable")
Acked-by: Hui Wang <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/bluetooth/btusb.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 52683fd22e05..5cbfbd948f67 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4849,8 +4849,8 @@ static int btusb_probe(struct usb_interface *intf,
data->diag = NULL;
}

- if (!enable_autosuspend)
- usb_disable_autosuspend(data->udev);
+ if (enable_autosuspend)
+ usb_enable_autosuspend(data->udev);

err = hci_register_dev(hdev);
if (err < 0)
@@ -4910,9 +4910,6 @@ static void btusb_disconnect(struct usb_interface *intf)
gpiod_put(data->reset_gpio);

hci_free_dev(hdev);
-
- if (!enable_autosuspend)
- usb_enable_autosuspend(data->udev);
}

#ifdef CONFIG_PM
--
2.30.2

2021-04-06 09:38:47

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: btusb: Revert Fix the autosuspend enable and disable

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=461009

---Test result---

##############################
Test: CheckPatch - FAIL
Bluetooth: btusb: Revert Fix the autosuspend enable and disable
WARNING: Unknown commit id '7bd9fb058d77', maybe rebased or not pulled?
#25:
Fixes: 7bd9fb058d77 ("Bluetooth: btusb: Fix the autosuspend enable and disable")

total: 0 errors, 1 warnings, 19 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] Bluetooth: btusb: Revert Fix the autosuspend enable and" has style problems, please review.

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - PASS


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (42.33 kB)
bnep-tester.log (3.45 kB)
mgmt-tester.log (533.87 kB)
rfcomm-tester.log (11.38 kB)
sco-tester.log (9.66 kB)
smp-tester.log (11.52 kB)
userchan-tester.log (5.30 kB)
Download all attachments

2021-04-09 13:46:30

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 5.12 regression fix resend 1/1] Bluetooth: btusb: Revert Fix the autosuspend enable and disable

Hi Hans,

> drivers/usb/core/hub.c: usb_new_device() contains the following:
>
> /* By default, forbid autosuspend for all devices. It will be
> * allowed for hubs during binding.
> */
> usb_disable_autosuspend(udev);
>
> So for anything which is not a hub, such as btusb devices, autosuspend is
> disabled by default and we must call usb_enable_autosuspend(udev) to
> enable it.
>
> This means that the "Fix the autosuspend enable and disable" commit,
> which drops the usb_enable_autosuspend() call when the enable_autosuspend
> module option is true, is completely wrong, revert it.
>
> This reverts commit 7bd9fb058d77213130e4b3e594115c028b708e7e.
>
> Cc: Hui Wang <[email protected]>
> Fixes: 7bd9fb058d77 ("Bluetooth: btusb: Fix the autosuspend enable and disable")
> Acked-by: Hui Wang <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/bluetooth/btusb.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)

since we are already at -rc6, I think it makes more sense that you send it directly to Linus for inclusion.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel