USB: move usbcore away from hcd->state

The hcd->state variable is a disaster.  It's not clearly owned by
either usbcore or the host controller drivers, and they both change it
from time to time, potentially stepping on each other's toes.  It's
not protected by any locks.  And there's no mechanism to prevent it
from going through an invalid transition.

This patch (as1451) takes a first step toward fixing these problems.
As it turns out, usbcore uses hcd->state for essentially only two
things: checking whether the controller's root hub is running and
checking whether the controller has died.  Therefore the patch adds
two new atomic bitflags to the hcd structure, to store these pieces of
information.  The new flags are used only by usbcore, and a private
spinlock prevents invalid combinations (a dead controller's root hub
cannot be running).

The patch does not change the places where usbcore sets hcd->state,
since HCDs may depend on them.  Furthermore, there is one place in
usb_hcd_irq() where usbcore still must use hcd->state: An HCD's
interrupt handler can implicitly indicate that the controller died by
setting hcd->state to HC_STATE_HALT.  Nevertheless, the new code is a
big improvement over the current code.

The patch makes one other change.  The hcd_bus_suspend() and
hcd_bus_resume() routines now check first whether the host controller
has died; if it has then they return immediately without calling the
HCD's bus_suspend or bus_resume methods.

This fixes the major problem reported in Bugzilla #29902: The system
fails to suspend after a host controller dies during system resume.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Alex Terekhov <a.terekhov@gmail.com>
CC: <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 6 insertions, 7 deletions

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index f71e8e307e0f..d37088591d9a 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -363,8 +363,7 @@ static int check_root_hub_suspended(struct device *dev)
 	struct pci_dev		*pci_dev = to_pci_dev(dev);
 	struct usb_hcd		*hcd = pci_get_drvdata(pci_dev);
 
-	if (!(hcd->state == HC_STATE_SUSPENDED ||
-			hcd->state == HC_STATE_HALT)) {
+	if (HCD_RH_RUNNING(hcd)) {
 		dev_warn(dev, "Root hub is not suspended\n");
 		return -EBUSY;
 	}
@@ -386,7 +385,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
 	if (retval)
 		return retval;
 
-	if (hcd->driver->pci_suspend) {
+	if (hcd->driver->pci_suspend && !HCD_DEAD(hcd)) {
 		/* Optimization: Don't suspend if a root-hub wakeup is
 		 * pending and it would cause the HCD to wake up anyway.
 		 */
@@ -427,7 +426,7 @@ static int resume_common(struct device *dev, int event)
 	struct usb_hcd		*hcd = pci_get_drvdata(pci_dev);
 	int			retval;
 
-	if (hcd->state != HC_STATE_SUSPENDED) {
+	if (HCD_RH_RUNNING(hcd)) {
 		dev_dbg(dev, "can't resume, not suspended!\n");
 		return 0;
 	}
@@ -442,7 +441,7 @@ static int resume_common(struct device *dev, int event)
 
 	clear_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
 
-	if (hcd->driver->pci_resume) {
+	if (hcd->driver->pci_resume && !HCD_DEAD(hcd)) {
 		if (event != PM_EVENT_AUTO_RESUME)
 			wait_for_companions(pci_dev, hcd);
 
@@ -475,10 +474,10 @@ static int hcd_pci_suspend_noirq(struct device *dev)
 
 	pci_save_state(pci_dev);
 
-	/* If the root hub is HALTed rather than SUSPENDed,
+	/* If the root hub is dead rather than suspended,
 	 * disallow remote wakeup.
 	 */
-	if (hcd->state == HC_STATE_HALT)
+	if (HCD_DEAD(hcd))
 		device_set_wakeup_enable(dev, 0);
 	dev_dbg(dev, "wakeup: %d\n", device_may_wakeup(dev));