[PATCH] usbcore and system sleep states

This patch makes usbcore behave better with system sleep states, fixing
some PM/PCI integration problems as well as some PM/USB ones.

 - PCI HCDs should behave better, even if they don't support PCI PM;
   tested with both ACPI and APM suspend cycles, and swsusp.

 - Systems that advertise ACPI S1 support don't necessarily support the
   PCI D1 or D2 states for their USB controllers.  (Examples include
   Centrino laptops.)  This patch makes them suspend into D3hot, which
   always works (and won't take much longer to resume, either).

 - Handle the "go into deeper suspend" transition the way PCI drivers
   are supposed to.

 - Understand that USB only has one suspend state:  if the device is
   suspended, it can't do any better.

 - Export some symbols to HCDs, so they can reuse more of the existing
   usbcore framework.

Plus minor related cleanups.

The PCI D1/D2 state issue is a variant of an earlier problem, with the
same root cause:  the PM core thinking there's a one-to-one mapping
between system states (roughly:  ACPI S0/S1/S3/S4) and PCI states that
a given device can support (only D0 is required, D3hot is common).
That same issue comes up again with USB.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/usb/core/hcd-pci.c

@@ -78,6 +78,7 @@ int usb_hcd_pci_probe (struct pci_dev *dev, const struct pci_device_id *id)
 
 	if (pci_enable_device (dev) < 0)
 		return -ENODEV;
+	dev->current_state = 0;
 	
         if (!dev->irq) {
         	dev_err (&dev->dev,
@@ -268,6 +269,18 @@ EXPORT_SYMBOL (usb_hcd_pci_remove);
 
 #ifdef	CONFIG_PM
 
+static char __attribute_used__ *pci_state(u32 state)
+{
+	switch (state) {
+	case 0:		return "D0";
+	case 1:		return "D1";
+	case 2:		return "D2";
+	case 3:		return "D3hot";
+	case 4:		return "D3cold";
+	}
+	return NULL;
+}
+
 /**
  * usb_hcd_pci_suspend - power management suspend of a PCI-based HCD
  * @dev: USB Host Controller being suspended
@@ -288,16 +301,32 @@ int usb_hcd_pci_suspend (struct pci_dev *dev, u32 state)
 	 * PM-sensitive HCDs may already have done this.
 	 */
 	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (has_pci_pm)
-		dev_dbg(hcd->self.controller, "suspend D%d --> D%d\n",
-			dev->current_state, state);
+	if (state > 4)
+		state = 4;
 
 	switch (hcd->state) {
 	case USB_STATE_HALT:
 		dev_dbg (hcd->self.controller, "halted; hcd not suspended\n");
 		break;
 	case HCD_STATE_SUSPENDED:
-		dev_dbg (hcd->self.controller, "hcd already suspended\n");
+		dev_dbg (hcd->self.controller, "PCI %s --> %s\n",
+				pci_state(dev->current_state),
+				pci_state(has_pci_pm ? state : 0));
+		if (state > 3)
+			state = 3;
+
+		if (state == dev->current_state)
+			break;
+		else if (state < dev->current_state)
+			retval = -EIO;
+		else if (has_pci_pm)
+			retval = pci_set_power_state (dev, state);
+
+		if (retval == 0)
+			dev->dev.power.power_state = state;
+		else
+			dev_dbg (hcd->self.controller, 
+					"re-suspend fail, %d\n", retval);
 		break;
 	default:
 		retval = hcd->driver->suspend (hcd, state);
@@ -308,22 +337,47 @@ int usb_hcd_pci_suspend (struct pci_dev *dev, u32 state)
 		else {
 			hcd->state = HCD_STATE_SUSPENDED;
 			pci_save_state (dev);
-#ifdef	CONFIG_USB_SUSPEND
-			pci_enable_wake (dev, state, hcd->remote_wakeup);
-			pci_enable_wake (dev, 4, hcd->remote_wakeup);
-#endif
+
 			/* no DMA or IRQs except in D0 */
 			pci_disable_device (dev);
 			free_irq (hcd->irq, hcd);
 			
-			if (has_pci_pm)
+			if (has_pci_pm) {
 				retval = pci_set_power_state (dev, state);
-			dev->dev.power.power_state = state;
+
+				/* POLICY: ignore D1/D2/D3hot differences;
+				 * we know D3hot will always work.
+				 */
+				if (retval < 0 && state < 3) {
+					retval = pci_set_power_state (dev, 3);
+					if (retval == 0)
+						state = 3;
+				}
+				if (retval == 0) {
+					dev->dev.power.power_state = state;
+#ifdef	CONFIG_USB_SUSPEND
+					pci_enable_wake (dev, state,
+							hcd->remote_wakeup);
+					pci_enable_wake (dev, 4,
+							hcd->remote_wakeup);
+#endif
+				}
+			} else {
+				if (state > 3)
+					state = 3;
+				dev->dev.power.power_state = state;
+			}
 			if (retval < 0) {
 				dev_dbg (&dev->dev,
-						"PCI suspend fail, %d\n",
+						"PCI %s suspend fail, %d\n",
+						pci_state(state),
 						retval);
 				(void) usb_hcd_pci_resume (dev);
+			} else {
+				dev_dbg(hcd->self.controller,
+					"suspended to PCI %s%s\n",
+					pci_state(dev->current_state),
+					has_pci_pm ? "" : " (legacy)");
 			}
 		}
 	}
@@ -345,9 +399,11 @@ int usb_hcd_pci_resume (struct pci_dev *dev)
 
 	hcd = pci_get_drvdata(dev);
 	has_pci_pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (has_pci_pm)
-		dev_dbg(hcd->self.controller, "resume from state D%d\n",
-				dev->current_state);
+
+	/* D3cold resume isn't usually reported this way... */
+	dev_dbg(hcd->self.controller, "resume from PCI %s%s\n",
+			pci_state(dev->current_state),
+			has_pci_pm ? "" : " (legacy)");
 
 	if (hcd->state != HCD_STATE_SUSPENDED) {
 		dev_dbg (hcd->self.controller, 
@@ -366,6 +422,7 @@ int usb_hcd_pci_resume (struct pci_dev *dev)
 			"can't restore IRQ after resume!\n");
 		return retval;
 	}
+	hcd->saw_irq = 0;
 	pci_set_master (dev);
 	pci_restore_state (dev);
 #ifdef	CONFIG_USB_SUSPEND

drivers/usb/core/hcd.c

@@ -435,8 +435,6 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
 		/* non-generic request */
 		if (HCD_IS_SUSPENDED (hcd->state))
 			urb->status = -EAGAIN;
-		else if (!HCD_IS_RUNNING (hcd->state))
-			urb->status = -ENODEV;
 		else
 			urb->status = hcd->driver->hub_control (hcd,
 				typeReq, wValue, wIndex,
@@ -445,8 +443,6 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
 error:
 		/* "protocol stall" on error */
 		urb->status = -EPIPE;
-		dev_dbg (hcd->self.controller, "unsupported hub control message (maxchild %d)\n",
-				urb->dev->maxchild);
 	}
 	if (urb->status) {
 		urb->actual_length = 0;
@@ -540,7 +536,7 @@ static void rh_report_status (unsigned long ptr)
 		urb->actual_length = length;
 		urb->status = 0;
 		urb->hcpriv = NULL;
-	} else
+	} else if (!urb->dev->dev.power.power_state)
 		mod_timer (&hcd->rh_timer, jiffies + HZ/4);
 	spin_unlock (&hcd_data_lock);
 	spin_unlock (&urb->lock);

drivers/usb/core/hub.c

@@ -1516,7 +1516,7 @@ static int hub_port_suspend(struct usb_device *hdev, int port)
  * Linux (2.6) currently has NO mechanisms to initiate that:  no khubd
  * timer, no SRP, no requests through sysfs.
  */
-static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
+int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 {
 	int	status;
 
@@ -1524,9 +1524,7 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 	if (port < 0)
 		return port;
 
-	if (state <= udev->dev.power.power_state
-			|| state < PM_SUSPEND_MEM
-			|| udev->state == USB_STATE_SUSPENDED
+	if (udev->dev.power.power_state
 			|| udev->state == USB_STATE_NOTATTACHED) {
 		return 0;
 	}
@@ -1590,7 +1588,6 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 	 */
 	if (state > PM_SUSPEND_MEM) {
 		dev_warn(&udev->dev, "no poweroff yet, suspending instead\n");
-		state = PM_SUSPEND_MEM;
 	}
 
 	/* "global suspend" of the HC-to-USB interface (root hub), or
@@ -1606,9 +1603,11 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
 		status = hub_port_suspend(udev->parent, port);
 
 	if (status == 0)
-		udev->dev.power.power_state = state;
+		udev->dev.power.power_state = PM_SUSPEND_MEM;
+	up(&udev->serialize);
 	return status;
 }
+EXPORT_SYMBOL(__usb_suspend_device);
 
 /**
  * usb_suspend_device - suspend a usb device
@@ -1821,6 +1820,7 @@ int usb_resume_device(struct usb_device *udev)
 					->actconfig->interface[0]);
 		}
 	} else if (udev->state == USB_STATE_SUSPENDED) {
+		// NOTE this fails if parent is also suspended...
 		status = hub_port_resume(udev->parent, port);
 	} else {
 		status = 0;
@@ -1834,10 +1834,11 @@ int usb_resume_device(struct usb_device *udev)
 	usb_unlock_device(udev);
 
 	/* rebind drivers that had no suspend() */
-	usb_lock_all_devices();
-	bus_rescan_devices(&usb_bus_type);
-	usb_unlock_all_devices();
-
+	if (status == 0) {
+		usb_lock_all_devices();
+		bus_rescan_devices(&usb_bus_type);
+		usb_unlock_all_devices();
+	}
 	return status;
 }
 
@@ -1895,6 +1896,9 @@ static int hub_resume(struct usb_interface *intf)
 	unsigned		port;
 	int			status;
 
+	if (intf->dev.power.power_state == PM_SUSPEND_ON)
+		return 0;
+
 	for (port = 0; port < hdev->maxchild; port++) {
 		struct usb_device	*udev;
 		u16			portstat, portchange;
@@ -1913,7 +1917,7 @@ static int hub_resume(struct usb_interface *intf)
 				continue;
 		}
 
-		if (!udev)
+		if (!udev || status < 0)
 			continue;
 		down (&udev->serialize);
 		if (portstat & USB_PORT_STAT_SUSPEND)

drivers/usb/core/usb.c

@@ -63,7 +63,8 @@ const char *usbcore_name = "usbcore";
 int nousb;		/* Disable USB when built into kernel image */
 			/* Not honored on modular build */
 
-static DECLARE_RWSEM(usb_all_devices_rwsem);
+DECLARE_RWSEM(usb_all_devices_rwsem);
+EXPORT_SYMBOL(usb_all_devices_rwsem);
 
 
 static int generic_probe (struct device *dev)
@@ -97,6 +98,7 @@ int usb_probe_interface(struct device *dev)
 
 	if (!driver->probe)
 		return error;
+	/* FIXME we'd much prefer to just resume it ... */
 	if (interface_to_usbdev(intf)->state == USB_STATE_SUSPENDED)
 		return -EHOSTUNREACH;
 
@@ -1393,6 +1395,10 @@ 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);