Re: [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers

[David Gibson <dwg@au1.ibm.com>]

On Tue, Nov 13, 2007 at 11:27:31PM +0300, Valentine Barshak wrote:
> This patch makes PowerPC 4xx UIC use generic edge and level irq handlers
> instead of a custom handle_uic_irq() function. Acking a level irq on UIC
> has no effect if the interrupt is still asserted by the device, even if
> the interrupt is already masked. So, to really de-assert the interrupt
> we need to de-assert the external source first *and* ack it on UIC then.
> The handle_level_irq() function masks and ack's the interrupt with mask_ack
> callback prior to calling the actual ISR and unmasks it at the end.
> So, to use it with UIC level interrupts we need to ack in the unmask
> callback instead, after the ISR has de-asserted the external interrupt source.
> Even if we ack the interrupt that we didn't handle (unmask/ack it at
> the end of the handler, while next irq is already pending) it will not
> de-assert the irq, untill we de-assert its exteral source.

Hrm.  I *think* I'm convinced this is safe, although acking in a
callback which doesn't say it acks is rather yucky.  Essentially this
code is trading flow readability (because just reading
handle_level_irq will tell you something other than what it does in
our case) for smaller code size.  I'm not sure if this is a good trade
or not.

There's also one definite problem: according to the discussions I had
with Thomas Gleixner when I wrote uic.c, handle_edge_irq is not what
we want for edge interrupts.

Apparently handle_edge_irq is only for edge interrupts on "broken"
PICs which won't latch new interrupts while the irq is masked.  UIC is
not in this category, so handle_level_irq is actually what we want,
even for an edge irq.

Yes, I thought the naming was more than a little confusing, too.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson