[RFC 11/22] USB: Set usb_hcd->state and flags for shared roothubs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The hcd->flags are in a sorry state.  Some of them are clearly specific to
the particular roothub (HCD_POLL_RH, HCD_POLL_PENDING, and
HCD_WAKEUP_PENDING), but some flags are related to PCI device state
(HCD_HW_ACCESSIBLE and HCD_SAW_IRQ).  This is an issue when one PCI device
can have two roothubs that share the same IRQ line and hardware.

Make sure to set HCD_FLAG_SAW_IRQ for both roothubs when an interrupt is
serviced, or an URB is unlinked without an interrupt.  (We can't tell if
the host actually serviced an interrupt for a particular bus, but we can
tell it serviced some interrupt.)

HCD_HW_ACCESSIBLE is set once by usb_add_hcd(), which is set for both
roothubs as they are added, so it doesn't need to be modified.
HCD_POLL_RH and HCD_POLL_PENDING are only checked by the USB core, and
they are never set by the xHCI driver, since the roothub never needs to be
polled.

The usb_hcd's state field is a similar mess.  Sometimes the state applies
to the underlying hardware: HC_STATE_HALT, HC_STATE_RUNNING, and
HC_STATE_QUIESCING.  But sometimes the state refers to the roothub state:
HC_STATE_RESUMING and HC_STATE_SUSPENDED.

This poses an issue with the xHCI split roothub, where two buses are
registered for one PCI device.  Each bus in the xHCI split roothub can be
suspended separately, but both buses must be suspended before the PCI
device can be suspended.  Therefore, make sure that the USB core checks
hcd->state equals HC_STATE_SUSPENDED for both roothubs before suspending
the PCI host.

Make sure to kill off the shared roothub when the PCI resume fails.

The xHCI driver will need to ensure that HC_STATE_HALT, HC_STATE_RUNNING,
and HC_STATE_QUIESCING will be set for both the roothubs.

I'm not quite sure if the code in hcd_pci_suspend_noirq() is correct,
please check.  If one roothub is halted, then both roothubs should be
halted (since they share the same hardware).  But I suppose there could be
a race condition where one usb_hcd->state is set to HC_STATE_HALT, but the
other isn't yet?

Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/core/hcd-pci.c   |   35 ++++++++++++++++++++++++++++++-----
 drivers/usb/core/hcd.c       |    4 ++++
 drivers/usb/host/xhci-ring.c |    2 ++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index cf3fcbe..e8d36e7 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -406,6 +406,14 @@ static int check_root_hub_suspended(struct device *dev)
 		dev_warn(dev, "Root hub is not suspended\n");
 		return -EBUSY;
 	}
+	if (hcd->shared_hcd) {
+		hcd = hcd->shared_hcd;
+		if (!(hcd->state == HC_STATE_SUSPENDED ||
+					hcd->state == HC_STATE_HALT)) {
+			dev_warn(dev, "Secondary root hub is not suspended\n");
+			return -EBUSY;
+		}
+	}
 	return 0;
 }
 
@@ -430,11 +438,16 @@ static int suspend_common(struct device *dev, bool do_wakeup)
 		 */
 		if (do_wakeup && HCD_WAKEUP_PENDING(hcd))
 			return -EBUSY;
+		if (do_wakeup && hcd->shared_hcd &&
+				HCD_WAKEUP_PENDING(hcd->shared_hcd))
+			return -EBUSY;
 		retval = hcd->driver->pci_suspend(hcd, do_wakeup);
 		suspend_report_result(hcd->driver->pci_suspend, retval);
 
 		/* Check again in case wakeup raced with pci_suspend */
-		if (retval == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) {
+		if ((retval == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) ||
+				(retval == 0 && do_wakeup && hcd->shared_hcd &&
+				 HCD_WAKEUP_PENDING(hcd->shared_hcd))) {
 			if (hcd->driver->pci_resume)
 				hcd->driver->pci_resume(hcd, false);
 			retval = -EBUSY;
@@ -465,7 +478,9 @@ 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->state != HC_STATE_SUSPENDED ||
+			(hcd->shared_hcd &&
+			 hcd->shared_hcd->state != HC_STATE_SUSPENDED)) {
 		dev_dbg(dev, "can't resume, not suspended!\n");
 		return 0;
 	}
@@ -479,6 +494,8 @@ static int resume_common(struct device *dev, int event)
 	pci_set_master(pci_dev);
 
 	clear_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
+	if (hcd->shared_hcd)
+		clear_bit(HCD_FLAG_SAW_IRQ, &hcd->shared_hcd->flags);
 
 	if (hcd->driver->pci_resume) {
 		if (event != PM_EVENT_AUTO_RESUME)
@@ -488,6 +505,8 @@ static int resume_common(struct device *dev, int event)
 				event == PM_EVENT_RESTORE);
 		if (retval) {
 			dev_err(dev, "PCI post-resume error %d!\n", retval);
+			if (hcd->shared_hcd)
+				usb_hc_died(hcd->shared_hcd);
 			usb_hc_died(hcd);
 		}
 	}
@@ -513,11 +532,17 @@ static int hcd_pci_suspend_noirq(struct device *dev)
 
 	pci_save_state(pci_dev);
 
-	/* If the root hub is HALTed rather than SUSPENDed,
+	/* If both roothubs are HALTed rather than SUSPENDed,
 	 * disallow remote wakeup.
 	 */
-	if (hcd->state == HC_STATE_HALT)
-		device_set_wakeup_enable(dev, 0);
+	if (!hcd->shared_hcd) {
+		if (hcd->state == HC_STATE_HALT)
+			device_set_wakeup_enable(dev, 0);
+	} else {
+		if (hcd->state == HC_STATE_HALT &&
+				hcd->shared_hcd->state == HC_STATE_HALT)
+			device_set_wakeup_enable(dev, 0);
+	}
 	dev_dbg(dev, "wakeup: %d\n", device_may_wakeup(dev));
 
 	/* Possibly enable remote wakeup,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index a061c31..48717b6 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1156,6 +1156,8 @@ int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
 		dev_warn(hcd->self.controller, "Unlink after no-IRQ?  "
 			"Controller is probably using the wrong IRQ.\n");
 		set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
+		if (hcd->shared_hcd)
+			set_bit(HCD_FLAG_SAW_IRQ, &hcd->shared_hcd->flags);
 	}
 
 	return 0;
@@ -2089,6 +2091,8 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
 		rc = IRQ_NONE;
 	} else {
 		set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
+		if (hcd->shared_hcd)
+			set_bit(HCD_FLAG_SAW_IRQ, &hcd->shared_hcd->flags);
 
 		if (unlikely(hcd->state == HC_STATE_HALT))
 			usb_hc_died(hcd);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0a58be8..e9f1aec 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2190,6 +2190,8 @@ irqreturn_t xhci_msi_irq(int irq, struct usb_hcd *hcd)
 	irqreturn_t ret;
 
 	set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
+	if (hcd->shared_hcd)
+		set_bit(HCD_FLAG_SAW_IRQ, &hcd->shared_hcd->flags);
 
 	ret = xhci_irq(hcd);
 
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux