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 <[email protected]>
> Signed-off-by: Richard Fitzgerald <[email protected]>
> ---
> 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
