On Fri, Jan 10, 2020 at 5:33 PM Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> wrote: > > On 3.1.2020 10.40, Kai-Heng Feng wrote: > > The xHCI spec doesn't specify the upper bound of U3 transition time. For > > some devices 20ms is not enough, so we need to make sure the link state > > is in U3 before further actions. > > Looking at the USB 3.2 spec (7.2.4.2 Link Power management and Flow) it seems > most timers related to U3 entry are a lot less than a millisecond (4-16us) > If port is in U1/U2, Ux_EXIT_TIMER is 6ms which seems to be the longest timeout. > > If we anyway are polling for the U3 state we could shorten the initial 20ms sleep > down to 10ms. I think many devices, especially if they are already in U0 > could manage this. Ok. > > are > > > I've tried to use U3 Entry Capability by setting U3 Entry Enable in > > config register, however the port change event for U3 transition > > interrupts the system suspend process. > > > > For now let's use the less ideal method by polling PLS. > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > --- > > drivers/usb/host/xhci-hub.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c > > index 7a3a29e5e9d2..2b2e9d004dbf 100644 > > --- a/drivers/usb/host/xhci-hub.c > > +++ b/drivers/usb/host/xhci-hub.c > > @@ -1228,6 +1228,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, > > break; > > case USB_PORT_FEAT_LINK_STATE: > > temp = readl(ports[wIndex]->addr); > > + xhci_dbg(xhci, "before setting link state, actual port %d-%d status = 0x%0x\n", hcd->self.busnum, wIndex + 1, temp); > > xhci_set_link_state() already shows similar debugging, Ok, will remove it. > > > /* Disable port */ > > if (link_state == USB_SS_PORT_LS_SS_DISABLED) { > > xhci_dbg(xhci, "Disable port %d\n", wIndex); > > @@ -1316,9 +1317,17 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, > > msleep(20); /* wait device to enter */ > > spin_lock_irqsave(&xhci->lock, flags); > > > > - temp = readl(ports[wIndex]->addr); > > - if (link_state == USB_SS_PORT_LS_U3) > > + if (link_state == USB_SS_PORT_LS_U3) { > > + retval = xhci_handshake(ports[wIndex]->addr, PORT_PLS_MASK, XDEV_U3, 80 * 1000); > > + if (retval) > > + xhci_dbg(xhci, "polling XDEV_U3 on port %d-%d timeout\n", hcd->self.busnum, wIndex + 1); > > In worst case we are busylooping for 80ms here, keeping the cpu busy. > It should be ok to sleep here, so how about just reading the register > every 10ms max 10 times, sleeping in between. Ok. Is the polling safe outside of spin_lock_irqsave()? Kai-Heng > > -Mathias