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

[Tony Lindgren]

* Mark Brown <broonie@xxxxxxxxxx> [170327 10:52]:
> 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:

Yes.. But then again we might avoid piling up yet more driver
specific hacks. I don't know what the genirq solution would look
like, handle until we get IRQ_NONE? :)

> > +	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.

Good idea. How about something like below where handle_reread checks
for the total time spent in the thread loop with a large enough
timeout? Or do you have some better ideas in mind?

I tested I can hit that warning with timeout set to much lower
10 ms with retries being 1 or 2 at that point.

Regards,

Tony

8< -------------------------------