OHCI IRQ issues

[Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>]

I have a problem with the current ohci_irq() implementation:

- The optimisation below breaks if the controller has an edge triggered
interrupt which is the case on some embedded controllers such as the
AMCC PowerPC 440EP:

	/* we can eliminate a (slow) ohci_readl()
	 * if _only_ WDH caused this irq
	 */
	if ((ohci->hcca->done_head != 0)
			&& ! (hc32_to_cpup (ohci, &ohci->hcca->done_head)
				& 0x01)) {
		ints =  OHCI_INTR_WDH;

	/* cardbus/... hardware gone before remove() */
	} else if ((ints = ohci_readl (ohci, &regs->intrstatus)) == ~(u32)0) {
		disable (ohci);
		ohci_dbg (ohci, "device removed!\n");
		return IRQ_HANDLED;

We must not do that on edge interrupts because the interrupts we "miss"
will not be re-triggered as they don't stay asserted.

- There may also be issues with edge interrupts here:

	if (ints & OHCI_INTR_RHSC) {

	.../...

	}
	else if (ints & OHCI_INTR_RD) {

If for some reason both RHSC and RD are set, we'll never notice RD, is
that expected ?

- I can't understand why we do that masking/unmasking later on:

	if (ints & OHCI_INTR_WDH) {
		if (HC_IS_RUNNING(hcd->state))
			ohci_writel (ohci, OHCI_INTR_WDH, &regs->intrdisable);
		spin_lock (&ohci->lock);
		dl_done_list (ohci);
		spin_unlock (&ohci->lock);
		if (HC_IS_RUNNING(hcd->state))
			ohci_writel (ohci, OHCI_INTR_WDH, &regs->intrenable);
	}

Those 2 writes to intrdisable look totally useless to me. First they are
posted so they won't actually protect anything, then, we -are- in the
interrupt handler for the OHCI, so we cannot take that interrupt again
recursively anyway. So they are just a waste of cycles as far as I'm
concerned.

Now, regarding the first problem, my proposal would be to just remove
the optimisation, I can't see it being very useful in practice. We
-could- add some flag whether the driver is using a level or an edge
interrupt and have the SoC glue set it, but that doesn't seem like a
worthy optimisation to me. Beside, that's also going to get us into
trouble if the implementation has subtle DMA ordering issues, i'd rather
-guarantee- that DMAs are flushed with a read before going to mess
around with the done list (I suspect that may explain some so far
unexplained problem we've been having on some other weird
implementations).

As for the two others, what do you guys think ? I believe the masking in
WDH handling can just be removed, and I'm not sure about the else around
RD handling.

Cheers,
Ben.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-devel@xxxxxxxxxxxxxxxxxxxxx
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel