[PATCH 0/2] Renesas RZ/G2L IRQC support

[PATCH 0/2] Renesas RZ/G2L IRQC support

[Lad Prabhakar]

Hi All,

The RZ/G2L Interrupt Controller is a front-end for the GIC found on
Renesas RZ/G2L SoC's with below pins:
- IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
- GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
  maximum of only 32 can be mapped to 32 GIC SPI interrupts,
- NMI edge select.

                                                                _____________
                                                                |    GIC     |
                                                                |  ________  |
                                         ____________           | |        | |
NMI ------------------------------------>|          |  SPI0-479 | | GIC-600| |
                _______                  |          |------------>|        | |
                |      |                 |          |  PPI16-31 | |        | |
                |      | IRQ0-IRQ7       |   IRQC   |------------>|        | |
P0_P48_4 ------>| GPIO |---------------->|          |           | |________| |
                |      |GPIOINT0-122     |          |           |            |
                |      |---------------->| TINT0-31 |           |            |
                |______|                 |__________|           |____________|

The proposed patches add hierarchical IRQ domain, one in IRQC driver and another in
pinctrl driver. Upon interrupt requests map the interrupt to GIC. Out of GPIOINT0-122
only 32 can be mapped to GIC SPI, this mapping is handled by the pinctrl and IRQC driver.

Cheers,
Prabhakar

Changes for RFCV4 -> V1:
* Used unevaluatedProperties.
* Altered the sequence of reg property
* Set the parent type
* Used raw_spin_lock() instead of raw_spin_lock_irqsave()
* Simplified parsing IRQ map.
* Will send the GPIO and pinctrl changes as part of separate series

Changes for v4:
* Used locking while RMW
* Now using interrupts property instead of interrupt-map
* Patch series depends on [0]
* Updated binding doc
* Fixed comments pointed by Andy

[0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
20220316200633.28974-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Changes for v3:
-> Re-structured the driver as a hierarchical irq domain instead of chained
-> made use of IRQCHIP_* macros
-> dropped locking
-> Added support for IRQ0-7 interrupts
-> Introduced 2 new patches for GPIOLIB
-> Switched to using GPIOLIB for irqdomains in pinctrl

RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210921193028.13099-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210803175109.1729-1-prabhakar.mahadev-lad.rj@bp.renesas.com/


Lad Prabhakar (2):
  dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
    Controller
  irqchip: Add RZ/G2L IA55 Interrupt Controller driver

 .../renesas,rzg2l-irqc.yaml                   | 131 +++++
 drivers/irqchip/Kconfig                       |   8 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-renesas-rzg2l.c           | 447 ++++++++++++++++++
 4 files changed, 587 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
 create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

-- 
2.17.1

[PATCH 1/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Lad Prabhakar]

Add DT bindings for the Renesas RZ/G2L Interrupt Controller.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../renesas,rzg2l-irqc.yaml                   | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
new file mode 100644
index 000000000000..5f2e1dd1d42a
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+  - Geert Uytterhoeven <geert+renesas@glider.be>
+
+description: |
+  IA55 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 IP. And it notifies the interrupt to the GIC
+    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
+    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
+    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
+      stand-up edge detection interrupts)
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-irqc    # RZ/G2L
+      - const: renesas,rzg2l-irqc
+
+  '#interrupt-cells':
+    const: 2
+
+  '#address-cells':
+    const: 0
+
+  interrupt-controller: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 41
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: clk
+      - const: pclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - '#address-cells'
+  - interrupt-controller
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+
+    irqc: interrupt-controller@110a0000 {
+            compatible = "renesas,r9a07g044-irqc", "renesas,rzg2l-irqc";
+            reg = <0x110a0000 0x10000>;
+            #interrupt-cells = <2>;
+            #address-cells = <0>;
+            interrupt-controller;
+            interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD R9A07G044_IA55_CLK>,
+                     <&cpg CPG_MOD R9A07G044_IA55_PCLK>;
+            clock-names = "clk", "pclk";
+            power-domains = <&cpg>;
+            resets = <&cpg R9A07G044_IA55_RESETN>;
+    };
-- 
2.17.1

Re: [PATCH 1/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +  - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description: |
> +  IA55 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 IP. And it notifies the interrupt to the GIC
> +    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> +    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> +    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> +      stand-up edge detection interrupts)
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a07g044-irqc    # RZ/G2L
> +      - const: renesas,rzg2l-irqc
> +
> +  '#interrupt-cells':
> +    const: 2

