On Thu, 10 Feb 2005, Russell King wrote:
> On Thu, Feb 10, 2005 at 02:08:16PM -0800, Christopher Hoover wrote:
> > > Greg:
> > >
> > > This patch contains the changes to the ohci-sa1111 driver.
> > >
> > > Alan Stern
> >
> >
> > This patch introduces several paths in the interrupt handler that return
> > IRQ_NONE. In the original code, we took care (in usb_hcd_sa1111_hcim_irq)
> > to always return IRQ_HANDLED, no matter what. I don't recall offhand why we
> > did this, but I'm mildly hesitant about this part of the patch given that
> > the SA-1111 is a quirky beast, especially re: interrupts.
>
> It's because edge triggered interrupts are fundamentally unreliable with
> "bad interrupt" detection.
>
> Consider:
>
> while (interrupt pending) {
> clear it;
> service it;
> }
>
> and consider what happens in hardware when an interrupt occurs after
> clearing the original source. The new interrupt triggers the interrupt
> circuitry, and, because this interrupt is being serviced, we remember
> that it happened. Meanwhile, we loop and find this new interrupt
> pending and service it. However, the generic IRQ code has no way to
> clear this condition.
>
> Once the interrupt has completed, we leave the driver handler. The
> generic IRQ code notices we have a pending interrupt, and re-calls
> the handler. The handler finds no work to do, and would normally
> return IRQ_NONE. At that point, we scream about a bad interrupt.
>
> One thing someone might suggest is why we remember such interrupts.
> Consider the alternative case where the interrupt occurs immediately
> after we've determined that there is no interrupt pending in the
> driver specific handler, but before we've returned to the generic
> IRQ code. Not remembering the pending interrupt means we drop
> pending interrupts, possibly causing that device to stop responding.
>
> So, essentially, the whole irqreturn_t/IRQ_HANDLED/IRQ_NONE doesn't
> suit edge-triggered interrupt sources on ARM. I've been debating
> for some time to make the return value irrelevant with such
> triggered sources - not doing so currently means people end up
> passing noirqdebug to the kernel just to get a stable system. This
> is obviously not the point of the original idea behind this.
Until you do, we could add a small extra patch to catch this sort of
problem. Something like this:
--- 1.134/drivers/usb/host/ohci-hcd.c 2005-02-05 01:06:47 -05:00
+++ edited/drivers/usb/host/ohci-hcd.c 2005-02-14 11:50:18 -05:00
@@ -148,6 +148,14 @@
#include "ohci-q.c"
+/* If interrupts are edge-triggered, we must never return IRQ_NONE */
+#if defined(CONFIG_SA1111)
+#define EDGE_TRIGGERED_INTS 1
+#else
+#define EDGE_TRIGGERED_INTS 0
+#endif
+
+
/* Some boards misreport power switching/overcurrent */
static int distrust_firmware = 1;
module_param (distrust_firmware, bool, 0);
@@ -703,6 +711,8 @@
/* interrupt for some other device? */
} else if ((ints &= ohci_readl (ohci, ®s->intrenable)) == 0) {
+ if (EDGE_TRIGGERED_INTS)
+ return IRQ_HANDLED;
return IRQ_NONE;
}
The other path introduced by my patch where IRQ_NONE can be returned
gets used only at times when the controller is not running and hence is
incapable of generating interrupts. So it shouldn't be a problem.
Would this be satisfactory? Or do you prefer to change the core interrupt
routines?
Alan Stern
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
