Message-ID:
 <TYYPR01MB1395511BA753B32266C15CFEE85D0A@TYYPR01MB13955.jpnprd01.prod.outlook.com>
Date: Mon, 24 Nov 2025 16:25:49 +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: 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.

> 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

> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#interrupt-cells'
> > +  - '#address-cells'
> > +  - interrupt-controller
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - power-domains
> 
> 
> 
> Best regards,
> Krzysztof

Powered by blists - more mailing lists