| Message ID | 20200715033049.254564-4-shane.lin@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Wed, 15 Jul 2020 11:30:49 +0800 Hsuan-Yu Lin <shane.lin@canonical.com> wrote: > From: Kai-Heng Feng <kai.heng.feng@canonical.com> > > BugLink: https://bugs.launchpad.net/bugs/1880519 > > USB2 devices with LPM enabled may interrupt the system suspend: > [ 932.510475] usb 1-7: usb suspend, wakeup 0 > [ 932.510549] hub 1-0:1.0: hub_suspend > [ 932.510581] usb usb1: bus suspend, wakeup 0 > [ 932.510590] xhci_hcd 0000:00:14.0: port 9 not suspended > [ 932.510593] xhci_hcd 0000:00:14.0: port 8 not suspended > .. > [ 932.520323] xhci_hcd 0000:00:14.0: Port change event, 1-7, id 7, portsc: 0x400e03 > .. > [ 932.591405] PM: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16 > [ 932.591414] PM: dpm_run_callback(): pci_pm_suspend+0x0/0x160 returns -16 > [ 932.591418] PM: Device 0000:00:14.0 failed to suspend async: error -16 > > During system suspend, USB core will let HC suspends the device if it > doesn't have remote wakeup enabled and doesn't have any children. > However, from the log above we can see that the usb 1-7 doesn't get bus > suspended due to not in U0. After a while the port finished U2 -> U0 > transition, interrupts the suspend process. > > The observation is that after disabling LPM, port doesn't transit to U0 > immediately and can linger in U2. xHCI spec 4.23.5.2 states that the > maximum exit latency for USB2 LPM should be BESL + 10us. The BESL for > the affected device is advertised as 400us, which is still not enough > based on my testing result. > > So let's use the maximum permitted latency, 10000, to poll for U0 > status to solve the issue. > > Cc: stable@vger.kernel.org > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> > Link: https://lore.kernel.org/r/20200624135949.22611-6-mathias.nyman@linux.intel.com > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (backported from commit b3d71abd135e6919ca0b6cab463738472653ddfb) We typically like to see some info about why this wasn't a clean cherry pick in the form of: [<nick>: blabla] so a reviewer has a chance to judge how different the backport is from the original. like: [juergh: Trivial context adjustments due to SAUCE patch.] ...Juerg > Signed-off-by: Hsuan-Yu Lin <shane.lin@canonical.com> > --- > drivers/usb/host/xhci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 984ed8181bc2..5e8b4054aa0e 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4249,6 +4249,9 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd, > mutex_lock(hcd->bandwidth_mutex); > xhci_change_max_exit_latency(xhci, udev, 0); > mutex_unlock(hcd->bandwidth_mutex); > + readl_poll_timeout(port_array[port_num], pm_val, > + (pm_val & PORT_PLS_MASK) == XDEV_U0, > + 100, 10000); > return 0; > } > }
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 984ed8181bc2..5e8b4054aa0e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4249,6 +4249,9 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd, mutex_lock(hcd->bandwidth_mutex); xhci_change_max_exit_latency(xhci, udev, 0); mutex_unlock(hcd->bandwidth_mutex); + readl_poll_timeout(port_array[port_num], pm_val, + (pm_val & PORT_PLS_MASK) == XDEV_U0, + 100, 10000); return 0; } }