Re: [PATCH RFC 1/5] dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/G2UL SoC

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH RFC 1/5] dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/G2UL SoC
Date: Mon, 19 Dec 2022 14:25:01 +0000	[thread overview]
Message-ID: <CA+V-a8uQFiU2KRcsoC5--tjfuWRj3VRJAUaZtv0+U0DziZQOwg@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdW_QuBUUypyrAbLqWPdZ81bWeYDyPbBf=2KmDht1X44bA@mail.gmail.com>

Hi Geert,

On Mon, Dec 19, 2022 at 1:50 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Dec 19, 2022 at 1:57 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > > > used as a fallback.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > > > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > > > domain) -> RISCV INTC
> > > >
> > > > I think this difference is purely a software difference, and abstracted
> > > > in DTS through the interrupt hierarchy.
> > > > Does it have any impact on the bindings?
> > > >
> > > > > - On the RZ/Five we have additional registers for IRQC block
> > > >
> > > > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > > > warranting separate compatible values.
> > > >
> > > > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> > > >
> > > > Can you please elaborate? I may have missed something, but to me it
> > > > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> > > >
> > > Now that we have to update the binding doc with the BUS_ERR_INT too,
> > > do you think it would make sense to add interrupt-names too?
>
> > Gentle ping.
>
> Thanks for the ping, I had missed you were waiting on input from me.
> Sorry for that...
>
No worries.

> As there are three different groups of parent interrupts, adding
> interrupt-names makes sense.
Ok.

> However, as this binding is already in active use since v6.1, you
> probably need to keep on supporting the
> ack of interrupt-names.  Or do you think there are no real users yet,
> and we can drop support for that?
>
Sorry can you please elaborate on "ack of interrupt-names".

So moving forward the driver will first check for interrupt-names
property and if that exists it will map the IRQ0-7 and GPIO-TINIT
interrupts (based on the names it will create a hierarchy domain) and
for the NMI and BUS_ERR_INT we request the IRQ numbers and register
the IRQ handler in IRQC driver itself.

And for backward compatibility we parse the IRQ numbers based on
indexes i.e. 0 = NMI, 1-8  = IRQ 0-7  and 9-41 GPIO TINT interrupts.

> > > BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> > > register a handler for it).
>
> Do you mean you will need a fourth parent type for that?
>
No something like what we have for NMI we can add something similar
below for bus error interrupts:
interrupts = ....
              <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
interrupt-names = ....,
             "bus-error-int";

As the registers to handle the NMI and BUS_ERR_INT are present on the
IRQC block, the interrupt handler will have to be registered by the
IRQC block itself by requesting the IRQ. So we will have to skip
mapping of BUS_ERR_INT as we do for the NMI case. Does that make
sense?

Cheers,
Prabhakar