usbcore: resume device resume recursion

This patch (as717b) removes the existing recursion in hub resume code:
Resuming a hub will no longer automatically resume the devices attached
to the hub.

At the same time, it adds one level of recursion: Suspending a USB
device will automatically suspend all the device's interfaces.  Failure
at an intermediate stage will cause all the already-suspended interfaces
to be resumed. Attempts to suspend or resume an interface by itself will
do nothing, although they won't return an error.  Thus the regular
system-suspend and system-resume procedures should continue to work as
before; only runtime PM will be affected.

The patch also removes the code that tests state of the interfaces
before suspending a device.  It's no longer needed, since everything
gets suspended together.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index b0db1583c522289a50a02a48d75d87450ebb4e68..eefc98584eac92e86be28c55836f532a60e9065a 100644 (file)
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -783,7 +783,7 @@ static int resume_device(struct usb_device *udev)
        return udriver->resume(udev);
 }
 
-/* Caller has locked intf */
+/* Caller has locked intf's usb_device */
 static int suspend_interface(struct usb_interface *intf, pm_message_t msg)
 {
        struct usb_driver       *driver;
@@ -815,7 +815,7 @@ static int suspend_interface(struct usb_interface *intf, pm_message_t msg)
        return status;
 }
 
-/* Caller has locked intf */
+/* Caller has locked intf's usb_device */
 static int resume_interface(struct usb_interface *intf)
 {
        struct usb_driver       *driver;
@@ -856,14 +856,59 @@ static int resume_interface(struct usb_interface *intf)
        return 0;
 }
 
+/* Caller has locked udev */
+int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
+{
+       int                     status = 0;
+       int                     i = 0;
+       struct usb_interface    *intf;
+
+       if (udev->actconfig) {
+               for (; i < udev->actconfig->desc.bNumInterfaces; i++) {
+                       intf = udev->actconfig->interface[i];
+                       status = suspend_interface(intf, msg);
+                       if (status != 0)
+                               break;
+               }
+       }
+       if (status == 0)
+               status = suspend_device(udev, msg);
+
+       /* If the suspend failed, resume interfaces that did get suspended */
+       if (status != 0) {
+               while (--i >= 0) {
+                       intf = udev->actconfig->interface[i];
+                       resume_interface(intf);
+               }
+       }
+       return status;
+}
+
+/* Caller has locked udev */
+int usb_resume_both(struct usb_device *udev)
+{
+       int                     status;
+       int                     i;
+       struct usb_interface    *intf;
+
+       status = resume_device(udev);
+       if (status == 0 && udev->actconfig) {
+               for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
+                       intf = udev->actconfig->interface[i];
+                       resume_interface(intf);
+               }
+       }
+       return status;
+}
+
 static int usb_suspend(struct device *dev, pm_message_t message)
 {
        int     status;
 
        if (is_usb_device(dev))
-               status = suspend_device(to_usb_device(dev), message);
+               status = usb_suspend_both(to_usb_device(dev), message);
        else
-               status = suspend_interface(to_usb_interface(dev), message);
+               status = 0;
        return status;
 }
 
@@ -871,10 +916,12 @@ static int usb_resume(struct device *dev)
 {
        int     status;
 
-       if (is_usb_device(dev))
-               status = resume_device(to_usb_device(dev));
-       else
-               status = resume_interface(to_usb_interface(dev));
+       if (is_usb_device(dev)) {
+               status = usb_resume_both(to_usb_device(dev));
+
+               /* Rebind drivers that had no suspend method? */
+       } else
+               status = 0;
        return status;
 }
 

diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 1522195de7158d81f153534bf1af1045340c71fc..b6dacd7551d2e0d843a3d6998bf67a2a4b838ba6 100644 (file)
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -184,22 +184,8 @@ static void generic_disconnect(struct usb_device *udev)
 
 #ifdef CONFIG_PM
 
-static int verify_suspended(struct device *dev, void *unused)
-{
-       if (dev->driver == NULL)
-               return 0;
-       return (dev->power.power_state.event == PM_EVENT_ON) ? -EBUSY : 0;
-}
-
 static int generic_suspend(struct usb_device *udev, pm_message_t msg)
 {
-       int     status;
-
-       /* rule out bogus requests through sysfs */
-       status = device_for_each_child(&udev->dev, NULL, verify_suspended);
-       if (status)
-               return status;
-
        /* USB devices enter SUSPEND state through their hubs, but can be
         * marked for FREEZE as soon as their children are already idled.
         * But those semantics are useless, so we equate the two (sigh).

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a372332440b21d7897330e61632e1c2da9f84958..a39112041e6932131fc12db5e6042eb4527c2f66 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1662,9 +1662,6 @@ static int finish_port_resume(struct usb_device *udev)
                        "gone after usb resume? status %d\n",
                        status);
        else if (udev->actconfig) {
-               unsigned        i;
-               int             (*resume)(struct device *);
-
                le16_to_cpus(&devstatus);
                if ((devstatus & (1 << USB_DEVICE_REMOTE_WAKEUP))
                                && udev->parent) {
@@ -1675,24 +1672,9 @@ static int finish_port_resume(struct usb_device *udev)
                                        USB_DEVICE_REMOTE_WAKEUP, 0,
                                        NULL, 0,
                                        USB_CTRL_SET_TIMEOUT);
-                       if (status) {
+                       if (status)
                                dev_dbg(&udev->dev, "disable remote "
                                        "wakeup, status %d\n", status);
-                               status = 0;
-                       }
-               }
-
-               /* resume interface drivers; if this is a hub, it
-                * may have a child resume event to deal with soon
-                */
-               resume = udev->dev.bus->resume;
-               for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
-                       struct device *dev =
-                                       &udev->actconfig->interface[i]->dev;
-
-                       down(&dev->sem);
-                       (void) resume(dev);
-                       up(&dev->sem);
                }
                status = 0;
 
@@ -1802,15 +1784,7 @@ int usb_port_resume(struct usb_device *udev)
        } else
                status = finish_port_resume(udev);
        if (status < 0)
-               dev_dbg(&udev->dev, "can't resume, status %d\n",
-                       status);
-
-       /* rebind drivers that had no suspend() */
-       if (status == 0) {
-               usb_unlock_device(udev);
-               bus_rescan_devices(&usb_bus_type);
-               usb_lock_device(udev);
-       }
+               dev_dbg(&udev->dev, "can't resume, status %d\n", status);
        return status;
 }
 
@@ -1830,6 +1804,9 @@ static int remote_wakeup(struct usb_device *udev)
                msleep(10);
                status = finish_port_resume(udev);
        }
+
+       if (status == 0)
+               usb_resume_both(udev);
        usb_unlock_device(udev);
 #endif
        return status;
@@ -1901,51 +1878,8 @@ static int hub_resume(struct usb_interface *intf)
                }
        }
 
+       /* tell khubd to look for changes on this hub */
        hub_activate(hub);
-
-       /* REVISIT:  this recursion probably shouldn't exist.  Remove
-        * this code sometime, after retesting with different root and
-        * external hubs.
-        */
-#ifdef CONFIG_USB_SUSPEND
-       {
-       unsigned                port1;
-
-       for (port1 = 1; port1 <= hdev->maxchild; port1++) {
-               struct usb_device       *udev;
-               u16                     portstat, portchange;
-
-               udev = hdev->children [port1-1];
-               status = hub_port_status(hub, port1, &portstat, &portchange);
-               if (status == 0) {
-                       if (portchange & USB_PORT_STAT_C_SUSPEND) {
-                               clear_port_feature(hdev, port1,
-                                       USB_PORT_FEAT_C_SUSPEND);
-                               portchange &= ~USB_PORT_STAT_C_SUSPEND;
-                       }
-
-                       /* let khubd handle disconnects etc */
-                       if (portchange)
-                               continue;
-               }
-
-               if (!udev || status < 0)
-                       continue;
-               usb_lock_device(udev);
-               if (portstat & USB_PORT_STAT_SUSPEND)
-                       status = hub_port_resume(hub, port1, udev);
-               else {
-                       status = finish_port_resume(udev);
-                       if (status < 0) {
-                               dev_dbg(&intf->dev, "resume port %d --> %d\n",
-                                       port1, status);
-                               hub_port_logical_disconnect(hub, port1);
-                       }
-               }
-               usb_unlock_device(udev);
-       }
-       }
-#endif
        return 0;
 }
 
@@ -2602,17 +2536,6 @@ static void hub_events(void)
                usb_get_intf(intf);
                spin_unlock_irq(&hub_event_lock);
 
-               /* Is this is a root hub wanting to reactivate the downstream
-                * ports?  If so, be sure the interface resumes even if its
-                * stub "device" node was never suspended.
-                */
-               if (i) {
-                       dpm_runtime_resume(&hdev->dev);
-                       dpm_runtime_resume(&intf->dev);
-                       usb_put_intf(intf);
-                       continue;
-               }
-
                /* Lock the device, then check to see if we were
                 * disconnected while waiting for the lock to succeed. */
                if (locktree(hdev) < 0) {
@@ -2629,6 +2552,13 @@ static void hub_events(void)
                        goto loop;
                }
 
+               /* Is this is a root hub wanting to reactivate the downstream
+                * ports?  If so, be sure the interface resumes even if its
+                * stub "device" node was never suspended.
+                */
+               if (i)
+                       usb_resume_both(hdev);
+
                /* If this is an inactive or suspended hub, do nothing */
                if (hub->quiescing)
                        goto loop;

diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 1d25ccac783276f47fe7b9b26dd3025f356d1b70..cc42972b6bb0439db8c8f3caab7445b03500c7ea 100644 (file)
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -30,6 +30,8 @@ extern void usb_major_cleanup(void);
 extern int usb_host_init(void);
 extern void usb_host_cleanup(void);
 
+extern int usb_suspend_both(struct usb_device *udev, pm_message_t msg);
+extern int usb_resume_both(struct usb_device *udev);
 extern int usb_port_suspend(struct usb_device *dev);
 extern int usb_port_resume(struct usb_device *dev);