On 19.6.2020 17.19, Kai-Heng Feng wrote: > Hi Mathias, > >> On Jun 9, 2020, at 18:15, Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote: >> >> >> >>> On Jun 8, 2020, at 19:21, Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> wrote: >>> >>> On 20.5.2020 13.18, Kai-Heng Feng wrote: >>>> 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 >>> >>> 400e03 = Connected, Enabled, U0 with port ink state change flag (PLC) set. >>> >>>> .. >>>> [ 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. >>> >>> In USB2 HW link PM the PLC flag should not be set in U2Exit -> U0 transitions. >>> Only case we should see a port change event is U2Entry -> U0 due to STALL or >>> error/timeout. (see xhci 4.23.5.1.1.1) >>> >>>> >>>> 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. >>> >>> I don't recall all details, but it could be that disabling LPM before suspend >>> is unnecessary. >>> At least xhci should be able to set a port to U3 from U1 and U2 (see xhci 4.15.1) >>> so that is one change that could be done to xhci_bus_suspend() >> >> Yes, put the device to U3 directly does the trick. > > As discussed, will you pick this series over the v2? > Or is there anything I should improve for this one? > > Kai-Heng Added this to queueI Thanks -Mathias
