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

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

[Hans de Goede]

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

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

[Hans de Goede]

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 <hui.wang@canonical.com>
Fixes: 7bd9fb058d77 ("Bluetooth: btusb: Fix the autosuspend enable and disable")
Acked-by: Hui Wang <hui.wang@canonical.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 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

RE: Bluetooth: btusb: Revert Fix the autosuspend enable and disable

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2108 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=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


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43341 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3532 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 546679 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11652 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9887 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11798 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5428 bytes --]

Re: Bluetooth: btusb: Revert Fix the autosuspend enable and disable

[Hans de Goede]

Hi,

On 4/5/21 11:09 PM, bluez.test.bot@gmail.com wrote:
> 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: 

I just checked and '7bd9fb058d77' is the hash of the fixed (reverted) commit in Linus' tree.

Regards,

Hans



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

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

[Marcel Holtmann]

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 <hui.wang@canonical.com>
> Fixes: 7bd9fb058d77 ("Bluetooth: btusb: Fix the autosuspend enable and disable")
> Acked-by: Hui Wang <hui.wang@canonical.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> 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 <marcel@holtmann.org>

Regards

Marcel