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

On 2/18/21 8:37 PM, Hans de Goede wrote:

Hi All,

quoted

From the commit msg:

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

Hui, I guess that what you were seeing is caused by:
/lib/udev/hwdb.d/60-autosuspend-chromiumos.hwdb

Hi Hans,

You are right, the VID:PID of the BT adapter on my machine is in that 
file, the autosuspend is enabled by udev instead of kernel. I tested on 
another machine whose BT adapter's ID is not in that file, the 
autosuspend is controlled by btusb.enable_autosuspend=0.

Your reverting patch is correct.

Which enables autosuspend on a bunch of USB devices based on VID:PID,
overruling the kernel defaults. This is done to get better power-consumption
with devices where it is known that it is safe to do this.
  
I guess that that the device you were testing this with is on that list.
So the proper fix would be to edit that file and remove your VID:PID from it.

Hui, also next time please try to Cc the original author of the code you
are modifying. A simple "git blame drivers/bluetooth/btusb.c" would have
found you commit eff2d68ca738 ("Bluetooth: btusb: Add a Kconfig option to
enable USB autosuspend by default") and then you could have added me to
the Cc and I could have nacked the patch before it got merged.

OK, got it. will be careful and will Cc to the original author next time.

I happen to spot this this time since I was looking into some other
btusb issue. But if I had not spotted this, this would have caused
a significant power-consumption regression on many laptops.

Btusb might not look like a big consumer, but if it does not autosuspend
it often is the only USB device not autosuspending, keeping the XHCI
controller awake, which in turn is keeping a whole power-plane awake on
what once used to be the southbridge. At least on Skylake era hw this
could lead to an extra idle powerconsumption of 1W. So a small change
can cause a big impact.

Regards,

Hans