[PATCH 0/5] Add IAX45 support for RZ/Five SoC

[PATCH 0/5] Add IAX45 support for RZ/Five SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

The IAX45 block on RZ/Five SoC is almost identical to the IRQC bock found
on the RZ/G2L family of SoCs.

IAX45 performs various interrupt controls including synchronization for the
external interrupts of NMI, IRQ, and GPIOINT and the interrupts of the
built-in peripheral interrupts output by each module. And it notifies the
interrupt to the PLIC.
- Select 32 TINT from 82 GPIOINT.
- Integration of bus error interrupts from system bus.
- Integration of ECC error interrupts from On-chip RAM.
- Indicate interrupt status. (NMI, IRQ, TINT, integrated bus error interrupt
  and integrated ECC error interrupt)
- Setting of interrupt detection method. (NMI, IRQ and TINT)
- All interrupts are masked by INTMASK.
- Mask function for NMI, IRQ and TINT

This patch series adds support for IAX45 in the IRQC driver and enables this
on RZ/Five SoC.

Cheers,
Prabhakar

Lad Prabhakar (5):
  dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document
    RZ/Five SoC
  irqchip/renesas-rzg2l: Add support for RZ/Five SoC
  riscv: dts: renesas: r9a07g043f: Add IRQC node to RZ/Five SoC DTSI
  arm64: dts: renesas: r9a07g043: Move interrupt-parent property to
    common DTSI
  riscv: dts: renesas: rzfive-smarc-som: Drop deleting interrupt
    properties from ETH0/1 nodes

 .../renesas,rzg2l-irqc.yaml                   |  27 ++++
 arch/arm64/boot/dts/renesas/r9a07g043.dtsi    |   1 +
 arch/arm64/boot/dts/renesas/r9a07g043u.dtsi   |   4 -
 arch/riscv/boot/dts/renesas/r9a07g043f.dtsi   |  76 ++++++++++
 .../boot/dts/renesas/rzfive-smarc-som.dtsi    |  16 ---
 drivers/irqchip/irq-renesas-rzg2l.c           | 132 +++++++++++++++++-
 6 files changed, 232 insertions(+), 24 deletions(-)

-- 
2.34.1

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

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
is almost identical to one found on the RZ/G2L SoC with below differences,
* Additional BUS error interrupt
* Additional ECCRAM error interrupt
* Has additional mask control registers for NMI/IRQ/TINT

Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../renesas,rzg2l-irqc.yaml                   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
index d3b5aec0a3f7..3abc01e48934 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -23,6 +23,7 @@ properties:
   compatible:
     items:
       - enum:
+          - renesas,r9a07g043f-irqc   # RZ/Five
           - renesas,r9a07g043u-irqc   # RZ/G2UL
           - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
           - renesas,r9a07g054-irqc    # RZ/V2L
@@ -88,6 +89,12 @@ properties:
       - description: GPIO interrupt, TINT30
       - description: GPIO interrupt, TINT31
       - description: Bus error interrupt
+      - description: ECCRAM0 TIE1 interrupt
+      - description: ECCRAM0 TIE2 interrupt
+      - description: ECCRAM0 overflow interrupt
+      - description: ECCRAM1 TIE1 interrupt
+      - description: ECCRAM1 TIE2 interrupt
+      - description: ECCRAM1 overflow interrupt
 
   interrupt-names:
     minItems: 41
@@ -134,6 +141,12 @@ properties:
       - const: tint30
       - const: tint31
       - const: bus-err
+      - const: eccram0-tie1
+      - const: eccram0-tie2
+      - const: eccram0-ovf
+      - const: eccram1-tie1
+      - const: eccram1-tie2
+      - const: eccram1-ovf
 
   clocks:
     maxItems: 2
@@ -180,6 +193,20 @@ allOf:
       required:
         - interrupt-names
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a07g043f-irqc
+    then:
+      properties:
+        interrupts:
+          minItems: 48
+        interrupt-names:
+          minItems: 48
+      required:
+        - interrupt-names
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3541 bytes --]

On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> is almost identical to one found on the RZ/G2L SoC with below differences,
> * Additional BUS error interrupt
> * Additional ECCRAM error interrupt
> * Has additional mask control registers for NMI/IRQ/TINT
> 
> Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> SoC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../renesas,rzg2l-irqc.yaml                   | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> index d3b5aec0a3f7..3abc01e48934 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -23,6 +23,7 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - renesas,r9a07g043f-irqc   # RZ/Five
>            - renesas,r9a07g043u-irqc   # RZ/G2UL
>            - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
>            - renesas,r9a07g054-irqc    # RZ/V2L
> @@ -88,6 +89,12 @@ properties:
>        - description: GPIO interrupt, TINT30
>        - description: GPIO interrupt, TINT31
>        - description: Bus error interrupt
> +      - description: ECCRAM0 TIE1 interrupt
> +      - description: ECCRAM0 TIE2 interrupt
> +      - description: ECCRAM0 overflow interrupt
> +      - description: ECCRAM1 TIE1 interrupt
> +      - description: ECCRAM1 TIE2 interrupt
> +      - description: ECCRAM1 overflow interrupt
>  
>    interrupt-names:
>      minItems: 41
> @@ -134,6 +141,12 @@ properties:
>        - const: tint30
>        - const: tint31
>        - const: bus-err
> +      - const: eccram0-tie1
> +      - const: eccram0-tie2
> +      - const: eccram0-ovf
> +      - const: eccram1-tie1
> +      - const: eccram1-tie2
> +      - const: eccram1-ovf

I think the restrictions already in the file become incorrect with this
patch:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - renesas,r9a07g043u-irqc
              - renesas,r9a08g045-irqc
    then:
      properties:
        interrupts:
          minItems: 42
        interrupt-names:
          minItems: 42
      required:
        - interrupt-names

This used to require all 42 interrupts for the two compatibles here
and at least the first 41 otherwise. Now you've increased the number of
interrupts to 48 thereby removing the upper limits on the existing
devices.

Given the commit message, I figure that providing 48 interrupts for
(at least some of) those devices would be incorrect?

Cheers,
Conor.

>  
>    clocks:
>      maxItems: 2
> @@ -180,6 +193,20 @@ allOf:
>        required:
>          - interrupt-names
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a07g043f-irqc
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 48
> +        interrupt-names:
> +          minItems: 48
> +      required:
> +        - interrupt-names
> +
>  unevaluatedProperties: false
>  
>  examples:
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:
> On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> > is almost identical to one found on the RZ/G2L SoC with below differences,
> > * Additional BUS error interrupt
> > * Additional ECCRAM error interrupt
> > * Has additional mask control registers for NMI/IRQ/TINT
> >
> > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> > SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > @@ -23,6 +23,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > +          - renesas,r9a07g043f-irqc   # RZ/Five
> >            - renesas,r9a07g043u-irqc   # RZ/G2UL
> >            - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
> >            - renesas,r9a07g054-irqc    # RZ/V2L
> > @@ -88,6 +89,12 @@ properties:
> >        - description: GPIO interrupt, TINT30
> >        - description: GPIO interrupt, TINT31
> >        - description: Bus error interrupt
> > +      - description: ECCRAM0 TIE1 interrupt

ECCRAM0 1bit error interrupt?

> > +      - description: ECCRAM0 TIE2 interrupt

ECCRAM0 2bit error interrupt?

> > +      - description: ECCRAM0 overflow interrupt

ECCRAM0 error overflow interrupt?

> > +      - description: ECCRAM1 TIE1 interrupt
> > +      - description: ECCRAM1 TIE2 interrupt
> > +      - description: ECCRAM1 overflow interrupt

Likewise.

> >    interrupt-names:
> >      minItems: 41
> > @@ -134,6 +141,12 @@ properties:
> >        - const: tint30
> >        - const: tint31
> >        - const: bus-err
> > +      - const: eccram0-tie1
> > +      - const: eccram0-tie2
> > +      - const: eccram0-ovf
> > +      - const: eccram1-tie1
> > +      - const: eccram1-tie2
> > +      - const: eccram1-ovf

Why not use the naming from the docs (all 6 include "ti")?
EC7TIE1_0, EC7TIE2_0, EC7TIOVF_0, EC7TIE1_1, EC7TIE2_1, EC7TIOVF_1
=> ec7tie1-0, ec7tie2-0, ec7tiovf-0, ...?

> I think the restrictions already in the file become incorrect with this
> patch:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - renesas,r9a07g043u-irqc
>               - renesas,r9a08g045-irqc
>     then:
>       properties:
>         interrupts:
>           minItems: 42
>         interrupt-names:
>           minItems: 42
>       required:
>         - interrupt-names
>
> This used to require all 42 interrupts for the two compatibles here
> and at least the first 41 otherwise. Now you've increased the number of
> interrupts to 48 thereby removing the upper limits on the existing
> devices.

I'm gonna repeat (and extend) my question from [1]: How come we thought
RZ/G2L and RZ/V2L do not have the bus error and ECCRAM interrupts?
Looks like most of the conditional handling can be removed (see below).

> Given the commit message, I figure that providing 48 interrupts for
> (at least some of) those devices would be incorrect?

Looks like all of RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five support
all 48 interrupts.  RZ/G3S lacks the final three for ECCRAM1.

[1] "Re: [PATCH v3 8/9] dt-bindings: interrupt-controller:
renesas,rzg2l-irqc: Document RZ/G3S"
https://lore.kernel.org/r/CAMuHMdX88KRnvJchUwrWcgmPooAESOT2492Nr1Z_5UMng3q__Q@mail.gmail.com

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Jan 30, 2024 at 11:13 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:
> > On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> > > is almost identical to one found on the RZ/G2L SoC with below differences,
> > > * Additional BUS error interrupt
> > > * Additional ECCRAM error interrupt
> > > * Has additional mask control registers for NMI/IRQ/TINT
> > >
> > > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> > > SoC.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > @@ -23,6 +23,7 @@ properties:
> > >    compatible:
> > >      items:
> > >        - enum:
> > > +          - renesas,r9a07g043f-irqc   # RZ/Five
> > >            - renesas,r9a07g043u-irqc   # RZ/G2UL
> > >            - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
> > >            - renesas,r9a07g054-irqc    # RZ/V2L
> > > @@ -88,6 +89,12 @@ properties:
> > >        - description: GPIO interrupt, TINT30
> > >        - description: GPIO interrupt, TINT31
> > >        - description: Bus error interrupt
> > > +      - description: ECCRAM0 TIE1 interrupt
>
> ECCRAM0 1bit error interrupt?
>
OK.

> > > +      - description: ECCRAM0 TIE2 interrupt
>
> ECCRAM0 2bit error interrupt?
>
OK.
> > > +      - description: ECCRAM0 overflow interrupt
>
> ECCRAM0 error overflow interrupt?
>
> > > +      - description: ECCRAM1 TIE1 interrupt
> > > +      - description: ECCRAM1 TIE2 interrupt
> > > +      - description: ECCRAM1 overflow interrupt
>
> Likewise.
>
OK.

> > >    interrupt-names:
> > >      minItems: 41
> > > @@ -134,6 +141,12 @@ properties:
> > >        - const: tint30
> > >        - const: tint31
> > >        - const: bus-err
> > > +      - const: eccram0-tie1
> > > +      - const: eccram0-tie2
> > > +      - const: eccram0-ovf
> > > +      - const: eccram1-tie1
> > > +      - const: eccram1-tie2
> > > +      - const: eccram1-ovf
>
> Why not use the naming from the docs (all 6 include "ti")?
> EC7TIE1_0, EC7TIE2_0, EC7TIOVF_0, EC7TIE1_1, EC7TIE2_1, EC7TIOVF_1
> => ec7tie1-0, ec7tie2-0, ec7tiovf-0, ...?
>
Agreed.

> > I think the restrictions already in the file become incorrect with this
> > patch:
> >   - if:
> >       properties:
> >         compatible:
> >           contains:
> >             enum:
> >               - renesas,r9a07g043u-irqc
> >               - renesas,r9a08g045-irqc
> >     then:
> >       properties:
> >         interrupts:
> >           minItems: 42
> >         interrupt-names:
> >           minItems: 42
> >       required:
> >         - interrupt-names
> >
> > This used to require all 42 interrupts for the two compatibles here
> > and at least the first 41 otherwise. Now you've increased the number of
> > interrupts to 48 thereby removing the upper limits on the existing
> > devices.
>
> I'm gonna repeat (and extend) my question from [1]: How come we thought
> RZ/G2L and RZ/V2L do not have the bus error and ECCRAM interrupts?
>
Hmm not sure how this was missed earlier.

> Looks like most of the conditional handling can be removed (see below).
>
> > Given the commit message, I figure that providing 48 interrupts for
> > (at least some of) those devices would be incorrect?
>
> Looks like all of RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five support
> all 48 interrupts.  RZ/G3S lacks the final three for ECCRAM1.
>
Agreed for RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five, but for RZ/G3S it
becomes tricky the interrupts for ECCRAM0/1 are combined hence they
have just 3 interrupts. How do you propose the above interrupt naming?

> [1] "Re: [PATCH v3 8/9] dt-bindings: interrupt-controller:
> renesas,rzg2l-irqc: Document RZ/G3S"
> https://lore.kernel.org/r/CAMuHMdX88KRnvJchUwrWcgmPooAESOT2492Nr1Z_5UMng3q__Q@mail.gmail.com
>
Sorry I missed this thread.

Cheers,
Prabhakar

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

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Jan 30, 2024 at 1:59 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Tue, Jan 30, 2024 at 11:13 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> > > > is almost identical to one found on the RZ/G2L SoC with below differences,
> > > > * Additional BUS error interrupt
> > > > * Additional ECCRAM error interrupt
> > > > * Has additional mask control registers for NMI/IRQ/TINT
> > > >
> > > > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> > > > SoC.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > @@ -134,6 +141,12 @@ properties:
> > > >        - const: tint30
> > > >        - const: tint31
> > > >        - const: bus-err
> > > > +      - const: eccram0-tie1
> > > > +      - const: eccram0-tie2
> > > > +      - const: eccram0-ovf
> > > > +      - const: eccram1-tie1
> > > > +      - const: eccram1-tie2
> > > > +      - const: eccram1-ovf
> >
> > Why not use the naming from the docs (all 6 include "ti")?
> > EC7TIE1_0, EC7TIE2_0, EC7TIOVF_0, EC7TIE1_1, EC7TIE2_1, EC7TIOVF_1
> > => ec7tie1-0, ec7tie2-0, ec7tiovf-0, ...?
> >
> Agreed.
>
> > > I think the restrictions already in the file become incorrect with this
> > > patch:
> > >   - if:
> > >       properties:
> > >         compatible:
> > >           contains:
> > >             enum:
> > >               - renesas,r9a07g043u-irqc
> > >               - renesas,r9a08g045-irqc
> > >     then:
> > >       properties:
> > >         interrupts:
> > >           minItems: 42
> > >         interrupt-names:
> > >           minItems: 42
> > >       required:
> > >         - interrupt-names
> > >
> > > This used to require all 42 interrupts for the two compatibles here
> > > and at least the first 41 otherwise. Now you've increased the number of
> > > interrupts to 48 thereby removing the upper limits on the existing
> > > devices.
> >
> > I'm gonna repeat (and extend) my question from [1]: How come we thought
> > RZ/G2L and RZ/V2L do not have the bus error and ECCRAM interrupts?
> >
> Hmm not sure how this was missed earlier.
>
> > Looks like most of the conditional handling can be removed (see below).
> >
> > > Given the commit message, I figure that providing 48 interrupts for
> > > (at least some of) those devices would be incorrect?
> >
> > Looks like all of RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five support
> > all 48 interrupts.  RZ/G3S lacks the final three for ECCRAM1.
> >
> Agreed for RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five, but for RZ/G3S it
> becomes tricky the interrupts for ECCRAM0/1 are combined hence they
> have just 3 interrupts. How do you propose the above interrupt naming?

I guess it doesn't hurt to have an index 0 on a part that has only a
single set?

Alternatives would be to
  1. Drop the index completely on RZ/G3S, complicating bindings and
     driver,
  1. Drop the index for the first set, and use index 2 for the second set,
     causing the names to differ even more on parts with 2 sets.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Lad, Prabhakar]

Hi Geert,

On Tue, Jan 30, 2024 at 1:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Jan 30, 2024 at 1:59 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Tue, Jan 30, 2024 at 11:13 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Jan 29, 2024 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> > > > > is almost identical to one found on the RZ/G2L SoC with below differences,
> > > > > * Additional BUS error interrupt
> > > > > * Additional ECCRAM error interrupt
> > > > > * Has additional mask control registers for NMI/IRQ/TINT
> > > > >
> > > > > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> > > > > SoC.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > > @@ -134,6 +141,12 @@ properties:
> > > > >        - const: tint30
> > > > >        - const: tint31
> > > > >        - const: bus-err
> > > > > +      - const: eccram0-tie1
> > > > > +      - const: eccram0-tie2
> > > > > +      - const: eccram0-ovf
> > > > > +      - const: eccram1-tie1
> > > > > +      - const: eccram1-tie2
> > > > > +      - const: eccram1-ovf
> > >
> > > Why not use the naming from the docs (all 6 include "ti")?
> > > EC7TIE1_0, EC7TIE2_0, EC7TIOVF_0, EC7TIE1_1, EC7TIE2_1, EC7TIOVF_1
> > > => ec7tie1-0, ec7tie2-0, ec7tiovf-0, ...?
> > >
> > Agreed.
> >
> > > > I think the restrictions already in the file become incorrect with this
> > > > patch:
> > > >   - if:
> > > >       properties:
> > > >         compatible:
> > > >           contains:
> > > >             enum:
> > > >               - renesas,r9a07g043u-irqc
> > > >               - renesas,r9a08g045-irqc
> > > >     then:
> > > >       properties:
> > > >         interrupts:
> > > >           minItems: 42
> > > >         interrupt-names:
> > > >           minItems: 42
> > > >       required:
> > > >         - interrupt-names
> > > >
> > > > This used to require all 42 interrupts for the two compatibles here
> > > > and at least the first 41 otherwise. Now you've increased the number of
> > > > interrupts to 48 thereby removing the upper limits on the existing
> > > > devices.
> > >
> > > I'm gonna repeat (and extend) my question from [1]: How come we thought
> > > RZ/G2L and RZ/V2L do not have the bus error and ECCRAM interrupts?
> > >
> > Hmm not sure how this was missed earlier.
> >
> > > Looks like most of the conditional handling can be removed (see below).
> > >
> > > > Given the commit message, I figure that providing 48 interrupts for
> > > > (at least some of) those devices would be incorrect?
> > >
> > > Looks like all of RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five support
> > > all 48 interrupts.  RZ/G3S lacks the final three for ECCRAM1.
> > >
> > Agreed for RZ/G2L{,C}, RZ/V2L, RZ/G2UL, and RZ/Five, but for RZ/G3S it
> > becomes tricky the interrupts for ECCRAM0/1 are combined hence they
> > have just 3 interrupts. How do you propose the above interrupt naming?
>
> I guess it doesn't hurt to have an index 0 on a part that has only a
> single set?
>
Let's go with this option...

> Alternatives would be to
>   1. Drop the index completely on RZ/G3S, complicating bindings and
>      driver,
>   1. Drop the index for the first set, and use index 2 for the second set,
>      causing the names to differ even more on parts with 2 sets.
>
...instead of complicating.

Cheers,
Prabhakar

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

[Lad, Prabhakar]

Hi Conor,

Thank you for the review.

