David:
Please take a look at this proposed patch and see what you think. It
removes the recursion from the usbcore suspend/resume routines. There's
just a couple of small exceptions; the big one is that resuming a device
will cause all its interfaces to be resumed as well, and the small one is
that before suspending a device all the interfaces are checked to make
sure they are already suspended.
In addition, the code for handling interfaces has been moved from hub.c
into the generic suspend/resume routines near the end of usb.c. However I
didn't bother to change the part in usb_resume_device where the single
interface of a root hub gets resumed along with the root hub; that code
still calls hub_resume directly.
All this should work perfectly well with system sleep transitions, as the
PM core does the recursion for us. Runtime power management will be a
little more difficult; for testing purposes it will be handy to have some
scripts available.
Alan Stern
Index: usb-2.6/drivers/usb/core/hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hub.c
+++ usb-2.6/drivers/usb/core/hub.c
@@ -452,6 +452,7 @@ static void hub_quiesce(struct usb_hub *
{
/* stop khubd and related activity */
hub->quiescing = 1;
+ hub->activating = 0;
usb_kill_urb(hub->urb);
if (hub->has_indicators)
cancel_delayed_work(&hub->leds);
@@ -1553,50 +1554,23 @@ static int __usb_suspend_device (struct
return 0;
}
- /* suspend interface drivers; if this is a hub, it
- * suspends the child devices
+ /* check to be sure the interfaces really are suspended;
+ * the caller should have suspended them already
+ *
+ * FIXME: Should this check all the child devices?
*/
if (udev->actconfig) {
int i;
for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
struct usb_interface *intf;
- struct usb_driver *driver;
intf = udev->actconfig->interface[i];
- if (state <= intf->dev.power.power_state)
- continue;
- if (!intf->dev.driver)
- continue;
- driver = to_usb_driver(intf->dev.driver);
-
- if (driver->suspend) {
- status = driver->suspend(intf, state);
- if (intf->dev.power.power_state != state
- || status)
- dev_err(&intf->dev,
- "suspend %d fail, code %d\n",
- state, status);
- }
-
- /* only drivers with suspend() can ever resume();
- * and after power loss, even they won't.
- * bus_rescan_devices() can rebind drivers later.
- *
- * FIXME the PM core self-deadlocks when unbinding
- * drivers during suspend/resume ... everything grabs
- * dpm_sem (not a spinlock, ugh). we want to unbind,
- * since we know every driver's probe/disconnect works
- * even for drivers that can't suspend.
- */
- if (!driver->suspend || state > PM_SUSPEND_MEM) {
-#if 1
- dev_warn(&intf->dev, "resume is unsafe!\n");
-#else
- down_write(&usb_bus_type.rwsem);
- device_release_driver(&intf->dev);
- up_write(&usb_bus_type.rwsem);
-#endif
+ if (intf->dev.power.power_state == PMSG_ON) {
+ dev_err(&udev->dev, "suspend %d failed, "
+ "interface %s still awake\n",
+ state, intf->dev.bus_id);
+ return -EBUSY;
}
}
}
@@ -1719,33 +1693,13 @@ static int finish_port_resume(struct usb
}
}
- /* resume interface drivers; if this is a hub, it
- * resumes the child devices
- */
+ /* resume interface drivers */
+ /* FIXME: Is this single-level recursion needed? */
for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
struct usb_interface *intf;
- struct usb_driver *driver;
intf = udev->actconfig->interface[i];
- if (intf->dev.power.power_state == PMSG_SUSPEND)
- continue;
- if (!intf->dev.driver) {
- /* FIXME maybe force to alt 0 */
- continue;
- }
- driver = to_usb_driver(intf->dev.driver);
-
- /* bus_rescan_devices() may rebind drivers */
- if (!driver->resume)
- continue;
-
- /* can we do better than just logging errors? */
- status = driver->resume(intf);
- if (intf->dev.power.power_state != PMSG_ON
- || status)
- dev_dbg(&intf->dev,
- "resume fail, state %d code %d\n",
- intf->dev.power.power_state, status);
+ intf->dev.bus->resume(&intf->dev);
}
status = 0;
@@ -1897,24 +1851,20 @@ static int hub_suspend(struct usb_interf
struct usb_hub *hub = usb_get_intfdata (intf);
struct usb_device *hdev = hub->hdev;
unsigned port1;
- int status;
/* stop khubd and related activity */
hub_quiesce(hub);
- /* then suspend every port */
+ /* warn if some port isn't suspended */
for (port1 = 1; port1 <= hdev->maxchild; port1++) {
struct usb_device *udev;
udev = hdev->children [port1-1];
if (!udev)
continue;
- down(&udev->serialize);
- status = __usb_suspend_device(udev, port1, state);
- up(&udev->serialize);
- if (status < 0)
- dev_dbg(&intf->dev, "suspend port %d --> %d\n",
- port1, status);
+ if (udev->dev.power.power_state == PMSG_ON)
+ dev_dbg(&intf->dev, "port %d isn't suspended\n",
+ port1);
}
intf->dev.power.power_state = state;
@@ -1923,47 +1873,10 @@ static int hub_suspend(struct usb_interf
static int hub_resume(struct usb_interface *intf)
{
- struct usb_device *hdev = interface_to_usbdev(intf);
struct usb_hub *hub = usb_get_intfdata (intf);
- unsigned port1;
- int status;
- if (intf->dev.power.power_state == PM_SUSPEND_ON)
+ if (intf->dev.power.power_state == PMSG_ON)
return 0;
-
- 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;
- down (&udev->serialize);
- 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);
- }
- }
- up(&udev->serialize);
- }
intf->dev.power.power_state = PMSG_ON;
hub->resume_root_hub = 0;
Index: usb-2.6/drivers/usb/core/usb.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/usb.c
+++ usb-2.6/drivers/usb/core/usb.c
@@ -1379,23 +1379,49 @@ static int usb_generic_suspend(struct de
{
struct usb_interface *intf;
struct usb_driver *driver;
+ int status;
+ /* there's only one USB suspend state */
+ if (dev->power.power_state != PMSG_ON)
+ return 0;
+
+ /* devices suspend through their hub */
if (dev->driver == &usb_generic_driver)
return usb_suspend_device (to_usb_device(dev), message);
- if ((dev->driver == NULL) ||
- (dev->driver_data == &usb_generic_driver_data))
+ if (dev->driver == NULL) {
+ dev->power.power_state = message;
return 0;
+ }
intf = to_usb_interface(dev);
driver = to_usb_driver(dev->driver);
-
- /* there's only one USB suspend state */
- if (intf->dev.power.power_state)
- return 0;
-
- if (driver->suspend)
- return driver->suspend(intf, message);
+ if (driver->suspend) {
+ status = driver->suspend(intf, message);
+ if (intf->dev.power.power_state != message || status)
+ dev_err(&intf->dev, "suspend %d fail, code %d\n",
+ message, status);
+ } else {
+ dev->power.power_state = message;
+
+ /* only drivers with suspend() can ever resume();
+ * and after power loss, even they won't.
+ * bus_rescan_devices() can rebind drivers later.
+ *
+ * FIXME the PM core self-deadlocks when unbinding
+ * drivers during suspend/resume ... everything grabs
+ * dpm_sem (not a spinlock, ugh). we want to unbind,
+ * since we know every driver's probe/disconnect works
+ * even for drivers that can't suspend.
+ */
+#if 1
+ dev_warn(&intf->dev, "resume is unsafe!\n");
+#else
+ down_write(&usb_bus_type.rwsem);
+ device_release_driver(&intf->dev);
+ up_write(&usb_bus_type.rwsem);
+#endif
+ }
return 0;
}
@@ -1403,20 +1429,40 @@ static int usb_generic_resume(struct dev
{
struct usb_interface *intf;
struct usb_driver *driver;
+ int status;
+
+ if (dev->power.power_state == PMSG_ON)
+ return 0;
/* devices resume through their hub */
if (dev->driver == &usb_generic_driver)
return usb_resume_device (to_usb_device(dev));
- if ((dev->driver == NULL) ||
- (dev->driver_data == &usb_generic_driver_data))
+ intf = to_usb_interface(dev);
+ if (dev->driver == NULL) {
+ dev->power.power_state = PMSG_ON;
+
+ /* Force to altsetting 0 for the next driver */
+ if (intf->cur_altsetting->desc.bAlternateSetting != 0)
+ usb_set_interface(interface_to_usbdev(intf),
+ intf->cur_altsetting->desc.bInterfaceNumber,
+ 0);
+
+ /* bus_rescan_devices() may rebind drivers */
return 0;
+ }
- intf = to_usb_interface(dev);
driver = to_usb_driver(dev->driver);
+ if (driver->resume) {
+
+ /* can we do better than just logging errors? */
+ status = driver->resume(intf);
+ if (intf->dev.power.power_state != PMSG_ON || status)
+ dev_dbg(&intf->dev, "resume fail, state %d code %d\n",
+ intf->dev.power.power_state, status);
+ } else
+ dev->power.power_state = PMSG_ON;
- if (driver->resume)
- return driver->resume(intf);
return 0;
}
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
