Messages in this thread | | | Date | Tue, 2 Sep 2014 15:26:52 +0100 | From | Lee Jones <> | Subject | Re: [PATCH 1/4] mfd: arizona: Add additional dummy IRQ callbacks |
| |
On Tue, 02 Sep 2014, Charles Keepax wrote:
> 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?
I don't pretend to be an expert on the IRQ framework, but this certainly looks a lot less hacky than your previous submission.
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |