Re: [PATCH v2 2/5] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller

[Geert Uytterhoeven]

Hi Fabrizio,

On Mon, Sep 30, 2024 at 4:53 PM Fabrizio Castro
<fabrizio.castro.jz@xxxxxxxxxxx> wrote:
> Add DT bindings for the Renesas RZ/V2H(P) Interrupt Controller.
>
> Also add macros for the NMI and IRQ0-15 interrupts which map the
> SPI0-16 interrupts on the RZ/V2H(P) SoC so that they can be
> used in the first cell of the interrupt specifiers.
>
> For the second cell of the interrupt specifier, since NMI, IRQn
> and TINTn support different types of interrupts between themselves,
> add helper macros to make it easier for the user to work out what's
> available.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> ---
> v1->v2:
> * Removed '|' from main description
> * Reworked main description
> * Fixed indentation of #interrupt-cells
> * Reworked description of #interrupt-cells
> * Dropped file include/dt-bindings/interrupt-controller/icu-rzv2h.h

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml

> +properties:
> +  compatible:
> +    const: renesas,r9a09g057-icu          # RZ/V2H(P)

Too many spaces before "#"?

> +
> +  '#interrupt-cells':
> +    description: The first cell is the SPI number of the NMI or the
> +      PORT_IRQ[0-15] interrupt, as per user manual. The second cell is used to
> +      specify the flag.
> +    const: 2
> +
> +  '#address-cells':
> +    const: 0
> +
> +  interrupt-controller: true
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 58
> +    items:
> +      - description: NMI interrupt
> +      - description: IRQ0 interrupt
> +      - description: IRQ1 interrupt
> +      - description: IRQ2 interrupt
> +      - description: IRQ3 interrupt
> +      - description: IRQ4 interrupt
> +      - description: IRQ5 interrupt
> +      - description: IRQ6 interrupt
> +      - description: IRQ7 interrupt
> +      - description: IRQ8 interrupt
> +      - description: IRQ9 interrupt
> +      - description: IRQ10 interrupt
> +      - description: IRQ11 interrupt
> +      - description: IRQ12 interrupt
> +      - description: IRQ13 interrupt
> +      - description: IRQ14 interrupt
> +      - description: IRQ15 interrupt

"PORT_IRQ<n>", to match Table 4.6-22 ("List of Input Events")
and '#interrupt-cells' above.

> +      - description: GPIO interrupt, TINT0
> +      - description: GPIO interrupt, TINT1
> +      - description: GPIO interrupt, TINT2
> +      - description: GPIO interrupt, TINT3
> +      - description: GPIO interrupt, TINT4
> +      - description: GPIO interrupt, TINT5
> +      - description: GPIO interrupt, TINT6
> +      - description: GPIO interrupt, TINT7
> +      - description: GPIO interrupt, TINT8
> +      - description: GPIO interrupt, TINT9
> +      - description: GPIO interrupt, TINT10
> +      - description: GPIO interrupt, TINT11
> +      - description: GPIO interrupt, TINT12
> +      - description: GPIO interrupt, TINT13
> +      - description: GPIO interrupt, TINT14
> +      - description: GPIO interrupt, TINT15
> +      - description: GPIO interrupt, TINT16
> +      - description: GPIO interrupt, TINT17
> +      - description: GPIO interrupt, TINT18
> +      - description: GPIO interrupt, TINT19
> +      - description: GPIO interrupt, TINT20
> +      - description: GPIO interrupt, TINT21
> +      - description: GPIO interrupt, TINT22
> +      - description: GPIO interrupt, TINT23
> +      - description: GPIO interrupt, TINT24
> +      - description: GPIO interrupt, TINT25
> +      - description: GPIO interrupt, TINT26
> +      - description: GPIO interrupt, TINT27
> +      - description: GPIO interrupt, TINT28
> +      - description: GPIO interrupt, TINT29
> +      - description: GPIO interrupt, TINT30
> +      - description: GPIO interrupt, TINT31
> +      - description: Software interrupt, INTA55_0
> +      - description: Software interrupt, INTA55_1
> +      - description: Software interrupt, INTA55_2
> +      - description: Software interrupt, INTA55_3
> +      - description: Error interrupt to CA55
> +      - description: GTCCRA compare match/input capture (U0)
> +      - description: GTCCRB compare match/input capture (U0)
> +      - description: GTCCRA compare match/input capture (U1)
> +      - description: GTCCRB compare match/input capture (U1)
> +
> +  interrupt-names:
> +    minItems: 58
> +    items:
> +      - const: nmi
> +      - 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

port_irq<n>?

The rest LGTM, although I think you may want to add more interrupts
later, for various events? However, it's not really clear to me which
interrupts go through the ICU, and which go directly to the GIC.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds