From stern@rowland.harvard.edu Sat Jul 1 19:11:08 2006 Date: Sat, 1 Jul 2006 22:11:02 -0400 (EDT) From: Alan Stern To: Greg KH Subject: [PATCH 09/15] usbcore: resume device resume recursion Message-ID: 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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 63 +++++++++++++++++++++++++---- drivers/usb/core/generic.c | 14 ------ drivers/usb/core/hub.c | 96 ++++++--------------------------------------- drivers/usb/core/usb.h | 2 4 files changed, 70 insertions(+), 105 deletions(-) --- gregkh-2.6.orig/drivers/usb/core/driver.c +++ gregkh-2.6/drivers/usb/core/driver.c @@ -783,7 +783,7 @@ static int resume_device(struct usb_devi 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_ 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_i 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; } --- gregkh-2.6.orig/drivers/usb/core/generic.c +++ gregkh-2.6/drivers/usb/core/generic.c @@ -184,22 +184,8 @@ static void generic_disconnect(struct us #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). --- gregkh-2.6.orig/drivers/usb/core/hub.c +++ gregkh-2.6/drivers/usb/core/hub.c @@ -1662,9 +1662,6 @@ static int finish_port_resume(struct usb "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 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 *u } 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_devi 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_interfa } } + /* 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; --- gregkh-2.6.orig/drivers/usb/core/usb.h +++ gregkh-2.6/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);