What is the meaning of the cells? IRQ number + flags, I assume?
How are the numbers mapped, do you need a DT bindings header?
Perhaps it would make sense to increase to 3 cells, so you can use
one cell for the type (cfr. e.g. GIC_SPI), and the second for the
plain index within the type?

The rest LGTM, but I'm not an interrupt expert, so I'm curious in
hearing Marc's opinion.

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/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Apr 28, 2022 at 10:32 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > @@ -0,0 +1,131 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +  - Geert Uytterhoeven <geert+renesas@glider.be>
> > +
> > +description: |
> > +  IA55 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 IP. And it notifies the interrupt to the GIC
> > +    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> > +    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> > +    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> > +      stand-up edge detection interrupts)
> > +
> > +allOf:
> > +  - $ref: /schemas/interrupt-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - renesas,r9a07g044-irqc    # RZ/G2L
> > +      - const: renesas,rzg2l-irqc
> > +
> > +  '#interrupt-cells':
> > +    const: 2
>
> What is the meaning of the cells? IRQ number + flags, I assume?
IRQ number and the type.

> How are the numbers mapped, do you need a DT bindings header?
No, just plain numbers are used (driver handles the validation of the
interrupt numbering), for example like below,

&eth0 {
   ...
   status = "okay";

   phy0: ethernet-phy@7 {
     compatible = "ethernet-phy-id0022.1640",
                           "ethernet-phy-ieee802.3-c22";
     reg = <7>;
     interrupt-parent = <&irqc>;
     interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
    ...
  };
};

And for the GPIO:

key-1 {
      gpios = <&pinctrl RZG2L_GPIO(43, 0) GPIO_ACTIVE_HIGH>;
      linux,code = <KEY_1>;
      linux,input-type = <EV_KEY>;
      wakeup-source;
      label = "SW1";
};

> Perhaps it would make sense to increase to 3 cells, so you can use
> one cell for the type (cfr. e.g. GIC_SPI), and the second for the
> plain index within the type?
>
Could you please elaborate on this. Are you referring to the type here
as the type to be set up in the GIC?

Cheers,
Prabhakar

> The rest LGTM, but I'm not an interrupt expert, so I'm curious in
> hearing Marc's opinion.
>
> 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/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 29, 2022 at 10:38 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Thu, Apr 28, 2022 at 10:32 AM Geert Uytterhoeven
> > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > @@ -0,0 +1,131 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> > > +
> > > +maintainers:
> > > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > +  - Geert Uytterhoeven <geert+renesas@glider.be>
> > > +
> > > +description: |
> > > +  IA55 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 IP. And it notifies the interrupt to the GIC
> > > +    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> > > +    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> > > +    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> > > +      stand-up edge detection interrupts)
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/interrupt-controller.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - renesas,r9a07g044-irqc    # RZ/G2L
> > > +      - const: renesas,rzg2l-irqc
> > > +
> > > +  '#interrupt-cells':
> > > +    const: 2
> >
> > What is the meaning of the cells? IRQ number + flags, I assume?
> IRQ number and the type.
>
> > How are the numbers mapped, do you need a DT bindings header?
> No, just plain numbers are used (driver handles the validation of the
> interrupt numbering), for example like below,
>
> &eth0 {
>    ...
>    status = "okay";
>
>    phy0: ethernet-phy@7 {
>      compatible = "ethernet-phy-id0022.1640",
>                            "ethernet-phy-ieee802.3-c22";
>      reg = <7>;
>      interrupt-parent = <&irqc>;
>      interrupts = <3 IRQ_TYPE_LEVEL_LOW>;

OK, so the number must be an external interrupt number (0..7).

>     ...
>   };
> };
>
> And for the GPIO:
>
> key-1 {
>       gpios = <&pinctrl RZG2L_GPIO(43, 0) GPIO_ACTIVE_HIGH>;
>       linux,code = <KEY_1>;
>       linux,input-type = <EV_KEY>;
>       wakeup-source;
>       label = "SW1";
> };

OK, so in this case the interrupt number is obtained implicitly, and
no interrupts property is used.

> > Perhaps it would make sense to increase to 3 cells, so you can use
> > one cell for the type (cfr. e.g. GIC_SPI), and the second for the
> > plain index within the type?
> >
> Could you please elaborate on this. Are you referring to the type here
> as the type to be set up in the GIC?

Please ignore, you don't need the type, as it's always an external
interrupt number, right?

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/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Lad, Prabhakar]

Hi Geert,

On Fri, Apr 29, 2022 at 9:44 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 29, 2022 at 10:38 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Thu, Apr 28, 2022 at 10:32 AM Geert Uytterhoeven
> > > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > > > @@ -0,0 +1,131 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> > > > +
> > > > +maintainers:
> > > > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > +  - Geert Uytterhoeven <geert+renesas@glider.be>
> > > > +
> > > > +description: |
> > > > +  IA55 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 IP. And it notifies the interrupt to the GIC
> > > > +    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> > > > +    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> > > > +    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> > > > +      stand-up edge detection interrupts)
> > > > +
> > > > +allOf:
> > > > +  - $ref: /schemas/interrupt-controller.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - renesas,r9a07g044-irqc    # RZ/G2L
> > > > +      - const: renesas,rzg2l-irqc
> > > > +
> > > > +  '#interrupt-cells':
> > > > +    const: 2
> > >
> > > What is the meaning of the cells? IRQ number + flags, I assume?
> > IRQ number and the type.
> >
> > > How are the numbers mapped, do you need a DT bindings header?
> > No, just plain numbers are used (driver handles the validation of the
> > interrupt numbering), for example like below,
> >
> > &eth0 {
> >    ...
> >    status = "okay";
> >
> >    phy0: ethernet-phy@7 {
> >      compatible = "ethernet-phy-id0022.1640",
> >                            "ethernet-phy-ieee802.3-c22";
> >      reg = <7>;
> >      interrupt-parent = <&irqc>;
> >      interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>
> OK, so the number must be an external interrupt number (0..7).
>
Yep that's right.

> >     ...
> >   };
> > };
> >
> > And for the GPIO:
> >
> > key-1 {
> >       gpios = <&pinctrl RZG2L_GPIO(43, 0) GPIO_ACTIVE_HIGH>;
> >       linux,code = <KEY_1>;
> >       linux,input-type = <EV_KEY>;
> >       wakeup-source;
> >       label = "SW1";
> > };
>
> OK, so in this case the interrupt number is obtained implicitly, and
> no interrupts property is used.
>
Indeed.

> > > Perhaps it would make sense to increase to 3 cells, so you can use
> > > one cell for the type (cfr. e.g. GIC_SPI), and the second for the
> > > plain index within the type?
> > >
> > Could you please elaborate on this. Are you referring to the type here
> > as the type to be set up in the GIC?
>
> Please ignore, you don't need the type, as it's always an external
> interrupt number, right?
>
Yep that's right.

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

Re: [PATCH 1/2] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

[Rob Herring]

On Thu, 21 Apr 2022 23:11:58 +0100, Lad Prabhakar wrote:
> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../renesas,rzg2l-irqc.yaml                   | 131 ++++++++++++++++++
>  1 file changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Lad Prabhakar]

Add a driver for the Renesas RZ/G2L Interrupt Controller.

This supports external pins being used as interrupts. It supports
one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
to be used as IRQ lines.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/irqchip/Kconfig             |   8 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/irq-renesas-rzg2l.c | 447 ++++++++++++++++++++++++++++
 3 files changed, 456 insertions(+)
 create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a3f450c52efa..e21f085f8c8c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
 	  Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
 	  to 8 external interrupts with configurable sense select.
 
