Re: OHCI IRQ issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

> I hope you mean the "test low bit of hcca->done_head" optimization...
> FWIW, that also breaks the SA-1111 OHCI controller.  I'm not sure that
> it's worth avoiding the register read here, especially since it does
> cause problems now and then. 

Yes, that's the one I'm talking about. I think it needs to go :-)

> Seeing that optimization called out in the OHCI spec was actually a
> surprise to me ... it wouldn't seem to play well with the usual
> implementation flexibility goals, and I've not seen similar things
> done with other drivers.  (Which is why for example neither EHCI
> nor UHCI could ever try such a thing!)

Yeah, I don't think it brings much benefit anyway.

> > We must not do that on edge interrupts because the interrupts we "miss"
> > will not be re-triggered as they don't stay asserted.
> If so, why aren't you arguing that this should be looping until
> no IRQ status bits are set?

Because it's expected that if a new bit gets set after the one that
triggered the interrupt, a new interrupt is emitted. The IRQ controller
is supposed to latch any subsequent edge interrupt happening after the
first one have been acked on the PIC (which should happen before the
driver's handler is called for egde interrupts).

So if 3 edge interrupts happened before the ack for example, the driver
will only be called once though all 3 conditions will be visible via a
single read of the status register. Anything that happens afterward
(such as after the read of the status register) should be latched
properly by the PIC and the driver will be called again.

If that wasn't the case, then there would still be a race, even with a
loop, if the IRQ occurs after the exit of the loop ... so the loop is
not necessary if the PIC is behaving properly. (In some case, that
effect is achieved with a SW trick btw, by not masking the interrupt
while handling, and having the PIC code do the latching in SW if a new
occurence of the IRQ happens while in the handler)

> I'm not sure what exactly you mean by edge triggering here.  Which
> edge(s)?  I've seen individual interrupt status bits act as if they
> used rising-edge triggers, but never the primary OHCI interrupt.

rising or falling edge, whatever a given hardware implements. Edge
triggered interrupts, unlike level triggered interrupts, don't "stick"
if the initial condition is still active. They are one shot events. They
are common in SoCs. In the specific case of the PPC440EP (and similar)
chips, the internal OHCI emits interrupts as edge conditions, not level
(this could be considered a HW misdesign I suppose but that's there and
we need to cope).

> Even in SOCs, that primary interrupt has been level triggered.

Well, at least not on that SoC :-)

> I think that's at least implicit in the IRQ handling model, in the
> way that IRQs need to be acked.
> > - There may also be issues with edge interrupts here:
> > 
> > 	if (ints & OHCI_INTR_RHSC) {
> > 
> > 	.../...
> > 
> > 	}
> > 	else if (ints & OHCI_INTR_RD) {
> Notice the CodingStyle violation ... "else" should be snugged
> up against the bracket!

I think the coding style violation comes from a big comment between the
backet and the else that I haven't pasted :-) In which case it's fair I
suppose ...

> > If for some reason both RHSC and RD are set, we'll never notice RD, is
> > that expected ?
> That's newish 2.6 behavior, and Alan's answer is right.
> The 2.4 code didn't use either interrupt.  Switching over
> to IRQs for root hub management made the dynamic tick
> stuff work better on ARM.  (That's the NO_HZ precursor.)
> Those hub events can be either level or edge triggered,
> confusingly enough...

Heh. Allright.

> The thing is, the 2.4 code tended to just throw lots of oddball
> logic in there to work around some chip quirk ... without so
> much as a comment about the issue being worked around.  It's
> no fun to find out via bug reports that some odd construct was
> actually an undocumented workaround for a chip bug.
> This one does look silly enough that it'd be worth seeing if
> removing it hurts anything.  Let it cook for a while.

Ok. I'll send a separate fix for removing the no-status-read
optimisation, which can go in now, and we can do a patch that removes
those 2 writes that can simmer in -mm for a little while.


This email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
To unsubscribe, use the last form field at:

[Home]     [Video for Linux]     [Photo]     [Yosemite Forum]     [Yosemite Photos]    [Video Projectors]     [PDAs]     [Hacking TiVo]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Devices]     [Big List of Linux Books]     [Free Dating]

  Powered by Linux