[PATCH] ehci-hcd more polite on cardbus

This patch makes the EHCI driver behave reasonably well in the
cardbus configurations I can test ... basically, it now sees
when a card is gone, and cleans up accordingly.  There are also
some related cleanups:  hardware handshakes will time out (not
that I've ever seen them fail), and some state management puts
a bit more effort into being strictly to-spec.

drivers/usb/host/ehci-hcd.c

@@ -65,6 +65,8 @@
  *
  * HISTORY:
  *
+ * 2002-07-25	Sanity check PCI reads, mostly for better cardbus support,
+ * 	clean up HC run state handshaking.
  * 2002-05-24	Preliminary FS/LS interrupts, using scheduling shortcuts
  * 2002-05-11	Clear TT errors for FS/LS ctrl/bulk.  Fill in some other
  *	missing pieces:  enabling 64bit dma, handoff from BIOS/SMM.
@@ -83,7 +85,7 @@
  * 2001-June	Works with usb-storage and NEC EHCI on 2.4
  */
 
-#define DRIVER_VERSION "2002-May-24"
+#define DRIVER_VERSION "2002-Jul-25"
 #define DRIVER_AUTHOR "David Brownell"
 #define DRIVER_DESC "USB 2.0 'Enhanced' Host Controller (EHCI) Driver"
 
@@ -112,6 +114,40 @@ MODULE_PARM_DESC (log2_irq_thresh, "log2 IRQ latency, 1-64 microframes");
 
 /*-------------------------------------------------------------------------*/
 
+/*
+ * handshake - spin reading hc until handshake completes or fails
+ * @ptr: address of hc register to be read
+ * @mask: bits to look at in result of read
+ * @done: value of those bits when handshake succeeds
+ * @usec: timeout in microseconds
+ *
+ * Returns negative errno, or zero on success
+ *
+ * Success happens when the "mask" bits have the specified value (hardware
+ * handshake done).  There are two failure modes:  "usec" have passed (major
+ * hardware flakeout), or the register reads as all-ones (hardware removed).
+ *
+ * That last failure should_only happen in cases like physical cardbus eject
+ * before driver shutdown. But it also seems to be caused by bugs in cardbus
+ * bridge shutdown:  shutting down the bridge before the devices using it.
+ */
+static int handshake (u32 *ptr, u32 mask, u32 done, int usec)
+{
+	u32	result;
+
+	do {
+		result = readl (ptr);
+		if (result == ~(u32)0)		/* card removed */
+			return -ENODEV;
+		result &= mask;
+		if (result == done)
+			return 0;
+		udelay (1);
+		usec--;
+	} while (usec > 0);
+	return -ETIMEDOUT;
+}
+
 /*
  * hc states include: unknown, halted, ready, running
  * transitional states are messy just now
@@ -119,36 +155,65 @@ MODULE_PARM_DESC (log2_irq_thresh, "log2 IRQ latency, 1-64 microframes");
  * a "ready" hc can be finishing prefetched work
  */
 
-/* halt a non-running controller */
-static void ehci_reset (struct ehci_hcd *ehci)
+/* force HC to halt state from unknown (EHCI spec section 2.3) */
+static int ehci_halt (struct ehci_hcd *ehci)
+{
+	u32	temp = readl (&ehci->regs->status);
+
+	if ((temp & STS_HALT) != 0)
+		return 0;
+
+	temp = readl (&ehci->regs->command);
+	temp &= ~CMD_RUN;
+	writel (temp, &ehci->regs->command);
+	return handshake (&ehci->regs->status, STS_HALT, STS_HALT, 16 * 125);
+}
+
+/* reset a non-running (STS_HALT == 1) controller */
+static int ehci_reset (struct ehci_hcd *ehci)
 {
 	u32	command = readl (&ehci->regs->command);
 
 	command |= CMD_RESET;
 	dbg_cmd (ehci, "reset", command);
 	writel (command, &ehci->regs->command);
-	while (readl (&ehci->regs->command) & CMD_RESET)
-		continue;
 	ehci->hcd.state = USB_STATE_HALT;
+	return handshake (&ehci->regs->command, CMD_RESET, 0, 050);
 }
 
 /* idle the controller (from running) */
 static void ehci_ready (struct ehci_hcd *ehci)
 {
-	u32	command;
+	u32	temp;
 
 #ifdef DEBUG
 	if (!HCD_IS_RUNNING (ehci->hcd.state))
 		BUG ();
 #endif
 
-	while (!(readl (&ehci->regs->status) & (STS_ASS | STS_PSS)))
-		udelay (100);
-	command = readl (&ehci->regs->command);
-	command &= ~(CMD_ASE | CMD_IAAD | CMD_PSE);
-	writel (command, &ehci->regs->command);
+	/* wait for any schedule enables/disables to take effect */
+	temp = 0;
+	if (ehci->async)
+		temp = STS_ASS;
+	if (ehci->next_uframe != -1)
+		temp |= STS_PSS;
+	if (handshake (&ehci->regs->status, STS_ASS | STS_PSS,
+				temp, 16 * 125) != 0) {
+		ehci->hcd.state = USB_STATE_HALT;
+		return;
+	}
 
-	// hardware can take 16 microframes to turn off ...
+	/* then disable anything that's still active */
+	temp = readl (&ehci->regs->command);
+	temp &= ~(CMD_ASE | CMD_IAAD | CMD_PSE);
+	writel (temp, &ehci->regs->command);
+
+	/* hardware can take 16 microframes to turn off ... */
+	if (handshake (&ehci->regs->status, STS_ASS | STS_PSS,
+				0, 16 * 125) != 0) {
+		ehci->hcd.state = USB_STATE_HALT;
+		return;
+	}
 	ehci->hcd.state = USB_STATE_READY;
 }
 
@@ -236,6 +301,10 @@ static int ehci_start (struct usb_hcd *hcd)
 	/* cache this readonly data; minimize PCI reads */
 	ehci->hcs_params = readl (&ehci->caps->hcs_params);
 
+	/* force HC to halt state */
+	if ((retval = ehci_halt (ehci)) != 0)
+		return retval;
+
 	/*
 	 * hw default: 1K periodic list heads, one per frame.
 	 * periodic_size can shrink by USBCMD update if hcc_params allows.
@@ -257,8 +326,10 @@ static int ehci_start (struct usb_hcd *hcd)
 	/* controller state:  unknown --> reset */
 
 	/* EHCI spec section 4.1 */
-	// FIXME require STS_HALT before reset...
-	ehci_reset (ehci);
+	if ((retval = ehci_reset (ehci)) != 0) {
+		ehci_mem_cleanup (ehci);
+		return retval;
+	}
 	writel (INTR_MASK, &ehci->regs->intr_enable);
 	writel (ehci->periodic_dma, &ehci->regs->frame_list);
 
@@ -335,8 +406,6 @@ static int ehci_start (struct usb_hcd *hcd)
 	if (usb_register_root_hub (udev, &ehci->hcd.pdev->dev) != 0) {
 		if (hcd->state == USB_STATE_RUNNING)
 			ehci_ready (ehci);
-		while (readl (&ehci->regs->status) & (STS_ASS | STS_PSS))
-			udelay (100);
 		ehci_reset (ehci);
 		hcd->self.root_hub = 0;
 		usb_free_dev (udev); 
@@ -355,16 +424,14 @@ static void ehci_stop (struct usb_hcd *hcd)
 
 	dbg ("%s: stop", hcd->self.bus_name);
 
+	/* no more interrupts ... */
 	if (hcd->state == USB_STATE_RUNNING)
 		ehci_ready (ehci);
-	while (readl (&ehci->regs->status) & (STS_ASS | STS_PSS))
-		udelay (100);
 	ehci_reset (ehci);
 
-	// root hub is shut down separately (first, when possible)
-	scan_async (ehci);
-	if (ehci->next_uframe != -1)
-		scan_periodic (ehci);
+	/* root hub is shut down separately (first, when possible) */
+	tasklet_disable (&ehci->tasklet);
+	ehci_tasklet ((unsigned long) ehci);
 	ehci_mem_cleanup (ehci);
 
 	dbg_status (ehci, "ehci_stop completed", readl (&ehci->regs->status));
@@ -412,8 +479,6 @@ dbg ("%s: suspend port %d", hcd->self.bus_name, i);
 
 	if (hcd->state == USB_STATE_RUNNING)
 		ehci_ready (ehci);
-	while (readl (&ehci->regs->status) & (STS_ASS | STS_PSS))
-		udelay (100);
 	writel (readl (&ehci->regs->command) & ~CMD_RUN, &ehci->regs->command);
 
 // save pci FLADJ value
@@ -489,6 +554,12 @@ static void ehci_irq (struct usb_hcd *hcd)
 	u32			status = readl (&ehci->regs->status);
 	int			bh;
 
+	/* e.g. cardbus physical eject */
+	if (status == ~(u32) 0) {
+		dbg ("%s: device removed!", hcd->self.bus_name);
+		goto dead;
+	}
+
 	status &= INTR_MASK;
 	if (!status)			/* irq sharing? */
 		return;
@@ -517,10 +588,13 @@ static void ehci_irq (struct usb_hcd *hcd)
 
 	/* PCI errors [4.15.2.4] */
 	if (unlikely ((status & STS_FATAL) != 0)) {
-		err ("%s: fatal error, state %x", hcd->self.bus_name, hcd->state);
+		err ("%s: fatal error, state %x",
+			hcd->self.bus_name, hcd->state);
+dead:
 		ehci_reset (ehci);
-		// generic layer kills/unlinks all urbs
-		// then tasklet cleans up the rest
+		/* generic layer kills/unlinks all urbs, then
+		 * uses ehci_stop to clean up the rest
+		 */
 		bh = 1;
 	}
 

drivers/usb/host/ehci-q.c

@@ -718,8 +718,7 @@ static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 		u32	cmd = readl (&ehci->regs->command);
 
 		/* in case a clear of CMD_ASE didn't take yet */
-		while (readl (&ehci->regs->status) & STS_ASS)
-			udelay (100);
+		(void) handshake (&ehci->regs->status, STS_ASS, 0, 150);
 
 		qh->hw_info1 |= __constant_cpu_to_le32 (QH_HEAD); /* [4.8] */
 		qh->qh_next.qh = qh;
@@ -917,11 +916,8 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 		if (ehci->hcd.state != USB_STATE_HALT) {
 			if (cmd & CMD_PSE)
 				writel (cmd & ~CMD_ASE, &ehci->regs->command);
-			else {
+			else
 				ehci_ready (ehci);
-				while (readl (&ehci->regs->status) & STS_ASS)
-					udelay (100);
-			}
 		}
 		qh->qh_next.qh = ehci->async = 0;
 

drivers/usb/host/ehci-sched.c

@@ -171,15 +171,19 @@ periodic_usecs (struct ehci_hcd *ehci, unsigned frame, unsigned uframe)
 
 /*-------------------------------------------------------------------------*/
 
-static void enable_periodic (struct ehci_hcd *ehci)
+static int enable_periodic (struct ehci_hcd *ehci)
 {
 	u32	cmd;
+	int	status;
 
 	/* did clearing PSE did take effect yet?
 	 * takes effect only at frame boundaries...
 	 */
-	while (readl (&ehci->regs->status) & STS_PSS)
-		udelay (20);
+	status = handshake (&ehci->regs->status, STS_PSS, 0, 9 * 125);
+	if (status != 0) {
+		ehci->hcd.state = USB_STATE_HALT;
+		return status;
+	}
 
 	cmd = readl (&ehci->regs->command) | CMD_PSE;
 	writel (cmd, &ehci->regs->command);
@@ -189,23 +193,29 @@ static void enable_periodic (struct ehci_hcd *ehci)
 	/* make sure tasklet scans these */
 	ehci->next_uframe = readl (&ehci->regs->frame_index)
 				% (ehci->periodic_size << 3);
+	return 0;
 }
 
-static void disable_periodic (struct ehci_hcd *ehci)
+static int disable_periodic (struct ehci_hcd *ehci)
 {
 	u32	cmd;
+	int	status;
 
 	/* did setting PSE not take effect yet?
 	 * takes effect only at frame boundaries...
 	 */
-	while (!(readl (&ehci->regs->status) & STS_PSS))
-		udelay (20);
+	status = handshake (&ehci->regs->status, STS_PSS, STS_PSS, 9 * 125);
+	if (status != 0) {
+		ehci->hcd.state = USB_STATE_HALT;
+		return status;
+	}
 
 	cmd = readl (&ehci->regs->command) & ~CMD_PSE;
 	writel (cmd, &ehci->regs->command);
 	/* posted write ... */
 
 	ehci->next_uframe = -1;
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -217,6 +227,7 @@ static void intr_deschedule (
 	unsigned	period
 ) {
 	unsigned long	flags;
+	int		status;
 
 	period >>= 3;		// FIXME microframe periods not handled yet
 
@@ -234,9 +245,11 @@ static void intr_deschedule (
 
 	/* maybe turn off periodic schedule */
 	if (!ehci->periodic_urbs)
-		disable_periodic (ehci);
-	else
+		status = disable_periodic (ehci);
+	else {
+		status = 0;
 		vdbg ("periodic schedule still enabled");
+	}
 
 	spin_unlock_irqrestore (&ehci->lock, flags);
 
@@ -245,7 +258,7 @@ static void intr_deschedule (
 	 * (yeech!) to be sure it's done.
 	 * No other threads may be mucking with this qh.
 	 */
-	if (((ehci_get_frame (&ehci->hcd) - frame) % period) == 0)
+	if (!status && ((ehci_get_frame (&ehci->hcd) - frame) % period) == 0)
 		udelay (125);
 
 	qh->qh_state = QH_STATE_IDLE;
@@ -501,7 +514,7 @@ static int intr_submit (
 
 			/* maybe enable periodic schedule processing */
 			if (!ehci->periodic_urbs++)
-				enable_periodic (ehci);
+				status = enable_periodic (ehci);
 			break;
 
 		} while (frame);
@@ -913,8 +926,12 @@ itd_schedule (struct ehci_hcd *ehci, struct urb *urb)
 		usb_claim_bandwidth (urb->dev, urb, usecs, 1);
 
 		/* maybe enable periodic schedule processing */
-		if (!ehci->periodic_urbs++)
-			enable_periodic (ehci);
+		if (!ehci->periodic_urbs++) {
+			if ((status =  enable_periodic (ehci)) != 0) {
+				// FIXME deschedule right away
+				err ("itd_schedule, enable = %d", status);
+			}
+		}
 
 		return 0;
 
@@ -994,7 +1011,7 @@ itd_complete (
 	/* defer stopping schedule; completion can submit */
 	ehci->periodic_urbs--;
 	if (!ehci->periodic_urbs)
-		disable_periodic (ehci);
+		(void) disable_periodic (ehci);
 
 	return flags;
 }