Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USB_AUTOSUSPEND=1 default setting causes issues with USB devices #311

Closed
jonathonf opened this issue Dec 10, 2017 · 13 comments
Closed

USB_AUTOSUSPEND=1 default setting causes issues with USB devices #311

jonathonf opened this issue Dec 10, 2017 · 13 comments

Comments

@jonathonf
Copy link

jonathonf commented Dec 10, 2017

Symptom data

With USB_AUTOSUSPEND=1 as the default, suspend is force-enabled even when devices may not support suspend.

This has caused many issues for Manjaro users, e.g.:

While we can repackage the defaults (https://forum.manjaro.org/t/changes-to-etc-default-tlp/33700) I'd like to float the idea of altering them here to help other distros too.

(There are some other defaults which possibly aren't ideal but I'll start with this one so I don't flood the issue tracker and in case there's a better way, e.g. just submitting a PR - let me know.)

Expected behaviour

Devices should continue to function under default settings.

Actual behaviour

Certain devices do not function under default settings.

Steps to reproduce the problem

Insert XBOX360 controller during boot with default settings (USB_AUTOSUSPEND=1). Observe that controller will not operate until disconnected and reconnected.

Switch to USB_AUTOSUSPEND=0, boot with controller connected, observe controller operates correctly without need for disconnect and reconnect.

@linrunner
Copy link
Owner

The underlying problem here are faulty kernel drivers and/or faulty device firmware causing devices to fail when power saving is activated.

I see two approaches to this:

  1. Causal solution: fix those drivers – this is none of my business, I suggest to file specific bug reports
  2. Workaround: change TLP's default configuration to USB_AUTOSUSPEND=0

Solution 2 will sacrifice significant power saving for users of hardware with proper drivers/firmware. So I decline to go that way. The same applies to all other defaults.

If you want to exclude specific device categories, then feel free to send pull requests with code to filter them.

You may of course change the default(s) in Manjaro, but keep in mind that you render power saving useless by this strategy.

@jonathonf
Copy link
Author

jonathonf commented Dec 10, 2017

By default, USB power saving is active in the kernel, but not force-enabled for incompatible drivers. That is, devices that support suspension will suspend, drivers that do not, will not.

USB_AUTOSUSPEND=1 force-enables suspension for incompatible drivers, hence causing the issues. From what I've read, USB_AUTOSUSPEND=0 does not disable USB power saving, rather it uses the defaults provided by device drivers (as determined by the kernel).

As far as I'm aware, the only way to fully disable the kernel's built-in USB autosuspend is to pass usbcore.autosuspend=-1 to the kernel.

@linrunner
Copy link
Owner

I don't agree all drivers that don't enable autosuspend by default are "incompatible".

@jonathonf
Copy link
Author

I'd also agree with that. It doesn't change the fact that USB_AUTOSUSPEND=1 force-enables suspension for incompatible drivers (as well as any driver that is compatible but not listed as such).

As long as the potential issues with default-on are known (see e.g. Warnings in the kernel power management doc) then that's fine. 🙂

@linrunner
Copy link
Owner

I'm pondering a change like

USB_AUTOSUSPEND_ON_AC=0
USB_AUTOSUSPEND_ON_BAT=1

Any thoughts?

(Reference: Manjaro: call for testing new TLP defaults)

@jonathonf
Copy link
Author

I like the idea very much. 👍

Seems like a good middle-ground.

@linrunner
Copy link
Owner

Since this requires some major restructuring and testing, I plan it for 1.2.

@rmilecki
Copy link

I've run into this problem after installing openSUSE 15.0 which comes with the tlp installed by default. For details please check: https://bugzilla.opensuse.org/show_bug.cgi?id=1124909

What I find the worst in this issue is that it completely conflicts with libsane and its workarounds for selected scanners. Basically it results in:

  1. libsane writing on on the power/control (fixing problematic scanner)
  2. tlp writing auto half a second later (invalidating the libsane workaround)

This is pretty confusing and I think that adding USB_AUTOSUSPEND_ON_AC and USB_AUTOSUSPEND_ON_BAT will make it only worse (harder to debug). It took me days to debug this problem.

I suggest to not set auto for unknown USB devices. Consider following:

From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Subject: [PATCH] set power/control to "auto" only for known devices
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

So far 20-tlp-func-usb was assuming every unknown device works correctly
with autosuspending enabled. All unknown devices (other than bluetooth,
phone, printers, etc.) were getting "auto" written to the power/control.

That behavior was breaking some devices including A LOT of scanners.
It's pretty common for scanners to require autosuspending *disabled*.

What's worse tlp was completely breaking libsane and its workarounds
provided by udev rule (see tools/sane-desc.c for a reference). While
libsane was setting power/control to "on" for specific scanners half a
second later tlp was setting it to the non-working "auto" value.

Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=1124909
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 func.d/20-tlp-func-usb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/func.d/20-tlp-func-usb b/func.d/20-tlp-func-usb
index 8766c2a..e7d05b1 100644
--- a/func.d/20-tlp-func-usb
+++ b/func.d/20-tlp-func-usb
@@ -39,7 +39,7 @@ usb_suspend_device () { # enable/disable usb autosuspend for a single device
         local usbid="$vendor:$(read_sysf $usbdev/idProduct)"
         local busdev="Bus $(read_sysf $usbdev/busnum) Dev $(read_sysf $usbdev/devnum)"
         local dclass="$(read_sysf $usbdev/bDeviceClass)"
-        local control="${3:-auto}"
+        local control="$3"
         local caller="$2"
         local exc=""
         local chg=0 rc1=0 rc2=0
@@ -200,7 +200,7 @@ usb_suspend_device () { # enable/disable usb autosuspend for a single device
             fi # wwan
         fi # !device blacklist
 
-        if [ "$(read_sysf $usbdev/power/control)" != "$control" ]; then
+        if [ -n "$control" -a "$(read_sysf $usbdev/power/control)" != "$control" ]; then
             # set control, write actual changes only
             write_sysf "$control" $usbdev/power/control; rc1=$?
             chg=1

@linrunner
Copy link
Owner

linrunner commented Feb 28, 2019

It took me days to debug this problem.

@rmilecki : I can understand your embarassment about that. Let me explain my point of view.

I started this project in 2009 when almost no device driver had power saving enabled by default. Despite people often telling me kernel development will catch up and tools like TLP will be obsolete soon, real progress in this respect has been quite slow. Although I understand the reasons for it – too many hardware variations, too few testers/devs – it's still frustrating.

If I would exclude all unknown devices (USB or else) by default, then TLP is superfluous, because we end up with just the plain kernel defaults. A power saving tool that deserves its name must be more aggressive than the defaults.

@linrunner linrunner removed the planned label Feb 28, 2019
@linrunner linrunner removed this from the 1.2 Release milestone Feb 28, 2019
linrunner added a commit that referenced this issue Mar 5, 2019
Rationale: TLP autosuspends all devices not blacklisted, overriding
libsane's udev rules that implement blacklisting for scanners.

Solution: check udev environment for 'libsane_matched=yes' and
exclude corresponding devices from autosuspend.

References:
* #311 (comment)

Credits:
* Rafal Milecki
@linrunner
Copy link
Owner

@rmilecki : exception implemented for devices managed by libsane. Thanks for your hint.

@userofryzen
Copy link

I'm pondering a change like

USB_AUTOSUSPEND_ON_AC=0
USB_AUTOSUSPEND_ON_BAT=1

Any thoughts?

(Reference: Manjaro: call for testing new TLP defaults)

I feel like there is a need to do this xD more options are always welcom

@linrunner
Copy link
Owner

linrunner commented Apr 29, 2020

I think i'll abandon the idea to split the settings for AC and BAT. It won't really help users when we limit the trouble to BAT. Better to have specific measures like the one for scanners above.

Btw: I have no problem with Manjaro's approach USB_AUTOSUSPEND=0 but won't adopt it upstream.

@LinuxOnTheDesktop
Copy link

LinuxOnTheDesktop commented Aug 13, 2023

May I ask here whether the 'your system is too slow' libinput warnings (see e.g. here [EDIT:] and here) might owe something to tlp's usb-suspending feature? I ask because of this (and because I use tlp and I see the relevant warnings).

I could try to test the idea myself, I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants