[PATCH] USB: ohci-hcd PM fixes

This patch primarily fixes PM-related bugs in the OHCI driver.

It gets rid of some flags that duplicated state between usbcore
and the HCD.  The duplication wasn't correct, and wasn't tested
correctly ... this fixes both issues.  So now the driver avoids
writing to hardware when it's suspended (as required by older
PowerBook hardware) or halted, and treats all non-running states
the same (as required by all hardware).

This includes the last generic parts of a patch sent a while back
by Benjamin Herrenschmidt, which weren't at that time testable on a
x86 kernel because the generic PM code was in flux (and broken).
There may still be some PMAC-specific issues to resolve.

With this patch, and a device_resume() deadlock fix, I've seen
OHCI suspend/resume work on hardware it's not worked on since the
PM changes started to merge into the 2.6.0-test kernels.

drivers/usb/host/ohci-hcd.c

@@ -102,14 +102,8 @@
 #include <asm/unaligned.h>
 #include <asm/byteorder.h>
 
-/*
- * TO DO:
- *
- *	- "disabled" and "sleeping" should be in hcd->state
- *	- lots more testing!!
- */
 
-#define DRIVER_VERSION "2003 Feb 24"
+#define DRIVER_VERSION "2003 Oct 13"
 #define DRIVER_AUTHOR "Roman Weissgaerber, David Brownell"
 #define DRIVER_DESC "USB 1.1 'Open' Host Controller (OHCI) Driver"
 
@@ -131,7 +125,6 @@ static const char	hcd_name [] = "ohci_hcd";
 
 static inline void disable (struct ohci_hcd *ohci)
 {
-	ohci->disabled = 1;
 	ohci->hcd.state = USB_STATE_HALT;
 }
 
@@ -222,7 +215,7 @@ static int ohci_urb_enqueue (
 	spin_lock_irqsave (&ohci->lock, flags);
 
 	/* don't submit to a dead HC */
-	if (ohci->disabled || ohci->sleeping) {
+	if (!HCD_IS_RUNNING(ohci->hcd.state)) {
 		retval = -ENODEV;
 		goto fail;
 	}
@@ -278,7 +271,7 @@ static int ohci_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
 #endif		  
 
 	spin_lock_irqsave (&ohci->lock, flags);
-	if (!ohci->disabled) {
+ 	if (HCD_IS_RUNNING(ohci->hcd.state)) {
 		urb_priv_t  *urb_priv;
 
 		/* Unless an IRQ completed the unlink while it was being
@@ -287,7 +280,6 @@ static int ohci_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
 		 */
 		urb_priv = urb->hcpriv;
 		if (urb_priv) {
-			urb_priv->state = URB_DEL; 
 			if (urb_priv->ed->state == ED_OPER)
 				start_urb_unlink (ohci, urb_priv->ed);
 		}
@@ -334,7 +326,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct hcd_dev *dev, int ep)
 	if (!ed)
 		goto done;
 
-	if (!HCD_IS_RUNNING (ohci->hcd.state) || ohci->disabled)
+	if (!HCD_IS_RUNNING (ohci->hcd.state))
 		ed->state = ED_IDLE;
 	switch (ed->state) {
 	case ED_UNLINK:		/* wait for hw to finish? */
@@ -355,9 +347,9 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct hcd_dev *dev, int ep)
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  can't recover; must leak ed.
 		 */
-		ohci_err (ohci, "ed %p (#%d) state %d%s\n",
+		ohci_err (ohci, "leak ed %p (#%d) state %d%s\n",
 			ed, epnum, ed->state,
-			list_empty (&ed->td_list) ? "" : "(has tds)");
+			list_empty (&ed->td_list) ? "" : " (has tds)");
 		td_free (ohci, ed->dummy);
 		break;
 	}
@@ -466,8 +458,7 @@ static int hc_start (struct ohci_hcd *ohci)
   	struct usb_bus		*bus;
 
 	spin_lock_init (&ohci->lock);
-	ohci->disabled = 1;
-	ohci->sleeping = 0;
+	disable (ohci);
 
 	/* Tell the controller where the control and bulk lists are
 	 * The lists are empty now. */
@@ -496,8 +487,8 @@ static int hc_start (struct ohci_hcd *ohci)
  	/* start controller operations */
 	ohci->hc_control &= OHCI_CTRL_RWC;
  	ohci->hc_control |= OHCI_CONTROL_INIT | OHCI_USB_OPER;
-	ohci->disabled = 0;
  	writel (ohci->hc_control, &ohci->regs->control);
+	ohci->hcd.state = USB_STATE_RUNNING;
 
 	/* Choose the interrupts we care about now, others later on demand */
 	mask = OHCI_INTR_MIE | OHCI_INTR_UE | OHCI_INTR_WDH;
@@ -586,9 +577,11 @@ static void ohci_irq (struct usb_hcd *hcd, struct pt_regs *ptregs)
 	}
   
 	if (ints & OHCI_INTR_WDH) {
-		writel (OHCI_INTR_WDH, &regs->intrdisable);	
+		if (HCD_IS_RUNNING(hcd->state))
+			writel (OHCI_INTR_WDH, &regs->intrdisable);	
 		dl_done_list (ohci, dl_reverse_done_list (ohci), ptregs);
-		writel (OHCI_INTR_WDH, &regs->intrenable); 
+		if (HCD_IS_RUNNING(hcd->state))
+			writel (OHCI_INTR_WDH, &regs->intrenable); 
 	}
   
 	/* could track INTR_SO to reduce available PCI/... bandwidth */
@@ -600,14 +593,17 @@ static void ohci_irq (struct usb_hcd *hcd, struct pt_regs *ptregs)
 	if (ohci->ed_rm_list)
 		finish_unlinks (ohci, le16_to_cpu (ohci->hcca->frame_no),
 				ptregs);
-	if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list)
+	if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list
+			&& HCD_IS_RUNNING(ohci->hcd.state))
 		writel (OHCI_INTR_SF, &regs->intrdisable);	
 	spin_unlock (&ohci->lock);
 
