ChangeSet 1.1352.1.3, 2003/10/13 16:21:38-07:00, david-b@pacbell.net [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 | 54 +++++++++++++++++++------------------------- drivers/usb/host/ohci-hub.c | 2 - drivers/usb/host/ohci-pci.c | 44 ++++++++++++++++------------------- drivers/usb/host/ohci-q.c | 23 +++++++++++++----- drivers/usb/host/ohci.h | 5 ---- 5 files changed, 62 insertions(+), 66 deletions(-) diff -Nru a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c --- a/drivers/usb/host/ohci-hcd.c Wed Oct 15 11:08:03 2003 +++ b/drivers/usb/host/ohci-hcd.c Wed Oct 15 11:08:03 2003 @@ -102,14 +102,8 @@ #include #include -/* - * 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 inline void disable (struct ohci_hcd *ohci) { - ohci->disabled = 1; ohci->hcd.state = USB_STATE_HALT; } @@ -222,7 +215,7 @@ 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 @@ #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 @@ */ 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 @@ 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 @@ /* 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 @@ 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 @@ /* 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 @@ } if (ints & OHCI_INTR_WDH) { - writel (OHCI_INTR_WDH, ®s->intrdisable); + if (HCD_IS_RUNNING(hcd->state)) + writel (OHCI_INTR_WDH, ®s->intrdisable); dl_done_list (ohci, dl_reverse_done_list (ohci), ptregs); - writel (OHCI_INTR_WDH, ®s->intrenable); + if (HCD_IS_RUNNING(hcd->state)) + writel (OHCI_INTR_WDH, ®s->intrenable); } /* could track INTR_SO to reduce available PCI/... bandwidth */ @@ -600,14 +593,17 @@ 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, ®s->intrdisable); spin_unlock (&ohci->lock); - writel (ints, ®s->intrstatus); - writel (OHCI_INTR_MIE, ®s->intrenable); - // flush those pci writes - (void) readl (&ohci->regs->control); + if (HCD_IS_RUNNING(ohci->hcd.state)) { + writel (ints, ®s->intrstatus); + writel (OHCI_INTR_MIE, ®s->intrenable); + // flush those pci writes + (void) readl (&ohci->regs->control); + } } /*-------------------------------------------------------------------------*/ @@ -616,13 +612,12 @@ { 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 @@ 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 -Nru a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c --- a/drivers/usb/host/ohci-hub.c Wed Oct 15 11:08:03 2003 +++ b/drivers/usb/host/ohci-hub.c Wed Oct 15 11:08:03 2003 @@ -73,7 +73,7 @@ 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 -Nru a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c --- a/drivers/usb/host/ohci-pci.c Wed Oct 15 11:08:03 2003 +++ b/drivers/usb/host/ohci-pci.c Wed Oct 15 11:08:03 2003 @@ -117,7 +117,6 @@ 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 @@ /* 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 @@ &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 @@ struct ohci_hcd *ohci = hcd_to_ohci (hcd); int temp; int retval = 0; - unsigned long flags; #ifdef CONFIG_PMAC_PBOOK { @@ -226,6 +227,7 @@ switch (temp) { case OHCI_USB_RESET: // lost power +restart: ohci_info (ohci, "USB restart\n"); retval = hc_restart (ohci); break; @@ -235,31 +237,28 @@ 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 @@ 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 -Nru a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c --- a/drivers/usb/host/ohci-q.c Wed Oct 15 11:08:03 2003 +++ b/drivers/usb/host/ohci-q.c Wed Oct 15 11:08:03 2003 @@ -449,7 +449,7 @@ 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 @@ * 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 @@ usb_pipein (urb->pipe) ? "in" : "out", le32_to_cpu (td->hwINFO), cc, cc_to_error [cc]); + } return rev; } @@ -871,7 +881,8 @@ /* 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 @@ 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 @@ /* 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 @@ } /* 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 -Nru a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h --- a/drivers/usb/host/ohci.h Wed Oct 15 11:08:03 2003 +++ b/drivers/usb/host/ohci.h Wed Oct 15 11:08:03 2003 @@ -314,13 +314,10 @@ 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 @@ /* * 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 */