+config RENESAS_RZG2L_IRQC
+	bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
+	  for external devices.
+
 config SL28CPLD_INTC
 	bool "Kontron sl28cpld IRQ controller"
 	depends on MFD_SL28CPLD=y || COMPILE_TEST
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 5d8e21d3dc6d..9a83bf4003e2 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)			+= irq-rda-intc.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_RENESAS_RZA1_IRQC)		+= irq-renesas-rza1.o
+obj-$(CONFIG_RENESAS_RZG2L_IRQC)	+= irq-renesas-rzg2l.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
new file mode 100644
index 000000000000..1eae53e5d717
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -0,0 +1,447 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L IRQC Driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation.
+ *
+ * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#define IRQC_IRQ_START			1
+#define IRQC_IRQ_COUNT			8
+#define IRQC_TINT_START			9
+#define IRQC_TINT_COUNT			32
+#define IRQC_NUM_IRQ			41
+
+#define ISCR				0x10
+#define IITSR				0x14
+#define TSCR				0x20
+#define TITSR0				0x24
+#define TITSR1				0x28
+#define TITSR0_MAX_INT			16
+#define TITSEL_WIDTH			0x2
+#define TSSR(n)				(0x30 + ((n) * 4))
+#define TIEN				BIT(7)
+#define TSSEL_SHIFT(n)			(8 * (n))
+#define TSSEL_MASK			GENMASK(7, 0)
+#define IRQ_MASK			0x3
+
+#define TSSR_OFFSET(n)			((n) % 4)
+#define TSSR_INDEX(n)			((n) / 4)
+
+#define TITSR_TITSEL_EDGE_RISING	0
+#define TITSR_TITSEL_EDGE_FALLING	1
+#define TITSR_TITSEL_LEVEL_HIGH		2
+#define TITSR_TITSEL_LEVEL_LOW		3
+
+#define IITSR_IITSEL(n, sense)		((sense) << ((n) * 2))
+#define IITSR_IITSEL_LEVEL_LOW		0
+#define IITSR_IITSEL_EDGE_FALLING	1
+#define IITSR_IITSEL_EDGE_RISING	2
+#define IITSR_IITSEL_EDGE_BOTH		3
+#define IITSR_IITSEL_MASK(n)		IITSR_IITSEL((n), 3)
+
+#define TINT_EXTRACT_HWIRQ(x)		((x) & ~GENMASK(31, 16))
+#define TINT_EXTRACT_GPIOINT(x)		((x) >> 16)
+
+struct rzg2l_irqc_priv {
+	void __iomem *base;
+	struct of_phandle_args map[IRQC_NUM_IRQ];
+	raw_spinlock_t lock;
+};
+
+struct rzg2l_irqc_chip_data {
+	int tint;
+};
+
+static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
+{
+	return data->domain->host_data;
+}
+
+static void rzg2l_irq_eoi(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
+	u16 bit = BIT(hw_irq);
+	u32 reg;
+
+	reg = readl_relaxed(priv->base + ISCR);
+	if (reg & bit)
+		writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
+			       priv->base + ISCR);
+}
+
+static void rzg2l_tint_eoi(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hw_irq = irqd_to_hwirq(d);
+	u32 bit = BIT(hw_irq - IRQC_TINT_START);
+	u32 reg;
+
+	reg = readl_relaxed(priv->base + TSCR);
+	if (reg & bit)
+		writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
+			       priv->base + TSCR);
+}
+
+static void rzg2l_irqc_eoi(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	raw_spin_lock(&priv->lock);
+	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
+		rzg2l_irq_eoi(d);
+	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
+		rzg2l_tint_eoi(d);
+	raw_spin_unlock(&priv->lock);
+	irq_chip_eoi_parent(d);
+}
+
+static void rzg2l_irqc_irq_disable(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
+		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);
+		reg = readl_relaxed(priv->base + TSSR(tssr_index));
+		reg &= ~(TSSEL_MASK << tssr_offset);
+		writel_relaxed(reg, priv->base + TSSR(tssr_index));
+		raw_spin_unlock(&priv->lock);
+	}
+	irq_chip_disable_parent(d);
+}
+
+static void rzg2l_irqc_irq_enable(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
+		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
+		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);
+		reg = readl_relaxed(priv->base + TSSR(tssr_index));
+		reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
+		writel_relaxed(reg, priv->base + TSSR(tssr_index));
+		raw_spin_unlock(&priv->lock);
+	}
+	irq_chip_enable_parent(d);
+}
+
+static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	u16 sense, tmp;
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_LEVEL_LOW:
+		sense = IITSR_IITSEL_LEVEL_LOW;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		sense = IITSR_IITSEL_EDGE_FALLING;
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+		sense = IITSR_IITSEL_EDGE_RISING;
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		sense = IITSR_IITSEL_EDGE_BOTH;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	raw_spin_lock(&priv->lock);
+	tmp = readl_relaxed(priv->base + IITSR);
+	tmp &= ~IITSR_IITSEL_MASK(hw_irq);
+	tmp |= IITSR_IITSEL(hw_irq, sense);
+	writel_relaxed(tmp, priv->base + IITSR);
+	raw_spin_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hwirq = irqd_to_hwirq(d);
+	u32 titseln = hwirq - IRQC_TINT_START;
+	u32 offset;
+	u8 sense;
+	u32 reg;
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_RISING:
+		sense = TITSR_TITSEL_EDGE_RISING;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		sense = TITSR_TITSEL_EDGE_FALLING;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (titseln < TITSR0_MAX_INT) {
+		offset = TITSR0;
+	} else {
+		titseln /= TITSEL_WIDTH;
+		offset  = TITSR1;
+	}
+
+	raw_spin_lock(&priv->lock);
+	reg = readl_relaxed(priv->base + offset);
+	reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
+	reg |= sense << (titseln * TITSEL_WIDTH);
+	writel_relaxed(reg, priv->base + offset);
+	raw_spin_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+	int ret = -EINVAL;
+
+	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
+		ret = rzg2l_irq_set_type(d, type);
+	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
+		ret = rzg2l_tint_set_edge(d, type);
+	if (ret)
+		return ret;
+
+	return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH);
+}
+
+static 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_disable		= rzg2l_irqc_irq_disable,
+	.irq_enable		= rzg2l_irqc_irq_enable,
+	.irq_get_irqchip_state	= irq_chip_get_parent_state,
+	.irq_set_irqchip_state	= irq_chip_set_parent_state,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= rzg2l_irqc_set_type,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_SET_TYPE_MASKED |
+				  IRQCHIP_SKIP_SET_WAKE,
+};
+
+static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
+			    unsigned int nr_irqs, void *arg)
+{
+	struct rzg2l_irqc_priv *priv = domain->host_data;
+	struct rzg2l_irqc_chip_data *chip_data = NULL;
+	struct irq_fwspec spec;
+	irq_hw_number_t hwirq;
+	int tint = -EINVAL;
+	unsigned int type;
+	unsigned int i;
+	int ret;
+
+	ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	/*
+	 * For TINIT interrupts ie where pinctrl driver is child of irqc domain
+	 * the hwirq and TINT are encoded in fwspec->param[0].
+	 * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and TINT
+	 * from 16-31 bits. TINIT from the pinctrl driver needs to be programmed
+	 * in IRQC registers to enable a given gpio pin as interrupt.
+	 */
+	if (hwirq > IRQC_IRQ_COUNT) {
+		tint = TINT_EXTRACT_GPIOINT(hwirq);
+		hwirq = TINT_EXTRACT_HWIRQ(hwirq);
+	}
+
+	if (hwirq > (IRQC_NUM_IRQ - 1))
+		return -EINVAL;
+
+	if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq > (IRQC_NUM_IRQ - 1)))
+		return -EINVAL;
+
+	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
+	if (!chip_data)
+		return -ENOMEM;
+	chip_data->tint = tint;
+
+	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
+					    chip_data);
+	if (ret) {
+		kfree(chip_data);
+		return ret;
+	}
+
+	spec.fwnode = domain->parent->fwnode;
+	spec.param_count = priv->map[hwirq].args_count;
+	for (i = 0; i < spec.param_count; i++)
+		spec.param[i] = priv->map[hwirq].args[i];
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
+	if (ret)
+		kfree(chip_data);
+
+	return ret;
+}
+
+static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs)
+{
+	struct irq_data *d;
+
+	d = irq_domain_get_irq_data(domain, virq);
+	if (d) {
+		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
+
+		kfree(chip_data);
+	}
+	irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
+	.alloc = rzg2l_irqc_alloc,
+	.free = rzg2l_irqc_domain_free,
+	.translate = irq_domain_translate_twocell,
+};
+
+static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
+				struct device_node *np)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < IRQC_NUM_IRQ; i++) {
+		ret = of_irq_parse_one(np, i, &priv->map[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
+{
+	struct irq_domain *irq_domain, *parent_domain;
+	struct reset_control *resetn;
+	struct rzg2l_irqc_priv *priv;
+	struct clk *clk;
+	struct clk *pclk;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = of_iomap(node, 0);
+	if (!priv->base) {
+		ret = -ENXIO;
+		goto free_priv;
+	}
+
+	clk = of_clk_get_by_name(node, "clk");
+	if (IS_ERR(clk)) {
+		ret = IS_ERR(clk);
+		goto iounmap_base;
+	}
+
+	pclk = of_clk_get_by_name(node, "pclk");
+	if (IS_ERR(pclk)) {
+		ret = IS_ERR(pclk);
+		goto iounmap_base;
+	}
+
+	resetn = of_reset_control_get_exclusive_by_index(node, 0);
+	if (IS_ERR(resetn)) {
+		ret = IS_ERR(resetn);
+		goto iounmap_base;
+	}
+
+	parent_domain = irq_find_host(parent);
+	if (!parent_domain) {
+		pr_err("%pOF: cannot find parent domain\n", node);
+		ret = -ENODEV;
+		goto iounmap_base;
+	}
+
+	ret = rzg2l_irqc_parse_map(priv, node);
+	if (ret) {
+		pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
+		goto iounmap_base;
+	}
+
+	ret = reset_control_deassert(resetn);
+	if (ret) {
+		pr_err("%pOF: failed to deassert resetn pin, %d\n", node, ret);
+		goto iounmap_base;
+	}
+
+	raw_spin_lock_init(&priv->lock);
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto assert_reset;
+
+	ret = clk_prepare_enable(pclk);
+	if (ret)
+		goto disable_clk;
+
+	irq_domain = irq_domain_add_hierarchy(parent_domain, 0, IRQC_NUM_IRQ,
+					      node, &rzg2l_irqc_domain_ops,
+					      priv);
+	if (!irq_domain) {
+		pr_err("%pOF: cannot initialize irq domain\n", node);
+		ret = -ENOMEM;
+		goto fail_irq_domain;
+	}
+
+	return 0;
+
+fail_irq_domain:
+	clk_disable_unprepare(pclk);
+disable_clk:
+	clk_disable_unprepare(clk);
+assert_reset:
+	reset_control_assert(resetn);
+iounmap_base:
+	iounmap(priv->base);
+free_priv:
+	kfree(priv);
+	return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
+IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
+IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH 2/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add a driver for the Renesas RZ/G2L Interrupt Controller.
>
> This supports external pins being used as interrupts. It supports
> one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> to be used as IRQ lines.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -0,0 +1,447 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G2L IRQC Driver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation.
> + *
> + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#define IRQC_IRQ_START                 1
> +#define IRQC_IRQ_COUNT                 8
> +#define IRQC_TINT_START                        9

= IRQC_IRQ_START + IRQC_IRQ_COUNT

> +#define IRQC_TINT_COUNT                        32
> +#define IRQC_NUM_IRQ                   41

= IRQC_TINT_START + IRQC_TINT_COUNT

Should these be in a DT binding header file?

Combining all types into a single linear number space makes it hard
to extend the range, when reusing for an SoC that supports more
interrupt sources.

> +static void rzg2l_irq_eoi(struct irq_data *d)
> +{
> +       struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +       unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> +       u16 bit = BIT(hw_irq);

I guess you can just use u32?

> +       u32 reg;
> +
> +       reg = readl_relaxed(priv->base + ISCR);
> +       if (reg & bit)
> +               writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,

As writes to the unused upper bits are ignored, you can drop the
masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more
interrupt sources.

> +                              priv->base + ISCR);
> +}
> +
> +static void rzg2l_tint_eoi(struct irq_data *d)
> +{
> +       struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +       unsigned int hw_irq = irqd_to_hwirq(d);

"irqd_to_hwirq(d) - IRQC_TINT_START", for symmetry with
rzg2l_irq_eoi()?

> +       u32 bit = BIT(hw_irq - IRQC_TINT_START);
> +       u32 reg;
> +
> +       reg = readl_relaxed(priv->base + TSCR);
> +       if (reg & bit)
> +               writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,

Drop the masking with all-ones?

> +                              priv->base + TSCR);
> +}

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/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Apr 28, 2022 at 10:42 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> >
> > This supports external pins being used as interrupts. It supports
> > one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> > to be used as IRQ lines.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -0,0 +1,447 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G2L IRQC Driver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > + *
> > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of_address.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define IRQC_IRQ_START                 1
> > +#define IRQC_IRQ_COUNT                 8
> > +#define IRQC_TINT_START                        9
>
> = IRQC_IRQ_START + IRQC_IRQ_COUNT
>
OK

> > +#define IRQC_TINT_COUNT                        32
> > +#define IRQC_NUM_IRQ                   41
>
> = IRQC_TINT_START + IRQC_TINT_COUNT
>
OK.

> Should these be in a DT binding header file?
>
> Combining all types into a single linear number space makes it hard
> to extend the range, when reusing for an SoC that supports more
> interrupt sources.
>
Or  DT data maybe?

> > +static void rzg2l_irq_eoi(struct irq_data *d)
> > +{
> > +       struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +       unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > +       u16 bit = BIT(hw_irq);
>
> I guess you can just use u32?
>
OK, will do

> > +       u32 reg;
> > +
> > +       reg = readl_relaxed(priv->base + ISCR);
> > +       if (reg & bit)
> > +               writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
>
> As writes to the unused upper bits are ignored, you can drop the
> masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more
> interrupt sources.
>
Agreed.

> > +                              priv->base + ISCR);
> > +}
> > +
> > +static void rzg2l_tint_eoi(struct irq_data *d)
> > +{
> > +       struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +       unsigned int hw_irq = irqd_to_hwirq(d);
>
> "irqd_to_hwirq(d) - IRQC_TINT_START", for symmetry with
> rzg2l_irq_eoi()?
>
OK.

> > +       u32 bit = BIT(hw_irq - IRQC_TINT_START);
> > +       u32 reg;
> > +
> > +       reg = readl_relaxed(priv->base + TSCR);
> > +       if (reg & bit)
> > +               writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
>
> Drop the masking with all-ones?
>
You mean instead of a mask just use the reg instead?

Cheers,
Prabhakar

> > +                              priv->base + TSCR);
> > +}
>
> 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/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Geert Uytterhoeven]