On Mon, Jan 29, 2024 at 5:30 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jan 29, 2024 at 03:16:14PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document RZ/Five (R9A07G043F) IRQC bindings. The IRQC block on RZ/Five SoC
> > is almost identical to one found on the RZ/G2L SoC with below differences,
> > * Additional BUS error interrupt
> > * Additional ECCRAM error interrupt
> > * Has additional mask control registers for NMI/IRQ/TINT
> >
> > Hence new compatible string "renesas,r9a07g043f-irqc" is added for RZ/Five
> > SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../renesas,rzg2l-irqc.yaml                   | 27 +++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > index d3b5aec0a3f7..3abc01e48934 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > @@ -23,6 +23,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > +          - renesas,r9a07g043f-irqc   # RZ/Five
> >            - renesas,r9a07g043u-irqc   # RZ/G2UL
> >            - renesas,r9a07g044-irqc    # RZ/G2{L,LC}
> >            - renesas,r9a07g054-irqc    # RZ/V2L
> > @@ -88,6 +89,12 @@ properties:
> >        - description: GPIO interrupt, TINT30
> >        - description: GPIO interrupt, TINT31
> >        - description: Bus error interrupt
> > +      - description: ECCRAM0 TIE1 interrupt
> > +      - description: ECCRAM0 TIE2 interrupt
> > +      - description: ECCRAM0 overflow interrupt
> > +      - description: ECCRAM1 TIE1 interrupt
> > +      - description: ECCRAM1 TIE2 interrupt
> > +      - description: ECCRAM1 overflow interrupt
> >
> >    interrupt-names:
> >      minItems: 41
> > @@ -134,6 +141,12 @@ properties:
> >        - const: tint30
> >        - const: tint31
> >        - const: bus-err
> > +      - const: eccram0-tie1
> > +      - const: eccram0-tie2
> > +      - const: eccram0-ovf
> > +      - const: eccram1-tie1
> > +      - const: eccram1-tie2
> > +      - const: eccram1-ovf
>
> I think the restrictions already in the file become incorrect with this
> patch:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - renesas,r9a07g043u-irqc
>               - renesas,r9a08g045-irqc
>     then:
>       properties:
>         interrupts:
>           minItems: 42
>         interrupt-names:
>           minItems: 42
>       required:
>         - interrupt-names
>
> This used to require all 42 interrupts for the two compatibles here
> and at least the first 41 otherwise. Now you've increased the number of
> interrupts to 48 thereby removing the upper limits on the existing
> devices.
>
> Given the commit message, I figure that providing 48 interrupts for
> (at least some of) those devices would be incorrect?
>
Agreed, I will fix this in the next version.

Cheers,
Prabhakar

[PATCH 2/5] irqchip/renesas-rzg2l: Add support for RZ/Five SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The IX45 block has additional mask registers (NMSK/IMSK/TMSK) as compared
to the RZ/G2L (family) SoC.

Introduce masking/unmasking support for IRQ and TINT interrupts in IRQC
controller driver. Two new registers, IMSK and TMSK, are defined to
handle masking on RZ/Five SoC. The implementation utilizes a new data
structure, `struct rzg2l_irqc_data`, to determine mask support for a
specific controller instance.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/irqchip/irq-renesas-rzg2l.c | 132 +++++++++++++++++++++++++++-
 1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 9494fc26259c..949280f95c29 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -37,6 +37,8 @@
 #define TSSEL_SHIFT(n)			(8 * (n))
 #define TSSEL_MASK			GENMASK(7, 0)
 #define IRQ_MASK			0x3
+#define IMSK				0x10010
+#define TMSK				0x10020
 
 #define TSSR_OFFSET(n)			((n) % 4)
 #define TSSR_INDEX(n)			((n) / 4)
@@ -66,15 +68,25 @@ struct rzg2l_irqc_reg_cache {
 	u32	titsr[2];
 };
 
+/**
+ * struct rzg2l_irqc_data - OF data structure
+ * @mask_supported: Indicates if mask registers are available
+ */
+struct rzg2l_irqc_data {
+	bool	mask_supported;
+};
+
 /**
  * struct rzg2l_irqc_priv - IRQ controller private data structure
  * @base:	Controller's base address
+ * @data:	OF data pointer
  * @fwspec:	IRQ firmware specific data
  * @lock:	Lock to serialize access to hardware registers
  * @cache:	Registers cache for suspend/resume
  */
 static struct rzg2l_irqc_priv {
 	void __iomem			*base;
+	const struct rzg2l_irqc_data	*data;
 	struct irq_fwspec		fwspec[IRQC_NUM_IRQ];
 	raw_spinlock_t			lock;
 	struct rzg2l_irqc_reg_cache	cache;
@@ -129,44 +141,136 @@ static void rzg2l_irqc_eoi(struct irq_data *d)
 	irq_chip_eoi_parent(d);
 }
 
+static void rzg2l_irqc_mask_irq_interrupt(struct rzg2l_irqc_priv *priv,
+					  unsigned int hwirq)
+{
+	u32 imsk = readl_relaxed(priv->base + IMSK);
+	u32 bit = BIT(hwirq - IRQC_IRQ_START);
+
+	writel_relaxed(imsk | bit, priv->base + IMSK);
+}
+
+static void rzg2l_irqc_unmask_irq_interrupt(struct rzg2l_irqc_priv *priv,
+					    unsigned int hwirq)
+{
+	u32 imsk = readl_relaxed(priv->base + IMSK);
+	u32 bit = BIT(hwirq - IRQC_IRQ_START);
+
+	writel_relaxed(imsk & ~bit, priv->base + IMSK);
+}
+
+static void rzg2l_irqc_mask_tint_interrupt(struct rzg2l_irqc_priv *priv,
+					   unsigned int hwirq)
+{
+	u32 tmsk = readl_relaxed(priv->base + TMSK);
+	u32 bit = BIT(hwirq - IRQC_TINT_START);
+
+	writel_relaxed(tmsk | bit, priv->base + TMSK);
+}
+
+static void rzg2l_irqc_unmask_tint_interrupt(struct rzg2l_irqc_priv *priv,
+					     unsigned int hwirq)
+{
+	u32 tmsk = readl_relaxed(priv->base + TMSK);
+	u32 bit = BIT(hwirq - IRQC_TINT_START);
+
+	writel_relaxed(tmsk & ~bit, priv->base + TMSK);
+}
+
+/* Must be called while priv->lock is held */
+static void rzg2l_irqc_mask_once(struct rzg2l_irqc_priv *priv, unsigned int hwirq)
+{
+	if (!priv->data->mask_supported)
+		return;
+
+	if (hwirq >= IRQC_IRQ_START && hwirq <= IRQC_IRQ_COUNT)
+		rzg2l_irqc_mask_irq_interrupt(priv, hwirq);
+	else if (hwirq >= IRQC_TINT_START && hwirq < IRQC_NUM_IRQ)
+		rzg2l_irqc_mask_tint_interrupt(priv, hwirq);
+}
+
+static void rzg2l_irqc_mask(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+
+	raw_spin_lock(&priv->lock);
+	rzg2l_irqc_mask_once(priv, irqd_to_hwirq(d));
+	raw_spin_unlock(&priv->lock);
+	irq_chip_mask_parent(d);
+}
+
+/* Must be called while priv->lock is held */
+static void rzg2l_irqc_unmask_once(struct rzg2l_irqc_priv *priv, unsigned int hwirq)
+{
+	if (!priv->data->mask_supported)
+		return;
+
+	if (hwirq >= IRQC_IRQ_START && hwirq <= IRQC_IRQ_COUNT)
+		rzg2l_irqc_unmask_irq_interrupt(priv, hwirq);
+	else if (hwirq >= IRQC_TINT_START && hwirq < IRQC_NUM_IRQ)
+		rzg2l_irqc_unmask_tint_interrupt(priv, hwirq);
+}
+
+static void rzg2l_irqc_unmask(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+
+	raw_spin_lock(&priv->lock);
+	rzg2l_irqc_unmask_once(priv, irqd_to_hwirq(d));
+	raw_spin_unlock(&priv->lock);
+	irq_chip_unmask_parent(d);
+}
+
 static void rzg2l_irqc_irq_disable(struct irq_data *d)
 {
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
 	unsigned int hw_irq = irqd_to_hwirq(d);
 
 	if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) {
-		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
 		u32 offset = hw_irq - IRQC_TINT_START;
 		u32 tssr_offset = TSSR_OFFSET(offset);
 		u8 tssr_index = TSSR_INDEX(offset);
 		u32 reg;
 
 		raw_spin_lock(&priv->lock);
+		rzg2l_irqc_mask_once(priv, hw_irq);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
 		reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset));
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
+	} else {
+		raw_spin_lock(&priv->lock);
+		rzg2l_irqc_mask_once(priv, hw_irq);
+		raw_spin_unlock(&priv->lock);
 	}
