Messages in this thread | | | Date | Mon, 25 Apr 2016 11:50:13 +0100 | From | Lee Jones <> | Subject | Re: [PATCH] mfd: arizona: Check if AOD interrupts are pending before dispatching |
| |
On Thu, 14 Apr 2016, Richard Fitzgerald wrote:
> Previously the arizona_irq_thread implementation would call > handle_nested_irqs() to handle AOD interrupts without checking if any > were actually pending. The kernel will see these as spurious IRQs and > will eventually disable the IRQ. > > This patch ensures we only launch the nested handler if there are AOD > interrupts pending in the codec. > > Signed-off-by: Simon Trimmer <simont@opensource.wolfsonmicro.com> > Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com> > --- > drivers/mfd/arizona-irq.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c > index edeb495..edaf592 100644 > --- a/drivers/mfd/arizona-irq.c > +++ b/drivers/mfd/arizona-irq.c > @@ -97,7 +97,7 @@ static irqreturn_t arizona_irq_thread(int irq, void *data) > { > struct arizona *arizona = data; > bool poll; > - unsigned int val; > + unsigned int val, nest_irq; > int ret; > > ret = pm_runtime_get_sync(arizona->dev); > @@ -109,8 +109,23 @@ static irqreturn_t arizona_irq_thread(int irq, void *data) > do { > poll = false; > > - if (arizona->aod_irq_chip) > - handle_nested_irq(irq_find_mapping(arizona->virq, 0)); > + if (arizona->aod_irq_chip) { > + /* > + * Check the AOD status register to determine whether > + * the nested IRQ handler should be called. > + */ > + ret = regmap_read(arizona->regmap, > + ARIZONA_AOD_IRQ1, > + &val);
Nit: Pop this on the line above.
> + if (ret == 0 && val != 0) { > + nest_irq = irq_find_mapping(arizona->virq, 0); > + handle_nested_irq(nest_irq); > + } else if (ret != 0) { > + dev_err(arizona->dev,
It's only a valid 'error' if you're return an error condition. AFAICT, you're not doing that, so use dev_warn() instead.
> + "Failed to read AOD IRQ1 %d\n", > + ret);
Nit: Pop this on the line above.
> + } > + }
I think the whole thing would be better written like:
if (arizona->aod_irq_chip) { /* * Check the AOD status register to determine whether * the nested IRQ handler should be called. */ ret = regmap_read(arizona->regmap, ARIZONA_AOD_IRQ1, &val); if (ret) dev_warn(arizona->dev, "Failed to read AOD IRQ1 %d\n", ret); else if (val) handle_nested_irq( irq_find_mapping(arizona->virq, 0)); }
> > /* > * Check if one of the main interrupts is asserted and only
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |