Re: [PATCH 1/4] regmap: irq: Fix lost interrupts by introducing handle_reread

From: Mark Brown
Date: Mon Mar 27 2017 - 14:24:09 EST


On Wed, Mar 22, 2017 at 10:10:49AM -0700, Tony Lindgren wrote:
> At least Motorola CPCAP PMIC needs it's device interrupts re-read
> until there are no more interrupts. Otherwise the PMIC interrupt to
> the SoC will eventually stop toggling. This seems to be a bug in the
> CPCAP PMIC where it can stop driving the GPIO interrupt to the SoC
> with pending CPCAP interrupts.

> Let's allow handling issues like this by introducing a flag for
> handle_reread and by splitting regmap_irq_thread() into two separate
> functions for regmap_read_irq_status() and regmap_irq_handle_pending().

So, I see your use case but the fact is that as Charles observed this is
exactly the code used for emulating level triggered IRQs with edge
triggered interrupt controllers. This means someone is doubtless going
to end up using it for precisely that. This makes me uncomfortable, we
do have this open coded into various drivers already but this is more of
a core thing and it feels like this should be in genirq rather than
here... that said, looking at the code:

> + do {
> + ret = regmap_read_irq_status(data);
> + if (ret)
> + goto out_runtime_put;
> +
> + ret = regmap_irq_handle_pending(data);
> + if (ret < 0)
> + goto out_runtime_put;
> +
> + if (!ret)
> + break;
> +
> + handled += ret;
> + } while (chip->handle_reread);

There's no protection against screaming interrupts here, I'd really like
to see that. Also some tracing of the number of times we spin.

Attachment: signature.asc
Description: PGP signature