Hi Prabhakar,

On Fri, Apr 29, 2022 at 11:43 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Thu, Apr 28, 2022 at 10:42 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> > >
> > > This supports external pins being used as interrupts. It supports
> > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> > > to be used as IRQ lines.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > @@ -0,0 +1,447 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Renesas RZ/G2L IRQC Driver
> > > + *
> > > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > > + *
> > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/err.h>
> > > +#include <linux/io.h>
> > > +#include <linux/irqchip.h>
> > > +#include <linux/irqdomain.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#define IRQC_IRQ_START                 1
> > > +#define IRQC_IRQ_COUNT                 8
> > > +#define IRQC_TINT_START                        9
> >
> > = IRQC_IRQ_START + IRQC_IRQ_COUNT
> >
> OK
>
> > > +#define IRQC_TINT_COUNT                        32
> > > +#define IRQC_NUM_IRQ                   41
> >
> > = IRQC_TINT_START + IRQC_TINT_COUNT
> >
> OK.
>
> > Should these be in a DT binding header file?
> >
> > Combining all types into a single linear number space makes it hard
> > to extend the range, when reusing for an SoC that supports more
> > interrupt sources.
> >
> Or  DT data maybe?

Let's leave it for now. As I missed that DT consumers will refer to
external interrupt numbers only (is that actually enforced?), there
won't be an issue.

