Skip to content

Commit

Permalink
usbcore: fix up device and power state tests
Browse files Browse the repository at this point in the history 
This patch (as734) rationalizes the various tests of device state and
power states.  There are duplications and mistaken tests in several
places.

Perhaps the most interesting challenge is where the hub driver tests to
see that all the child devices are suspended before allowing itself to
be suspended.  When CONFIG_USB_SUSPEND is set the test is
straightforward, since we expect that the children _will_ be suspended.
But when CONFIG_USB_SUSPEND isn't set, it's not so clear what should be
done.  The code compromises by checking the child's
power.power_state.event field.


Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Alan Stern authored and Greg Kroah-Hartman committed Sep 27, 2006
1 parent 2bf4086 commit 114b368
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 61 deletions.
40 changes: 19 additions & 21 deletions drivers/usb/core/driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,11 +757,13 @@ static int suspend_device(struct usb_device *udev, pm_message_t msg)
struct usb_device_driver *udriver;
int status = 0;

if (udev->state == USB_STATE_NOTATTACHED ||
udev->state == USB_STATE_SUSPENDED)
goto done;

if (udev->dev.driver == NULL)
goto done;
udriver = to_usb_device_driver(udev->dev.driver);
if (udev->dev.power.power_state.event == msg.event)
goto done;
status = udriver->suspend(udev, msg);

done:
Expand All @@ -776,14 +778,13 @@ static int resume_device(struct usb_device *udev)
struct usb_device_driver *udriver;
int status = 0;

if (udev->dev.power.power_state.event == PM_EVENT_ON)
if (udev->state == USB_STATE_NOTATTACHED ||
udev->state != USB_STATE_SUSPENDED)
goto done;

if (udev->dev.driver == NULL)
goto done;
udriver = to_usb_device_driver(udev->dev.driver);
if (udev->state == USB_STATE_NOTATTACHED)
goto done;
status = udriver->resume(udev);

done:
Expand All @@ -798,14 +799,14 @@ static int suspend_interface(struct usb_interface *intf, pm_message_t msg)
struct usb_driver *driver;
int status = 0;

if (intf->dev.driver == NULL)
/* with no hardware, USB interfaces only use FREEZE and ON states */
if (interface_to_usbdev(intf)->state == USB_STATE_NOTATTACHED ||
!is_active(intf))
goto done;

driver = to_usb_driver(intf->dev.driver);

/* with no hardware, USB interfaces only use FREEZE and ON states */
if (!is_active(intf))
if (intf->dev.driver == NULL)
goto done;
driver = to_usb_driver(intf->dev.driver);

if (driver->suspend && driver->resume) {
status = driver->suspend(intf, msg);
Expand All @@ -831,25 +832,16 @@ static int suspend_interface(struct usb_interface *intf, pm_message_t msg)
static int resume_interface(struct usb_interface *intf)
{
struct usb_driver *driver;
struct usb_device *udev;
int status = 0;

if (intf->dev.power.power_state.event == PM_EVENT_ON)
if (interface_to_usbdev(intf)->state == USB_STATE_NOTATTACHED ||
is_active(intf))
goto done;

if (intf->dev.driver == NULL)
goto done;

driver = to_usb_driver(intf->dev.driver);

udev = interface_to_usbdev(intf);
if (udev->state == USB_STATE_NOTATTACHED)
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)
*/
if (driver->resume) {
status = driver->resume(intf);
if (status)
Expand Down Expand Up @@ -904,6 +896,12 @@ int usb_resume_both(struct usb_device *udev)
int i;
struct usb_interface *intf;

/* Can't resume if the 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;
}

status = resume_device(udev);
if (status == 0 && udev->actconfig) {
for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) {
Expand Down
3 changes: 0 additions & 3 deletions drivers/usb/core/generic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,6 @@ static int generic_suspend(struct usb_device *udev, pm_message_t msg)

static int generic_resume(struct usb_device *udev)
{
if (udev->state == USB_STATE_NOTATTACHED)
return 0;

return usb_port_resume(udev);
}

Expand Down
47 changes: 10 additions & 37 deletions drivers/usb/core/hub.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1556,26 +1556,6 @@ static int __usb_port_suspend (struct usb_device *udev, int port1)
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.
*/
Expand Down Expand Up @@ -1614,8 +1594,6 @@ static int __usb_port_suspend (struct usb_device *udev, int port1)
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
return 0;
Expand Down Expand Up @@ -1761,24 +1739,17 @@ hub_port_resume(struct usb_hub *hub, int port1, struct usb_device *udev)
*/
int usb_port_resume(struct usb_device *udev)
{
int status;

if (udev->state == USB_STATE_NOTATTACHED)
return -ENODEV;
int status = 0;

/* 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
// NOTE this fails if parent is also suspended...
status = hub_port_resume(hdev_to_hub(udev->parent),
udev->portnum, udev);
#endif
status = 0;
} else
status = finish_port_resume(udev);
if (status < 0)
Expand Down Expand Up @@ -1821,12 +1792,14 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
struct usb_device *udev;

udev = hdev->children [port1-1];
if (udev && (udev->dev.power.power_state.event
== PM_EVENT_ON
if (udev && msg.event == PM_EVENT_SUSPEND &&
#ifdef CONFIG_USB_SUSPEND
|| udev->state != USB_STATE_SUSPENDED
udev->state != USB_STATE_SUSPENDED
#else
udev->dev.power.power_state.event
== PM_EVENT_ON
#endif
)) {
) {
dev_dbg(&intf->dev, "port %d nyet suspended\n", port1);
return -EBUSY;
}
Expand Down

0 comments on commit 114b368

Please sign in to comment.