[PATCH 1/2] usb: hub: use shorter 120ms post resume hold for SS root hubs

[PATCH 1/2] usb: hub: use shorter 120ms post resume hold for SS root hubs

[Lovekesh Solanki]

Commit 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices
behind suspended hubs") holds a runtime PM reference for
USB_SS_PORT_U0_WAKE_TIME (200ms) after every SS hub resume, so
downstream links have time to finish trianing before the hub can
suspend again.

But 200ms is excessive for root hubs since a root hub has no upstream
suspended hub whose wake propagation needs to be accounted for.
The root hub still needs some time for USB3 link training to complete,
And root hubs also default to zero auto suspend delay and get
periodically resumed by userspace USB enumeration opening and
closing their usbfs nodes (adb, libusb).
This extra 200ms hold stretches this resume into a suspend/resume
cycle of roughly once per second, which on idle AMD systems have
been reported to cause system freezes.

Shorten the post resume hold to 120ms only for root hubs but
keep the 200ms for external hubs that the original commit is
intended for.

Reported-by: Mathieu Fluhr <mathieu.fluhr@gmail.com>
Link: https://lore.kernel.org/all/CAPyJwA_D9qw0T72f8zwM1yKjP+To=maVANbcdsWM7yRmbBxYvw@mail.gmail.com/
Cc: stable@vger.kernel.org
Fixes: 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")
Signed-off-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
---
 drivers/usb/core/hub.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3345b3298daf..e1415ad5f908 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -76,6 +76,12 @@
  */
 #define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
 
+/*
+ * Root hubs have no upstream hub whose wake propagation needs to be
+ * accounted for, but still needs time for USB3 link training to complete.
+ */
+#define USB_SS_ROOT_HUB_U0_WAKE_TIME 120 /* ms */
+
 /* Protect struct usb_device->state and ->children members
  * Note: Both are also protected by ->dev.sem, except that ->state can
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
@@ -1358,7 +1364,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 
 		queue_delayed_work(system_power_efficient_wq,
 				   &hub->post_resume_work,
-				   msecs_to_jiffies(USB_SS_PORT_U0_WAKE_TIME));
+				   msecs_to_jiffies(hdev->parent ?
+			USB_SS_PORT_U0_WAKE_TIME :
+			USB_SS_ROOT_HUB_U0_WAKE_TIME));
 		return;
 	}
 
-- 
2.55.0

[PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Lovekesh Solanki]

On auto resume (is_auto_resume), xhci_resume() used to msleep(120)
when there were suspended USB3 devices and no pending port event,
to give USB3 devices time to resend their U3 LFPS wake (at 100ms
intervals) if the first wake signalling failed.

The hub driver now holds the SS root hub awake for
USB_SS_ROOT_HUB_U0_WAKE_TIME (120ms) after a resume, which covers that
same window and lets the pending port event be picked up through the
normal hub event path. Therefore, this legacy wait in xhci_resume()
is redundant and can be removed.

Cc: stable@vger.kernel.org
Signed-off-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
---
 drivers/usb/host/xhci.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..b0caba8ebf18 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1083,7 +1083,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
 	struct xhci_segment	*seg;
 	int			retval = 0;
 	bool			pending_portevent = false;
-	bool			suspended_usb3_devs = false;
 	bool			reset_registers = false;
 
 	if (!hcd->state)
@@ -1245,18 +1244,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
 
 	/*
 	 * Resume roothubs only if there are pending events.
-	 * USB 3 devices resend U3 LFPS wake after a 100ms delay if
-	 * the first wake signalling failed, give it that chance if
-	 * there are suspended USB 3 devices.
 	 */
-	if (xhci->usb3_rhub.bus_state.suspended_ports || xhci->usb3_rhub.bus_state.bus_suspended)
-		suspended_usb3_devs = true;
-
 	pending_portevent = xhci_pending_portevent(xhci);
