| Message ID | 20210311115353.2137560-3-mathias.nyman@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | 253f588c70f66184b1f3a9bbb428b49bbda73e80 |
| Headers | show |
| Series | xhci fixes for usb-linus | expand |
Hi Mathias, On Thu, Mar 11, 2021 at 01:53:51PM +0200, Mathias Nyman wrote: > A xHC USB 3 port might miss the first wake signal from a USB 3 device > if the port LFPS reveiver isn't enabled fast enough after xHC resume. > > xHC host will anyway be resumed by a PME# signal, but will go back to > suspend if no port activity is seen. > The device resends the U3 LFPS wake signal after a 100ms delay, but > by then host is already suspended, starting all over from the > beginning of this issue. > > USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device > needs to delay 100ms before resending the wake. > > Don't suspend immediately if port activity isn't detected in resume. > Instead add a retry. If there is no port activity then delay for 120ms, > and re-check for port activity. We have a use case with which this change is causing unnecessary delay. Consider a USB2* device is attached and host is initiating the resume. Since this is not a device initiated wakeup there wouldn't be any pending event seen on the PORTSC registers, yet this adds an additional 120ms delay to re-check the PORTSC before returning and allowing the USB core to perform resume signaling. Is there a way to avoid this delay in that case? Perhaps could we distinguish whether we arrive here at xhci_resume() due to a host-initiated resume vs. a device remote wakeup? * I think it should be similar for attached USB3 devices as well, since the host-initiated exit from U3 wouldn't happen until usb_port_resume(). Thanks, Jack > Cc: <stable@vger.kernel.org> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> > --- > drivers/usb/host/xhci.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index bd27bd670104..48a68fcf2b36 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1088,6 +1088,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > struct usb_hcd *secondary_hcd; > int retval = 0; > bool comp_timer_running = false; > + bool pending_portevent = false; > > if (!hcd->state) > return 0; > @@ -1226,13 +1227,22 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > done: > if (retval == 0) { > - /* Resume root hubs only when have pending events. */ > - if (xhci_pending_portevent(xhci)) { > + /* > + * 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. > + */ > + pending_portevent = xhci_pending_portevent(xhci); > + if (!pending_portevent) { > + msleep(120); > + pending_portevent = xhci_pending_portevent(xhci); > + } > + > + if (pending_portevent) { > usb_hcd_resume_root_hub(xhci->shared_hcd); > usb_hcd_resume_root_hub(hcd); > } > }
On Tue, Sep 21, 2021 at 06:16:43PM -0700, Jack Pham wrote: > Hi Mathias, > > On Thu, Mar 11, 2021 at 01:53:51PM +0200, Mathias Nyman wrote: > > A xHC USB 3 port might miss the first wake signal from a USB 3 device > > if the port LFPS reveiver isn't enabled fast enough after xHC resume. > > > > xHC host will anyway be resumed by a PME# signal, but will go back to > > suspend if no port activity is seen. > > The device resends the U3 LFPS wake signal after a 100ms delay, but > > by then host is already suspended, starting all over from the > > beginning of this issue. > > > > USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device > > needs to delay 100ms before resending the wake. > > > > Don't suspend immediately if port activity isn't detected in resume. > > Instead add a retry. If there is no port activity then delay for 120ms, > > and re-check for port activity. > > We have a use case with which this change is causing unnecessary delay. > Consider a USB2* device is attached and host is initiating the resume. > Since this is not a device initiated wakeup there wouldn't be any > pending event seen on the PORTSC registers, yet this adds an additional > 120ms delay to re-check the PORTSC before returning and allowing the USB > core to perform resume signaling. > > Is there a way to avoid this delay in that case? Perhaps could we > distinguish whether we arrive here at xhci_resume() due to a > host-initiated resume vs. a device remote wakeup? Do you have a proposed patch that would do such a thing? Given that you are seeing the problem it should be easy for you to test :) > * I think it should be similar for attached USB3 devices as well, since > the host-initiated exit from U3 wouldn't happen until usb_port_resume(). Can you reliably detect "attached" devices? thanks, greg k-h
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index bd27bd670104..48a68fcf2b36 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1088,6 +1088,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) struct usb_hcd *secondary_hcd; int retval = 0; bool comp_timer_running = false; + bool pending_portevent = false; if (!hcd->state) return 0; @@ -1226,13 +1227,22 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) done: if (retval == 0) { - /* Resume root hubs only when have pending events. */ - if (xhci_pending_portevent(xhci)) { + /* + * 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. + */ + pending_portevent = xhci_pending_portevent(xhci); + if (!pending_portevent) { + msleep(120); + pending_portevent = xhci_pending_portevent(xhci); + } + + if (pending_portevent) { usb_hcd_resume_root_hub(xhci->shared_hcd); usb_hcd_resume_root_hub(hcd); } } - /* * If system is subject to the Quirk, Compliance Mode Timer needs to * be re-initialized Always after a system resume. Ports are subject
A xHC USB 3 port might miss the first wake signal from a USB 3 device if the port LFPS reveiver isn't enabled fast enough after xHC resume. xHC host will anyway be resumed by a PME# signal, but will go back to suspend if no port activity is seen. The device resends the U3 LFPS wake signal after a 100ms delay, but by then host is already suspended, starting all over from the beginning of this issue. USB 3 specs say U3 wake LFPS signal is sent for max 10ms, then device needs to delay 100ms before resending the wake. Don't suspend immediately if port activity isn't detected in resume. Instead add a retry. If there is no port activity then delay for 120ms, and re-check for port activity. Cc: <stable@vger.kernel.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> --- drivers/usb/host/xhci.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)