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@xxxxxxxxxxxxxxx>
> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> ---
> 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);
> }
> }
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
