Re: [PATCH 1/4] mfd: arizona: Add additional dummy IRQ callbacks

[Charles Keepax <ckeepax@opensource.wolfsonmicro.com>]

On Thu, Aug 21, 2014 at 12:56:31PM +0100, Lee Jones wrote:
> On Tue, 12 Aug 2014, Charles Keepax wrote:
> 
> > We use a dummy IRQ chip to dispatch interrupts to the two seperate IRQ
> > domains on the Arizona devices. Currently only the enable and disable
> > callbacks are defined however, there are some situations where additional
> > callbacks will be used from the IRQ core, which currently results in an
> > NULL pointer deference. Add handlers for more of the IRQ callbacks and
> > combine these into a single function since they are all identical.
> > 
> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > ---
> 
> If you provide .irq_enable(), then .irq_unmask becomes redundant
> and/or is checked for before invoking.  There is a chance of
> .irq_mask() being called, but if this is a problem, it should be fixed
> in the IRQ Chip code.  There is also one unprotected invocation of
> .irq_ack(), but I think this should be fixed rather than forcing each
> user of IRQ Chip to provide all of these call-backs.

So I have been looking at this further and going back over things
from when the issue was discovered and it looks like it was
probably the unprotected irq_ack call (in handle_edge_irq) that
was the problem. But thinking about this more I am fairly
convinced that the call is unprotected because it is expected that
an edge IRQ will always have an ack, as it doesn't really make
sense for an edge IRQ to not have an ack.

The IRQ chip here is just a software device to distribute the IRQ
to the two sub-domains. As such I think the problem lies here:

	irq_set_chip_and_handler(virq, &arizona_irq_chip, handle_edge_irq);

I am pretty sure the correct fix is to change this to a
handle_simple_irq as it is just a software dummy and there is
nothing really edge triggered about it. Do you see any problems
with that as a solution?

Thanks,
Charles
\x16