Message-ID: <0ca08039-73fb-4c4b-ad10-15be8129d1b7@aosc.io>
Date: Fri, 7 Feb 2025 11:23:46 +0800
From: Mingcong Bai <jeffbai@...c.io>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Huacai Chen <chenhuacai@...nel.org>, Huacai Chen
 <chenhuacai@...ngson.cn>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org, Kexy Biscuit <kexybiscuit@...c.io>
Subject: Re: [PATCH] USB: core: Enable root_hub's remote wakeup for wakeup
 sources

Hi Alan,

在 2025/2/7 05:20, Alan Stern 写道:
> On Thu, Feb 06, 2025 at 11:49:49AM +0800, Mingcong Bai wrote:
>> On both unpatched and patched kernels, I have set power/control to "auto"
>> for both the root hub and the external hub and left the keyboard for 60
>> seconds. Regardless if I plug the keyboard into the chassis or the external
>> hub, the keyboard continues to work from the first key strike (no delay or
>> lost input).
> 
> It's not necessary to wait 60 seconds; 10 seconds would be enough.
> 
> For the case where the keyboard is plugged into the hub, it would be
> best if you removed the r8152 device (network or wifi, I guess).
> Leaving it plugged in will prevent the external hub from going into
> runtime suspend unless the network interface is turned off.
> 
> You can check whether these devices have gone into runtime suspend by
> looking at the contents of the .../power/runtime_status attribute
> file.  There are a couple of ways you can do this without disturbing the
> keyboard's status, such as by using ssh or by doing something like:
> 
> 	sleep 10 ; cat .../power/runtime_status
> 
> Or if you want to see the status of all your USB devices,
> 
> 	sleep 10 ; grep . /sys/bus/usb/devices/*/power/runtime_status
> 

Got it, thanks for the tip.

>>> This means there's something different between the way the keyboard
>>> sends its wakeup signal and the way the Genesys Logic hub sends its
>>> wakeup signal.
>>>
>>> Can you post the output from "lsusb -t" for this system?
>>
>> Keyboard plugged into the chassis:
>>
>> /:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>>      |__ Port 001: Dev 002, If 0, Class=Human Interface Device,
>> Driver=usbhid, 1.5M
>> /:  Bus 002.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
>> /:  Bus 003.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
>> /:  Bus 004.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 005.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 006.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 007.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 008.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>>
>> Keyboard plugged into the hub:
>>
>> /:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 002.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
>>      |__ Port 001: Dev 003, If 0, Class=Hub, Driver=hub/4p, 480M
>>          |__ Port 001: Dev 004, If 0, Class=Vendor Specific Class,
>> Driver=r8152, 480M
>>          |__ Port 004: Dev 005, If 0, Class=Human Interface Device,
>> Driver=usbhid, 1.5M
> 
> Ah, okay, there's an important difference.  The hub connects to an EHCI
> controller whereas the keyboard by itself connects to UHCI.
> 
> Also the output from "grep . /sys/bus/usb/devices/*/serial"?
> 
> And the contents of /sys/kernel/debug/usb/uhci/0000:00:1d.0?
> 

The enumerated USB tree changed (now to a controller at 1a.0) so I have 
reproduced a new set of outputs (also for the log you requested below).

`lsusb -t`:

/:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
/:  Bus 002.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
/:  Bus 003.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
     |__ Port 001: Dev 002, If 0, Class=Human Interface Device, 
Driver=usbhid, 1.5M
/:  Bus 004.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 005.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 006.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 007.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 008.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M

`grep . /sys/bus/usb/devices/*/serial`:

/sys/bus/usb/devices/usb1/serial:0000:00:1a.7
/sys/bus/usb/devices/usb2/serial:0000:00:1d.7
/sys/bus/usb/devices/usb3/serial:0000:00:1a.0
/sys/bus/usb/devices/usb4/serial:0000:00:1a.1
/sys/bus/usb/devices/usb5/serial:0000:00:1a.2
/sys/bus/usb/devices/usb6/serial:0000:00:1d.0
/sys/bus/usb/devices/usb7/serial:0000:00:1d.1
/sys/bus/usb/devices/usb8/serial:0000:00:1d.2

`cat /sys/kernel/debug/usb/uhci/0000:00:1a.0`:
Root-hub state: running   FSBR: 0
HC status
   usbcmd    =     00c1   Maxp64 CF RS
   usbstat   =     0000
   usbint    =     000f
   usbfrnum  =   (1)c14
   flbaseadd = 03147c14
   sof       =       40
   stat1     =     01a5   LowSpeed Enabled Connected
   stat2     =     0080
Most recent frame: 6fe6f (623)   Last ISO frame: 6fe6f (623)
Periodic load table
	0	0	0	0	118	0	0	0
	0	0	0	0	118	0	0	0
	0	0	0	0	118	0	0	0
	0	0	0	0	118	0	0	0
Total: 472, #INT: 1, #ISO: 0

