USB: OHCI: fix endless polling behavior
authorAlan Stern <stern@rowland.harvard.edu>
Thu, 9 Oct 2008 19:40:23 +0000 (15:40 -0400)
committerGreg Kroah-Hartman <gregkh@suse.de>
Fri, 17 Oct 2008 21:41:04 +0000 (14:41 -0700)
This patch (as1149) fixes an obscure problem in OHCI polling.  In the
current code, if the RHSC interrupt status flag turns on at a time
when RHSC interrupts are disabled, it will remain on forever:

The interrupt handler is the only place where RHSC status
gets turned back off;

The interrupt handler won't turn RHSC status off because it
doesn't turn off status flags if the corresponding interrupt
isn't enabled;

RHSC interrupts will never get enabled because
ohci_root_hub_state_changes() doesn't reenable RHSC if RHSC
status is on!

As a result we will continue polling indefinitely instead of reverting
to interrupt-driven operation, and the root hub will not autosuspend.
This particular sequence of events is not at all unusual; in fact
plugging a USB device into an OHCI controller will usually cause it to
occur.

Of course, this is a bug.  The proper thing to do is to turn off RHSC
status just before reading the actual port status values.  That way
either a port status change will be detected (if it occurs before the
status read) or it will turn RHSC back on.  Possibly both, but that
won't hurt anything.

We can still check for systems in which RHSC is totally broken, by
re-reading RHSC after clearing it and before reading the port
statuses.  (This re-read has to be done anyway, to post the earlier
write.)  If RHSC is on but no port-change statuses are set, then we
know that RHSC is broken and we can avoid re-enabling it.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/host/ohci-hub.c

index a150e85c901ac5321e282a9d19eefaca3e37837d..32bbce9718f0626b98cc37e09b14faa51a5cc16f 100644 (file)
@@ -359,17 +359,15 @@ static void ohci_finish_controller_resume(struct usb_hcd *hcd)
 
 /* Carry out polling-, autostop-, and autoresume-related state changes */
 static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed,
-               int any_connected)
+               int any_connected, int rhsc_status)
 {
        int     poll_rh = 1;
-       int     rhsc_status, rhsc_enable;
+       int     rhsc_enable;
 
        /* Some broken controllers never turn off RHCS in the interrupt
         * status register.  For their sake we won't re-enable RHSC
         * interrupts if the interrupt bit is already active.
         */
-       rhsc_status = ohci_readl(ohci, &ohci->regs->intrstatus) &
-                       OHCI_INTR_RHSC;
        rhsc_enable = ohci_readl(ohci, &ohci->regs->intrenable) &
                        OHCI_INTR_RHSC;
 
@@ -421,14 +419,23 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed,
                                ohci_rh_resume(ohci);
                        else
                                usb_hcd_resume_root_hub(ohci_to_hcd(ohci));
+
+               /* If remote wakeup is disabled, stop polling */
+               } else if (!ohci->autostop &&
+                               !ohci_to_hcd(ohci)->self.root_hub->
+                                       do_remote_wakeup) {
+                       poll_rh = 0;
+
                } else {
-                       if (!rhsc_enable && !rhsc_status && (ohci->autostop ||
-                                       ohci_to_hcd(ohci)->self.root_hub->
-                                               do_remote_wakeup)) {
+                       /* If no status changes are pending,
+                        * enable RHSC interrupts
+                        */
+                       if (!rhsc_enable && !rhsc_status) {
                                rhsc_enable = OHCI_INTR_RHSC;
                                ohci_writel(ohci, rhsc_enable,
                                                &ohci->regs->intrenable);
                        }
+                       /* Keep polling until RHSC is enabled */
                        if (rhsc_enable)
                                poll_rh = 0;
                }
@@ -448,22 +455,22 @@ static inline int ohci_rh_resume(struct ohci_hcd *ohci)
  * autostop isn't used when CONFIG_PM is turned off.
  */
 static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed,
-               int any_connected)
+               int any_connected, int rhsc_status)
 {
-       int     rhsc_status;
-
        /* If RHSC is enabled, don't poll */
        if (ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC)
                return 0;
 
-       /* If no status changes are pending, enable RHSC interrupts */
-       rhsc_status = ohci_readl(ohci, &ohci->regs->intrstatus) &
-                       OHCI_INTR_RHSC;
-       if (!changed && !rhsc_status) {
-               ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable);
-               return 0;
-       }
-       return 1;
+       /* If status changes are pending, continue polling.
+        * Conversely, if no status changes are pending but the RHSC
+        * status bit was set, then RHSC may be broken so continue polling.
+        */
+       if (changed || rhsc_status)
+               return 1;
+
+       /* It's safe to re-enable RHSC interrupts */
+       ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable);
+       return 0;
 }
 
 #endif /* CONFIG_PM */
@@ -478,6 +485,7 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
        struct ohci_hcd *ohci = hcd_to_ohci (hcd);
        int             i, changed = 0, length = 1;
        int             any_connected = 0;
+       int             rhsc_status;
        unsigned long   flags;
 
        spin_lock_irqsave (&ohci->lock, flags);
@@ -503,6 +511,11 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
                length++;
        }
 
+       /* Clear the RHSC status flag before reading the port statuses */
+       ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrstatus);
+       rhsc_status = ohci_readl(ohci, &ohci->regs->intrstatus) &
+                       OHCI_INTR_RHSC;
+
        /* look at each port */
        for (i = 0; i < ohci->num_ports; i++) {
                u32     status = roothub_portstatus (ohci, i);
@@ -521,7 +534,7 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
        }
 
        hcd->poll_rh = ohci_root_hub_state_changes(ohci, changed,
-                       any_connected);
+                       any_connected, rhsc_status);
 
 done:
        spin_unlock_irqrestore (&ohci->lock, flags);