-	if (suspended_usb3_devs && !pending_portevent && is_auto_resume) {
-		msleep(120);
-		pending_portevent = xhci_pending_portevent(xhci);
-	}
 
 	if (pending_portevent) {
 		if (xhci->shared_hcd)
-- 
2.55.0

Re: [PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Mathias Nyman]

On 8/31/26 22:03, Lovekesh Solanki wrote:
> On auto resume (is_auto_resume), xhci_resume() used to msleep(120)
> when there were suspended USB3 devices and no pending port event,
> to give USB3 devices time to resend their U3 LFPS wake (at 100ms
> intervals) if the first wake signalling failed.
> 
> The hub driver now holds the SS root hub awake for
> USB_SS_ROOT_HUB_U0_WAKE_TIME (120ms) after a resume, which covers that
> same window and lets the pending port event be picked up through the
> normal hub event path. Therefore, this legacy wait in xhci_resume()
> is redundant and can be removed.

I looked into this again and remember now why we didn't remove the msleep(120)
earlier.

If we want the hub driver to keep us awake to detect delayed port activity,
then we need to wake the (root) hub driver in xhci resume.

That means always calling usb_hcd_resume_root_hub() in runtime resume cases where
resume is caused by usb device remote wake. (xHCI PME# in PCI case), even if there
isn't any port activity yet.

We don't currently check if runtime resume is caused by xHC PME# wake in the xhci-pci
driver. Not sure if there's a driver api for it.
Instead we in resume sleep for 120ms if there exists connected and suspended USB3
devices without port activity, and then check again.

This msleep(120) is unnecessary if runtime resume was caused by userspace instead
of usb device initiated wake (seen as PME for PCI hosts)

Probably best to just take Patch 1/1 and look at this xhci resume part separately.

Thanks
Mathias

Re: [PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Lovekesh Solanki]

On Tue, Sep 01, 2026 at 03:53:47PM +0300, Mathias Nyman wrote:
> I looked into this again and remember now why we didn't remove the msleep(120)
> earlier.
> 
> If we want the hub driver to keep us awake to detect delayed port activity,
> then we need to wake the (root) hub driver in xhci resume.
> 
> That means always calling usb_hcd_resume_root_hub() in runtime resume cases where
> resume is caused by usb device remote wake. (xHCI PME# in PCI case), even if there
> isn't any port activity yet.
Got it, thanks for the explanation.

> We don't currently check if runtime resume is caused by xHC PME# wake in the xhci-pci
> driver. Not sure if there's a driver api for it.
> Instead we in resume sleep for 120ms if there exists connected and suspended USB3
> devices without port activity, and then check again.
> 
> This msleep(120) is unnecessary if runtime resume was caused by userspace instead
> of usb device initiated wake (seen as PME for PCI hosts)
So the better implementation will cross into the PCI side, are there any
plans to implement that? Or any interest in it at all that you know of?

> Probably best to just take Patch 1/1 and look at this xhci resume part separately.
Do I need to resend the 1/2 Patch independently now that this one is
dropped? Or it'll be taken as is..? I'm still figuring the process out.

Thanks,
Lovekesh

Re: [PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Mathias Nyman]

On 9/1/26 18:50, Lovekesh Solanki wrote:
> On Tue, Sep 01, 2026 at 03:53:47PM +0300, Mathias Nyman wrote:
>> I looked into this again and remember now why we didn't remove the msleep(120)
>> earlier.
>>
>> If we want the hub driver to keep us awake to detect delayed port activity,
>> then we need to wake the (root) hub driver in xhci resume.
>>
>> That means always calling usb_hcd_resume_root_hub() in runtime resume cases where
>> resume is caused by usb device remote wake. (xHCI PME# in PCI case), even if there
>> isn't any port activity yet.
> Got it, thanks for the explanation.
> 
>> We don't currently check if runtime resume is caused by xHC PME# wake in the xhci-pci
>> driver. Not sure if there's a driver api for it.
>> Instead we in resume sleep for 120ms if there exists connected and suspended USB3
>> devices without port activity, and then check again.
>>
>> This msleep(120) is unnecessary if runtime resume was caused by userspace instead
>> of usb device initiated wake (seen as PME for PCI hosts)
> So the better implementation will cross into the PCI side, are there any
> plans to implement that? Or any interest in it at all that you know of?

It's possible PCI side or pm core already provides some easy way for xhci driver to check this.
This specific issue is currently not on my todo list, yet.

> 
>> Probably best to just take Patch 1/1 and look at this xhci resume part separately.
> Do I need to resend the 1/2 Patch independently now that this one is
> dropped? Or it'll be taken as is..? I'm still figuring the process out.

Maybe better to send it again, remember to include Greg k-h and Alan Stern

I have some minor, mostly personal taste based nitpicks that could be changed
in the patch. I'll reply with those comments directly to it.

Thanks

Mathias

Re: [PATCH 1/2] usb: hub: use shorter 120ms post resume hold for SS root hubs

[Mathias Nyman]

Hi

Some minor nitpicks/personal preferences if you are sending a v2.

I don't really object taking this as is either.

On 8/31/26 22:03, Lovekesh Solanki wrote:
> Commit 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices
> behind suspended hubs") holds a runtime PM reference for
> USB_SS_PORT_U0_WAKE_TIME (200ms) after every SS hub resume, so
> downstream links have time to finish trianing before the hub can
> suspend again.
> 
> But 200ms is excessive for root hubs since a root hub has no upstream
> suspended hub whose wake propagation needs to be accounted for.
> The root hub still needs some time for USB3 link training to complete,
> And root hubs also default to zero auto suspend delay and get
> periodically resumed by userspace USB enumeration opening and
> closing their usbfs nodes (adb, libusb).
> This extra 200ms hold stretches this resume into a suspend/resume
> cycle of roughly once per second, which on idle AMD systems have
> been reported to cause system freezes.

I'd highlight the fact that this patch mitigates the AMD platform issue.

Maybe begin commit message with something like

"Holding a USB3 hub PM runtime reference for 200ms at hub resume triggers a
  AMD platform issue. Users running Android adb report crashes after adb has
  been polling and waking up the USB subsystem once a second for some time.
  Vendors are working on a solution. Disabling USB runtime PM is one way to
  prevent this issue, but it is also proven that reducing the hold time to 120ms in
  resume also mitigates it. See Link for more details.

  Reducing the hold time to 120ms for the USB3 roothub is in itself a valid change
  and optimization as the current 200ms is excessive...
"

> 
> Shorten the post resume hold to 120ms only for root hubs but
> keep the 200ms for external hubs that the original commit is
> intended for.
> 
> Reported-by: Mathieu Fluhr <mathieu.fluhr@gmail.com>
> Link: https://lore.kernel.org/all/CAPyJwA_D9qw0T72f8zwM1yKjP+To=maVANbcdsWM7yRmbBxYvw@mail.gmail.com/
> Cc: stable@vger.kernel.org
> Fixes: 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")
> Signed-off-by: Lovekesh Solanki <lovekeshsolanki00@gmail.com>
> ---
>   drivers/usb/core/hub.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 3345b3298daf..e1415ad5f908 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -76,6 +76,12 @@
>    */
>   #define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
>   
> +/*
> + * Root hubs have no upstream hub whose wake propagation needs to be
> + * accounted for, but still needs time for USB3 link training to complete.
> + */
> +#define USB_SS_ROOT_HUB_U0_WAKE_TIME 120 /* ms */

I'd do minor changes to the original code comment above the first #define,
and just add the second define right after it

/*
  * modified original comment
  */
#define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
#define USB_SS_RH_PORT_U0_WAKE_TIME	120  /* ms */

Thanks
Mathias

Re: [PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Lovekesh Solanki]

On Wed, Sep 02, 2026 at 03:10:41PM +0300, Mathias Nyman wrote:
> It's possible PCI side or pm core already provides some easy way for xhci driver to check this.
> This specific issue is currently not on my todo list, yet.
Alright, would you like to me to work on this? I could look into
that side and possibly come up with an approach.

Thanks,
Lovekesh

Re: [PATCH 1/2] usb: hub: use shorter 120ms post resume hold for SS root hubs

[Lovekesh Solanki]

Thanks for the feedback,

On Wed, Sep 02, 2026 at 04:29:40PM +0300, Mathias Nyman wrote:
> I'd highlight the fact that this patch mitigates the AMD platform issue.
> 
> Maybe begin commit message with something like
> 
> "Holding a USB3 hub PM runtime reference for 200ms at hub resume triggers a
>  AMD platform issue. Users running Android adb report crashes after adb has
>  been polling and waking up the USB subsystem once a second for some time.
>  Vendors are working on a solution. Disabling USB runtime PM is one way to
>  prevent this issue, but it is also proven that reducing the hold time to 120ms in
>  resume also mitigates it. See Link for more details.
> 
>  Reducing the hold time to 120ms for the USB3 roothub is in itself a valid change
>  and optimization as the current 200ms is excessive...
> "
I will re work the commit message in v2 along those lines. But if the
mention of the commit 8f5b7e2bec1c is removed, should I keep the Fixes
tag or remove it as well?

> I'd do minor changes to the original code comment above the first #define,
> and just add the second define right after it
> 
> /*
>  * modified original comment
>  */
> #define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
> #define USB_SS_RH_PORT_U0_WAKE_TIME	120  /* ms */

I could change it to something like:

/*
 * Give SS hubs 200ms time after wake to train downstream links before
 * assuming no port activity and allowing hub to runtime suspend back.
 * Root hubs have no upstream hub whose wake propagation needs to be
 * accounted for, so they need less time, use 120ms for them.
 */
#define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
#define USB_SS_RH_PORT_U0_WAKE_TIME	120  /* ms */

If everything else's fine, I'll send a v2 with these changes.

Thanks,
Lovekesh

Re: [PATCH 2/2] xhci: drop 120ms U3 LFPS wake retry wait from runtime resume

[Mathias Nyman]

On 9/2/26 21:21, Lovekesh Solanki wrote:
> On Wed, Sep 02, 2026 at 03:10:41PM +0300, Mathias Nyman wrote:
>> It's possible PCI side or pm core already provides some easy way for xhci driver to check this.
>> This specific issue is currently not on my todo list, yet.
> Alright, would you like to me to work on this? I could look into
> that side and possibly come up with an approach.

Yes, please, that would be great.

Thanks
Mathias

Re: [PATCH 1/2] usb: hub: use shorter 120ms post resume hold for SS root hubs

[Mathias Nyman]

On 9/2/26 21:43, Lovekesh Solanki wrote:
> Thanks for the feedback,
> 
> On Wed, Sep 02, 2026 at 04:29:40PM +0300, Mathias Nyman wrote:
>> I'd highlight the fact that this patch mitigates the AMD platform issue.
>>
>> Maybe begin commit message with something like
>>
>> "Holding a USB3 hub PM runtime reference for 200ms at hub resume triggers a
>>   AMD platform issue. Users running Android adb report crashes after adb has
>>   been polling and waking up the USB subsystem once a second for some time.
>>   Vendors are working on a solution. Disabling USB runtime PM is one way to
>>   prevent this issue, but it is also proven that reducing the hold time to 120ms in
>>   resume also mitigates it. See Link for more details.
>>
>>   Reducing the hold time to 120ms for the USB3 roothub is in itself a valid change
>>   and optimization as the current 200ms is excessive...
>> "
> I will re work the commit message in v2 along those lines. But if the
> mention of the commit 8f5b7e2bec1c is removed, should I keep the Fixes
> tag or remove it as well?

Keep it.
It helps stable maintainers know how far back it makes sense to port this.

No need to remove the point about the commit message either.
It was just a suggestion to shuffle the content and highlight the real reason
why this patch is created.

> > I could change it to something like:
> 
> /*
>   * Give SS hubs 200ms time after wake to train downstream links before
>   * assuming no port activity and allowing hub to runtime suspend back.
>   * Root hubs have no upstream hub whose wake propagation needs to be
>   * accounted for, so they need less time, use 120ms for them.
>   */
> #define USB_SS_PORT_U0_WAKE_TIME	200  /* ms */
> #define USB_SS_RH_PORT_U0_WAKE_TIME	120  /* ms */
> 
> If everything else's fine, I'll send a v2 with these changes.

Looks good to me

Thanks
Mathias