diff mbox series

[2/2] USB: hub: Suspend and resume port with LPM enabled

Message ID 20200610064231.9454-2-kai.heng.feng@canonical.com (mailing list archive)
State New, archived
Headers show
Series [1/2] xhci: Suspend ports to U3 directly from U1 or U2 | expand

Commit Message

Kai-Heng Feng June 10, 2020, 6:42 a.m. UTC
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, because it requires a longer period for
disabling LPM. After a while the port finished its U2 -> U0 transition,
interrupts the suspend process.

Though PLC shouldn't be set for U2 -> U0 case, we can avoid all that by
directly put the port from U0/U1/U2 to U3, and solves this issue.

Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/usb/core/hub.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Sergei Shtylyov June 10, 2020, 9:21 a.m. UTC | #1
Hello!

On 10.06.2020 9:42, 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
> ..
> [  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

    Suspend. Perhaps can be fixed while applying...

> 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, because it requires a longer period for
> disabling LPM. After a while the port finished its U2 -> U0 transition,
> interrupts the suspend process.
> 
> Though PLC shouldn't be set for U2 -> U0 case, we can avoid all that by
> directly put the port from U0/U1/U2 to U3, and solves this issue.
            ^ putting                            ^ it?

> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
[...]

MBR, Sergei
diff mbox series

Patch

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b1e14beaac5f..882b54df6ef5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3285,9 +3285,6 @@  int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		}
 	}
 
-	/* disable USB2 hardware LPM */
-	usb_disable_usb2_hardware_lpm(udev);
-
 	if (usb_disable_ltm(udev)) {
 		dev_err(&udev->dev, "Failed to disable LTM before suspend\n");
 		status = -ENOMEM;
@@ -3323,9 +3320,6 @@  int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 		/* Try to enable USB3 LTM again */
 		usb_enable_ltm(udev);
  err_ltm:
-		/* Try to enable USB2 hardware LPM again */
-		usb_enable_usb2_hardware_lpm(udev);
-
 		if (udev->do_remote_wakeup)
 			(void) usb_disable_remote_wakeup(udev);
  err_wakeup:
@@ -3606,9 +3600,6 @@  int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 		dev_dbg(&udev->dev, "can't resume, status %d\n", status);
 		hub_port_logical_disconnect(hub, port1);
 	} else  {
-		/* Try to enable USB2 hardware LPM */
-		usb_enable_usb2_hardware_lpm(udev);
-
 		/* Try to enable USB3 LTM */
 		usb_enable_ltm(udev);
 	}