[RFC PATCH v2] USB: hub: fix port suspend/resume

From: Marco Felsch
Date: Thu Feb 27 2020 - 09:37:57 EST


At the momemnt the usb-port driver has only runime_pm hooks.
Suspending the port and turn off the VBUS supply should be triggered by
the hub device suspend callback usb_port_suspend() which calls the
pm_runtime_put_sync() if all pre-conditions are meet. This mechanism
don't work correctly due to the global PM behaviour, for more information
see [1]. According [1] I added the suspend/resume callbacks for the port
device to fix this.

[1] https://www.spinics.net/lists/linux-usb/msg190537.html

Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
---
Hi,

this v2 contains the fixes

Reported-by: kbuild test robot <lkp@xxxxxxxxx>

Regards,
Marco

Changes:
- init retval to zero
- keep CONFIG_PM due to do_remote_wakeup availability
- adapt commit message

drivers/usb/core/hub.c | 13 -------------
drivers/usb/core/port.c | 35 ++++++++++++++++++++++++++++++-----
2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3405b146edc9..c294484e478d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3323,10 +3323,6 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
usb_set_device_state(udev, USB_STATE_SUSPENDED);
}

- if (status == 0 && !udev->do_remote_wakeup && udev->persist_enabled
- && test_and_clear_bit(port1, hub->child_usage_bits))
- pm_runtime_put_sync(&port_dev->dev);
-
usb_mark_last_busy(hub->hdev);

usb_unlock_port(port_dev);
@@ -3514,15 +3510,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
int status;
u16 portchange, portstatus;

- if (!test_and_set_bit(port1, hub->child_usage_bits)) {
- status = pm_runtime_get_sync(&port_dev->dev);
- if (status < 0) {
- dev_dbg(&udev->dev, "can't resume usb port, status %d\n",
- status);
- return status;
- }
- }
-
usb_lock_port(port_dev);

/* Skip the initial Clear-Suspend step for a remote wakeup */
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index bbbb35fa639f..13f130b67efe 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -283,7 +283,34 @@ static int usb_port_runtime_suspend(struct device *dev)

return retval;
}
-#endif
+
+static int __maybe_unused _usb_port_suspend(struct device *dev)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+ struct usb_device *udev = port_dev->child;
+ int retval = 0;
+
+ if (!udev->do_remote_wakeup && udev->persist_enabled)
+ retval = usb_port_runtime_suspend(dev);
+
+ /* Do not force the user to enable the power-off feature */
+ if (retval && retval != -EAGAIN)
+ return retval;
+
+ return 0;
+}
+
+static int __maybe_unused _usb_port_resume(struct device *dev)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+ struct usb_device *udev = port_dev->child;
+
+ if (!udev->do_remote_wakeup && udev->persist_enabled)
+ return usb_port_runtime_resume(dev);
+
+ return 0;
+}
+#endif /* CONFIG_PM */

static void usb_port_shutdown(struct device *dev)
{
@@ -294,10 +321,8 @@ static void usb_port_shutdown(struct device *dev)
}

static const struct dev_pm_ops usb_port_pm_ops = {
-#ifdef CONFIG_PM
- .runtime_suspend = usb_port_runtime_suspend,
- .runtime_resume = usb_port_runtime_resume,
-#endif
+ SET_SYSTEM_SLEEP_PM_OPS(_usb_port_suspend, _usb_port_resume)
+ SET_RUNTIME_PM_OPS(usb_port_runtime_suspend, usb_port_runtime_resume, NULL)
};

struct device_type usb_port_device_type = {
--
2.20.1