Message-ID:
 <TYYPR01MB139554162E9D8DAAD69E8976585DFA@TYYPR01MB13955.jpnprd01.prod.outlook.com>
Date: Thu, 27 Nov 2025 14:44:16 +0000
From: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Thomas Gleixner
	<tglx@...utronix.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Geert Uytterhoeven
	<geert+renesas@...der.be>, magnus.damm <magnus.damm@...il.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH 1/4] dt-bindings: interrupt-controller: document
 RZ/{T2H,N2H} ICU

> From: Krzysztof Kozlowski <krzk@...nel.org>
> Sent: Thursday, November 27, 2025 9:19 AM
>
> On 24/11/2025 17:25, Cosmin-Gabriel Tanislav wrote:
> >> From: Krzysztof Kozlowski <krzk@...nel.org>
> >> Sent: Sunday, November 23, 2025 3:24 PM
> >>
> >> On 21/11/2025 12:14, Cosmin Tanislav wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - const: renesas,r9a09g077-icu # RZ/T2H
> >>> +
> >>> +      - items:
> >>> +          - enum:
> >>> +              - renesas,r9a09g087-icu # RZ/N2H
> >>> +          - const: renesas,r9a09g077-icu
> >>> +
> >>> +  '#interrupt-cells':
> >>> +    description: The first cell is the SPI number of the interrupt, as per user
> >>> +      manual. The second cell is used to specify the flag.
> >>> +    const: 2
> >>> +
> >>> +  '#address-cells':
> >>> +    const: 0
> >>> +
> >>> +  interrupt-controller: true
> >>> +
> >>> +  reg:
> >>> +    items:
> >>> +      - description: Non-safety registers (INTCPU0-13, IRQ0-13)
> >>> +      - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
> >>
> >> reg is always the second property. Please follow DTS coding style.
> >>
> >
> > Ack.
> >
> >>> +
> >>> +  interrupts:
> >>> +    items:
> >>> +      - description: Software interrupt 0
> >>> +      - description: Software interrupt 1
> >>> +      - description: Software interrupt 2
> >>> +      - description: Software interrupt 3
> >>> +      - description: Software interrupt 4
> >>> +      - description: Software interrupt 5
> >>> +      - description: Software interrupt 6
> >>> +      - description: Software interrupt 7
> >>> +      - description: Software interrupt 8
> >>> +      - description: Software interrupt 9
> >>> +      - description: Software interrupt 10
> >>> +      - description: Software interrupt 11
> >>> +      - description: Software interrupt 12
> >>> +      - description: Software interrupt 13
> >>> +      - description: Software interrupt 14
> >>> +      - description: Software interrupt 15
> >>> +      - description: External pin interrupt 0
> >>> +      - description: External pin interrupt 1
> >>> +      - description: External pin interrupt 2
> >>> +      - description: External pin interrupt 3
> >>> +      - description: External pin interrupt 4
> >>> +      - description: External pin interrupt 5
> >>> +      - description: External pin interrupt 6
> >>> +      - description: External pin interrupt 7
> >>> +      - description: External pin interrupt 8
> >>> +      - description: External pin interrupt 9
> >>> +      - description: External pin interrupt 10
> >>> +      - description: External pin interrupt 11
> >>> +      - description: External pin interrupt 12
> >>> +      - description: External pin interrupt 13
> >>> +      - description: External pin interrupt 14
> >>> +      - description: External pin interrupt 15
> >>> +      - description: System error interrupt
> >>> +      - description: Cortex-A55 error event 0
> >>> +      - description: Cortex-A55 error event 1
> >>> +      - description: Cortex-R52 CPU 0 error event 0
> >>> +      - description: Cortex-R52 CPU 0 error event 1
> >>> +      - description: Cortex-R52 CPU 1 error event 0
> >>> +      - description: Cortex-R52 CPU 1 error event 1
> >>> +      - description: Peripherals error event 0
> >>> +      - description: Peripherals error event 1
> >>> +      - description: DSMIF error event 0
> >>> +      - description: DSMIF error event 1
> >>> +      - description: ENCIF error event 0
> >>> +      - description: ENCIF error event 1
> >>> +
> >>> +  interrupt-names:
> >>> +    items:
> >>> +      - const: intcpu0
> >>> +      - const: intcpu1
> >>> +      - const: intcpu2
> >>> +      - const: intcpu3
> >>> +      - const: intcpu4
> >>> +      - const: intcpu5
> >>> +      - const: intcpu6
> >>> +      - const: intcpu7
> >>> +      - const: intcpu8
> >>> +      - const: intcpu9
> >>> +      - const: intcpu10
> >>> +      - const: intcpu11
> >>> +      - const: intcpu12
> >>> +      - const: intcpu13
> >>> +      - const: intcpu14
> >>> +      - const: intcpu15
> >>> +      - const: irq0
> >>> +      - const: irq1
> >>> +      - const: irq2
> >>> +      - const: irq3
> >>> +      - const: irq4
> >>> +      - const: irq5
> >>> +      - const: irq6
> >>> +      - const: irq7
> >>> +      - const: irq8
> >>> +      - const: irq9
> >>> +      - const: irq10
> >>> +      - const: irq11
> >>> +      - const: irq12
> >>> +      - const: irq13
> >>> +      - const: irq14
> >>> +      - const: irq15
> >>> +      - const: sei
> >>> +      - const: ca55-err0
> >>> +      - const: ca55-err1
> >>> +      - const: cr520-err0
> >>> +      - const: cr520-err1
> >>> +      - const: cr521-err0
> >>> +      - const: cr521-err1
> >>> +      - const: peri-err0
> >>> +      - const: peri-err1
> >>> +      - const: dsmif-err0
> >>> +      - const: dsmif-err1
> >>> +      - const: encif-err0
> >>> +      - const: encif-err1
> >>
> >> Why all the interrupt names have nothing in common with previous ICU
> >> (renesas,rzv2h-icu.yaml)?
> >
> > Unfortunately, the functionality is different compared to what was
> > present on RZ/V2H, hence the different names, descriptions, and order,
> > which I've taken straight from the User Manual of the SoC.
> >
> > If the ICUs were similar, I would have tried to reuse the bindings and
> > drivers, but it would have quickly become too complex for what it's
> > worth.
> >
> >> These names are supposed to share, not
> >> re-invent every time the name.
> >>
> >
> > Do you think it is worth diverging from the User Manual to bring the
> > definition more in line with past SoCs?
> >
> > The advantage of sticking with the User Manual naming scheme is that
> > you can easily cross-reference these descriptions with the User Manual
> > and find what you need, whereas "PORT_IRQ0" / "GPIO interrupt" would
> > give you no information for RZ/T2H.
> 
> User manuals come with all sorts of different namings, so following it
> blindly would made impossible to make any common parts of bindings.
> 

True, and it makes sense to share them when the blocks are similar and
even more so if they can be made part of the same binding, but it's not
the case here.

> New devices should in general build on top of old ones, so lists like
> that can be shared. Of course how much can be shared here is different
> question, but person reading previous binding should be able to find
> similar things named similarly.
> 

In this case, I would keep the naming scheme different, as the ones that
are similarly named on RZ/V2H are different in functionality, it would
only cause confusion if we were to use the RZ/V2H naming scheme.

> >
> >> Isn't external interrupt the same as GPIO interrupt? How do they differ
> >> for this particular device?
> >>
> >
> > External pin interrupts on RZ/T2H are more like the PORT_IRQn on RZ/V2H,
> > since the pin is non-selectable (as opposed to "GPIO interrupt, TINTn"
> > on RZ/V2H, which has selectable pins). Also, on RZ/T2H, IRQ is a separate
> > function entirely, once you switch a pin to the IRQ function it is no
> > longer a GPIO.
> >
> >> And "Error interrupt to CA55" is "icu-error-ca55", but here THE SAME is
> >> called "ca55-err0"?
> >>
> >
> > Same reason as before, I used the naming scheme from the User Manual.
> >
> >> No, please start using unified naming, not re-inventing this every time.
> >> Order also is supposed to follow older generation, so bindings share
> >> common parts.
> >>
> >
> > How do you want me to shuffle the order for it to be more like the older
> > generation?
> >
> > I chose the current ordering because it matches the User Manual (and it
> > coincidentally results in an ascending GIC SPI numbering).
> >
> > Do you want me to put the software interrupts (intcpuN) after the
> > external pin interrupts (SEI included)?
> >
> > Eg:
> >   interrupt-names:
> >     items:
> >       - const: irq0
> >       ...
> >       - const: irq15
> >       - const: sei
> >       - const: intcpu0
> >       ...
> >       - const: intcpu15
> >       - const: ca55-err0
> >       - const: ca55-err1
> >       - const: cr520-err0
> >       - const: cr520-err1
> >       - const: cr521-err0
> >       - const: cr521-err1
> >       - const: peri-err0
> >       - const: peri-err1
> >       - const: dsmif-err0
> >       - const: dsmif-err1
> >       - const: encif-err0
> >       - const: encif-err1
> 
> If only the "err" interrupts are similar, then the order indeed does not
> matter much, because nothing is in common.
> 

Only the ca55-err0 & ca55-err1 interrupts are similar to icu-error-ca55,
but as you can see RZ/T2H has two of them. I think it is better to keep
the User Manual naming and order here. irqN and intcpuN also match up
nicely with the underlying GIC SPI numbering if we keep the original
order.

Maybe other people can chime in about what they think too, and we can
reach a consensus.

> 
> 
> Best regards,
> Krzysztof

Powered by blists - more mailing lists