usb: fix hub-port pm_runtime_enable() vs runtime pm transitions

Commit 9262c19d14c4 "usb: disable port power control if not supported in
wHubCharacteristics" gated enabling runtime pm for usb_port devices on
whether the parent hub supports power control, which causes a
regression.  The port must still be allowed to carry out runtime pm
callbacks and receive a -EAGAIN or -EBUSY result.  Otherwise the
usb_port device will transition to the pm error state and trigger the
same for the child usb_device.

Prior to the offending commit usb_hub_create_port_device() arranged for
runtime pm to be disabled is dev_pm_qos_expose_flags() failed.  Instead,
force the default state of PM_QOS_FLAG_NO_POWER_OFF flag to be set prior
to enabling runtime pm.  If that policy can not be set then fail
registration.

Report: http://marc.info/?l=linux-usb&m=140290586301336&w=2
Fixes: 9262c19d14c4 ("usb: disable port power control if not supported in wHubCharacteristics")
Reported-by: Bjørn Mork <bjorn@mork.no>
Reported-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 78c3cd20d7aed6b1f2057e5a6aa8877884750711..21b99b4b4082e5563430f464a9f8999bff03b0f4 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1577,6 +1577,12 @@ static int hub_configure(struct usb_hub *hub,
                }
        }
        hdev->maxchild = i;
+       for (i = 0; i < hdev->maxchild; i++) {
+               struct usb_port *port_dev = hub->ports[i];
+
+               pm_runtime_put(&port_dev->dev);
+       }
+
        mutex_unlock(&usb_port_peer_mutex);
        if (ret < 0)
                goto fail;

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 0a7cdc0ef0a9b76913fa98330f363960a217d6ec..326308e53961932e6297d2b9a0007c1ac65f0d3d 100644 (file)
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -84,6 +84,7 @@ struct usb_hub {
  * @dev: generic device interface
  * @port_owner: port's owner
  * @peer: related usb2 and usb3 ports (share the same connector)
+ * @req: default pm qos request for hubs without port power control
  * @connect_type: port's connect type
  * @location: opaque representation of platform connector location
  * @status_lock: synchronize port_event() vs usb_port_{suspend|resume}
@@ -95,6 +96,7 @@ struct usb_port {
        struct device dev;
        struct usb_dev_state *port_owner;
        struct usb_port *peer;
+       struct dev_pm_qos_request *req;
        enum usb_port_connect_type connect_type;
        usb_port_location_t location;
        struct mutex status_lock;

diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 9347ade7d5fe3637fcb2aa8514831e1b5da2247c..fe1b6d0967e32a95e270fcccee8e8b72eb844db3 100644 (file)
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -68,6 +68,7 @@ static void usb_port_device_release(struct device *dev)
 {
        struct usb_port *port_dev = to_usb_port(dev);
 
+       kfree(port_dev->req);
        kfree(port_dev);
 }
 
@@ -400,9 +401,13 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
        int retval;
 
        port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
-       if (!port_dev) {
-               retval = -ENOMEM;
-               goto exit;
+       if (!port_dev)
+               return -ENOMEM;
+
+       port_dev->req = kzalloc(sizeof(*(port_dev->req)), GFP_KERNEL);
+       if (!port_dev->req) {
+               kfree(port_dev);
+               return -ENOMEM;
        }
 
        hub->ports[port1 - 1] = port_dev;
@@ -418,31 +423,53 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
                        port1);
        mutex_init(&port_dev->status_lock);
        retval = device_register(&port_dev->dev);
-       if (retval)
-               goto error_register;
+       if (retval) {
+               put_device(&port_dev->dev);
+               return retval;
+       }
+
+       /* Set default policy of port-poweroff disabled. */
+       retval = dev_pm_qos_add_request(&port_dev->dev, port_dev->req,
+                       DEV_PM_QOS_FLAGS, PM_QOS_FLAG_NO_POWER_OFF);
+       if (retval < 0) {
+               device_unregister(&port_dev->dev);
+               return retval;
+       }
 
        find_and_link_peer(hub, port1);
 
+       /*
+        * Enable runtime pm and hold a refernce that hub_configure()
+        * will drop once the PM_QOS_NO_POWER_OFF flag state has been set
+        * and the hub has been fully registered (hdev->maxchild set).
+        */
        pm_runtime_set_active(&port_dev->dev);
+       pm_runtime_get_noresume(&port_dev->dev);
+       pm_runtime_enable(&port_dev->dev);
+       device_enable_async_suspend(&port_dev->dev);
 
        /*
-        * Do not enable port runtime pm if the hub does not support
-        * power switching.  Also, userspace must have final say of
-        * whether a port is permitted to power-off.  Do not enable
-        * runtime pm if we fail to expose pm_qos_no_power_off.
+        * Keep hidden the ability to enable port-poweroff if the hub
+        * does not support power switching.
         */
-       if (hub_is_port_power_switchable(hub)
-                       && dev_pm_qos_expose_flags(&port_dev->dev,
-                       PM_QOS_FLAG_NO_POWER_OFF) == 0)
-               pm_runtime_enable(&port_dev->dev);
+       if (!hub_is_port_power_switchable(hub))
+               return 0;
 
-       device_enable_async_suspend(&port_dev->dev);
-       return 0;
+       /* Attempt to let userspace take over the policy. */
+       retval = dev_pm_qos_expose_flags(&port_dev->dev,
+                       PM_QOS_FLAG_NO_POWER_OFF);
+       if (retval < 0) {
+               dev_warn(&port_dev->dev, "failed to expose pm_qos_no_poweroff\n");
+               return 0;
+       }
 
-error_register:
-       put_device(&port_dev->dev);
-exit:
-       return retval;
+       /* Userspace owns the policy, drop the kernel 'no_poweroff' request. */
+       retval = dev_pm_qos_remove_request(port_dev->req);
+       if (retval >= 0) {
+               kfree(port_dev->req);
+               port_dev->req = NULL;
+       }
+       return 0;
 }
 
 void usb_hub_remove_port_device(struct usb_hub *hub, int port1)