* [PATCH] usb: hub: Resume hubs to find newly connected device
@ 2021-12-08 7:08 Kai-Heng Feng
2021-12-08 21:45 ` Alan Stern
0 siblings, 1 reply; 7+ messages in thread
From: Kai-Heng Feng @ 2021-12-08 7:08 UTC (permalink / raw)
To: gregkh
Cc: stern, mathias.nyman, Kai-Heng Feng, Thinh Nguyen, Bixuan Cui,
Andrew Lunn, Chris Chiu, Rajat Jain, linux-usb, linux-kernel
When a new USB device gets plugged to nested hubs, the affected hub,
which connects to usb 2-1.4-port2, doesn't report there's any change,
hence the nested hubs go back to runtime suspend like nothing happened:
[ 281.032951] usb usb2: usb wakeup-resume
[ 281.032959] usb usb2: usb auto-resume
[ 281.032974] hub 2-0:1.0: hub_resume
[ 281.033011] usb usb2-port1: status 0263 change 0000
[ 281.033077] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
[ 281.049797] usb 2-1: usb wakeup-resume
[ 281.069800] usb 2-1: Waited 0ms for CONNECT
[ 281.069810] usb 2-1: finish resume
[ 281.070026] hub 2-1:1.0: hub_resume
[ 281.070250] usb 2-1-port4: status 0203 change 0000
[ 281.070272] usb usb2-port1: resume, status 0
[ 281.070282] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
[ 281.089813] usb 2-1.4: usb wakeup-resume
[ 281.109792] usb 2-1.4: Waited 0ms for CONNECT
[ 281.109801] usb 2-1.4: finish resume
[ 281.109991] hub 2-1.4:1.0: hub_resume
[ 281.110147] usb 2-1.4-port2: status 0263 change 0000
[ 281.110234] usb 2-1-port4: resume, status 0
[ 281.110239] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
[ 281.110266] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
[ 281.110426] hub 2-1.4:1.0: hub_suspend
[ 281.110565] usb 2-1.4: usb auto-suspend, wakeup 1
[ 281.130998] hub 2-1:1.0: hub_suspend
[ 281.137788] usb 2-1: usb auto-suspend, wakeup 1
[ 281.142935] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
[ 281.177828] usb 2-1: usb wakeup-resume
[ 281.197839] usb 2-1: Waited 0ms for CONNECT
[ 281.197850] usb 2-1: finish resume
[ 281.197984] hub 2-1:1.0: hub_resume
[ 281.198203] usb 2-1-port4: status 0203 change 0000
[ 281.198228] usb usb2-port1: resume, status 0
[ 281.198237] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
[ 281.217835] usb 2-1.4: usb wakeup-resume
[ 281.237834] usb 2-1.4: Waited 0ms for CONNECT
[ 281.237845] usb 2-1.4: finish resume
[ 281.237990] hub 2-1.4:1.0: hub_resume
[ 281.238067] usb 2-1.4-port2: status 0263 change 0000
[ 281.238148] usb 2-1-port4: resume, status 0
[ 281.238152] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
[ 281.238166] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
[ 281.238385] hub 2-1.4:1.0: hub_suspend
[ 281.238523] usb 2-1.4: usb auto-suspend, wakeup 1
[ 281.258076] hub 2-1:1.0: hub_suspend
[ 281.265744] usb 2-1: usb auto-suspend, wakeup 1
[ 281.285976] hub 2-0:1.0: hub_suspend
[ 281.285988] usb usb2: bus auto-suspend, wakeup 1
So in addition to change event, wakes up the port if it's a hub to find
newly connected device:
[ 232.069881] usb usb2: usb wakeup-resume
[ 232.069889] usb usb2: usb auto-resume
[ 232.069904] hub 2-0:1.0: hub_resume
[ 232.069941] usb usb2-port1: status 0263 change 0000
[ 232.069962] hub 2-1:1.0: state 8 ports 4 chg 0000 evt 0000
[ 232.070034] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
[ 232.112701] usb 2-1: Waited 0ms for CONNECT
[ 232.112711] usb 2-1: finish resume
[ 232.112900] hub 2-1:1.0: hub_resume
[ 232.113218] usb 2-1-port4: status 0203 change 0000
[ 232.113267] hub 2-1.4:1.0: state 8 ports 4 chg 0000 evt 0000
[ 232.152679] usb 2-1.4: Waited 0ms for CONNECT
[ 232.152691] usb 2-1.4: finish resume
[ 232.152829] hub 2-1.4:1.0: hub_resume
[ 232.153057] usb 2-1.4-port2: status 0263 change 0000
[ 232.153094] hub 2-1.4.2:1.0: state 8 ports 3 chg 0000 evt 0000
[ 232.153155] usb 2-1-port4: resume, status 0
[ 232.153160] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
[ 232.153192] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
[ 232.153235] hub 2-1:1.0: state 7 ports 4 chg 0000 evt 0000
[ 232.153244] usb usb2-port1: resume, status 0
[ 232.153274] usb 2-1.4.2: usb auto-resume
[ 232.153444] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
[ 232.220690] usb 2-1.4.2: Waited 0ms for CONNECT
[ 232.220702] usb 2-1.4.2: finish resume
[ 232.220849] hub 2-1.4.2:1.0: hub_resume
[ 232.220870] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
[ 232.220949] usb 2-1.4.2-port2: status 0203 change 0001
[ 232.328747] usb 2-1.4.2-port2: status 0203, change 0000, 5.0 Gb/s
[ 232.408838] usb 2-1.4.2.2: new SuperSpeed USB device number 5 using xhci_hcd
[ 232.429734] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
[ 232.429745] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
[ 232.429827] usb 2-1.4.2.2: default language 0x0409
[ 232.430192] usb 2-1.4.2.2: udev 5, busnum 2, minor = 132
[ 232.430197] usb 2-1.4.2.2: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
[ 232.430202] usb 2-1.4.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 232.430206] usb 2-1.4.2.2: Product: Ultra
[ 232.430209] usb 2-1.4.2.2: Manufacturer: SanDisk
[ 232.430212] usb 2-1.4.2.2: SerialNumber: 4C530001170905105491
[ 232.430488] usb 2-1.4.2.2: usb_probe_device
[ 232.430493] usb 2-1.4.2.2: configuration #1 chosen from 1 choice
[ 232.431196] usb 2-1.4.2.2: Disabling U1 link state for device below second-tier hub.
[ 232.431199] usb 2-1.4.2.2: Plug device into first-tier hub to decrease power consumption.
[ 232.431469] usb 2-1.4.2.2: adding 2-1.4.2.2:1.0 (config #1, interface 0)
[ 232.431610] hub 2-1.4.2:1.0: state 7 ports 3 chg 0000 evt 0004
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/usb/core/hub.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 00070a8a65079..0c9442a8eab05 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1227,6 +1227,10 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
port_resumed))
set_bit(port1, hub->change_bits);
+ if ((portstatus & USB_PORT_STAT_CONNECTION) &&
+ usb_hub_to_struct_hub(udev))
+ usb_kick_hub_wq(udev);
+
} else if (udev->persist_enabled) {
#ifdef CONFIG_PM
udev->reset_resume = 1;
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-08 7:08 [PATCH] usb: hub: Resume hubs to find newly connected device Kai-Heng Feng
@ 2021-12-08 21:45 ` Alan Stern
2021-12-09 1:19 ` Kai-Heng Feng
0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2021-12-08 21:45 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> When a new USB device gets plugged to nested hubs, the affected hub,
> which connects to usb 2-1.4-port2, doesn't report there's any change,
> hence the nested hubs go back to runtime suspend like nothing happened:
That's a bug in the hub. When there's a change in the connection status
of one of its ports, it should report this change to the kernel.
> [ 281.032951] usb usb2: usb wakeup-resume
> [ 281.032959] usb usb2: usb auto-resume
> [ 281.032974] hub 2-0:1.0: hub_resume
> [ 281.033011] usb usb2-port1: status 0263 change 0000
> [ 281.033077] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 281.049797] usb 2-1: usb wakeup-resume
> [ 281.069800] usb 2-1: Waited 0ms for CONNECT
> [ 281.069810] usb 2-1: finish resume
> [ 281.070026] hub 2-1:1.0: hub_resume
> [ 281.070250] usb 2-1-port4: status 0203 change 0000
> [ 281.070272] usb usb2-port1: resume, status 0
> [ 281.070282] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
> [ 281.089813] usb 2-1.4: usb wakeup-resume
> [ 281.109792] usb 2-1.4: Waited 0ms for CONNECT
> [ 281.109801] usb 2-1.4: finish resume
> [ 281.109991] hub 2-1.4:1.0: hub_resume
> [ 281.110147] usb 2-1.4-port2: status 0263 change 0000
> [ 281.110234] usb 2-1-port4: resume, status 0
> [ 281.110239] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> [ 281.110266] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 281.110426] hub 2-1.4:1.0: hub_suspend
> [ 281.110565] usb 2-1.4: usb auto-suspend, wakeup 1
> [ 281.130998] hub 2-1:1.0: hub_suspend
> [ 281.137788] usb 2-1: usb auto-suspend, wakeup 1
> [ 281.142935] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 281.177828] usb 2-1: usb wakeup-resume
> [ 281.197839] usb 2-1: Waited 0ms for CONNECT
> [ 281.197850] usb 2-1: finish resume
> [ 281.197984] hub 2-1:1.0: hub_resume
> [ 281.198203] usb 2-1-port4: status 0203 change 0000
> [ 281.198228] usb usb2-port1: resume, status 0
> [ 281.198237] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
> [ 281.217835] usb 2-1.4: usb wakeup-resume
> [ 281.237834] usb 2-1.4: Waited 0ms for CONNECT
> [ 281.237845] usb 2-1.4: finish resume
> [ 281.237990] hub 2-1.4:1.0: hub_resume
> [ 281.238067] usb 2-1.4-port2: status 0263 change 0000
> [ 281.238148] usb 2-1-port4: resume, status 0
> [ 281.238152] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> [ 281.238166] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 281.238385] hub 2-1.4:1.0: hub_suspend
> [ 281.238523] usb 2-1.4: usb auto-suspend, wakeup 1
> [ 281.258076] hub 2-1:1.0: hub_suspend
> [ 281.265744] usb 2-1: usb auto-suspend, wakeup 1
> [ 281.285976] hub 2-0:1.0: hub_suspend
> [ 281.285988] usb usb2: bus auto-suspend, wakeup 1
>
> So in addition to change event, wakes up the port if it's a hub to find
> newly connected device:
> [ 232.069881] usb usb2: usb wakeup-resume
> [ 232.069889] usb usb2: usb auto-resume
> [ 232.069904] hub 2-0:1.0: hub_resume
> [ 232.069941] usb usb2-port1: status 0263 change 0000
> [ 232.069962] hub 2-1:1.0: state 8 ports 4 chg 0000 evt 0000
> [ 232.070034] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 232.112701] usb 2-1: Waited 0ms for CONNECT
> [ 232.112711] usb 2-1: finish resume
> [ 232.112900] hub 2-1:1.0: hub_resume
> [ 232.113218] usb 2-1-port4: status 0203 change 0000
> [ 232.113267] hub 2-1.4:1.0: state 8 ports 4 chg 0000 evt 0000
> [ 232.152679] usb 2-1.4: Waited 0ms for CONNECT
> [ 232.152691] usb 2-1.4: finish resume
> [ 232.152829] hub 2-1.4:1.0: hub_resume
> [ 232.153057] usb 2-1.4-port2: status 0263 change 0000
> [ 232.153094] hub 2-1.4.2:1.0: state 8 ports 3 chg 0000 evt 0000
> [ 232.153155] usb 2-1-port4: resume, status 0
> [ 232.153160] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> [ 232.153192] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 232.153235] hub 2-1:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 232.153244] usb usb2-port1: resume, status 0
> [ 232.153274] usb 2-1.4.2: usb auto-resume
> [ 232.153444] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 232.220690] usb 2-1.4.2: Waited 0ms for CONNECT
> [ 232.220702] usb 2-1.4.2: finish resume
> [ 232.220849] hub 2-1.4.2:1.0: hub_resume
> [ 232.220870] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> [ 232.220949] usb 2-1.4.2-port2: status 0203 change 0001
> [ 232.328747] usb 2-1.4.2-port2: status 0203, change 0000, 5.0 Gb/s
> [ 232.408838] usb 2-1.4.2.2: new SuperSpeed USB device number 5 using xhci_hcd
> [ 232.429734] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
> [ 232.429745] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
> [ 232.429827] usb 2-1.4.2.2: default language 0x0409
> [ 232.430192] usb 2-1.4.2.2: udev 5, busnum 2, minor = 132
> [ 232.430197] usb 2-1.4.2.2: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> [ 232.430202] usb 2-1.4.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 232.430206] usb 2-1.4.2.2: Product: Ultra
> [ 232.430209] usb 2-1.4.2.2: Manufacturer: SanDisk
> [ 232.430212] usb 2-1.4.2.2: SerialNumber: 4C530001170905105491
> [ 232.430488] usb 2-1.4.2.2: usb_probe_device
> [ 232.430493] usb 2-1.4.2.2: configuration #1 chosen from 1 choice
> [ 232.431196] usb 2-1.4.2.2: Disabling U1 link state for device below second-tier hub.
> [ 232.431199] usb 2-1.4.2.2: Plug device into first-tier hub to decrease power consumption.
> [ 232.431469] usb 2-1.4.2.2: adding 2-1.4.2.2:1.0 (config #1, interface 0)
> [ 232.431610] hub 2-1.4.2:1.0: state 7 ports 3 chg 0000 evt 0004
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
So because of this buggy hub, you now want to wake up _every_ hub in the
system whenever any wakeup event occurs? Is this really a good idea?
Is there a better way to solve the problem, such as a special quirk
flag?
Alan Stern
> ---
> drivers/usb/core/hub.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 00070a8a65079..0c9442a8eab05 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1227,6 +1227,10 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
> port_resumed))
> set_bit(port1, hub->change_bits);
>
> + if ((portstatus & USB_PORT_STAT_CONNECTION) &&
> + usb_hub_to_struct_hub(udev))
> + usb_kick_hub_wq(udev);
> +
> } else if (udev->persist_enabled) {
> #ifdef CONFIG_PM
> udev->reset_resume = 1;
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-08 21:45 ` Alan Stern
@ 2021-12-09 1:19 ` Kai-Heng Feng
2021-12-09 16:04 ` Alan Stern
0 siblings, 1 reply; 7+ messages in thread
From: Kai-Heng Feng @ 2021-12-09 1:19 UTC (permalink / raw)
To: Alan Stern
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Thu, Dec 9, 2021 at 5:45 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> > When a new USB device gets plugged to nested hubs, the affected hub,
> > which connects to usb 2-1.4-port2, doesn't report there's any change,
> > hence the nested hubs go back to runtime suspend like nothing happened:
>
> That's a bug in the hub. When there's a change in the connection status
> of one of its ports, it should report this change to the kernel.
I think it should, but when I searched through the USB spec and I
can't find anywhere specify hub requires to report it in change
status.
>
> > [ 281.032951] usb usb2: usb wakeup-resume
> > [ 281.032959] usb usb2: usb auto-resume
> > [ 281.032974] hub 2-0:1.0: hub_resume
> > [ 281.033011] usb usb2-port1: status 0263 change 0000
> > [ 281.033077] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 281.049797] usb 2-1: usb wakeup-resume
> > [ 281.069800] usb 2-1: Waited 0ms for CONNECT
> > [ 281.069810] usb 2-1: finish resume
> > [ 281.070026] hub 2-1:1.0: hub_resume
> > [ 281.070250] usb 2-1-port4: status 0203 change 0000
> > [ 281.070272] usb usb2-port1: resume, status 0
> > [ 281.070282] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
> > [ 281.089813] usb 2-1.4: usb wakeup-resume
> > [ 281.109792] usb 2-1.4: Waited 0ms for CONNECT
> > [ 281.109801] usb 2-1.4: finish resume
> > [ 281.109991] hub 2-1.4:1.0: hub_resume
> > [ 281.110147] usb 2-1.4-port2: status 0263 change 0000
> > [ 281.110234] usb 2-1-port4: resume, status 0
> > [ 281.110239] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> > [ 281.110266] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 281.110426] hub 2-1.4:1.0: hub_suspend
> > [ 281.110565] usb 2-1.4: usb auto-suspend, wakeup 1
> > [ 281.130998] hub 2-1:1.0: hub_suspend
> > [ 281.137788] usb 2-1: usb auto-suspend, wakeup 1
> > [ 281.142935] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 281.177828] usb 2-1: usb wakeup-resume
> > [ 281.197839] usb 2-1: Waited 0ms for CONNECT
> > [ 281.197850] usb 2-1: finish resume
> > [ 281.197984] hub 2-1:1.0: hub_resume
> > [ 281.198203] usb 2-1-port4: status 0203 change 0000
> > [ 281.198228] usb usb2-port1: resume, status 0
> > [ 281.198237] hub 2-1:1.0: state 7 ports 4 chg 0010 evt 0000
> > [ 281.217835] usb 2-1.4: usb wakeup-resume
> > [ 281.237834] usb 2-1.4: Waited 0ms for CONNECT
> > [ 281.237845] usb 2-1.4: finish resume
> > [ 281.237990] hub 2-1.4:1.0: hub_resume
> > [ 281.238067] usb 2-1.4-port2: status 0263 change 0000
> > [ 281.238148] usb 2-1-port4: resume, status 0
> > [ 281.238152] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> > [ 281.238166] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 281.238385] hub 2-1.4:1.0: hub_suspend
> > [ 281.238523] usb 2-1.4: usb auto-suspend, wakeup 1
> > [ 281.258076] hub 2-1:1.0: hub_suspend
> > [ 281.265744] usb 2-1: usb auto-suspend, wakeup 1
> > [ 281.285976] hub 2-0:1.0: hub_suspend
> > [ 281.285988] usb usb2: bus auto-suspend, wakeup 1
> >
> > So in addition to change event, wakes up the port if it's a hub to find
> > newly connected device:
> > [ 232.069881] usb usb2: usb wakeup-resume
> > [ 232.069889] usb usb2: usb auto-resume
> > [ 232.069904] hub 2-0:1.0: hub_resume
> > [ 232.069941] usb usb2-port1: status 0263 change 0000
> > [ 232.069962] hub 2-1:1.0: state 8 ports 4 chg 0000 evt 0000
> > [ 232.070034] hub 2-0:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 232.112701] usb 2-1: Waited 0ms for CONNECT
> > [ 232.112711] usb 2-1: finish resume
> > [ 232.112900] hub 2-1:1.0: hub_resume
> > [ 232.113218] usb 2-1-port4: status 0203 change 0000
> > [ 232.113267] hub 2-1.4:1.0: state 8 ports 4 chg 0000 evt 0000
> > [ 232.152679] usb 2-1.4: Waited 0ms for CONNECT
> > [ 232.152691] usb 2-1.4: finish resume
> > [ 232.152829] hub 2-1.4:1.0: hub_resume
> > [ 232.153057] usb 2-1.4-port2: status 0263 change 0000
> > [ 232.153094] hub 2-1.4.2:1.0: state 8 ports 3 chg 0000 evt 0000
> > [ 232.153155] usb 2-1-port4: resume, status 0
> > [ 232.153160] usb 2-1-port4: status 0203, change 0000, 10.0 Gb/s
> > [ 232.153192] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 232.153235] hub 2-1:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 232.153244] usb usb2-port1: resume, status 0
> > [ 232.153274] usb 2-1.4.2: usb auto-resume
> > [ 232.153444] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 232.220690] usb 2-1.4.2: Waited 0ms for CONNECT
> > [ 232.220702] usb 2-1.4.2: finish resume
> > [ 232.220849] hub 2-1.4.2:1.0: hub_resume
> > [ 232.220870] hub 2-1.4:1.0: state 7 ports 4 chg 0000 evt 0000
> > [ 232.220949] usb 2-1.4.2-port2: status 0203 change 0001
> > [ 232.328747] usb 2-1.4.2-port2: status 0203, change 0000, 5.0 Gb/s
> > [ 232.408838] usb 2-1.4.2.2: new SuperSpeed USB device number 5 using xhci_hcd
> > [ 232.429734] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
> > [ 232.429745] usb 2-1.4.2.2: skipped 1 descriptor after endpoint
> > [ 232.429827] usb 2-1.4.2.2: default language 0x0409
> > [ 232.430192] usb 2-1.4.2.2: udev 5, busnum 2, minor = 132
> > [ 232.430197] usb 2-1.4.2.2: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> > [ 232.430202] usb 2-1.4.2.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > [ 232.430206] usb 2-1.4.2.2: Product: Ultra
> > [ 232.430209] usb 2-1.4.2.2: Manufacturer: SanDisk
> > [ 232.430212] usb 2-1.4.2.2: SerialNumber: 4C530001170905105491
> > [ 232.430488] usb 2-1.4.2.2: usb_probe_device
> > [ 232.430493] usb 2-1.4.2.2: configuration #1 chosen from 1 choice
> > [ 232.431196] usb 2-1.4.2.2: Disabling U1 link state for device below second-tier hub.
> > [ 232.431199] usb 2-1.4.2.2: Plug device into first-tier hub to decrease power consumption.
> > [ 232.431469] usb 2-1.4.2.2: adding 2-1.4.2.2:1.0 (config #1, interface 0)
> > [ 232.431610] hub 2-1.4.2:1.0: state 7 ports 3 chg 0000 evt 0004
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> So because of this buggy hub, you now want to wake up _every_ hub in the
> system whenever any wakeup event occurs? Is this really a good idea?
> Is there a better way to solve the problem, such as a special quirk
> flag?
If there's no other activities, the USB hub should go back to suspend
immediately, so the impact is minimal.
I've seen several similar bug reports so I think this solution should
be applied for all hubs.
Kai-Heng
>
> Alan Stern
>
> > ---
> > drivers/usb/core/hub.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 00070a8a65079..0c9442a8eab05 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -1227,6 +1227,10 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
> > port_resumed))
> > set_bit(port1, hub->change_bits);
> >
> > + if ((portstatus & USB_PORT_STAT_CONNECTION) &&
> > + usb_hub_to_struct_hub(udev))
> > + usb_kick_hub_wq(udev);
> > +
> > } else if (udev->persist_enabled) {
> > #ifdef CONFIG_PM
> > udev->reset_resume = 1;
> > --
> > 2.32.0
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-09 1:19 ` Kai-Heng Feng
@ 2021-12-09 16:04 ` Alan Stern
2021-12-10 6:06 ` Kai-Heng Feng
0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2021-12-09 16:04 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Thu, Dec 09, 2021 at 09:19:24AM +0800, Kai-Heng Feng wrote:
> On Thu, Dec 9, 2021 at 5:45 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> > > When a new USB device gets plugged to nested hubs, the affected hub,
> > > which connects to usb 2-1.4-port2, doesn't report there's any change,
> > > hence the nested hubs go back to runtime suspend like nothing happened:
> >
> > That's a bug in the hub. When there's a change in the connection status
> > of one of its ports, it should report this change to the kernel.
>
> I think it should, but when I searched through the USB spec and I
> can't find anywhere specify hub requires to report it in change
> status.
USB-2.0 spec, section 11.24.2.7.2.1 (C_PORT_CONNECTION):
This bit is set when the PORT_CONNECTION bit changes because of an
attach or detach detect event (see Section 7.1.7.3). This bit will be
cleared to zero by a ClearPortFeature(C_PORT_CONNECTION) request or
while the port is in the Powered-off state.
> > So because of this buggy hub, you now want to wake up _every_ hub in the
> > system whenever any wakeup event occurs? Is this really a good idea?
> > Is there a better way to solve the problem, such as a special quirk
> > flag?
>
> If there's no other activities, the USB hub should go back to suspend
> immediately, so the impact is minimal.
Not immediately, but after a few seconds. However your patch will affect every
hub, not just the one that the new device was plugged into.
> I've seen several similar bug reports so I think this solution should
> be applied for all hubs.
Maybe those bug reports all had something in common, such as the type of hub or
the bus speed they were running at. Did you check?
Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-09 16:04 ` Alan Stern
@ 2021-12-10 6:06 ` Kai-Heng Feng
2021-12-10 16:27 ` Alan Stern
0 siblings, 1 reply; 7+ messages in thread
From: Kai-Heng Feng @ 2021-12-10 6:06 UTC (permalink / raw)
To: Alan Stern
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Fri, Dec 10, 2021 at 12:04 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, Dec 09, 2021 at 09:19:24AM +0800, Kai-Heng Feng wrote:
> > On Thu, Dec 9, 2021 at 5:45 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> > > > When a new USB device gets plugged to nested hubs, the affected hub,
> > > > which connects to usb 2-1.4-port2, doesn't report there's any change,
> > > > hence the nested hubs go back to runtime suspend like nothing happened:
> > >
> > > That's a bug in the hub. When there's a change in the connection status
> > > of one of its ports, it should report this change to the kernel.
> >
> > I think it should, but when I searched through the USB spec and I
> > can't find anywhere specify hub requires to report it in change
> > status.
>
> USB-2.0 spec, section 11.24.2.7.2.1 (C_PORT_CONNECTION):
>
> This bit is set when the PORT_CONNECTION bit changes because of an
> attach or detach detect event (see Section 7.1.7.3). This bit will be
> cleared to zero by a ClearPortFeature(C_PORT_CONNECTION) request or
> while the port is in the Powered-off state.
It's indeed set for the hub's downstream facing port, and that's why
wake up the hub and check its ports can still find connect event.
But I can't find anywhere stats how hub's upstream facing port should be set.
>
> > > So because of this buggy hub, you now want to wake up _every_ hub in the
> > > system whenever any wakeup event occurs? Is this really a good idea?
> > > Is there a better way to solve the problem, such as a special quirk
> > > flag?
> >
> > If there's no other activities, the USB hub should go back to suspend
> > immediately, so the impact is minimal.
>
> Not immediately, but after a few seconds. However your patch will affect every
> hub, not just the one that the new device was plugged into.
Yes, that's the case here.
>
> > I've seen several similar bug reports so I think this solution should
> > be applied for all hubs.
>
> Maybe those bug reports all had something in common, such as the type of hub or
> the bus speed they were running at. Did you check?
The hub in question is a SuperSpeed hub. Let me scan bug reports on
Launchpad and see what I can find.
Kai-Heng
>
> Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-10 6:06 ` Kai-Heng Feng
@ 2021-12-10 16:27 ` Alan Stern
2021-12-14 5:53 ` Kai-Heng Feng
0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2021-12-10 16:27 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Fri, Dec 10, 2021 at 02:06:10PM +0800, Kai-Heng Feng wrote:
> On Fri, Dec 10, 2021 at 12:04 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Thu, Dec 09, 2021 at 09:19:24AM +0800, Kai-Heng Feng wrote:
> > > On Thu, Dec 9, 2021 at 5:45 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > > >
> > > > On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> > > > > When a new USB device gets plugged to nested hubs, the affected hub,
> > > > > which connects to usb 2-1.4-port2, doesn't report there's any change,
> > > > > hence the nested hubs go back to runtime suspend like nothing happened:
> > > >
> > > > That's a bug in the hub. When there's a change in the connection status
> > > > of one of its ports, it should report this change to the kernel.
> > >
> > > I think it should, but when I searched through the USB spec and I
> > > can't find anywhere specify hub requires to report it in change
> > > status.
> >
> > USB-2.0 spec, section 11.24.2.7.2.1 (C_PORT_CONNECTION):
> >
> > This bit is set when the PORT_CONNECTION bit changes because of an
> > attach or detach detect event (see Section 7.1.7.3). This bit will be
> > cleared to zero by a ClearPortFeature(C_PORT_CONNECTION) request or
> > while the port is in the Powered-off state.
>
> It's indeed set for the hub's downstream facing port, and that's why
> wake up the hub and check its ports can still find connect event.
> But I can't find anywhere stats how hub's upstream facing port should be set.
It looks like the port status-change bits don't get set in response to a
wakeup signal, for SuperSpeed links. Section C.1.2.3 in the USB-3.1
spec says:
Note that C_PORT_LINK_STATE is not asserted in the event of a
remote wakeup. As discussed previously, in the event of a
Remote Wakeup the associated function sends the host a Function
Wake device notification packet.
I don't know if we receive those Function Wake notification packets, or
what we do with them.
In any case, section C.1.4.5 says that during remote wakeup, all of the
links from the remote wakeup device up to the controlling hub transition
to U0. But your log extract showed:
[ 281.110147] usb 2-1.4-port2: status 0263 change 0000
So even though the 2-1.4.2 hub originated a wakeup signal, the upstream
link to the 2-1.4 hub remained in U3 according to these status bits.
Could it be that we need to include an extra delay, so the link has
enough time to get into the U0 state?
Maybe Mathias can help investigate this issue.
Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: hub: Resume hubs to find newly connected device
2021-12-10 16:27 ` Alan Stern
@ 2021-12-14 5:53 ` Kai-Heng Feng
0 siblings, 0 replies; 7+ messages in thread
From: Kai-Heng Feng @ 2021-12-14 5:53 UTC (permalink / raw)
To: Alan Stern
Cc: gregkh, mathias.nyman, Thinh Nguyen, Bixuan Cui, Andrew Lunn,
Chris Chiu, Rajat Jain, linux-usb, linux-kernel
On Sat, Dec 11, 2021 at 12:27 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Fri, Dec 10, 2021 at 02:06:10PM +0800, Kai-Heng Feng wrote:
> > On Fri, Dec 10, 2021 at 12:04 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Thu, Dec 09, 2021 at 09:19:24AM +0800, Kai-Heng Feng wrote:
> > > > On Thu, Dec 9, 2021 at 5:45 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > > > >
> > > > > On Wed, Dec 08, 2021 at 03:08:33PM +0800, Kai-Heng Feng wrote:
> > > > > > When a new USB device gets plugged to nested hubs, the affected hub,
> > > > > > which connects to usb 2-1.4-port2, doesn't report there's any change,
> > > > > > hence the nested hubs go back to runtime suspend like nothing happened:
> > > > >
> > > > > That's a bug in the hub. When there's a change in the connection status
> > > > > of one of its ports, it should report this change to the kernel.
> > > >
> > > > I think it should, but when I searched through the USB spec and I
> > > > can't find anywhere specify hub requires to report it in change
> > > > status.
> > >
> > > USB-2.0 spec, section 11.24.2.7.2.1 (C_PORT_CONNECTION):
> > >
> > > This bit is set when the PORT_CONNECTION bit changes because of an
> > > attach or detach detect event (see Section 7.1.7.3). This bit will be
> > > cleared to zero by a ClearPortFeature(C_PORT_CONNECTION) request or
> > > while the port is in the Powered-off state.
> >
> > It's indeed set for the hub's downstream facing port, and that's why
> > wake up the hub and check its ports can still find connect event.
> > But I can't find anywhere stats how hub's upstream facing port should be set.
>
> It looks like the port status-change bits don't get set in response to a
> wakeup signal, for SuperSpeed links. Section C.1.2.3 in the USB-3.1
> spec says:
>
> Note that C_PORT_LINK_STATE is not asserted in the event of a
> remote wakeup. As discussed previously, in the event of a
> Remote Wakeup the associated function sends the host a Function
> Wake device notification packet.
>
> I don't know if we receive those Function Wake notification packets, or
> what we do with them.
>
> In any case, section C.1.4.5 says that during remote wakeup, all of the
> links from the remote wakeup device up to the controlling hub transition
> to U0. But your log extract showed:
>
> [ 281.110147] usb 2-1.4-port2: status 0263 change 0000
>
> So even though the 2-1.4.2 hub originated a wakeup signal, the upstream
> link to the 2-1.4 hub remained in U3 according to these status bits.
> Could it be that we need to include an extra delay, so the link has
> enough time to get into the U0 state?
You are right, adding delay works and it can get to U0.
This only happens to USB3 devices, the hub has no problem detecting
USB2 devices.
Let me prepare v2 for this.
Kai-Heng
>
> Maybe Mathias can help investigate this issue.
>
> Alan Stern
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-14 5:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 7:08 [PATCH] usb: hub: Resume hubs to find newly connected device Kai-Heng Feng
2021-12-08 21:45 ` Alan Stern
2021-12-09 1:19 ` Kai-Heng Feng
2021-12-09 16:04 ` Alan Stern
2021-12-10 6:06 ` Kai-Heng Feng
2021-12-10 16:27 ` Alan Stern
2021-12-14 5:53 ` Kai-Heng Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).