>> /:  Bus 003.Port 001: Dev 001, Class=root_hub, Driver=ehci-pci/6p, 480M
>> /:  Bus 004.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 005.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 006.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 007.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>> /:  Bus 008.Port 001: Dev 001, Class=root_hub, Driver=uhci_hcd/2p, 12M
>>
>>>
>>> Also, can you enable dynamic debugging for usbcore by doing:
>>>
>>> 	echo module usbcore =p >/sys/kernel/debug/dynamic_debug/control
>>>
>>> and then post the dmesg log for a suspend/resume cycle?
>>
>> Keyboard plugged into the chassis (does not wake up via the external
>> keyboard, needed to strike Fn on the internal keyboard):
> 
> These logs are pretty much what I would expect, except for one thing:
> 
>> [  363.682633] ehci-pci 0000:00:1d.7: wakeup: 1
>> [  363.682714] uhci_hcd 0000:00:1d.2: wakeup: 1
>> [  363.682719] uhci_hcd 0000:00:1d.2: --> PCI D0
>> [  363.682757] uhci_hcd 0000:00:1d.1: wakeup: 1
>> [  363.682760] uhci_hcd 0000:00:1d.1: --> PCI D0
>> [  363.682796] uhci_hcd 0000:00:1d.0: wakeup: 1
>> [  363.682849] uhci_hcd 0000:00:1d.0: --> PCI D2
>> [  363.683087] ehci-pci 0000:00:1a.7: wakeup: 1
>> [  363.683153] uhci_hcd 0000:00:1a.2: wakeup: 1
>> [  363.683215] uhci_hcd 0000:00:1a.2: --> PCI D2
>> [  363.683254] uhci_hcd 0000:00:1a.1: wakeup: 1
>> [  363.683257] uhci_hcd 0000:00:1a.1: --> PCI D0
>> [  363.683293] uhci_hcd 0000:00:1a.0: wakeup: 1
>> [  363.683338] uhci_hcd 0000:00:1a.0: --> PCI D2
>> [  363.694561] ehci-pci 0000:00:1a.7: --> PCI D3hot
>> [  363.694597] ehci-pci 0000:00:1d.7: --> PCI D3hot
> 
> Why do the 1d.1, 1d.2, and 1a.1 UHCI controllers remain in D0 during
> suspend, whereas the 1d.0, 1a.0, and 1a.2 controllers get put in D2?
> That's odd.
> 
> Can you send the output from "lspci -vv -s 1d.0" and "lspci -vv -s 1d.1",
> running as root?  It may explain this behavior.
> 

`lspci -vv -s 1d.0`:

00:1a.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB UHCI 
Controller #4 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo ThinkPad T400
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 20
	Region 4: I/O ports at 1860 [size=32]
	Capabilities: [50] PCI Advanced Features
		AFCap: TP+ FLR+
		AFCtrl: FLR-
		AFStatus: TP-
	Kernel driver in use: uhci_hcd

`lspci -vv -s 1d.1`:

00:1a.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB UHCI 
Controller #5 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo ThinkPad T400
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin B routed to IRQ 21
	Region 4: I/O ports at 1880 [size=32]
	Capabilities: [50] PCI Advanced Features
		AFCap: TP+ FLR+
		AFCtrl: FLR-
		AFStatus: TP-
	Kernel driver in use: uhci_hcd

> Reading through the source code, I found a comment in the UHCI driver
> (drivers/usb/host/uhci-hcd.c, line 328) which is highly relevant:
> 
> 	/*
> 	 * UHCI doesn't distinguish between wakeup requests from downstream
> 	 * devices and local connect/disconnect events.  There's no way to
> 	 * enable one without the other; both are controlled by EGSM. Thus
> 	 * if wakeups are disallowed then EGSM must be turned off -- in which
> 	 * case remote wakeup requests from downstream during system sleep
> 	 * will be lost.
> 	 * ...
> 
> Most likely this explains what you are seeing.  In particular, it
> explains why the keyboard (when plugged directly into the computer)
> can't wake up the system unless wakeup is enabled on the root hub.  It
> even explains why wakeup from runtime suspend works, because wakeup is
> always enabled on all USB devices during runtime suspend regardless
> of the setting in .../power/wakeup.
> 
> (UHCI was the first USB host controller technology to be developed;
> the spec is from 1996.  It is very primitive compared to later
> controllers, in many ways.  Perhaps it shouldn't be surprising that
> UHCI controllers can't do all that we want them to.)
> 

This all makes sense. Since Huacai's patch was originally intended to 
fix Loongson's OHCI implementation, I was beginning to suspect if it 
exists for OHCI implementations found on older x86 platforms, say, AMD's 
SB600/700 series south bridges. Also to see if this issue is shared 
between OHCI and UHCI.

I have purchased a motherboard to test this and will report back as soon 
as I get my hands on it.

> Alan Stern
> 

Best Regards,
Mingcong Bai

Powered by blists - more mailing lists