-	writel (ints, &regs->intrstatus);
-	writel (OHCI_INTR_MIE, &regs->intrenable);	
-	// flush those pci writes
-	(void) readl (&ohci->regs->control);
+	if (HCD_IS_RUNNING(ohci->hcd.state)) {
+		writel (ints, &regs->intrstatus);
+		writel (OHCI_INTR_MIE, &regs->intrenable);	
+		// flush those pci writes
+		(void) readl (&ohci->regs->control);
+	}
 }
 
 /*-------------------------------------------------------------------------*/
@@ -616,13 +612,12 @@ static void ohci_stop (struct usb_hcd *hcd)
 {	
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
 
-	ohci_dbg (ohci, "stop %s controller%s\n",
+	ohci_dbg (ohci, "stop %s controller (state 0x%02x)\n",
 		hcfs2string (ohci->hc_control & OHCI_CTRL_HCFS),
-		ohci->disabled ? " (disabled)" : ""
-		);
+		ohci->hcd.state);
 	ohci_dump (ohci, 1);
 
-	if (!ohci->disabled)
+	if (HCD_IS_RUNNING(ohci->hcd.state))
 		hc_reset (ohci);
 	
 	remove_debug_files (ohci);
@@ -649,8 +644,7 @@ static int hc_restart (struct ohci_hcd *ohci)
 	int temp;
 	int i;
 
-	ohci->disabled = 1;
-	ohci->sleeping = 0;
+	disable (ohci);
 	if (hcd_to_bus (&ohci->hcd)->root_hub)
 		usb_disconnect (&hcd_to_bus (&ohci->hcd)->root_hub);
 	

drivers/usb/host/ohci-hub.c

@@ -73,7 +73,7 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
 
 	ports = roothub_a (ohci) & RH_A_NDP; 
 	if (ports > MAX_ROOT_PORTS) {
-		if (ohci->disabled)
+		if (!HCD_IS_RUNNING(ohci->hcd.state))
 			return -ESHUTDOWN;
 		ohci_err (ohci, "bogus NDP=%d, rereads as NDP=%d\n",
 			ports, readl (&ohci->regs->roothub.a) & RH_A_NDP);

drivers/usb/host/ohci-pci.c

@@ -117,7 +117,6 @@ ohci_pci_start (struct usb_hcd *hcd)
 static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state)
 {
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
-	unsigned long		flags;
 	u16			cmd;
 	u32			tmp;
 
@@ -129,16 +128,15 @@ static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state)
 
 	/* act as if usb suspend can always be used */
 	ohci_dbg (ohci, "suspend to %d\n", state);
-	ohci->sleeping = 1;
-
+	
 	/* First stop processing */
-  	spin_lock_irqsave (&ohci->lock, flags);
+  	spin_lock_irq (&ohci->lock);
 	ohci->hc_control &=
 		~(OHCI_CTRL_PLE|OHCI_CTRL_CLE|OHCI_CTRL_BLE|OHCI_CTRL_IE);
 	writel (ohci->hc_control, &ohci->regs->control);
 	writel (OHCI_INTR_SF, &ohci->regs->intrstatus);
 	(void) readl (&ohci->regs->intrstatus);
-  	spin_unlock_irqrestore (&ohci->lock, flags);
+  	spin_unlock_irq (&ohci->lock);
 
 	/* Wait a frame or two */
 	mdelay (1);
@@ -156,10 +154,14 @@ static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state)
 		&ohci->regs->intrenable);
 
 	/* Suspend chip and let things settle down a bit */
