Greg:
This patch (as718) includes a large cleanup of the suspend/resume code in
usbcore. The most notable change is the addition of a new field to the
usb_interface structure, for keeping track of whether the interface is
active or suspended. The intf->dev.power.power_state.event field can't be
used safely for this purpose because the PM core believes it owns the
value and can change it unilaterally.
Some duplicated code and unnecessary checks are removed and more code is
included under the "#ifdef CONFIG_PM" umbrella. The code that puts an
interface into altsetting 0 is moved so that it runs just before a driver
is bound (which is when it matters) instead of just after a driver is
unbound.
Alan Stern
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
---
Index: usb-2.6/include/linux/usb.h
===================================================================
--- usb-2.6.orig/include/linux/usb.h
+++ usb-2.6/include/linux/usb.h
@@ -101,6 +101,7 @@ enum usb_interface_condition {
* number from the USB core by calling usb_register_dev().
* @condition: binding state of the interface: not bound, binding
* (in probe()), bound to a driver, or unbinding (in disconnect())
+ * @is_active: flag set when the interface is bound and not suspended.
* @dev: driver model's view of this device
* @class_dev: driver model's class view of this device.
*
@@ -141,6 +142,8 @@ struct usb_interface {
int minor; /* minor number this interface is
* bound to */
enum usb_interface_condition condition; /* state of binding */
+ int is_active; /* the interface is not suspended */
+
struct device dev; /* interface specific device info */
struct class_device *class_dev;
};
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
@@ -1042,7 +1042,7 @@ void usb_set_device_state(struct usb_dev
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM
/**
* usb_root_hub_lost_power - called by HCD if the root hub lost Vbus power
@@ -1470,6 +1470,7 @@ static void hub_port_logical_disconnect(
kick_khubd(hub);
}
+#ifdef CONFIG_PM
#ifdef CONFIG_USB_SUSPEND
@@ -1555,40 +1556,19 @@ static int __usb_port_suspend (struct us
if (port1 < 0)
return port1;
- if (udev->state == USB_STATE_SUSPENDED
- || udev->state == USB_STATE_NOTATTACHED) {
- return 0;
- }
-
- /* all interfaces must already be suspended */
- if (udev->actconfig) {
- int i;
-
- for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
- struct usb_interface *intf;
-
- intf = udev->actconfig->interface[i];
- if (is_active(intf)) {
- dev_dbg(&intf->dev, "nyet suspended\n");
- return -EBUSY;
- }
- }
- }
-
/* we change the device's upstream USB link,
* but root hubs have no upstream USB link.
*/
if (udev->parent)
status = hub_port_suspend(hdev_to_hub(udev->parent), port1,
udev);
-
- if (status == 0)
- udev->dev.power.power_state = PMSG_SUSPEND;
+ else {
+ dev_dbg(&udev->dev, "usb suspend\n");
+ usb_set_device_state(udev, USB_STATE_SUSPENDED);
+ }
return status;
}
-#endif
-
/*
* usb_port_suspend - suspend a usb device's upstream port
* @udev: device that's no longer in active use
@@ -1611,17 +1591,19 @@ static int __usb_port_suspend (struct us
*/
int usb_port_suspend(struct usb_device *udev)
{
-#ifdef CONFIG_USB_SUSPEND
- if (udev->state == USB_STATE_NOTATTACHED)
- return -ENODEV;
return __usb_port_suspend(udev, udev->portnum);
-#else
- /* NOTE: udev->state unchanged, it's not lying ... */
- udev->dev.power.power_state = PMSG_SUSPEND;
+}
+
+#else /* CONFIG_USB_SUSPEND */
+
+/* When CONFIG_USB_SUSPEND isn't set, we never suspend any ports. */
+int usb_port_suspend(struct usb_device *udev)
+{
return 0;
-#endif
}
+#endif
+
/*
* If the USB "suspend" state is in use (rather than "global suspend"),
* many devices will be individually taken out of suspend state using
@@ -1646,7 +1628,6 @@ static int finish_port_resume(struct usb
usb_set_device_state(udev, udev->actconfig
? USB_STATE_CONFIGURED
: USB_STATE_ADDRESS);
- udev->dev.power.power_state = PMSG_ON;
/* 10.5.4.5 says be sure devices in the tree are still there.
* For now let's assume the device didn't go crazy on resume,
@@ -1740,6 +1721,15 @@ hub_port_resume(struct usb_hub *hub, int
return status;
}
+#else /* CONFIG_USB_SUSPEND */
+
+/* When CONFIG_USB_SUSPEND isn't set, we never resume any ports. */
+static inline int
+hub_port_resume(struct usb_hub *hub, int port1, struct usb_device *udev)
+{
+ return 0;
+}
+
#endif
/*
@@ -1759,34 +1749,18 @@ int usb_port_resume(struct usb_device *u
{
int status;
- if (udev->state == USB_STATE_NOTATTACHED)
- return -ENODEV;
-
/* we change the device's upstream USB link,
* but root hubs have no upstream USB link.
*/
if (udev->parent) {
-#ifdef CONFIG_USB_SUSPEND
- if (udev->state == USB_STATE_SUSPENDED) {
- // NOTE swsusp may bork us, device state being wrong...
- // NOTE this fails if parent is also suspended...
- status = hub_port_resume(hdev_to_hub(udev->parent),
- udev->portnum, udev);
- } else
-#endif
- status = 0;
+ // NOTE this fails if parent is also suspended...
+ status = hub_port_resume(hdev_to_hub(udev->parent),
+ udev->portnum, 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);
- }
+ if (status < 0)
+ dev_dbg(&udev->dev, "can't resume, status %d\n", status);
return status;
}
@@ -1820,17 +1794,16 @@ static int hub_suspend(struct usb_interf
struct usb_device *hdev = hub->hdev;
unsigned port1;
- /* fail if children aren't already suspended */
+ /* all children must already be suspended or frozen */
for (port1 = 1; port1 <= hdev->maxchild; port1++) {
struct usb_device *udev;
+ /* Regardless of whether CONFIG_USB_SUSPEND is set,
+ * the power_state member records the fictitious state
+ * we want to check here.
+ */
udev = hdev->children [port1-1];
- if (udev && (udev->dev.power.power_state.event
- == PM_EVENT_ON
-#ifdef CONFIG_USB_SUSPEND
- || udev->state != USB_STATE_SUSPENDED
-#endif
- )) {
+ if (udev && udev->dev.power.power_state.event == PM_EVENT_ON) {
dev_dbg(&intf->dev, "port %d nyet suspended\n", port1);
return -EBUSY;
}
@@ -1885,6 +1858,15 @@ static int hub_resume(struct usb_interfa
return 0;
}
+#else /* CONFIG_PM */
+
+static inline int remote_wakeup(struct usb_device *udev)
+{
+ return 0;
+}
+
+#endif
+
void usb_suspend_root_hub(struct usb_device *hdev)
{
struct usb_hub *hub = hdev_to_hub(hdev);
@@ -1894,7 +1876,7 @@ void usb_suspend_root_hub(struct usb_dev
* that the root hub status URB will be canceled.
*/
__hub_quiesce(hub);
- mark_quiesced(to_usb_interface(hub->intfdev));
+ to_usb_interface(hub->intfdev)->is_active = 0;
}
void usb_resume_root_hub(struct usb_device *hdev)
Index: usb-2.6/drivers/usb/core/driver.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/driver.c
+++ usb-2.6/drivers/usb/core/driver.c
@@ -182,42 +182,46 @@ static void generic_disconnect(struct us
usb_set_configuration(udev, 0);
usb_remove_sysfs_dev_files(udev);
-
- /* in case the call failed or the device was suspended */
- if (udev->state >= USB_STATE_CONFIGURED)
- usb_disable_device(udev, 0);
}
#ifdef CONFIG_PM
-static int verify_suspended(struct device *dev, void *unused)
-{
- return (dev->power.power_state.event == PM_EVENT_ON) ? -EBUSY : 0;
-}
-
static int generic_suspend(struct usb_interface *_udev, pm_message_t msg)
{
struct usb_device *udev = (struct usb_device *) _udev;
- int status;
- /* rule out bogus requests through sysfs */
- status = device_for_each_child(&udev->dev, NULL, verify_suspended);
- if (status)
- return status;
+ /* all interfaces must already be suspended */
+ if (udev->actconfig) {
+ int i;
+
+ for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *intf =
+ udev->actconfig->interface[i];
+
+ if (intf->is_active) {
+ dev_dbg(&intf->dev, "nyet suspended\n");
+ return -EBUSY;
+ }
+ }
+ }
/* 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).
*/
- return usb_port_suspend(udev);
+ if (msg.event == PM_EVENT_FREEZE)
+ return 0;
+ return usb_port_suspend(udev);
}
static int generic_resume(struct usb_interface *_udev)
{
struct usb_device *udev = (struct usb_device *) _udev;
- if (udev->state == USB_STATE_NOTATTACHED)
- return 0;
+ /* We can't resume if our parent is suspended */
+ if (udev->parent && udev->parent->state == USB_STATE_SUSPENDED) {
+ dev_warn(&udev->dev, "can't resume; parent is suspended\n");
+ return -EHOSTUNREACH;
+ }
return usb_port_resume(udev);
}
@@ -428,11 +432,19 @@ static int usb_probe_interface(struct de
* state whatsoever. We use it to record when it's bound to
* a driver that may start I/0: it's not frozen/quiesced.
*/
- mark_active(intf);
+ intf->is_active = 1;
intf->condition = USB_INTERFACE_BINDING;
+
+ /* Try to make sure the driver sees altsetting 0 */
+ if (intf->cur_altsetting->desc.bAlternateSetting != 0)
+ usb_set_interface(interface_to_usbdev(intf),
+ intf->altsetting[0].desc.
+ bInterfaceNumber,
+ 0);
+
error = driver->probe(intf, id);
if (error) {
- mark_quiesced(intf);
+ intf->is_active = 0;
intf->condition = USB_INTERFACE_UNBOUND;
} else
intf->condition = USB_INTERFACE_BOUND;
@@ -457,13 +469,9 @@ static int usb_unbind_interface(struct d
if (driver->disconnect)
driver->disconnect(intf);
- /* reset other interface state */
- usb_set_interface(interface_to_usbdev(intf),
- intf->altsetting[0].desc.bInterfaceNumber,
- 0);
usb_set_intfdata(intf, NULL);
intf->condition = USB_INTERFACE_UNBOUND;
- mark_quiesced(intf);
+ intf->is_active = 0;
return 0;
}
@@ -499,7 +507,7 @@ int usb_driver_claim_interface(struct us
dev->driver = &driver->driver;
usb_set_intfdata(iface, priv);
iface->condition = USB_INTERFACE_BOUND;
- mark_active(iface);
+ iface->is_active = 1;
/* if interface was already added, bind now; else let
* the future device_add() bind it, bypassing probe()
@@ -547,7 +555,7 @@ void usb_driver_release_interface(struct
dev->driver = NULL;
usb_set_intfdata(iface, NULL);
iface->condition = USB_INTERFACE_UNBOUND;
- mark_quiesced(iface);
+ iface->is_active = 0;
}
EXPORT_SYMBOL(usb_driver_release_interface);
@@ -930,14 +938,19 @@ EXPORT_SYMBOL_GPL_FUTURE(usb_deregister)
static int suspend_device(struct usb_device *udev, pm_message_t msg)
{
struct usb_driver *driver;
- int status;
+ int status = 0;
- if (udev->dev.power.power_state.event != PM_EVENT_ON
- || udev->dev.driver == NULL)
- return 0;
+ if (udev->state == USB_STATE_NOTATTACHED)
+ goto done;
+ if (udev->state == USB_STATE_SUSPENDED || udev->dev.driver == NULL)
+ goto done;
driver = to_usb_driver(udev->dev.driver);
status = driver->suspend((struct usb_interface *) udev, msg);
+
+done:
+ if (status == 0)
+ udev->dev.power.power_state.event = msg.event;
return status;
}
@@ -945,24 +958,19 @@ static int suspend_device(struct usb_dev
static int resume_device(struct usb_device *udev)
{
struct usb_driver *driver;
- int status;
-
- if (udev->dev.power.power_state.event == PM_EVENT_ON)
- return 0;
-
- /* mark things as "on" immediately, no matter what errors crop up */
- udev->dev.power.power_state.event = PM_EVENT_ON;
-
- if (udev->dev.driver == NULL) {
- udev->dev.power.power_state.event = PM_EVENT_FREEZE;
- return 0;
- }
+ int status = 0;
if (udev->state == USB_STATE_NOTATTACHED)
- return 0;
+ return -ENODEV;
+ if (udev->state != USB_STATE_SUSPENDED || udev->dev.driver == NULL)
+ goto done;
driver = to_usb_driver(udev->dev.driver);
status = driver->resume((struct usb_interface *) udev);
+
+done:
+ if (status == 0)
+ udev->dev.power.power_state.event = PM_EVENT_ON;
return status;
}
@@ -970,15 +978,12 @@ static int resume_device(struct usb_devi
static int suspend_interface(struct usb_interface *intf, pm_message_t msg)
{
struct usb_driver *driver;
- int status;
-
- if (intf->dev.power.power_state.event != PM_EVENT_ON
- || intf->dev.driver == NULL)
- return 0;
+ int status = 0;
- /* with no hardware, USB interfaces only use FREEZE and ON states */
- if (!is_active(intf))
- return 0;
+ if (interface_to_usbdev(intf)->state == USB_STATE_NOTATTACHED)
+ goto done;
+ if (!intf->is_active || intf->dev.driver == NULL)
+ goto done;
driver = to_usb_driver(intf->dev.driver);
if (driver->suspend && driver->resume) {
@@ -987,14 +992,18 @@ static int suspend_interface(struct usb_
dev_err(&intf->dev, "%s error %d\n",
"suspend", status);
else
- mark_quiesced(intf);
+ intf->is_active = 0;
} else {
// FIXME else if there's no suspend method, disconnect...
dev_warn(&intf->dev, "no suspend for driver %s?\n",
driver->name);
- mark_quiesced(intf);
status = 0;
+ intf->is_active = 0;
}
+
+done:
+ if (status == 0)
+ intf->dev.power.power_state.event = msg.event;
return status;
}
@@ -1002,38 +1011,32 @@ static int suspend_interface(struct usb_
static int resume_interface(struct usb_interface *intf)
{
struct usb_driver *driver;
- int status;
-
- if (intf->dev.power.power_state.event == PM_EVENT_ON)
- return 0;
-
- /* mark things as "on" immediately, no matter what errors crop up */
- intf->dev.power.power_state.event = PM_EVENT_ON;
-
- if (intf->dev.driver == NULL) {
- intf->dev.power.power_state.event = PM_EVENT_FREEZE;
- return 0;
- }
+ int status = 0;
if (interface_to_usbdev(intf)->state == USB_STATE_NOTATTACHED)
- return 0;
+ return -ENODEV;
+ if (intf->is_active || intf->dev.driver == NULL)
+ goto done;
- /* if driver was suspended, it has a resume method;
- * however, sysfs can wrongly mark things as suspended
- * (on the "no suspend method" FIXME path above)
- */
driver = to_usb_driver(intf->dev.driver);
if (driver->resume) {
status = driver->resume(intf);
- if (status) {
+ if (status)
dev_err(&intf->dev, "%s error %d\n",
"resume", status);
- mark_quiesced(intf);
- }
- } else
+ else
+ intf->is_active = 1;
+ } else {
dev_warn(&intf->dev, "no resume for driver %s?\n",
driver->name);
- return 0;
+ status = 0;
+ intf->is_active = 1;
+ }
+
+done:
+ if (status == 0)
+ intf->dev.power.power_state.event = PM_EVENT_ON;
+ return status;
}
/* Caller has locked udev */
@@ -1070,8 +1073,11 @@ static int usb_core_resume(struct device
if (is_usb_device(dev))
status = resume_device(to_usb_device(dev));
- else
+ else {
status = resume_interface(to_usb_interface(dev));
+
+ /* Rebind drivers that had no suspend method? */
+ }
return status;
}
Index: usb-2.6/drivers/usb/core/usb.h
===================================================================
--- usb-2.6.orig/drivers/usb/core/usb.h
+++ usb-2.6/drivers/usb/core/usb.h
@@ -27,10 +27,20 @@ extern void usb_major_cleanup(void);
extern int usb_host_init(void);
extern void usb_host_cleanup(void);
+#ifdef CONFIG_PM
+
extern void 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);
+#else
+
+#define usb_resume_both(udev) do {} while (0)
+#define usb_port_suspend(dev) do {} while (0)
+#define usb_port_resume(dev) do {} while (0)
+
+#endif
+
extern struct bus_type usb_bus_type;
extern struct usb_driver usb_generic_driver;
@@ -43,24 +53,6 @@ static inline int is_usb_device(struct d
return dev->platform_data == &usb_generic_driver;
}
-/* Interfaces and their "power state" are owned by usbcore */
-
-static inline void mark_active(struct usb_interface *f)
-{
- f->dev.power.power_state.event = PM_EVENT_ON;
-}
-
-static inline void mark_quiesced(struct usb_interface *f)
-{
- f->dev.power.power_state.event = PM_EVENT_FREEZE;
-}
-
-static inline int is_active(struct usb_interface *f)
-{
- return f->dev.power.power_state.event == PM_EVENT_ON;
-}
-
-
/* for labeling diagnostics */
extern const char *usbcore_name;
Index: usb-2.6/drivers/usb/core/message.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/message.c
+++ usb-2.6/drivers/usb/core/message.c
@@ -1474,7 +1474,6 @@ free_interfaces:
intf->dev.dma_mask = dev->dev.dma_mask;
intf->dev.release = release_interface;
device_initialize (&intf->dev);
- mark_quiesced(intf);
sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
dev->bus->busnum, dev->devpath,
configuration, alt->desc.bInterfaceNumber);
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
