Re: OHCI IRQ issues

[Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>]

> > We must not do that on edge interrupts because the interrupts we "miss"
> > will not be re-triggered as they don't stay asserted.
> 
> I'm confused.  Are you saying that the USB controller generates a
> normal interrupt request level but the interrupt controller interprets 
> it as edge-triggered?  How could that ever possibly work?
>
> Or are you saying that the USB controller generates a "spike" interrupt 
> request signal whenever one of the appropriate bits gets turned on?  In 
> that case the interrupts we miss _would_ be re-triggered, wouldn't 
> they?

The OHCI in this case generates an edge interrupt (what you call a
"spike", it's not that uncommon in SoC land). If you miss it it won't be
re-emitted. With edge interrupts, it's the responsibility of the driver
to make sure to harvest all conditions every time an interrupt happens.

 .../...

> Yes, it is.  The RHSC case calls usb_hcd_poll_rh_status(), which 
> indirectly calls ohci_root_hub_state_changes(), which does a resume if 
> the controller is suspended or auto-stopped.  Hence in this case the RD 
> isn't needed or wanted.

Ok, so that case is good.

> > 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.
> 
> Me too.  I don't know why those lines are there.

Allright, I'll send a patch removing them.

> If it does cause problems on some chipsets then removing it is 
> probably okay.  It looks like a very minor optimization.

Yes, I don't see the point at all. In fact, EHCI doesn't even try to do
something like that.

> > 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.
> 
> The "else" around RD is really needed, or at least it used to be
> needed.  Without it we would end up trying to resume a suspended
> controller twice, and the second resume would get into trouble.

Ok. I'll send a patch changing the 2 other cases then.

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