The driver can be changed later to derive IRQC_IRQ_COUNT from the
compatible value, when needed.

> > > +       u32 reg;
> > > +
> > > +       reg = readl_relaxed(priv->base + ISCR);
> > > +       if (reg & bit)
> > > +               writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
> >
> > As writes to the unused upper bits are ignored, you can drop the
> > masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more
> > interrupt sources.
> >
> Agreed.

> > > +       u32 bit = BIT(hw_irq - IRQC_TINT_START);
> > > +       u32 reg;
> > > +
> > > +       reg = readl_relaxed(priv->base + TSCR);
> > > +       if (reg & bit)
> > > +               writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
> >
> > Drop the masking with all-ones?
> >
> You mean instead of a mask just use the reg instead?

No, I meant to drop the masking with GENMASK(IRQC_TINT_COUNT - 1, 0),
cfr. for external interrupts.

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/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Lad, Prabhakar]

Hi Geert,

On Fri, Apr 29, 2022 at 10:53 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Apr 29, 2022 at 11:43 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Thu, Apr 28, 2022 at 10:42 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> > > >
> > > > This supports external pins being used as interrupts. It supports
> > > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> > > > to be used as IRQ lines.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > > > --- /dev/null
> > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > @@ -0,0 +1,447 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Renesas RZ/G2L IRQC Driver
> > > > + *
> > > > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > > > + *
> > > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > + */
> > > > +
> > > > +#include <linux/clk.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/io.h>
> > > > +#include <linux/irqchip.h>
> > > > +#include <linux/irqdomain.h>
> > > > +#include <linux/of_address.h>
> > > > +#include <linux/reset.h>
> > > > +#include <linux/spinlock.h>
> > > > +
> > > > +#define IRQC_IRQ_START                 1
> > > > +#define IRQC_IRQ_COUNT                 8
> > > > +#define IRQC_TINT_START                        9
> > >
> > > = IRQC_IRQ_START + IRQC_IRQ_COUNT
> > >
> > OK
> >
> > > > +#define IRQC_TINT_COUNT                        32
> > > > +#define IRQC_NUM_IRQ                   41
> > >
> > > = IRQC_TINT_START + IRQC_TINT_COUNT
> > >
> > OK.
> >
> > > Should these be in a DT binding header file?
> > >
> > > Combining all types into a single linear number space makes it hard
> > > to extend the range, when reusing for an SoC that supports more
> > > interrupt sources.
> > >
> > Or  DT data maybe?
>
> Let's leave it for now. As I missed that DT consumers will refer to
> external interrupt numbers only (is that actually enforced?), there
> won't be an issue.
>
> The driver can be changed later to derive IRQC_IRQ_COUNT from the
> compatible value, when needed.
>
Agreed.

