[RFT PATCH v2 0/1] Fix "detection of high tier USB3 devices" patch in usb-linus

[RFT PATCH v2 0/1] Fix "detection of high tier USB3 devices" patch in usb-linus

[Mathias Nyman]

Hi

This the second attempt on a fixup patch for
8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")
that is currently in Greg's usb-linus branch.

This one would need some additional testing and Ack that it works on
Raspberry Pi 3B+ and QC SC8280XP CRD board before pushing it forward

Thanks
Mathias

Mathias Nyman (1):
  usb: hub: Fix flushing and scheduling of delayed work tuning runtime
    pm

 drivers/usb/core/hub.c | 23 ++++++++++-------------
 drivers/usb/core/hub.h |  1 +
 2 files changed, 11 insertions(+), 13 deletions(-)

-- 
2.43.0

[RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mathias Nyman]

Delayed work to prevent USB3 hubs from runtime-suspending immediately
after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
of high tier USB3 devices behind suspended hubs").

This delayed work needs be flushed if system suspends, or hub needs to
be quiesced for other reasons right after resume. Not flushing it
triggered issues on QC SC8280XP CRD board during suspend/resume testing.

The same hub->init_work delayed work item is used for several purposes,
and simply fushing it in hub_quiesce() causes other issues, so fix
this by creating a dedicated work item for post resume work, and flush
that in hub_quiesce()

The delayed work item that allow hub runtime suspend is also scheduled
just before calling autopm get. Alan pointed out there is a small risk
that work is run before autopm get, which would call autopm put before
get, and mess up the runtime pm usage order.
Swap the order of work sheduling and calling autopm get to solve this.

Cc: stable@kernel.org
Fixes: 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")
Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Closes: https://lore.kernel.org/linux-usb/acaaa928-832c-48ca-b0ea-d202d5cd3d6c@oss.qualcomm.com
Reported-by: Alan Stern <stern@rowland.harvard.edu>
Closes: https://lore.kernel.org/linux-usb/c73fbead-66d7-497a-8fa1-75ea4761090a@rowland.harvard.edu
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/linux-usb/aF5rNp1l0LWITnEB@finisterre.sirena.org.uk
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 v2:
 - Add and use dedicated delayed work struct for post resume work
 - Add commit message section about dedicated work

 drivers/usb/core/hub.c | 23 ++++++++++-------------
 drivers/usb/core/hub.h |  1 +
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f981e365be36..256fe8c86828 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1074,12 +1074,11 @@ int usb_remove_device(struct usb_device *udev)
 
 enum hub_activation_type {
 	HUB_INIT, HUB_INIT2, HUB_INIT3,		/* INITs must come first */
-	HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME, HUB_POST_RESUME,
+	HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME,
 };
 
 static void hub_init_func2(struct work_struct *ws);
 static void hub_init_func3(struct work_struct *ws);
-static void hub_post_resume(struct work_struct *ws);
 
 static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 {
@@ -1103,12 +1102,6 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 		goto init3;
 	}
 
-	if (type == HUB_POST_RESUME) {
-		usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
-		hub_put(hub);
-		return;
-	}
-
 	hub_get(hub);
 
 	/* The superspeed hub except for root hub has to use Hub Depth
@@ -1359,11 +1352,12 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 
 	if (type == HUB_RESUME && hub_is_superspeed(hub->hdev)) {
 		/* give usb3 downstream links training time after hub resume */
-		INIT_DELAYED_WORK(&hub->init_work, hub_post_resume);
-		queue_delayed_work(system_power_efficient_wq, &hub->init_work,
-				   msecs_to_jiffies(USB_SS_PORT_U0_WAKE_TIME));
 		usb_autopm_get_interface_no_resume(
 			to_usb_interface(hub->intfdev));
+
+		queue_delayed_work(system_power_efficient_wq,
+				   &hub->post_resume_work,
+				   msecs_to_jiffies(USB_SS_PORT_U0_WAKE_TIME));
 		return;
 	}
 
@@ -1387,9 +1381,10 @@ static void hub_init_func3(struct work_struct *ws)
 
 static void hub_post_resume(struct work_struct *ws)
 {
-	struct usb_hub *hub = container_of(ws, struct usb_hub, init_work.work);
+	struct usb_hub *hub = container_of(ws, struct usb_hub, post_resume_work.work);
 
-	hub_activate(hub, HUB_POST_RESUME);
+	usb_autopm_put_interface_async(to_usb_interface(hub->intfdev));
+	hub_put(hub);
 }
 
 enum hub_quiescing_type {
@@ -1417,6 +1412,7 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
 
 	/* Stop hub_wq and related activity */
 	timer_delete_sync(&hub->irq_urb_retry);
+	flush_delayed_work(&hub->post_resume_work);
 	usb_kill_urb(hub->urb);
 	if (hub->has_indicators)
 		cancel_delayed_work_sync(&hub->leds);
@@ -1975,6 +1971,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hub->hdev = hdev;
 	INIT_DELAYED_WORK(&hub->leds, led_work);
 	INIT_DELAYED_WORK(&hub->init_work, NULL);
+	INIT_DELAYED_WORK(&hub->post_resume_work, hub_post_resume);
 	INIT_WORK(&hub->events, hub_event);
 	INIT_LIST_HEAD(&hub->onboard_devs);
 	spin_lock_init(&hub->irq_urb_lock);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index e6ae73f8a95d..9ebc5ef54a32 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -70,6 +70,7 @@ struct usb_hub {
 	u8			indicator[USB_MAXCHILDREN];
 	struct delayed_work	leds;
 	struct delayed_work	init_work;
+	struct delayed_work	post_resume_work;
 	struct work_struct      events;
 	spinlock_t		irq_urb_lock;
 	struct timer_list	irq_urb_retry;
-- 
2.43.0

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
> Delayed work to prevent USB3 hubs from runtime-suspending immediately
> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
> of high tier USB3 devices behind suspended hubs").

This doesn't apply for me against any of mainline, pending-fixes or
-next.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mathias Nyman]

On 27.6.2025 17.34, Mark Brown wrote:
> On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
>> Delayed work to prevent USB3 hubs from runtime-suspending immediately
>> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
>> of high tier USB3 devices behind suspended hubs").
> 
> This doesn't apply for me against any of mainline, pending-fixes or
> -next.

Ah, right, -next of course already has version 1 of  "usb: hub: Fix
flushing and scheduling of delayed work that tunes runtime pm"

I'll rebase this patch on top of that

Thanks
-Mathias

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Fri, Jun 27, 2025 at 05:52:14PM +0300, Mathias Nyman wrote:
> On 27.6.2025 17.34, Mark Brown wrote:

> > This doesn't apply for me against any of mainline, pending-fixes or
> > -next.

> Ah, right, -next of course already has version 1 of  "usb: hub: Fix
> flushing and scheduling of delayed work that tunes runtime pm"

> I'll rebase this patch on top of that

No problem, that's why I tried mainline as well but I guess there's some
other dependency.  I was able to get this version applied directly on
the Fixes: commit (which I should've thought of before sending that
mail, sorry) - a test result should emerge at some point in the next
half hour or so.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
> Delayed work to prevent USB3 hubs from runtime-suspending immediately
> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
> of high tier USB3 devices behind suspended hubs").

...

> Cc: stable@kernel.org
> Fixes: 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")

Tested-by: Mark Brown <broonie@kernel.org>

applied directly on the above commit, I'm happy to also test a rebased
version.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Konrad Dybcio]

On 6/27/25 4:52 PM, Mathias Nyman wrote:
> On 27.6.2025 17.34, Mark Brown wrote:
>> On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
>>> Delayed work to prevent USB3 hubs from runtime-suspending immediately
>>> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
>>> of high tier USB3 devices behind suspended hubs").
>>
>> This doesn't apply for me against any of mainline, pending-fixes or
>> -next.
> 
> Ah, right, -next of course already has version 1 of  "usb: hub: Fix
> flushing and scheduling of delayed work that tunes runtime pm"
> 
> I'll rebase this patch on top of that

FWIW I applied this one on next-20250624 (not containing the v1 fix)
and ran some testing, with no issues seen

Tested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> # SC8280XP CRD

Konrad

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mathias Nyman]

On 27.6.2025 18.23, Konrad Dybcio wrote:
> On 6/27/25 4:52 PM, Mathias Nyman wrote:
>> On 27.6.2025 17.34, Mark Brown wrote:
>>> On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
>>>> Delayed work to prevent USB3 hubs from runtime-suspending immediately
>>>> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
>>>> of high tier USB3 devices behind suspended hubs").
>>>
>>> This doesn't apply for me against any of mainline, pending-fixes or
>>> -next.
>>
>> Ah, right, -next of course already has version 1 of  "usb: hub: Fix
>> flushing and scheduling of delayed work that tunes runtime pm"
>>
>> I'll rebase this patch on top of that
> 
> FWIW I applied this one on next-20250624 (not containing the v1 fix)
> and ran some testing, with no issues seen
> 
> Tested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> # SC8280XP CRD
> 

That is good news. Thanks Konrad for testing

-Mathias

Re: [RFT PATCH v2 1/1] usb: hub: Fix flushing and scheduling of delayed work tuning runtime pm

[Mathias Nyman]

On 27.6.2025 18.19, Mark Brown wrote:
> On Fri, Jun 27, 2025 at 05:20:44PM +0300, Mathias Nyman wrote:
>> Delayed work to prevent USB3 hubs from runtime-suspending immediately
>> after resume was added in commit 8f5b7e2bec1c ("usb: hub: fix detection
>> of high tier USB3 devices behind suspended hubs").
> 
> ...
> 
>> Cc: stable@kernel.org
>> Fixes: 8f5b7e2bec1c ("usb: hub: fix detection of high tier USB3 devices behind suspended hubs")
> 
> Tested-by: Mark Brown <broonie@kernel.org>
> 
> applied directly on the above commit, I'm happy to also test a rebased
> version.

Thanks for testing, much appreciated.

I just finished the rebase and sent it as RFT PATCH v3

Thanks
Mathias