[PATCH] USB: usb PM updates, root hub stuff (2/4)

Makes usbcore handle some suspend scenarios more consistently, especially
when talking with root hubs.

 - Use usb_device->state, not device->power.power_state.  (The driver model
   power_state is still needed for interfaces.)  With USB_SUSPEND=n, there
   are also cases where the HCD_STATE_SUSPENDED needs to be used instead
   of testing for USB_STATE_SUSPENDED.

 - Updates usb_device->state for root hubs during suspend/resume.

 - Fixes a locking bug (extra "up") that got merged recently, affecting
   CONFIG_USB_SUSPEND.

 - Recover one of the "HC died" cases better.

 - Root hub timer updates, handling various suspend transitions more
   consistently:  don't duplicate earlier submit checks, only work
   when hub is configured, address various submit/resume races.

Together, these help hubs work better regardless of whether USB_SUSPEND
has been enabled (that's the only way USB_STATE_SUSPENDED appears), and
whether or not the HCD is marked as suspended (or maybe Alan would
want to call that "half suspended").
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/usb/core/hcd.c

@@ -479,6 +479,11 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
 /*
  * Root Hub interrupt transfers are synthesized with a timer.
  * Completions are called in_interrupt() but not in_irq().
+ *
+ * Note: some root hubs (including common UHCI based designs) can't
+ * correctly issue port change IRQs.  They're the ones that _need_ a
+ * timer; most other root hubs don't.  Some systems could save a
+ * lot of battery power by eliminating these root hub timer IRQs.
  */
 
 static void rh_report_status (unsigned long ptr);
@@ -488,10 +493,7 @@ static int rh_status_urb (struct usb_hcd *hcd, struct urb *urb)
 	int	len = 1 + (urb->dev->maxchild / 8);
 
 	/* rh_timer protected by hcd_data_lock */
-	if (hcd->rh_timer.data
-			|| urb->status != -EINPROGRESS
-			|| urb->transfer_buffer_length < len
-			|| !HCD_IS_RUNNING (hcd->state)) {
+	if (hcd->rh_timer.data || urb->transfer_buffer_length < len) {
 		dev_dbg (hcd->self.controller,
 				"not queuing rh status urb, stat %d\n",
 				urb->status);
@@ -530,19 +532,19 @@ static void rh_report_status (unsigned long ptr)
 		return;
 	}
 
-	if (!HCD_IS_SUSPENDED (hcd->state))
-		length = hcd->driver->hub_status_data (
-					hcd, urb->transfer_buffer);
-
 	/* complete the status urb, or retrigger the timer */
 	spin_lock (&hcd_data_lock);
+	if (urb->dev->state == USB_STATE_CONFIGURED) {
+		length = hcd->driver->hub_status_data (
+					hcd, urb->transfer_buffer);
 		if (length > 0) {
 			hcd->rh_timer.data = 0;
 			urb->actual_length = length;
 			urb->status = 0;
 			urb->hcpriv = NULL;
-	} else if (!urb->dev->dev.power.power_state)
+		} else
 			mod_timer (&hcd->rh_timer, jiffies + HZ/4);
+	}
 	spin_unlock (&hcd_data_lock);
 	spin_unlock (&urb->lock);
 
@@ -1103,13 +1105,17 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
 	spin_lock_irqsave (&hcd_data_lock, flags);
 	if (unlikely (urb->reject))
 		status = -EPERM;
-	else if (HCD_IS_RUNNING (hcd->state) &&
-			hcd->state != USB_STATE_QUIESCING) {
+	else switch (hcd->state) {
+	case USB_STATE_RUNNING:
+	case USB_STATE_RESUMING:
 		usb_get_dev (urb->dev);
 		list_add_tail (&urb->urb_list, &dev->urb_list);
 		status = 0;
-	} else
+		break;
+	default:
 		status = -ESHUTDOWN;
+		break;
+	}
 	spin_unlock_irqrestore (&hcd_data_lock, flags);
 	if (status) {
 		INIT_LIST_HEAD (&urb->urb_list);

drivers/usb/core/hub.c

@@ -1373,7 +1373,10 @@ static int hub_port_reset(struct usb_device *hdev, int port,
 			status = hub_port_wait_reset(hdev, port, udev, delay);
 
 		/* return on disconnect or reset */
-		if (status == -ENOTCONN || status == 0) {
+		switch (status) {
+		case 0:
+		case -ENOTCONN:
+		case -ENODEV:
 			clear_port_feature(hdev,
 				port + 1, USB_PORT_FEAT_C_RESET);
 			/* FIXME need disconnect() for NOTATTACHED device */
@@ -1524,7 +1527,7 @@ int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 	if (port < 0)
 		return port;
 
-	if (udev->dev.power.power_state
+	if (udev->state == USB_STATE_SUSPENDED
 			|| udev->state == USB_STATE_NOTATTACHED) {
 		return 0;
 	}
@@ -1595,16 +1598,16 @@ int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 	 */
 	if (!udev->parent) {
 		struct usb_bus	*bus = udev->bus;
-		if (bus && bus->op->hub_suspend)
+		if (bus && bus->op->hub_suspend) {
 			status = bus->op->hub_suspend (bus);
-		else
+			if (status == 0)
+				usb_set_device_state(udev,
+						USB_STATE_SUSPENDED);
+		} else
 			status = -EOPNOTSUPP;
 	} else
 		status = hub_port_suspend(udev->parent, port);
 
-	if (status == 0)
-		udev->dev.power.power_state = PM_SUSPEND_MEM;
-	up(&udev->serialize);
 	return status;
 }
 EXPORT_SYMBOL(__usb_suspend_device);
@@ -1652,7 +1655,6 @@ static int finish_port_resume(struct usb_device *udev)
 
 	/* caller owns the udev device lock */
 	dev_dbg(&udev->dev, "usb resume\n");
-	udev->dev.power.power_state = PM_SUSPEND_ON;
 
 	/* usb ch9 identifies four variants of SUSPENDED, based on what
 	 * state the device resumes to.  Linux currently won't see the
@@ -1809,14 +1811,15 @@ int usb_resume_device(struct usb_device *udev)
 	 */
 	if (!udev->parent) {
 		struct usb_bus	*bus = udev->bus;
-		if (bus && bus->op->hub_resume)
+		if (bus && bus->op->hub_resume) {
 			status = bus->op->hub_resume (bus);
-		else
+		} else
 			status = -EOPNOTSUPP;
 		if (status == 0) {
 			/* TRSMRCY = 10 msec */
 			msleep(10);
-			status = hub_resume (bus->root_hub
+			usb_set_device_state (udev, USB_STATE_CONFIGURED);
+			status = hub_resume (udev
 					->actconfig->interface[0]);
 		}
 	} else if (udev->state == USB_STATE_SUSPENDED) {
@@ -1824,7 +1827,6 @@ int usb_resume_device(struct usb_device *udev)
 		status = hub_port_resume(udev->parent, port);
 	} else {
 		status = 0;
-		udev->dev.power.power_state = PM_SUSPEND_ON;
 	}
 	if (status < 0) {
 		dev_dbg(&udev->dev, "can't resume, status %d\n",

drivers/usb/core/usb.c

@@ -1395,10 +1395,6 @@ static int usb_generic_suspend(struct device *dev, u32 state)
 	struct usb_interface *intf;
 	struct usb_driver *driver;
 
-	/* there's only one USB suspend state */
-	if (dev->power.power_state)
-		return 0;
-
 	if (dev->driver == &usb_generic_driver)
 		return usb_suspend_device (to_usb_device(dev), state);
 
@@ -1409,6 +1405,10 @@ static int usb_generic_suspend(struct device *dev, u32 state)
 	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, state);
 	return 0;