> > > > +       u32 reg;
> > > > +
> > > > +       reg = readl_relaxed(priv->base + ISCR);
> > > > +       if (reg & bit)
> > > > +               writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
> > >
> > > As writes to the unused upper bits are ignored, you can drop the
> > > masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more
> > > interrupt sources.
> > >
> > Agreed.
>
> > > > +       u32 bit = BIT(hw_irq - IRQC_TINT_START);
> > > > +       u32 reg;
> > > > +
> > > > +       reg = readl_relaxed(priv->base + TSCR);
> > > > +       if (reg & bit)
> > > > +               writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
> > >
> > > Drop the masking with all-ones?
> > >
> > You mean instead of a mask just use the reg instead?
>
> No, I meant to drop the masking with GENMASK(IRQC_TINT_COUNT - 1, 0),
> cfr. for external interrupts.
>
Ahh right, I missed that.

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

Re: [PATCH 2/2] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

[Lad, Prabhakar]

Hi Geert,

On Fri, Apr 29, 2022 at 10:59 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Geert,
>
> On Fri, Apr 29, 2022 at 10:53 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> >
> > Hi Prabhakar,
> >
> > On Fri, Apr 29, 2022 at 11:43 AM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > > On Thu, Apr 28, 2022 at 10:42 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Fri, Apr 22, 2022 at 12:12 AM Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> > > > >
> > > > > This supports external pins being used as interrupts. It supports
> > > > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> > > > > to be used as IRQ lines.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > > > > --- /dev/null
> > > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > > @@ -0,0 +1,447 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Renesas RZ/G2L IRQC Driver
> > > > > + *
> > > > > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > > > > + *
> > > > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/clk.h>
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/io.h>
> > > > > +#include <linux/irqchip.h>
> > > > > +#include <linux/irqdomain.h>
> > > > > +#include <linux/of_address.h>
> > > > > +#include <linux/reset.h>
> > > > > +#include <linux/spinlock.h>
> > > > > +
> > > > > +#define IRQC_IRQ_START                 1
> > > > > +#define IRQC_IRQ_COUNT                 8
> > > > > +#define IRQC_TINT_START                        9
> > > >
> > > > = IRQC_IRQ_START + IRQC_IRQ_COUNT
> > > >
> > > OK
> > >
> > > > > +#define IRQC_TINT_COUNT                        32
> > > > > +#define IRQC_NUM_IRQ                   41
> > > >
> > > > = IRQC_TINT_START + IRQC_TINT_COUNT
> > > >
> > > OK.
> > >
> > > > Should these be in a DT binding header file?
> > > >
> > > > Combining all types into a single linear number space makes it hard
> > > > to extend the range, when reusing for an SoC that supports more
> > > > interrupt sources.
> > > >
> > > Or  DT data maybe?
> >
> > Let's leave it for now. As I missed that DT consumers will refer to
> > external interrupt numbers only (is that actually enforced?), there
> > won't be an issue.
> >
> > The driver can be changed later to derive IRQC_IRQ_COUNT from the
> > compatible value, when needed.
> >
> Agreed.
>
> > > > > +       u32 reg;
> > > > > +
> > > > > +       reg = readl_relaxed(priv->base + ISCR);
> > > > > +       if (reg & bit)
> > > > > +               writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
> > > >
> > > > As writes to the unused upper bits are ignored, you can drop the
> > > > masking with GENMASK(IRQC_IRQ_COUNT - 1, 0), and be prepared for more
> > > > interrupt sources.
> > > >
> > > Agreed.
> >
> > > > > +       u32 bit = BIT(hw_irq - IRQC_TINT_START);
> > > > > +       u32 reg;
> > > > > +
> > > > > +       reg = readl_relaxed(priv->base + TSCR);
> > > > > +       if (reg & bit)
> > > > > +               writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
> > > >
> > > > Drop the masking with all-ones?
> > > >
> > > You mean instead of a mask just use the reg instead?
> >
> > No, I meant to drop the masking with GENMASK(IRQC_TINT_COUNT - 1, 0),
> > cfr. for external interrupts.
> >
> Ahh right, I missed that.
>
I would need reg to clear off the bit if I drop the mask.

Cheers,
Prabhakar