+
 	irq_chip_disable_parent(d);
 }
 
 static void rzg2l_irqc_irq_enable(struct irq_data *d)
 {
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
 	unsigned int hw_irq = irqd_to_hwirq(d);
 
 	if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) {
 		unsigned long tint = (uintptr_t)irq_data_get_irq_chip_data(d);
-		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
 		u32 offset = hw_irq - IRQC_TINT_START;
 		u32 tssr_offset = TSSR_OFFSET(offset);
 		u8 tssr_index = TSSR_INDEX(offset);
 		u32 reg;
 
 		raw_spin_lock(&priv->lock);
+		rzg2l_irqc_unmask_once(priv, hw_irq);
 		reg = readl_relaxed(priv->base + TSSR(tssr_index));
 		reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
 		writel_relaxed(reg, priv->base + TSSR(tssr_index));
 		raw_spin_unlock(&priv->lock);
+	} else {
+		raw_spin_lock(&priv->lock);
+		rzg2l_irqc_unmask_once(priv, hw_irq);
+		raw_spin_unlock(&priv->lock);
 	}
+
 	irq_chip_enable_parent(d);
 }
 
@@ -294,8 +398,8 @@ static struct syscore_ops rzg2l_irqc_syscore_ops = {
 static const struct irq_chip irqc_chip = {
 	.name			= "rzg2l-irqc",
 	.irq_eoi		= rzg2l_irqc_eoi,
-	.irq_mask		= irq_chip_mask_parent,
-	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_mask		= rzg2l_irqc_mask,
+	.irq_unmask		= rzg2l_irqc_unmask,
 	.irq_disable		= rzg2l_irqc_irq_disable,
 	.irq_enable		= rzg2l_irqc_irq_enable,
 	.irq_get_irqchip_state	= irq_chip_get_parent_state,
@@ -371,9 +475,23 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
 	return 0;
 }
 
+static const struct rzg2l_irqc_data rzfive_irqc_data = {
+	.mask_supported = true,
+};
+
+static const struct rzg2l_irqc_data rzg2l_irqc_default_data = {
+	.mask_supported = false,
+};
+
+static const struct of_device_id rzg2l_irqc_matches[] = {
+	{ .compatible = "renesas,r9a07g043f-irqc", .data = &rzfive_irqc_data },
+	{ }
+};
+
 static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
 {
 	struct irq_domain *irq_domain, *parent_domain;
+	const struct of_device_id *match;
 	struct platform_device *pdev;
 	struct reset_control *resetn;
 	int ret;
@@ -392,6 +510,12 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
 	if (!rzg2l_irqc_data)
 		return -ENOMEM;
 
+	match = of_match_node(rzg2l_irqc_matches, node);
+	if (match)
+		rzg2l_irqc_data->data = match->data;
+	else
+		rzg2l_irqc_data->data = &rzg2l_irqc_default_data;
+
 	rzg2l_irqc_data->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
 	if (IS_ERR(rzg2l_irqc_data->base))
 		return PTR_ERR(rzg2l_irqc_data->base);
-- 
2.34.1

Re: [PATCH 2/5] irqchip/renesas-rzg2l: Add support for RZ/Five SoC

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The IX45 block has additional mask registers (NMSK/IMSK/TMSK) as compared
> to the RZ/G2L (family) SoC.
>
> Introduce masking/unmasking support for IRQ and TINT interrupts in IRQC
> controller driver. Two new registers, IMSK and TMSK, are defined to
> handle masking on RZ/Five SoC. The implementation utilizes a new data
> structure, `struct rzg2l_irqc_data`, to determine mask support for a
> specific controller instance.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -66,15 +68,25 @@ struct rzg2l_irqc_reg_cache {
>         u32     titsr[2];
>  };
>
> +/**
> + * struct rzg2l_irqc_data - OF data structure
> + * @mask_supported: Indicates if mask registers are available
> + */
> +struct rzg2l_irqc_data {

This structure has the same name as the single static struct
rzg2l_irqc_priv instance, which is confusing.

> +       bool    mask_supported;
> +};
> +
>  /**
>   * struct rzg2l_irqc_priv - IRQ controller private data structure
>   * @base:      Controller's base address
> + * @data:      OF data pointer
>   * @fwspec:    IRQ firmware specific data
>   * @lock:      Lock to serialize access to hardware registers
>   * @cache:     Registers cache for suspend/resume
>   */
>  static struct rzg2l_irqc_priv {
>         void __iomem                    *base;
> +       const struct rzg2l_irqc_data    *data;

Replacing this by a bool would avoid a pointer dereference in each user,
and allows you to make rzg2l_irqc_data etc. __initconst.

>         struct irq_fwspec               fwspec[IRQC_NUM_IRQ];
>         raw_spinlock_t                  lock;
>         struct rzg2l_irqc_reg_cache     cache;

> @@ -371,9 +475,23 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
>         return 0;
>  }
>
> +static const struct rzg2l_irqc_data rzfive_irqc_data = {
> +       .mask_supported = true,
> +};
> +
> +static const struct rzg2l_irqc_data rzg2l_irqc_default_data = {
> +       .mask_supported = false,
> +};
> +
> +static const struct of_device_id rzg2l_irqc_matches[] = {
> +       { .compatible = "renesas,r9a07g043f-irqc", .data = &rzfive_irqc_data },
> +       { }
> +};
> +
>  static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
>  {
>         struct irq_domain *irq_domain, *parent_domain;
> +       const struct of_device_id *match;
>         struct platform_device *pdev;
>         struct reset_control *resetn;
>         int ret;
> @@ -392,6 +510,12 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
>         if (!rzg2l_irqc_data)
>                 return -ENOMEM;
>
> +       match = of_match_node(rzg2l_irqc_matches, node);
> +       if (match)
> +               rzg2l_irqc_data->data = match->data;
> +       else
> +               rzg2l_irqc_data->data = &rzg2l_irqc_default_data;

Instead of matching a second time, I'd rather add a second
IRQCHIP_MATCH() entry with a different init function, passing the
actual rzg2l_irqc_data pointer.

> +
>         rzg2l_irqc_data->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
>         if (IS_ERR(rzg2l_irqc_data->base))
>                 return PTR_ERR(rzg2l_irqc_data->base);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 2/5] irqchip/renesas-rzg2l: Add support for RZ/Five SoC

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Jan 30, 2024 at 11:38 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The IX45 block has additional mask registers (NMSK/IMSK/TMSK) as compared
> > to the RZ/G2L (family) SoC.
> >
> > Introduce masking/unmasking support for IRQ and TINT interrupts in IRQC
> > controller driver. Two new registers, IMSK and TMSK, are defined to
> > handle masking on RZ/Five SoC. The implementation utilizes a new data
> > structure, `struct rzg2l_irqc_data`, to determine mask support for a
> > specific controller instance.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -66,15 +68,25 @@ struct rzg2l_irqc_reg_cache {
> >         u32     titsr[2];
> >  };
> >
> > +/**
> > + * struct rzg2l_irqc_data - OF data structure
> > + * @mask_supported: Indicates if mask registers are available
> > + */
> > +struct rzg2l_irqc_data {
>
> This structure has the same name as the single static struct
> rzg2l_irqc_priv instance, which is confusing.
>
Agreed, I will rename it to rzg2l_irqc_of_data

> > +       bool    mask_supported;
> > +};
> > +
> >  /**
> >   * struct rzg2l_irqc_priv - IRQ controller private data structure
> >   * @base:      Controller's base address
> > + * @data:      OF data pointer
> >   * @fwspec:    IRQ firmware specific data
> >   * @lock:      Lock to serialize access to hardware registers
> >   * @cache:     Registers cache for suspend/resume
> >   */
> >  static struct rzg2l_irqc_priv {
> >         void __iomem                    *base;
> > +       const struct rzg2l_irqc_data    *data;
>
> Replacing this by a bool would avoid a pointer dereference in each user,
> and allows you to make rzg2l_irqc_data etc. __initconst.
>
Do you mean just add "bool mask_supported" here and get rid of struct
rzg2l_irqc_data ? Can you please elaborate here..

> >         struct irq_fwspec               fwspec[IRQC_NUM_IRQ];
> >         raw_spinlock_t                  lock;
> >         struct rzg2l_irqc_reg_cache     cache;
>
> > @@ -371,9 +475,23 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
> >         return 0;
> >  }
> >
> > +static const struct rzg2l_irqc_data rzfive_irqc_data = {
> > +       .mask_supported = true,
> > +};
> > +
> > +static const struct rzg2l_irqc_data rzg2l_irqc_default_data = {
> > +       .mask_supported = false,
> > +};
> > +
> > +static const struct of_device_id rzg2l_irqc_matches[] = {
> > +       { .compatible = "renesas,r9a07g043f-irqc", .data = &rzfive_irqc_data },
> > +       { }
> > +};
> > +
> >  static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> >  {
> >         struct irq_domain *irq_domain, *parent_domain;
> > +       const struct of_device_id *match;
> >         struct platform_device *pdev;
> >         struct reset_control *resetn;
> >         int ret;
> > @@ -392,6 +510,12 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> >         if (!rzg2l_irqc_data)
> >                 return -ENOMEM;
> >
> > +       match = of_match_node(rzg2l_irqc_matches, node);
> > +       if (match)
> > +               rzg2l_irqc_data->data = match->data;
> > +       else
> > +               rzg2l_irqc_data->data = &rzg2l_irqc_default_data;
>
> Instead of matching a second time, I'd rather add a second
> IRQCHIP_MATCH() entry with a different init function, passing the
> actual rzg2l_irqc_data pointer.
>
OK, or rather just pass true/false instead of rzg2l_irqc_of_data pointer.?

Cheers,
Prabhakar

Re: [PATCH 2/5] irqchip/renesas-rzg2l: Add support for RZ/Five SoC

[Geert Uytterhoeven]

Hi Prabhakar,

On Wed, Jan 31, 2024 at 7:36 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Tue, Jan 30, 2024 at 11:38 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > The IX45 block has additional mask registers (NMSK/IMSK/TMSK) as compared
> > > to the RZ/G2L (family) SoC.
> > >
> > > Introduce masking/unmasking support for IRQ and TINT interrupts in IRQC
> > > controller driver. Two new registers, IMSK and TMSK, are defined to
> > > handle masking on RZ/Five SoC. The implementation utilizes a new data
> > > structure, `struct rzg2l_irqc_data`, to determine mask support for a
> > > specific controller instance.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > @@ -66,15 +68,25 @@ struct rzg2l_irqc_reg_cache {
> > >         u32     titsr[2];
> > >  };
> > >
> > > +/**
> > > + * struct rzg2l_irqc_data - OF data structure
> > > + * @mask_supported: Indicates if mask registers are available
> > > + */
> > > +struct rzg2l_irqc_data {
> >
> > This structure has the same name as the single static struct
> > rzg2l_irqc_priv instance, which is confusing.
> >
> Agreed, I will rename it to rzg2l_irqc_of_data
>
> > > +       bool    mask_supported;
> > > +};
> > > +
> > >  /**
> > >   * struct rzg2l_irqc_priv - IRQ controller private data structure
> > >   * @base:      Controller's base address
> > > + * @data:      OF data pointer
> > >   * @fwspec:    IRQ firmware specific data
> > >   * @lock:      Lock to serialize access to hardware registers
> > >   * @cache:     Registers cache for suspend/resume
> > >   */
> > >  static struct rzg2l_irqc_priv {
> > >         void __iomem                    *base;
> > > +       const struct rzg2l_irqc_data    *data;
> >
> > Replacing this by a bool would avoid a pointer dereference in each user,
> > and allows you to make rzg2l_irqc_data etc. __initconst.
> >
> Do you mean just add "bool mask_supported" here and get rid of struct
> rzg2l_irqc_data ? Can you please elaborate here..

Either add "bool mask_supported" here, or add a copy of the full
struct rzg2l_irqc_data (see below).

>
> > >         struct irq_fwspec               fwspec[IRQC_NUM_IRQ];
> > >         raw_spinlock_t                  lock;
> > >         struct rzg2l_irqc_reg_cache     cache;
> >
> > > @@ -371,9 +475,23 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
> > >         return 0;
> > >  }
> > >
> > > +static const struct rzg2l_irqc_data rzfive_irqc_data = {
> > > +       .mask_supported = true,
> > > +};
> > > +
> > > +static const struct rzg2l_irqc_data rzg2l_irqc_default_data = {
> > > +       .mask_supported = false,
> > > +};
> > > +
> > > +static const struct of_device_id rzg2l_irqc_matches[] = {
> > > +       { .compatible = "renesas,r9a07g043f-irqc", .data = &rzfive_irqc_data },
> > > +       { }
> > > +};
> > > +
> > >  static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> > >  {
> > >         struct irq_domain *irq_domain, *parent_domain;
> > > +       const struct of_device_id *match;
> > >         struct platform_device *pdev;
> > >         struct reset_control *resetn;
> > >         int ret;
> > > @@ -392,6 +510,12 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> > >         if (!rzg2l_irqc_data)
> > >                 return -ENOMEM;
> > >
> > > +       match = of_match_node(rzg2l_irqc_matches, node);
> > > +       if (match)
> > > +               rzg2l_irqc_data->data = match->data;
> > > +       else
> > > +               rzg2l_irqc_data->data = &rzg2l_irqc_default_data;
> >
> > Instead of matching a second time, I'd rather add a second
> > IRQCHIP_MATCH() entry with a different init function, passing the
> > actual rzg2l_irqc_data pointer.
> >
> OK, or rather just pass true/false instead of rzg2l_irqc_of_data pointer.?

Yes, that would be fine for me, too.
It all depends on whether you plan to add, or see a need for adding,
more flags or other fields in the future (and even for flags, you could
combine them in an unsigned long).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 2/5] irqchip/renesas-rzg2l: Add support for RZ/Five SoC

[Lad, Prabhakar]

Hi Geert,

On Thu, Feb 1, 2024 at 8:34 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Wed, Jan 31, 2024 at 7:36 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Tue, Jan 30, 2024 at 11:38 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > The IX45 block has additional mask registers (NMSK/IMSK/TMSK) as compared
> > > > to the RZ/G2L (family) SoC.
> > > >
> > > > Introduce masking/unmasking support for IRQ and TINT interrupts in IRQC
> > > > controller driver. Two new registers, IMSK and TMSK, are defined to
> > > > handle masking on RZ/Five SoC. The implementation utilizes a new data
> > > > structure, `struct rzg2l_irqc_data`, to determine mask support for a
> > > > specific controller instance.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > @@ -66,15 +68,25 @@ struct rzg2l_irqc_reg_cache {
> > > >         u32     titsr[2];
> > > >  };
> > > >
> > > > +/**
> > > > + * struct rzg2l_irqc_data - OF data structure
> > > > + * @mask_supported: Indicates if mask registers are available
> > > > + */
> > > > +struct rzg2l_irqc_data {
> > >
> > > This structure has the same name as the single static struct
> > > rzg2l_irqc_priv instance, which is confusing.
> > >
> > Agreed, I will rename it to rzg2l_irqc_of_data
> >
> > > > +       bool    mask_supported;
> > > > +};
> > > > +
> > > >  /**
> > > >   * struct rzg2l_irqc_priv - IRQ controller private data structure
> > > >   * @base:      Controller's base address
> > > > + * @data:      OF data pointer
> > > >   * @fwspec:    IRQ firmware specific data
> > > >   * @lock:      Lock to serialize access to hardware registers
> > > >   * @cache:     Registers cache for suspend/resume
> > > >   */
> > > >  static struct rzg2l_irqc_priv {
> > > >         void __iomem                    *base;
> > > > +       const struct rzg2l_irqc_data    *data;
> > >
> > > Replacing this by a bool would avoid a pointer dereference in each user,
> > > and allows you to make rzg2l_irqc_data etc. __initconst.
> > >
> > Do you mean just add "bool mask_supported" here and get rid of struct
> > rzg2l_irqc_data ? Can you please elaborate here..
>
> Either add "bool mask_supported" here, or add a copy of the full
> struct rzg2l_irqc_data (see below).
>
OK, i'll keep a copy of struct rzg2l_irqc_data here...
> >
> > > >         struct irq_fwspec               fwspec[IRQC_NUM_IRQ];
> > > >         raw_spinlock_t                  lock;
> > > >         struct rzg2l_irqc_reg_cache     cache;
> > >
> > > > @@ -371,9 +475,23 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
> > > >         return 0;
> > > >  }
> > > >
> > > > +static const struct rzg2l_irqc_data rzfive_irqc_data = {
> > > > +       .mask_supported = true,
> > > > +};
> > > > +
> > > > +static const struct rzg2l_irqc_data rzg2l_irqc_default_data = {
> > > > +       .mask_supported = false,
> > > > +};
> > > > +
> > > > +static const struct of_device_id rzg2l_irqc_matches[] = {
> > > > +       { .compatible = "renesas,r9a07g043f-irqc", .data = &rzfive_irqc_data },
> > > > +       { }
> > > > +};
> > > > +
> > > >  static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> > > >  {
> > > >         struct irq_domain *irq_domain, *parent_domain;
> > > > +       const struct of_device_id *match;
> > > >         struct platform_device *pdev;
> > > >         struct reset_control *resetn;
> > > >         int ret;
> > > > @@ -392,6 +510,12 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
> > > >         if (!rzg2l_irqc_data)
> > > >                 return -ENOMEM;
> > > >
> > > > +       match = of_match_node(rzg2l_irqc_matches, node);
> > > > +       if (match)
> > > > +               rzg2l_irqc_data->data = match->data;
> > > > +       else
> > > > +               rzg2l_irqc_data->data = &rzg2l_irqc_default_data;
> > >
> > > Instead of matching a second time, I'd rather add a second
> > > IRQCHIP_MATCH() entry with a different init function, passing the
> > > actual rzg2l_irqc_data pointer.
> > >
> > OK, or rather just pass true/false instead of rzg2l_irqc_of_data pointer.?
>
> Yes, that would be fine for me, too.
> It all depends on whether you plan to add, or see a need for adding,
> more flags or other fields in the future (and even for flags, you could
> combine them in an unsigned long).
>
I'll keep the bool flag for now and update it  as  and when it grows.
I see future SoCs with a lot of other interrupts being supported by
this block.

Cheers,
Prabhakar

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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

[PATCH 3/5] riscv: dts: renesas: r9a07g043f: Add IRQC node to RZ/Five SoC DTSI

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add the IRQC node to RZ/Five (R9A07G043F) SoC DTSI.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 arch/riscv/boot/dts/renesas/r9a07g043f.dtsi | 76 +++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi b/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
index d7a66043f13b..d2272a0bfb61 100644
--- a/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
+++ b/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
@@ -50,6 +50,82 @@ &soc {
 	dma-noncoherent;
 	interrupt-parent = <&plic>;
 
+	irqc: interrupt-controller@110a0000 {
+		compatible = "renesas,r9a07g043f-irqc",
+			     "renesas,rzg2l-irqc";
+		reg = <0 0x110a0000 0 0x20000>;
+		#interrupt-cells = <2>;
+		#address-cells = <0>;
+		interrupt-controller;
+		interrupts = <SOC_PERIPHERAL_IRQ(0) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(1) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(2) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(3) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(4) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(5) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(6) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(7) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(8) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(444) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(445) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(446) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(447) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(448) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(449) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(450) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(451) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(452) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(453) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(454) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(455) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(456) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(457) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(458) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(459) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(460) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(461) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(462) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(463) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(464) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(465) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(466) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(467) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(468) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(469) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(470) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(471) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(472) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(473) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(474) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(475) IRQ_TYPE_LEVEL_HIGH>,
+			     <SOC_PERIPHERAL_IRQ(25) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(34) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(35) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(36) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(37) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(38) IRQ_TYPE_EDGE_RISING>,
+			     <SOC_PERIPHERAL_IRQ(39) IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "nmi",
+				  "irq0", "irq1", "irq2", "irq3",
+				  "irq4", "irq5", "irq6", "irq7",
+				  "tint0", "tint1", "tint2", "tint3",
+				  "tint4", "tint5", "tint6", "tint7",
+				  "tint8", "tint9", "tint10", "tint11",
+				  "tint12", "tint13", "tint14", "tint15",
+				  "tint16", "tint17", "tint18", "tint19",
+				  "tint20", "tint21", "tint22", "tint23",
+				  "tint24", "tint25", "tint26", "tint27",
+				  "tint28", "tint29", "tint30", "tint31",
+				  "bus-err", "eccram0-tie1", "eccram0-tie2",
+				  "eccram0-ovf", "eccram1-tie1", "eccram1-tie2",
+				  "eccram1-ovf";
+		clocks = <&cpg CPG_MOD R9A07G043_IAX45_CLK>,
+			 <&cpg CPG_MOD R9A07G043_IAX45_PCLK>;
+		clock-names = "clk", "pclk";
+		power-domains = <&cpg>;
+		resets = <&cpg R9A07G043_IAX45_RESETN>;
+	};
+
 	plic: interrupt-controller@12c00000 {
 		compatible = "renesas,r9a07g043-plic", "andestech,nceplic100";
 		#interrupt-cells = <2>;
-- 
2.34.1

Re: [PATCH 3/5] riscv: dts: renesas: r9a07g043f: Add IRQC node to RZ/Five SoC DTSI

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add the IRQC node to RZ/Five (R9A07G043F) SoC DTSI.

Thanks for your patch!

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> --- a/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
> +++ b/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
> @@ -50,6 +50,82 @@ &soc {
>         dma-noncoherent;
>         interrupt-parent = <&plic>;
>
> +       irqc: interrupt-controller@110a0000 {
> +               compatible = "renesas,r9a07g043f-irqc",
> +                            "renesas,rzg2l-irqc";
> +               reg = <0 0x110a0000 0 0x20000>;
> +               #interrupt-cells = <2>;
> +               #address-cells = <0>;
> +               interrupt-controller;
> +               interrupts = <SOC_PERIPHERAL_IRQ(0) IRQ_TYPE_LEVEL_HIGH>,

As this is the RZ/Five-specific .dtsi file, and not the common base
.dtsi, you could avoid using SOC_PERIPHERAL_IRQ() here.
I am not sure what is most readable...

The rest LGTM (pending interrupt names review comments).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 3/5] riscv: dts: renesas: r9a07g043f: Add IRQC node to RZ/Five SoC DTSI

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Jan 30, 2024 at 11:25 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add the IRQC node to RZ/Five (R9A07G043F) SoC DTSI.
>
> Thanks for your patch!
>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > --- a/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
> > +++ b/arch/riscv/boot/dts/renesas/r9a07g043f.dtsi
> > @@ -50,6 +50,82 @@ &soc {
> >         dma-noncoherent;
> >         interrupt-parent = <&plic>;
> >
> > +       irqc: interrupt-controller@110a0000 {
> > +               compatible = "renesas,r9a07g043f-irqc",
> > +                            "renesas,rzg2l-irqc";
> > +               reg = <0 0x110a0000 0 0x20000>;
> > +               #interrupt-cells = <2>;
> > +               #address-cells = <0>;
> > +               interrupt-controller;
> > +               interrupts = <SOC_PERIPHERAL_IRQ(0) IRQ_TYPE_LEVEL_HIGH>,
>
> As this is the RZ/Five-specific .dtsi file, and not the common base
> .dtsi, you could avoid using SOC_PERIPHERAL_IRQ() here.
> I am not sure what is most readable...
>
Ok, ill switch to the usual way here..

>+                            <SOC_PERIPHERAL_IRQ(25) IRQ_TYPE_EDGE_RISING>,
In RZ/Five HW manual this is documented as LEVEL_HIGH and RZ/G2UL this
is documented as EDGE. I ve internally asked for clarification with
the HW team. If it's the same for both the SoCs I'll move this node to
common SoC dtsi and just update the compat string in rz/five specific
dtsi.

Cheers,
Prabhakar

> The rest LGTM (pending interrupt names review comments).
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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

[PATCH 4/5] arm64: dts: renesas: r9a07g043: Move interrupt-parent property to common DTSI

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Now that we have added support for IRQC to both RZ/Five and RZ/G2UL SoCs
we can move the interrupt-parent for pinctrl node back to the common
shared r9a07g043.dtsi file.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g043.dtsi  | 1 +
 arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
index 8721f4c9fa0f..d2365def1059 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
@@ -598,6 +598,7 @@ pinctrl: pinctrl@11030000 {
 			gpio-ranges = <&pinctrl 0 0 152>;
 			#interrupt-cells = <2>;
 			interrupt-controller;
+			interrupt-parent = <&irqc>;
 			clocks = <&cpg CPG_MOD R9A07G043_GPIO_HCLK>;
 			power-domains = <&cpg>;
 			resets = <&cpg R9A07G043_GPIO_RSTN>,
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
index 2ab231572d95..0e931c88afa5 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
@@ -54,10 +54,6 @@ timer {
 	};
 };
 
-&pinctrl {
-	interrupt-parent = <&irqc>;
-};
-
 &soc {
 	interrupt-parent = <&gic>;
 
-- 
2.34.1

Re: [PATCH 4/5] arm64: dts: renesas: r9a07g043: Move interrupt-parent property to common DTSI

[Geert Uytterhoeven]

On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Now that we have added support for IRQC to both RZ/Five and RZ/G2UL SoCs
> we can move the interrupt-parent for pinctrl node back to the common
> shared r9a07g043.dtsi file.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

[PATCH 5/5] riscv: dts: renesas: rzfive-smarc-som: Drop deleting interrupt properties from ETH0/1 nodes

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Now that we have enabled IRQC support for RZ/Five SoC switch to interrupt
mode for ethernet0/1 PHYs instead of polling mode.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../riscv/boot/dts/renesas/rzfive-smarc-som.dtsi | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/riscv/boot/dts/renesas/rzfive-smarc-som.dtsi b/arch/riscv/boot/dts/renesas/rzfive-smarc-som.dtsi
index 72d9b6fba526..86b2f15375ec 100644
--- a/arch/riscv/boot/dts/renesas/rzfive-smarc-som.dtsi
+++ b/arch/riscv/boot/dts/renesas/rzfive-smarc-som.dtsi
@@ -7,22 +7,6 @@
 
 #include <arm64/renesas/rzg2ul-smarc-som.dtsi>
 
-#if (!SW_ET0_EN_N)
-&eth0 {
-	phy0: ethernet-phy@7 {
-		/delete-property/ interrupt-parent;
-		/delete-property/ interrupts;
-	};
-};
-#endif
-
-&eth1 {
-	phy1: ethernet-phy@7 {
-		/delete-property/ interrupt-parent;
-		/delete-property/ interrupts;
-	};
-};
-
 &sbc {
 	status = "disabled";
 };
-- 
2.34.1

Re: [PATCH 5/5] riscv: dts: renesas: rzfive-smarc-som: Drop deleting interrupt properties from ETH0/1 nodes

[Geert Uytterhoeven]

On Mon, Jan 29, 2024 at 4:16 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Now that we have enabled IRQC support for RZ/Five SoC switch to interrupt
> mode for ethernet0/1 PHYs instead of polling mode.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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