From b5fb454f69642f9d933b327b185a2ba06dd0945c Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 20 Aug 2008 17:22:05 -0400 Subject: USB: automatically enable RHSC interrupts This patch (as1069c) changes the way OHCI root-hub status-change interrupts are enabled. Currently a special HCD method, hub_irq_enable(), is called when the hub driver is finished using a root hub. This approach turns out to be subject to races, resulting in unnecessary polling. The patch does away with the method entirely. Instead, the driver automatically enables the RHSC interrupt when no more status changes are present. This scheme is safe with controllers using level-triggered semantics for their interrupt flags. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/ohci-hub.c | 53 ++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 22 deletions(-) (limited to 'drivers/usb/host/ohci-hub.c') diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 439beb784f3..7ea9a7b3115 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -36,18 +36,6 @@ /*-------------------------------------------------------------------------*/ -/* hcd->hub_irq_enable() */ -static void ohci_rhsc_enable (struct usb_hcd *hcd) -{ - struct ohci_hcd *ohci = hcd_to_ohci (hcd); - - spin_lock_irq(&ohci->lock); - if (!ohci->autostop) - del_timer(&hcd->rh_timer); /* Prevent next poll */ - ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable); - spin_unlock_irq(&ohci->lock); -} - #define OHCI_SCHED_ENABLES \ (OHCI_CTRL_CLE|OHCI_CTRL_BLE|OHCI_CTRL_PLE|OHCI_CTRL_IE) @@ -374,18 +362,28 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { int poll_rh = 1; + int rhsc; + rhsc = ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC; switch (ohci->hc_control & OHCI_CTRL_HCFS) { case OHCI_USB_OPER: - /* keep on polling until we know a device is connected - * and RHSC is enabled */ + /* If no status changes are pending, enable status-change + * interrupts. + */ + if (!rhsc && !changed) { + rhsc = OHCI_INTR_RHSC; + ohci_writel(ohci, rhsc, &ohci->regs->intrenable); + } + + /* Keep on polling until we know a device is connected + * and RHSC is enabled, or until we autostop. + */ if (!ohci->autostop) { if (any_connected || !device_may_wakeup(&ohci_to_hcd(ohci) ->self.root_hub->dev)) { - if (ohci_readl(ohci, &ohci->regs->intrenable) & - OHCI_INTR_RHSC) + if (rhsc) poll_rh = 0; } else { ohci->autostop = 1; @@ -398,12 +396,13 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, ohci->autostop = 0; ohci->next_statechange = jiffies + STATECHANGE_DELAY; - } else if (time_after_eq(jiffies, + } else if (rhsc && time_after_eq(jiffies, ohci->next_statechange) && !ohci->ed_rm_list && !(ohci->hc_control & OHCI_SCHED_ENABLES)) { ohci_rh_suspend(ohci, 1); + poll_rh = 0; } } break; @@ -417,6 +416,12 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, else usb_hcd_resume_root_hub(ohci_to_hcd(ohci)); } else { + if (!rhsc && (ohci->autostop || + ohci_to_hcd(ohci)->self.root_hub-> + do_remote_wakeup)) + ohci_writel(ohci, OHCI_INTR_RHSC, + &ohci->regs->intrenable); + /* everything is idle, no need for polling */ poll_rh = 0; } @@ -438,12 +443,16 @@ static inline int ohci_rh_resume(struct ohci_hcd *ohci) static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { - int poll_rh = 1; - - /* keep on polling until RHSC is enabled */ + /* If RHSC is enabled, don't poll */ if (ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC) - poll_rh = 0; - return poll_rh; + return 0; + + /* If no status changes are pending, enable status-change interrupts */ + if (!changed) { + ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable); + return 0; + } + return 1; } #endif /* CONFIG_PM */ -- cgit v1.2.3-70-g09d2 From 4a511bc3f5829bc18428bcf11c25417a79d09396 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 3 Sep 2008 16:38:32 -0400 Subject: OHCI: Allow broken controllers to auto-stop This patch (as1134) attempts to improve the way we handle OHCI controllers with broken Root Hub Status Change interrupt support. In these controllers the RHSC interrupt bit essentially never turns off, making RHSC interrupts useless -- they have to remain permanently disabled. Such controllers should still be allowed to turn off their root hubs when no devices are attached. Polling for new connections can continue while the root hub is suspended. The patch implements this feature. (It won't have much effect unless CONFIG_PM is enabled and CONFIG_USB_SUSPEND is disabled, but since the overhead is very small we may as well do it.) Signed-off-by: Alan Stern Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/ohci-hub.c | 60 ++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 28 deletions(-) (limited to 'drivers/usb/host/ohci-hub.c') diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 7ea9a7b3115..a150e85c901 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -362,18 +362,23 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { int poll_rh = 1; - int rhsc; + int rhsc_status, rhsc_enable; - rhsc = ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC; - switch (ohci->hc_control & OHCI_CTRL_HCFS) { + /* 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; + switch (ohci->hc_control & OHCI_CTRL_HCFS) { case OHCI_USB_OPER: - /* If no status changes are pending, enable status-change - * interrupts. - */ - if (!rhsc && !changed) { - rhsc = OHCI_INTR_RHSC; - ohci_writel(ohci, rhsc, &ohci->regs->intrenable); + /* If no status changes are pending, enable RHSC interrupts. */ + if (!rhsc_enable && !rhsc_status && !changed) { + rhsc_enable = OHCI_INTR_RHSC; + ohci_writel(ohci, rhsc_enable, &ohci->regs->intrenable); } /* Keep on polling until we know a device is connected @@ -383,7 +388,7 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, if (any_connected || !device_may_wakeup(&ohci_to_hcd(ohci) ->self.root_hub->dev)) { - if (rhsc) + if (rhsc_enable) poll_rh = 0; } else { ohci->autostop = 1; @@ -396,34 +401,36 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, ohci->autostop = 0; ohci->next_statechange = jiffies + STATECHANGE_DELAY; - } else if (rhsc && time_after_eq(jiffies, + } else if (time_after_eq(jiffies, ohci->next_statechange) && !ohci->ed_rm_list && !(ohci->hc_control & OHCI_SCHED_ENABLES)) { ohci_rh_suspend(ohci, 1); - poll_rh = 0; + if (rhsc_enable) + poll_rh = 0; } } break; - /* if there is a port change, autostart or ask to be resumed */ case OHCI_USB_SUSPEND: case OHCI_USB_RESUME: + /* if there is a port change, autostart or ask to be resumed */ if (changed) { if (ohci->autostop) ohci_rh_resume(ohci); else usb_hcd_resume_root_hub(ohci_to_hcd(ohci)); } else { - if (!rhsc && (ohci->autostop || + if (!rhsc_enable && !rhsc_status && (ohci->autostop || ohci_to_hcd(ohci)->self.root_hub-> - do_remote_wakeup)) - ohci_writel(ohci, OHCI_INTR_RHSC, + do_remote_wakeup)) { + rhsc_enable = OHCI_INTR_RHSC; + ohci_writel(ohci, rhsc_enable, &ohci->regs->intrenable); - - /* everything is idle, no need for polling */ - poll_rh = 0; + } + if (rhsc_enable) + poll_rh = 0; } break; } @@ -443,12 +450,16 @@ static inline int ohci_rh_resume(struct ohci_hcd *ohci) static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { + 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 status-change interrupts */ - if (!changed) { + /* 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; } @@ -492,13 +503,6 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf) length++; } - /* Some broken controllers never turn off RHCS in the interrupt - * status register. For their sake we won't re-enable RHSC - * interrupts if the flag is already set. - */ - if (ohci_readl(ohci, &ohci->regs->intrstatus) & OHCI_INTR_RHSC) - changed = 1; - /* look at each port */ for (i = 0; i < ohci->num_ports; i++) { u32 status = roothub_portstatus (ohci, i); -- cgit v1.2.3-70-g09d2 From 71b7497c078a97e2afb774ad7c1f8ff5bdda8a60 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 9 Oct 2008 15:40:23 -0400 Subject: USB: OHCI: fix endless polling behavior 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 Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/ohci-hub.c | 51 ++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 19 deletions(-) (limited to 'drivers/usb/host/ohci-hub.c') diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index a150e85c901..32bbce9718f 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -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); -- cgit v1.2.3-70-g09d2