Re: [PATCH 3/3] regmap: enhance regmap-irq to handle 1 IRQ feeding n chips

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Tue, Jul 31, 2012 at 05:18:36PM -0600, Stephen Warren wrote:

> I don't think it's appropriate to put this support into the IRQ core.
> The main issue is that all the handlers for any shared wired-or
> interrupt line have to be registered before the IRQ is enabled, to avoid
> some initially active interrupt continually firing before the IRQ is
> enabled. Co-ordinating this when the wired-or line is on a board outside
> a device driver rather than internal to a chip and one device driver is
> a bit more than the IRQ core should probably be doing, hence I imagine
> why it doesn't support it.

No, that's not the issue at all - none of the above is at all different
to any other shared interrupt and obviously we support shared IRQs quite
happily (we wouldn't run on a good chunk of PCs if we didn't).  Shared
interrupts do require the hardware design not be insane but generally
hardware engineers do manage to get that right.

We don't support this for threaded IRQs due to thorny synchronisation
issues in fast paths.

> Co-ordinating this setup where all the sources of the wired-or are in
> one chip seems to belong to the chip driver, which is where my patch did
> this.

Well, no.  It did this by having a piece of framework code add a round
robin irq_chip (essentially a shared threaded IRQ) layered on top of the
existing regmap-irq code which had nothing to do with the rest of that
code.  There's nothing at all about that framework code which is at all
specific to regmap-irq, it just calls a series of sub IRQs every time
the primary IRQ goes off.

This isn't the chip driver that's doing things, it's the regmap-irq
code.  With the current round robin implementation there's no reason for
regmap to implement it, other things can quite happily do the same thing.
Having a regmap helper which used a generic facility would be reasonable
but the actual demux is a generic thing.

[Suggestion to not bounce back into the IRQ core]

> but it seems a little hokey to short-circuit the IRQ core; it would
> prevent execution of any statistics gathering or stuck interrupt
> handling that handle_nested_irq() might do for example.

This seems like a better approach if doing things entirely in regmap.  I
can't see any impact on any of the IRQ core features here, we're always
going to call each of the sub IRQs exactly once for each call to the IRQ
handler and the stuck IRQ code is still going to identify the same set
of real IRQs as stuck.

> Now, if we made each child regmap_irq not be its own IRQ domain or
> irq_chip, but simply had one top-level domain/chip that aggregated them,
> that argument would be moot. However, that top-level domain/chip would
> become rather complex and just end up doing a bunch of demultiplexing
> code that's not needed if we do it like in my patch...

That demultiplexing seems excessively complex, yes.