Messages in this thread | | | Date | Wed, 1 Aug 2012 12:41:42 +0100 | From | Mark Brown <> | Subject | Re: [PATCH 3/3] regmap: enhance regmap-irq to handle 1 IRQ feeding n chips |
| |
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.
| |