| Message ID | 20220824160946.10128-1-jflf_kernel@gmx.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: add quirks for Lenovo OneLink+ Dock | expand |
On 30.08.22 15:56, jflf_kernel@gmx.com wrote: Hi, > I agree that USB_QUIRK_RESET_RESUME seems fishy with a hub. It's pretty much the last quirk I tried, and the only one that worked. I can't say I understand what it does exactly. Two things 1) force a reset after a resume and call reset_resume() instead of resume() 2) block autosuspend if remote wakeup is required I suspect you are actually using the second effect. Have you tested with "usbcore.autosuspend=-1" on the kernel command line. The hubs themselves don't seem to reset (or at least not fully), as there is no re-enumeration of existing children. Probably because they do not suspend at all. Generally hubs do need remote wakeup lest they forget the port whose activity woke the system. > I have not experienced a single problem or side effect since using those quirks. I use a mix of USB 2.0 and 3.0 devices, some bus- and some self-powered, some permanently connected (including ethernet and audio in the hub itself) and some not. The interesting test would be to see whether a keyboard, ethernet or mouse can wake your system from suspend. Have you tested WOL? Regards Oliver
On 30/08/2022 16.47, Oliver Neukum wrote: > 1) force a reset after a resume and call reset_resume() instead of resume() > 2) block autosuspend if remote wakeup is required > > I suspect you are actually using the second effect. Have you > tested with "usbcore.autosuspend=-1" on the kernel command line. After further testing, your suspicion is correct. TL;DR: the two VL812 hubs don't behave well when suspended. I'd like to prepare a better patch for that issue. What's the recommended strategy? The current patch works, even if only as a side effect and when there's a wakeup source downstream. It's currently in Greg KH's usb-linus branch, and will land in linux-next at some point. I'm tempted to let it be and undo it later in the better patch. Is that acceptable? Or should I ask Greg KH to pull it? > The interesting test would be to see whether a keyboard, ethernet > or mouse can wake your system from suspend. Have you tested WOL? Right, a bit of background to understand what's going on with that dock. There are actually three USB hubs in the dock: the two VL812, and a third Genesys Logic USB 2.0 only: $ lsusb -d 17ef: Bus 002 Device 005: ID 17ef:3054 Lenovo OneLink+ Giga Bus 002 Device 004: ID 17ef:1019 Lenovo USB3.0 Hub Bus 002 Device 002: ID 17ef:1018 Lenovo USB3.0 Hub Bus 001 Device 011: ID 17ef:3055 Lenovo ThinkPad OneLink Plus Dock Audio Bus 001 Device 006: ID 17ef:1019 Lenovo USB2.0 Hub Bus 001 Device 004: ID 17ef:1018 Lenovo USB2.0 Hub $ lsusb -d 05e3: Bus 001 Device 007: ID 05e3:0608 Genesys Logic, Inc. Hub $ lsusb -tv | grep -B1 -e 17ef: -e 05e3: |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M ID 17ef:1018 Lenovo |__ Port 1: Dev 4, If 0, Class=Hub, Driver=hub/4p, 5000M ID 17ef:1019 Lenovo |__ Port 3: Dev 5, If 1, Class=CDC Data, Driver=cdc_ether, 5000M ID 17ef:3054 Lenovo |__ Port 3: Dev 5, If 0, Class=Communications, Driver=cdc_ether, 5000M ID 17ef:3054 Lenovo -- |__ Port 6: Dev 4, If 0, Class=Hub, Driver=hub/4p, 480M ID 17ef:1018 Lenovo -- |__ Port 2: Dev 7, If 0, Class=Hub, Driver=hub/2p, 480M ID 05e3:0608 Genesys Logic, Inc. Hub -- |__ Port 1: Dev 6, If 0, Class=Hub, Driver=hub/4p, 480M ID 17ef:1019 Lenovo |__ Port 4: Dev 8, If 1, Class=Audio, Driver=snd-usb-audio, 12M ID 17ef:3055 Lenovo |__ Port 4: Dev 8, If 2, Class=Audio, Driver=snd-usb-audio, 12M ID 17ef:3055 Lenovo |__ Port 4: Dev 8, If 0, Class=Audio, Driver=snd-usb-audio, 12M ID 17ef:3055 Lenovo |__ Port 4: Dev 8, If 3, Class=Human Interface Device, Driver=usbhid, 12M ID 17ef:3055 Lenovo The user-friendly port view (front and back refer to the locations of USB-A ports): 1018 [p1] --> 1019 [p1] --> USB3 back 4 | | [p2] --> USB3 back 3 | | [p3] --> USB3 RTL8153 ethernet | | [p4] --> USB2 CM6533 audio | | [p2] --> GL850S [p1] --> USB2 back 2 | | [p2] --> USB2 back 1 "keyboard/mouse" | | [p3] --> USB3 front 1 "power" | [p4] --> USB3 front 2 I have always kept keyboard and mouse on the USB2 ports connected to the Genesys Logic hub. They work there without any problem. With WoL enabled in the BIOS, there are only two wakeup sources: $ grep . /sys/bus/usb/devices/*/power/wakeup | grep enabled /sys/bus/usb/devices/1-6.2.2/power/wakeup:enabled # keyboard on GL850S /sys/bus/usb/devices/2-1.1.3/power/wakeup:enabled # net While suspended I can wake up the laptop with the keyboard and everything works. Moving the keyboard to one of the VL812 ports moves the wakeup source:: $ grep . /sys/bus/usb/devices/*/power/wakeup | grep enabled /sys/bus/usb/devices/1-6.1.1/power/wakeup:enabled # keyboard on the :1019 VL812 (USB2) /sys/bus/usb/devices/2-1.1.3/power/wakeup:enabled # net Now here's the interesting bit. Without "usbcore.autosuspend=-1" I can still wakeup the laptop from suspend, but after waking up the keyboard is gone: kernel: hub 1-6.1:1.0: hub_ext_port_status failed (err = -71) kernel: hub 1-6:1.0: hub_ext_port_status failed (err = -71) kernel: hub 1-6.1:1.0: hub_ext_port_status failed (err = -71) kernel: hub 1-6.1:1.0: hub_ext_port_status failed (err = -71) kernel: usb 1-6.1-port4: cannot disable (err = -71) kernel: usb 1-6.1-port1: cannot disable (err = -71) After that the port to which the keyboard is connected is essentially dead. Replugging doesn't help, I need to shutdown the laptop (powered via the dock) and cut all power to the dock to get that port back to operational status. With "usbcore.autosuspend=-1", everything behaves as it should. I met the exact same behaviour ("dead" port) with USB 3.0 devices too, although in my experience so far only with bus-powered devices. Sample kernel output: kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: usb 2-1.4: device not accepting address 6, error -62 kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: usb 2-1.4: device not accepting address 7, error -62 kernel: usb 2-1-port4: attempt power cycle kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: usb 2-1.4: device not accepting address 8, error -62 kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: xhci_hcd 0000:00:14.0: Timeout while waiting for setup device command kernel: usb 2-1.4: device not accepting address 9, error -62 kernel: usb 2-1-port4: unable to enumerate USB device I'll run some more tests out of curiosity to see how things behave in corner cases. Thanks! JF
On 31/08/2022 09.31, Greg KH wrote: > On Tue, Aug 30, 2022 at 09:50:03PM +0200, jflf_kernel@gmx.com wrote: >> >> On 30/08/2022 16.47, Oliver Neukum wrote: >> >>> 1) force a reset after a resume and call reset_resume() instead of resume() >>> 2) block autosuspend if remote wakeup is required >>> >>> I suspect you are actually using the second effect. Have you >>> tested with "usbcore.autosuspend=-1" on the kernel command line. >> >> After further testing, your suspicion is correct. >> >> TL;DR: the two VL812 hubs don't behave well when suspended. >> >> I'd like to prepare a better patch for that issue. What's the recommended strategy? The current patch works, even if only as a side effect and when there's a wakeup source downstream. It's currently in Greg KH's usb-linus branch, and will land in linux-next at some point. I'm tempted to let it be and undo it later in the better patch. Is that acceptable? Or should I ask Greg KH to pull it? > > I can revert it if you want me to, just let me know. > > thanks, > > greg k-h [keeping the lists in CC this time] Please revert if possible, and apologies for the trouble. Thanks! JF
On 31/08/2022 11.16, Oliver Neukum wrote: >> I'll run some more tests out of curiosity to see how things behave in corner cases. > > While you do so, which is a good idea, could you restate your current > results in a more precise way? We should have 4 results for each hub. > It is not the case that RESET_RESUME has no effect if you give > usbcore.autosuspend=-1 if the system has been suspended. Hence we have > the cases of > > no RESET_RESUME/default autosuspend > no RESET_RESUME/autosuspend=-1 > with RESET_RESUME/default autosuspend > with RESET_RESUME/autosuspend=-1 Hi Oliver, Apologies for my last unclear message. Both VL812 hubs behave the same way so I have a single table for both. The settings are applied to both hubs, on both USB2 and USB3. I have separated USB2 and USB3 as they behave slightly differently. ======================================================================================= Settings USB2 hotplug USB3 hotplug State after waking up --------------------------------------------------------------------------------------- power/control=auto works (1) fails broken (2) usbcore.autosuspend=-1 works works broken (2) OR power/control=on power/control=auto works (3) works (3) works and USB_QUIRK_RESET_RESUME power/control=on works works works and USB_QUIRK_RESET_RESUME HUB_QUIRK_DISABLE_AUTOSUSPEND works works works and USB_QUIRK_RESET_RESUME ======================================================================================= (1) I have yet to find a device that would be rejected by the USB2 side, while I have reliable reproducers for USB3. The problem appears with specific devices only, which otherwise work flawlessly on other hubs, other systems, etc. (2) After a system wake-up the hubs are in a seemingly random state. Numerous errors appear in the syslog, and some ports may be dead (changes from wake-up to wake-up). This can affect both internal (to dock built-in devices) and external hub ports. (3) With RESET_RESUME the hubs (both USB2 and USB3) stay active and don't autosuspend, even when power/control==auto. In all situations above a keyboard and the integrated ethernet controller can wake up the laptop from suspend. In situations (2) they may not work anymore after waking up. The bottom line is: we need USB_QUIRK_RESET_RESUME to deal with the spurious hub state after waking up, and we need to prevent the hubs from autosuspending to work around the USB3 issue. RESET_RESUME does all of that right now, but can its behaviour be considered stable? Is there any chance that some day it would no longer block autosuspend? If RESET_RESUME is stable, then my current patch is enough and it can be re-merged. If not, I have an updated patch ready with HUB_QUIRK_DISABLE_AUTOSUSPEND. Also, RESET_RESUME keeps the device nodes active, but not the hub interface nodes. HUB_QUIRK_DISABLE_AUTOSUSPEND and usbcore.autosuspend=-1 keep both of them active: $ grep active /sys/bus/usb/devices/2*/power/runtime_status # USB_QUIRK_RESET_RESUME /sys/bus/usb/devices/2-1.1/power/runtime_status:active /sys/bus/usb/devices/2-1/power/runtime_status:active $ grep active /sys/bus/usb/devices/2*/power/runtime_status # HUB_QUIRK_DISABLE_AUTOSUSPEND /sys/bus/usb/devices/2-1:1.0/power/runtime_status:active /sys/bus/usb/devices/2-1.1:1.0/power/runtime_status:active /sys/bus/usb/devices/2-1.1/power/runtime_status:active /sys/bus/usb/devices/2-1/power/runtime_status:active I do not know what impact this could have. In my tests both accepted the hotplugged USB3 devices, so keeping the device nodes active is enough. Thanks! JF
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index f99a65a64..999b7c969 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -437,6 +437,10 @@ static const struct usb_device_id usb_quirk_list[] = { { USB_DEVICE(0x1532, 0x0116), .driver_info = USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL }, + /* Lenovo ThinkPad OneLink+ Dock twin hub controllers (VIA Labs VL812) */ + { USB_DEVICE(0x17ef, 0x1018), .driver_info = USB_QUIRK_RESET_RESUME }, + { USB_DEVICE(0x17ef, 0x1019), .driver_info = USB_QUIRK_RESET_RESUME }, + /* Lenovo USB-C to Ethernet Adapter RTL8153-04 */ { USB_DEVICE(0x17ef, 0x720c), .driver_info = USB_QUIRK_NO_LPM },
The Lenovo OneLink+ Dock contains two VL812 USB3.0 controllers: 17ef:1018 upstream 17ef:1019 downstream Those two controllers both have problems with some USB3.0 devices, particularly self-powered ones. Typical error messages include: Timeout while waiting for setup device command device not accepting address X, error -62 unable to enumerate USB device By process of elimination the controllers themselves were identified as the cause of the problem. Through trial and error the issue was solved by using USB_QUIRK_RESET_RESUME for both chips. Signed-off-by: JFLF <jflf_kernel@gmx.com> --- drivers/usb/core/quirks.c | 4 ++++ 1 file changed, 4 insertions(+) -- 2.34.1