Bug 218987

Created attachment 306493 [details]
usbmon traces when unplug/replug Genesys Logic Hub based devices causing SRE

Created attachment 306494 [details]
Patch attempt to get devices enumerated when a SRE occurs

Thanks for debugging this.

Issue 1, reason for SRE is unknown to me as well.
Issue 2 is probably due to PORTSC not immediately showing connected devices after host is restarted. This is especially true for USB3 devices where device only shows up in PORTSC after successful link training.   

A similar issue was solved in:
commit 33e321586e37 ("xhci: Add grace period after xHC start to prevent premature runtime suspend")

In your SRE case we never event start polling the roothub.

How about a solution that is similar to your patch but doesn't touch
pending_portevent. Instead we always resume roothubs in case xHC was reset during resume.

Does the oneliner below work for you?

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 37eb37b0affa..12b9c2c7f39f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1172,7 +1172,7 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
                        pending_portevent = xhci_pending_portevent(xhci);
                }
 
-               if (pending_portevent) {
+               if (pending_portevent || reinit_xhc) {
                        if (xhci->shared_hcd)
                                usb_hcd_resume_root_hub(xhci->shared_hcd);
                        usb_hcd_resume_root_hub(hcd);

Thanks for looking into this issue.

(In reply to Mathias Nyman from comment #3)
...
> Issue 2 is probably due to PORTSC not immediately showing connected devices
> after host is restarted. This is especially true for USB3 devices where
> device only shows up in PORTSC after successful link training.   

Weirdly enough this issue seems to happen only on the USB 2.0 root hub part of the port not the USB 3.0 one. I first tried to add delay to see if my port events showed but no luck on that. Anyway that does not matter much.

> How about a solution that is similar to your patch but doesn't touch
> pending_portevent. Instead we always resume roothubs in case xHC was reset
> during resume.
> 
> Does the oneliner below work for you?

Yes it works. While I did initially try that, I switched to pending_portevent patch version to avoid unneeded xhci_pending_portevent() and msleep(120) calls. But both are very fine with me.

> > 
> > Does the oneliner below work for you?
> 
> Yes it works. While I did initially try that, I switched to
> pending_portevent patch version to avoid unneeded xhci_pending_portevent()
> and msleep(120) calls. But both are very fine with me.

Good point, I'll attach a second version that skips the 120ms sleep.
After that patch it will also be easier to refactor reinit_xhc and 
!reinit_xhc into separate functions

Created attachment 306504 [details]
patch v2 to fix connect detect issue after xhc resume SRE case

Does this work?

Yes this works thanks.
Just a typo in the commit message s/Restore Errer/Restore Error/.

(In reply to Remi Pommarel from comment #7)
> Yes this works thanks.
> Just a typo in the commit message s/Restore Errer/Restore Error/.

Thanks, patch with fixed commit message (typo, tags etc) sent.
https://lore.kernel.org/linux-usb/20240627145523.1453155-2-mathias.nyman@linux.intel.com/

Thanks this is resolved for me then.
Still I will take a look at why SRE happen just out of curiosity.(In reply to Mathias Nyman from comment #8)
...
> Thanks, patch with fixed commit message (typo, tags etc) sent.
> https://lore.kernel.org/linux-usb/20240627145523.1453155-2-mathias.
> nyman@linux.intel.com/

Thanks this is resolved for me then.
Still I will take a look at why SRE happen (seem to be related to the presence of HDMI) just out of curiosity.

About SRE, 

Logs show transfer errors due to disconnect. This normal.

Driver will however try to recover the transfer by resending the same transfer 3 times (soft-reset internal endpoint) before giving up and and doing a hard reset of the endpoint.

Soft reset is useful if transfer error was caused by electrical interference, but this case the soft rest if futile, device is gone. 

Not sure if it makes a difference but could be worth testing if skipping the soft reset impacts SRE.

Can be done by setting quirk:
xhci->quirks |= XHCI_NO_SOFT_RETRY

(In reply to Mathias Nyman from comment #10)
> About SRE, 
> 

...

> Not sure if it makes a difference but could be worth testing if skipping the
> soft reset impacts SRE.
> 
> Can be done by setting quirk:
> xhci->quirks |= XHCI_NO_SOFT_RETRY

Thanks.

Unfortunately this does not help.

What is funny is that it is related to if the HDMI is plugged or not. So I think it is related to DP alt mode of type C. I don't know if xHC needs to even know about alternate mode, it does not look like so to me.