[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.

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index f978d143e09079fd94b2040f1c5438e5c0c6844a..4372ea78d09b4385860111e7cbb67bad56409f43 100644 (file)
--- a/drivers/usb/host/ohci-hcd.c
+++ b/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 @@ rescan:
        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 @@ rescan:
                /* 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);
        

diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index a7584a499af0a1ca0235cf5df0af2eec401a7d59..4f8260c91ba4b6e06d4b38410bc7480f19489b4e 100644 (file)
--- a/drivers/usb/host/ohci-hub.c
+++ b/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);

diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index a0820ee9f178a7582732fc104f8773778836e29e..431795328c264c63382009549af5e3e2f2868163 100644 (file)
--- a/drivers/usb/host/ohci-pci.c
+++ b/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:

diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index 0d1e634690300680e3cd9fa8c3b10b6e3c146104..7a6a1d4ec5c57ff133f66b854ebca5f48e670509 100644 (file)
--- a/drivers/usb/host/ohci-q.c
+++ b/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 @@ rescan_all:
                /* 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 @@ rescan_this:
                        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 @@ rescan_this:
 
                /* 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 @@ rescan_this:
        }
 
        /* 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) {

diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index a9c564b98897d16f34440c75d04e9a7dd48a42ea..7419f7f10741dd58a75db4c383cd019b17223e0b 100644 (file)
--- a/drivers/usb/host/ohci.h
+++ b/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 */