Re: [PATCH v3 4/6] dt-bindings: interrupt-controller: realtek,rtl-intc: require parents

From: Rob Herring <robh@kernel.org>
To: Sander Vanheule <sander@svanheule.net>
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>,
	Birger Koblitz <mail@birger-koblitz.de>,
	Bert Vermeulen <bert@biot.com>, John Crispin <john@phrozen.org>
Subject: Re: [PATCH v3 4/6] dt-bindings: interrupt-controller: realtek,rtl-intc: require parents
Date: Fri, 4 Feb 2022 20:08:09 -0600	[thread overview]
Message-ID: <Yf3cCTB5Qbz44nc/@robh.at.kernel.org> (raw)
In-Reply-To: <d291855a36f200b178aa9e7fb6e41ff438773e38.camel@svanheule.net>

On Sat, Jan 22, 2022 at 01:49:44PM +0100, Sander Vanheule wrote:
> Hi Rob,
> 
> On Fri, 2022-01-21 at 16:56 -0600, Rob Herring wrote:
> > On Sun, Jan 09, 2022 at 03:54:35PM +0100, Sander Vanheule wrote:
> > > The interrupt router has 32 inputs and up to 15 outputs, and the way
> > > these are mapped to each other is runtime configurable. The outputs of
> > > this interrupt router on the other hand, are connected to a fixed set of
> > > parent interrupts. This means that "interrupt-map" is inappropriate, and
> > > rather a list of parent interrupts should be specified.
> > 
> > I'm not sure why interrupt-map is not appropriate. It is not appropriate 
> > if you have to touch the interrupt router h/w in servicing the 
> > interrupts. If you just need one time configuration of the mapping, then 
> > it should be fine to use I think.
> 
> If interrupt-map is used, then AFAICT there are no hooks to inform the driver that a
> translation has occurred. How should the interrupt controller driver then know how to set
> up the routing? Commit de4adddcbcc2 ("of/irq: Add a quirk for controllers with their own
> definition of interrupt-map") added a quirk for the original binding/driver, but that
> requires open-coding an interrupt-map parser in the driver.

The issue was not open-coding parsing, but was the need for something in 
the middle to service the interrupt. As 'interrupt-map' should be a 
transparent remapping or routing.

> 
> What this binding doesn't mention (I can add it), is that there are also two IRQ status
> registers to:
>   - unmask/mask SoC interrupts
>   - read the current status of the SoC interrupts

That would not be transparent.

> In theory, if the routing is set up correctly (and the IRQ permanently unmasked), I think
> one could treat interrupt-map as intended, and connect SoC peripheral IRQ handlers
> directly to the parent interrupts. But then the interrupt subsystem would need to check
> all attached handlers. This interrupt router/controller allows to check which peripheral
> is triggering the parent IRQ, which should be more efficient.
>
> These interrupt controllers are also used on multi-threaded systems, where each hardware
> thread has its own IRQ controller. I'm still experimenting with the implementation, but 
> there the routing registers would be used to set the CPU affinity of SoC interrupts.
> 
> I have to say that I'm not very familiar with the kernel code that handles all this
> though, so maybe I'm just missing something?

Okay, seems 'interrupt-map' is indeed not appropriate here.

Rob