+  	spin_lock_irq (&ohci->lock);
  	ohci->hc_control = OHCI_USB_SUSPEND;
  	writel (ohci->hc_control, &ohci->regs->control);
 	(void) readl (&ohci->regs->control);
-	mdelay (500); /* No schedule here ! */
+  	spin_unlock_irq (&ohci->lock);
+
+	set_current_state (TASK_UNINTERRUPTIBLE);
+	schedule_timeout (HZ/2);
 
 	tmp = readl (&ohci->regs->control) | OHCI_CTRL_HCFS;
 	switch (tmp) {
@@ -199,7 +201,6 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
 	int			temp;
 	int			retval = 0;
-	unsigned long		flags;
 
 #ifdef CONFIG_PMAC_PBOOK
 	{
@@ -226,6 +227,7 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
 	switch (temp) {
 
 	case OHCI_USB_RESET:	// lost power
+restart:
 		ohci_info (ohci, "USB restart\n");
 		retval = hc_restart (ohci);
 		break;
@@ -235,31 +237,28 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
 		ohci_info (ohci, "USB continue from %s wakeup\n",
 			 (temp == OHCI_USB_SUSPEND)
 				? "host" : "remote");
+
+		/* we "should" only need RESUME if we're SUSPENDed ... */
 		ohci->hc_control = OHCI_USB_RESUME;
 		writel (ohci->hc_control, &ohci->regs->control);
 		(void) readl (&ohci->regs->control);
-		mdelay (20); /* no schedule here ! */
-		/* Some controllers (lucent) need a longer delay here */
-		mdelay (15);
+		/* Some controllers (lucent) need extra-long delays */
+		mdelay (35); /* no schedule here ! */
 
 		temp = readl (&ohci->regs->control);
 		temp = ohci->hc_control & OHCI_CTRL_HCFS;
 		if (temp != OHCI_USB_RESUME) {
 			ohci_err (ohci, "controller won't resume\n");
-			ohci->disabled = 1;
-			retval = -EIO;
-			break;
+			/* maybe we can reset */
+			goto restart;
 		}
 
-		/* Some chips likes being resumed first */
+		/* Then re-enable operations */
 		writel (OHCI_USB_OPER, &ohci->regs->control);
 		(void) readl (&ohci->regs->control);
 		mdelay (3);
 
-		/* Then re-enable operations */
-		spin_lock_irqsave (&ohci->lock, flags);
-		ohci->disabled = 0;
-		ohci->sleeping = 0;
+		spin_lock_irq (&ohci->lock);
 		ohci->hc_control = OHCI_CONTROL_INIT | OHCI_USB_OPER;
 		if (!ohci->ed_rm_list) {
 			if (ohci->ed_controltail)
@@ -274,25 +273,22 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
 		writel (OHCI_INTR_SF, &ohci->regs->intrstatus);
 		writel (OHCI_INTR_SF, &ohci->regs->intrenable);
 
-		/* Check for a pending done list */
 		writel (OHCI_INTR_WDH, &ohci->regs->intrdisable);	
 		(void) readl (&ohci->regs->intrdisable);
-		spin_unlock_irqrestore (&ohci->lock, flags);
+		spin_unlock_irq (&ohci->lock);
 
 #ifdef CONFIG_PMAC_PBOOK
 		if (_machine == _MACH_Pmac)
 			enable_irq (hcd->pdev->irq);
 #endif
+
+		/* Check for a pending done list */
 		if (ohci->hcca->done_head)
 			dl_done_list (ohci, dl_reverse_done_list (ohci), NULL);
 		writel (OHCI_INTR_WDH, &ohci->regs->intrenable); 
 
 		/* assume there are TDs on the bulk and control lists */
 		writel (OHCI_BLF | OHCI_CLF, &ohci->regs->cmdstatus);
-
-// ohci_dump_status (ohci);
-ohci_dbg (ohci, "sleeping = %d, disabled = %d\n",
-		ohci->sleeping, ohci->disabled);
 		break;
 
 	default:

drivers/usb/host/ohci-q.c

@@ -449,7 +449,7 @@ static void start_urb_unlink (struct ohci_hcd *ohci, struct ed *ed)
 	ohci->ed_rm_list = ed;
 
 	/* enable SOF interrupt */
-	if (!ohci->sleeping) {
+	if (HCD_IS_RUNNING (ohci->hcd.state)) {
 		writel (OHCI_INTR_SF, &ohci->regs->intrstatus);
 		writel (OHCI_INTR_SF, &ohci->regs->intrenable);
 		// flush those pci writes
@@ -794,7 +794,16 @@ ed_halted (struct ohci_hcd *ohci, struct td *td, int cc, struct td *rev)
 	 * looks odd ... that doesn't include protocol stalls
 	 * (or maybe some other things)
 	 */
-	if (cc != TD_CC_STALL || !usb_pipecontrol (urb->pipe))
+	switch (cc) {
+	case TD_DATAUNDERRUN:
+		if ((urb->transfer_flags & URB_SHORT_NOT_OK) == 0)
+			break;
+		/* fallthrough */
+	case TD_CC_STALL:
+		if (usb_pipecontrol (urb->pipe))
+			break;
+		/* fallthrough */
+	default:
 		ohci_dbg (ohci,
 			"urb %p path %s ep%d%s %08x cc %d --> status %d\n",
 			urb, urb->dev->devpath,
@@ -802,6 +811,7 @@ ed_halted (struct ohci_hcd *ohci, struct td *td, int cc, struct td *rev)
 			usb_pipein (urb->pipe) ? "in" : "out",
 			le32_to_cpu (td->hwINFO),
 			cc, cc_to_error [cc]);
+	}
 
 	return rev;
 }
@@ -871,7 +881,8 @@ finish_unlinks (struct ohci_hcd *ohci, u16 tick, struct pt_regs *regs)
 		/* only take off EDs that the HC isn't using, accounting for
 		 * frame counter wraps.
 		 */
-		if (tick_before (tick, ed->tick) && !ohci->disabled) {
+		if (tick_before (tick, ed->tick)
+				&& HCD_IS_RUNNING(ohci->hcd.state)) {
 			last = &ed->ed_next;
 			continue;
 		}
@@ -901,7 +912,7 @@ finish_unlinks (struct ohci_hcd *ohci, u16 tick, struct pt_regs *regs)
 			urb = td->urb;
 			urb_priv = td->urb->hcpriv;
 
-			if (urb_priv->state != URB_DEL) {
+			if (urb->status == -EINPROGRESS) {
 				prev = &td->hwNextTD;
 				continue;
 			}
@@ -938,7 +949,7 @@ finish_unlinks (struct ohci_hcd *ohci, u16 tick, struct pt_regs *regs)
 
 		/* but if there's work queued, reschedule */
 		if (!list_empty (&ed->td_list)) {
-			if (!ohci->disabled && !ohci->sleeping)
+			if (HCD_IS_RUNNING(ohci->hcd.state))
 				ed_schedule (ohci, ed);
 		}
 
@@ -947,7 +958,7 @@ finish_unlinks (struct ohci_hcd *ohci, u16 tick, struct pt_regs *regs)
    	}
 
 	/* maybe reenable control and bulk lists */ 
-	if (!ohci->disabled && !ohci->ed_rm_list) {
+	if (HCD_IS_RUNNING(ohci->hcd.state) && !ohci->ed_rm_list) {
 		u32	command = 0, control = 0;
 
 		if (ohci->ed_controltail) {

drivers/usb/host/ohci.h

@@ -314,13 +314,10 @@ typedef struct urb_priv {
 	struct ed		*ed;
 	__u16			length;		// # tds in this request
 	__u16			td_cnt;		// tds already serviced
-	int   			state;
 	struct td		*td [0];	// all TDs in this request
 
 } urb_priv_t;
 
-#define URB_DEL 1
-
 #define TD_HASH_SIZE    64    /* power'o'two */
 // sizeof (struct td) ~= 64 == 2^6 ... 
 #define TD_HASH_FUNC(td_dma) ((td_dma ^ (td_dma >> 6)) % TD_HASH_SIZE)
@@ -365,8 +362,6 @@ struct ohci_hcd {
 	/*
 	 * driver state
 	 */
-	int			disabled;	/* e.g. got a UE, we're hung */
-	int			sleeping;
 	int			load [NUM_INTS];
 	u32 			hc_control;	/* copy of hc control reg */