On 2/18/21 8:37 PM, Hans de Goede wrote:
Hi All,
>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
