ChangeSet 1.1220, 2003/05/23 13:40:42-07:00, stern@rowland.harvard.edu [PATCH] USB: uhci Interrupt Latency fix Paul: Okay, I think this patch ought to do the trick. I modified the PM suspend/resume code so that on buggy motherboards like yours the suspend routine really does a reset, while on normal motherboards the resume routine really does a resume. I haven't tried that part out because, truth to tell, I'm a little scared of doing an APM/ACPI suspend. Not long ago I walked away from my computer for about a half-hour, leaving 2.5.69 running. When I got back the screen was blank and the machine was totally non-responsive. I changed the delays in reset_hc() to use schedule_timeout() rather than wait_ms(), which should make it more friendly. Finally, I put the USBCMD_FGR back into wakeup_hc(). The reason for it is now evident: a wakeup might be the result of a system-initiated event as opposed to something requested by a device. drivers/usb/host/uhci-hcd.c | 163 ++++++++++++++++++++++++++++++++++---------- drivers/usb/host/uhci-hcd.h | 29 +++++++ 2 files changed, 157 insertions(+), 35 deletions(-) diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c --- a/drivers/usb/host/uhci-hcd.c Fri May 23 14:25:39 2003 +++ b/drivers/usb/host/uhci-hcd.c Fri May 23 14:25:39 2003 @@ -61,7 +61,7 @@ /* * Version Information */ -#define DRIVER_VERSION "v2.0" +#define DRIVER_VERSION "v2.1" #define DRIVER_AUTHOR "Linus 'Frodo Rabbit' Torvalds, Johannes Erdfelt, Randy Dunlap, Georg Acher, Deti Fliegl, Thomas Sailer, Roman Weissgaerber" #define DRIVER_DESC "USB Universal Host Controller Interface driver" @@ -91,9 +91,7 @@ static int uhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb); static void uhci_unlink_generic(struct uhci_hcd *uhci, struct urb *urb); -static int ports_active(struct uhci_hcd *uhci); -static void suspend_hc(struct uhci_hcd *uhci); -static void wakeup_hc(struct uhci_hcd *uhci); +static void hc_state_transitions(struct uhci_hcd *uhci); /* If a transfer is still active after this much time, turn off FSBR */ #define IDLE_TIMEOUT (HZ / 20) /* 50 ms */ @@ -1757,9 +1755,8 @@ uhci->skel_term_qh->link = UHCI_PTR_TERM; } - /* enter global suspend if nothing connected */ - if (!uhci->is_suspended && !ports_active(uhci)) - suspend_hc(uhci); + /* Poll for and perform state transitions */ + hc_state_transitions(uhci); init_stall_timer(hcd); } @@ -1884,14 +1881,14 @@ err("%x: host system error, PCI problems?", io_addr); if (status & USBSTS_HCPE) err("%x: host controller process error. something bad happened", io_addr); - if ((status & USBSTS_HCH) && !uhci->is_suspended) { + if ((status & USBSTS_HCH) && uhci->state > 0) { err("%x: host controller halted. very bad", io_addr); /* FIXME: Reset the controller, fix the offending TD */ } } if (status & USBSTS_RD) - wakeup_hc(uhci); + uhci->resume_detect = 1; uhci_free_pending_qhs(uhci); @@ -1922,10 +1919,18 @@ unsigned int io_addr = uhci->io_addr; /* Global reset for 50ms */ + uhci->state = UHCI_RESET; outw(USBCMD_GRESET, io_addr + USBCMD); - wait_ms(50); + set_current_state(TASK_UNINTERRUPTIBLE); + schedule_timeout((HZ*50+999) / 1000); + set_current_state(TASK_RUNNING); outw(0, io_addr + USBCMD); - wait_ms(10); + + /* Another 10ms delay */ + set_current_state(TASK_UNINTERRUPTIBLE); + schedule_timeout((HZ*10+999) / 1000); + set_current_state(TASK_RUNNING); + uhci->resume_detect = 0; } static void suspend_hc(struct uhci_hcd *uhci) @@ -1933,34 +1938,49 @@ unsigned int io_addr = uhci->io_addr; dbg("%x: suspend_hc", io_addr); - - uhci->is_suspended = 1; - smp_wmb(); - + uhci->state = UHCI_SUSPENDED; + uhci->resume_detect = 0; outw(USBCMD_EGSM, io_addr + USBCMD); } static void wakeup_hc(struct uhci_hcd *uhci) { unsigned int io_addr = uhci->io_addr; - unsigned int status; - dbg("%x: wakeup_hc", io_addr); + switch (uhci->state) { + case UHCI_SUSPENDED: /* Start the resume */ + dbg("%x: wakeup_hc", io_addr); + + /* Global resume for >= 20ms */ + outw(USBCMD_FGR | USBCMD_EGSM, io_addr + USBCMD); + uhci->state = UHCI_RESUMING_1; + uhci->state_end = jiffies + (20*HZ+999) / 1000; + break; - /* Global resume for 20ms */ - outw(USBCMD_FGR | USBCMD_EGSM, io_addr + USBCMD); - wait_ms(20); - outw(0, io_addr + USBCMD); - - /* wait for EOP to be sent */ - status = inw(io_addr + USBCMD); - while (status & USBCMD_FGR) - status = inw(io_addr + USBCMD); + case UHCI_RESUMING_1: /* End global resume */ + uhci->state = UHCI_RESUMING_2; + outw(0, io_addr + USBCMD); + /* Falls through */ - uhci->is_suspended = 0; + case UHCI_RESUMING_2: /* Wait for EOP to be sent */ + if (inw(io_addr + USBCMD) & USBCMD_FGR) + break; - /* Run and mark it configured with a 64-byte max packet */ - outw(USBCMD_RS | USBCMD_CF | USBCMD_MAXP, io_addr + USBCMD); + /* Run for at least 1 second, and + * mark it configured with a 64-byte max packet */ + uhci->state = UHCI_RUNNING_GRACE; + uhci->state_end = jiffies + HZ; + outw(USBCMD_RS | USBCMD_CF | USBCMD_MAXP, + io_addr + USBCMD); + break; + + case UHCI_RUNNING_GRACE: /* Now allowed to suspend */ + uhci->state = UHCI_RUNNING; + break; + + default: + break; + } } static int ports_active(struct uhci_hcd *uhci) @@ -1975,6 +1995,73 @@ return connection; } +static int suspend_allowed(struct uhci_hcd *uhci) +{ + unsigned int io_addr = uhci->io_addr; + int i; + + if (!uhci->hcd.pdev || + uhci->hcd.pdev->vendor != PCI_VENDOR_ID_INTEL || + uhci->hcd.pdev->device != PCI_DEVICE_ID_INTEL_82371AB_2) + return 1; + + /* This is a 82371AB/EB/MB USB controller which has a bug that + * causes false resume indications if any port has an + * over current condition. To prevent problems, we will not + * allow a global suspend if any ports are OC. + * + * Some motherboards using the 82371AB/EB/MB (but not the USB portion) + * appear to hardwire the over current inputs active to disable + * the USB ports. + */ + + /* check for over current condition on any port */ + for (i = 0; i < uhci->rh_numports; i++) { + if (inw(io_addr + USBPORTSC1 + i * 2) & USBPORTSC_OC) + return 0; + } + + return 1; +} + +static void hc_state_transitions(struct uhci_hcd *uhci) +{ + switch (uhci->state) { + case UHCI_RUNNING: + + /* global suspend if nothing connected for 1 second */ + if (!ports_active(uhci) && suspend_allowed(uhci)) { + uhci->state = UHCI_SUSPENDING_GRACE; + uhci->state_end = jiffies + HZ; + } + break; + + case UHCI_SUSPENDING_GRACE: + if (ports_active(uhci)) + uhci->state = UHCI_RUNNING; + else if (time_after_eq(jiffies, uhci->state_end)) + suspend_hc(uhci); + break; + + case UHCI_SUSPENDED: + + /* wakeup if requested by a device */ + if (uhci->resume_detect) + wakeup_hc(uhci); + break; + + case UHCI_RESUMING_1: + case UHCI_RESUMING_2: + case UHCI_RUNNING_GRACE: + if (time_after_eq(jiffies, uhci->state_end)) + wakeup_hc(uhci); + break; + + default: + break; + } +} + static void start_hc(struct uhci_hcd *uhci) { unsigned int io_addr = uhci->io_addr; @@ -2003,6 +2090,8 @@ outl(uhci->fl->dma_handle, io_addr + USBFLBASEADD); /* Run and mark it configured with a 64-byte max packet */ + uhci->state = UHCI_RUNNING_GRACE; + uhci->state_end = jiffies + HZ; outw(USBCMD_RS | USBCMD_CF | USBCMD_MAXP, io_addr + USBCMD); uhci->hcd.state = USB_STATE_READY; @@ -2101,8 +2190,6 @@ uhci->fsbr = 0; uhci->fsbrtimeout = 0; - uhci->is_suspended = 0; - spin_lock_init(&uhci->qh_remove_list_lock); INIT_LIST_HEAD(&uhci->qh_remove_list); @@ -2335,7 +2422,11 @@ { struct uhci_hcd *uhci = hcd_to_uhci(hcd); - suspend_hc(uhci); + /* Don't try to suspend broken motherboards, reset instead */ + if (suspend_allowed(uhci)) + suspend_hc(uhci); + else + reset_hc(uhci); return 0; } @@ -2345,8 +2436,12 @@ pci_set_master(uhci->hcd.pdev); - reset_hc(uhci); - start_hc(uhci); + if (uhci->state == UHCI_SUSPENDED) + uhci->resume_detect = 1; + else { + reset_hc(uhci); + start_hc(uhci); + } return 0; } #endif diff -Nru a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h --- a/drivers/usb/host/uhci-hcd.h Fri May 23 14:25:39 2003 +++ b/drivers/usb/host/uhci-hcd.h Fri May 23 14:25:39 2003 @@ -53,6 +53,7 @@ #define USBPORTSC_RD 0x0040 /* Resume Detect */ #define USBPORTSC_LSDA 0x0100 /* Low Speed Device Attached */ #define USBPORTSC_PR 0x0200 /* Port Reset */ +#define USBPORTSC_OC 0x0400 /* Over Current condition */ #define USBPORTSC_SUSP 0x1000 /* Suspend */ /* Legacy support register */ @@ -282,6 +283,29 @@ return 0; /* int128 for 128-255 ms (Max.) */ } +/* + * Device states for the host controller. + * + * To prevent "bouncing" in the presence of electrical noise, + * we insist on a 1-second "grace" period, before switching to + * the RUNNING or SUSPENDED states, during which the state is + * not allowed to change. + * + * The resume process is divided into substates in order to avoid + * potentially length delays during the timer handler. + * + * States in which the host controller is halted must have values <= 0. + */ +enum uhci_state { + UHCI_RESET, + UHCI_RUNNING_GRACE, /* Before RUNNING */ + UHCI_RUNNING, /* The normal state */ + UHCI_SUSPENDING_GRACE, /* Before SUSPENDED */ + UHCI_SUSPENDED = -10, /* When no devices are attached */ + UHCI_RESUMING_1, + UHCI_RESUMING_2 +}; + #define hcd_to_uhci(hcd_ptr) container_of(hcd_ptr, struct uhci_hcd, hcd) /* @@ -313,7 +337,10 @@ struct uhci_frame_list *fl; /* P: uhci->frame_list_lock */ int fsbr; /* Full speed bandwidth reclamation */ unsigned long fsbrtimeout; /* FSBR delay */ - int is_suspended; + + enum uhci_state state; /* FIXME: needs a spinlock */ + unsigned long state_end; /* Time of next transition */ + int resume_detect; /* Need a Global Resume */ /* Main list of URB's currently controlled by this HC */ spinlock_t urb_list_lock;