* [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H
@ 2025-11-21 11:14 Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU Cosmin Tanislav
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Cosmin Tanislav @ 2025-11-21 11:14 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
The Renesas RZ/T2H and RZ/N2H SoCs have an Interrupt Control Unit (ICU)
that supports interrupts from external pins IRQ0 to IRQ15, DREQ and SEI.
Cosmin Tanislav (4):
dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
arm64: dts: renesas: r9a09g077: add ICU support
arm64: dts: renesas: r9a09g087: add ICU support
.../renesas,r9a09g077-icu.yaml | 236 ++++++++++++++
arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 73 +++++
arch/arm64/boot/dts/renesas/r9a09g087.dtsi | 73 +++++
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzt2h.c | 288 ++++++++++++++++++
drivers/soc/renesas/Kconfig | 1 +
include/linux/irqchip/irq-renesas-rzt2h.h | 23 ++
8 files changed, 703 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
create mode 100644 drivers/irqchip/irq-renesas-rzt2h.c
create mode 100644 include/linux/irqchip/irq-renesas-rzt2h.h
--
2.52.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
2025-11-21 11:14 [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H Cosmin Tanislav
@ 2025-11-21 11:14 ` Cosmin Tanislav
2025-11-23 13:23 ` Krzysztof Kozlowski
2025-11-21 11:14 ` [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver Cosmin Tanislav
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Cosmin Tanislav @ 2025-11-21 11:14 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
The Renesas RZ/T2H (R9A09G077) and Renesas RZ/N2H (R9A09G087) SoCs have
an Interrupt Controller (ICU) block that routes external interrupts to
the GIC's SPIs, with the ability of level-translation, and can also
produce software interrupts and aggregate error interrupts.
It has 16 software triggered interrupts (INTCPUn), 16 external pin
interrupts (IRQn), a System error interrupt (SEI), two Cortex-A55 error
interrupts (CA55_ERRn), two Cortex-R52 error interrupts for each of the
two cores (CR52x_ERRn), two Peripheral error interrupts (PERI_ERRn),
two DSMIF error interrupts (DSMIF_ERRn), and two ENCIF error interrupts
(ENCIF_ERRn).
The IRQn and SEI interrupts are exposed externally, while the others
are software triggered.
INTCPU0 to INTCPU13, IRQ 0 to IRQ13 are non-safety interrupts, while
INTCPU14, INTCPU15, IRQ14, IRQ15 and SEI are safety interrupts, and are
exposed via a separate register space.
Document them, and use RZ/T2H as a fallback for RZ/N2H as the ICU is
entirely compatible.
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
.../renesas,r9a09g077-icu.yaml | 236 ++++++++++++++++++
1 file changed, 236 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
new file mode 100644
index 000000000000..8ccdfc51ae3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
@@ -0,0 +1,236 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,r9a09g077-icu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/{T2H,N2H} Interrupt Controller
+
+maintainers:
+ - Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+description:
+ The Interrupt Controller (ICU) handles software-triggered interrupts
+ (INTCPU), external interrupts (IRQ and SEI), error interrupts and DMAC
+ requests.
+
+properties:
+ compatible:
+ oneOf:
+ - const: renesas,r9a09g077-icu # RZ/T2H
+
+ - items:
+ - enum:
+ - renesas,r9a09g087-icu # RZ/N2H
+ - const: renesas,r9a09g077-icu
+
+ '#interrupt-cells':
+ description: The first cell is the SPI number of the interrupt, as per user
+ manual. The second cell is used to specify the flag.
+ const: 2
+
+ '#address-cells':
+ const: 0
+
+ interrupt-controller: true
+
+ reg:
+ items:
+ - description: Non-safety registers (INTCPU0-13, IRQ0-13)
+ - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
+
+ interrupts:
+ items:
+ - description: Software interrupt 0
+ - description: Software interrupt 1
+ - description: Software interrupt 2
+ - description: Software interrupt 3
+ - description: Software interrupt 4
+ - description: Software interrupt 5
+ - description: Software interrupt 6
+ - description: Software interrupt 7
+ - description: Software interrupt 8
+ - description: Software interrupt 9
+ - description: Software interrupt 10
+ - description: Software interrupt 11
+ - description: Software interrupt 12
+ - description: Software interrupt 13
+ - description: Software interrupt 14
+ - description: Software interrupt 15
+ - description: External pin interrupt 0
+ - description: External pin interrupt 1
+ - description: External pin interrupt 2
+ - description: External pin interrupt 3
+ - description: External pin interrupt 4
+ - description: External pin interrupt 5
+ - description: External pin interrupt 6
+ - description: External pin interrupt 7
+ - description: External pin interrupt 8
+ - description: External pin interrupt 9
+ - description: External pin interrupt 10
+ - description: External pin interrupt 11
+ - description: External pin interrupt 12
+ - description: External pin interrupt 13
+ - description: External pin interrupt 14
+ - description: External pin interrupt 15
+ - description: System error interrupt
+ - description: Cortex-A55 error event 0
+ - description: Cortex-A55 error event 1
+ - description: Cortex-R52 CPU 0 error event 0
+ - description: Cortex-R52 CPU 0 error event 1
+ - description: Cortex-R52 CPU 1 error event 0
+ - description: Cortex-R52 CPU 1 error event 1
+ - description: Peripherals error event 0
+ - description: Peripherals error event 1
+ - description: DSMIF error event 0
+ - description: DSMIF error event 1
+ - description: ENCIF error event 0
+ - description: ENCIF error event 1
+
+ interrupt-names:
+ items:
+ - const: intcpu0
+ - const: intcpu1
+ - const: intcpu2
+ - const: intcpu3
+ - const: intcpu4
+ - const: intcpu5
+ - const: intcpu6
+ - const: intcpu7
+ - const: intcpu8
+ - const: intcpu9
+ - const: intcpu10
+ - const: intcpu11
+ - const: intcpu12
+ - const: intcpu13
+ - const: intcpu14
+ - const: intcpu15
+ - const: irq0
+ - const: irq1
+ - const: irq2
+ - const: irq3
+ - const: irq4
+ - const: irq5
+ - const: irq6
+ - const: irq7
+ - const: irq8
+ - const: irq9
+ - const: irq10
+ - const: irq11
+ - const: irq12
+ - const: irq13
+ - const: irq14
+ - const: irq15
+ - const: sei
+ - const: ca55-err0
+ - const: ca55-err1
+ - const: cr520-err0
+ - const: cr520-err1
+ - const: cr521-err0
+ - const: cr521-err1
+ - const: peri-err0
+ - const: peri-err1
+ - const: dsmif-err0
+ - const: dsmif-err1
+ - const: encif-err0
+ - const: encif-err1
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - '#interrupt-cells'
+ - '#address-cells'
+ - interrupt-controller
+ - interrupts
+ - interrupt-names
+ - clocks
+ - power-domains
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
+
+ icu: interrupt-controller@802a0000 {
+ compatible = "renesas,r9a09g077-icu";
+ reg = <0x802a0000 0x10000>,
+ <0x812a0000 0x50>;
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 7 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 9 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 10 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 11 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 12 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 13 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 15 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 16 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 17 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 18 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 19 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 20 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 21 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 22 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 24 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 27 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 28 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 29 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 31 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 406 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 407 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 408 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 409 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 410 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 411 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 412 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 413 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 414 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 415 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 416 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 417 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 418 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "intcpu0", "intcpu1", "intcpu2",
+ "intcpu3", "intcpu4", "intcpu5",
+ "intcpu6", "intcpu7", "intcpu8",
+ "intcpu9", "intcpu10", "intcpu11",
+ "intcpu12", "intcpu13", "intcpu14",
+ "intcpu15",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "irq8", "irq9", "irq10", "irq11",
+ "irq12", "irq13", "irq14", "irq15",
+ "sei",
+ "ca55-err0", "ca55-err1",
+ "cr520-err0", "cr520-err1",
+ "cr521-err0", "cr521-err1",
+ "peri-err0", "peri-err1",
+ "dsmif-err0", "dsmif-err1",
+ "encif-err0", "encif-err1";
+ clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKM>;
+ power-domains = <&cpg>;
+ };
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-21 11:14 [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU Cosmin Tanislav
@ 2025-11-21 11:14 ` Cosmin Tanislav
2025-11-22 15:55 ` Thomas Gleixner
2025-11-21 11:14 ` [PATCH 3/4] arm64: dts: renesas: r9a09g077: add ICU support Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 4/4] arm64: dts: renesas: r9a09g087: " Cosmin Tanislav
3 siblings, 1 reply; 14+ messages in thread
From: Cosmin Tanislav @ 2025-11-21 11:14 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
The Renesas RZ/T2H (R9A09G077) and Renesas RZ/N2H (R9A09G087) SoCs have
an Interrupt Controller (ICU) that supports interrupts from external
pins IRQ0 to IRQ15, and SEI, and software-triggered interrupts INTCPU0
to INTCPU15.
INTCPU0 to INTCPU13, IRQ0 to IRQ13 are non-safety interrupts, while
INTCPU14, INTCPU15, IRQ14, IRQ15 and SEI are safety interrupts, and are
exposed via a separate register space.
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzt2h.c | 288 ++++++++++++++++++++++
drivers/soc/renesas/Kconfig | 1 +
include/linux/irqchip/irq-renesas-rzt2h.h | 23 ++
5 files changed, 321 insertions(+)
create mode 100644 drivers/irqchip/irq-renesas-rzt2h.c
create mode 100644 include/linux/irqchip/irq-renesas-rzt2h.h
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f334f49c9791..118d0c16e633 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -297,6 +297,14 @@ config RENESAS_RZG2L_IRQC
Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
for external devices.
+config RENESAS_RZT2H_ICU
+ bool "Renesas RZ/{T2H,N2H} ICU support" if COMPILE_TEST
+ select GENERIC_IRQ_CHIP
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Enable support for the Renesas RZ/{T2H,N2H} Interrupt Controller
+ (ICU).
+
config RENESAS_RZV2H_ICU
bool "Renesas RZ/V2H(P) ICU support" if COMPILE_TEST
select GENERIC_IRQ_CHIP
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 6a229443efe0..26aa3b6ec99f 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -54,6 +54,7 @@ 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_RENESAS_RZT2H_ICU) += irq-renesas-rzt2h.o
obj-$(CONFIG_RENESAS_RZV2H_ICU) += irq-renesas-rzv2h.o
obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o
diff --git a/drivers/irqchip/irq-renesas-rzt2h.c b/drivers/irqchip/irq-renesas-rzt2h.c
new file mode 100644
index 000000000000..68c163b7ba77
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-rzt2h.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bitfield.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/irq-renesas-rzt2h.h>
+#include <linux/irqdomain.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#define RZT2H_ICU_INTCPU_NS_START 0
+#define RZT2H_ICU_INTCPU_NS_COUNT 14
+
+#define RZT2H_ICU_INTCPU_S_START (RZT2H_ICU_INTCPU_NS_START + \
+ RZT2H_ICU_INTCPU_NS_COUNT)
+#define RZT2H_ICU_INTCPU_S_COUNT 2
+
+#define RZT2H_ICU_IRQ_NS_START (RZT2H_ICU_INTCPU_S_START + \
+ RZT2H_ICU_INTCPU_S_COUNT)
+#define RZT2H_ICU_IRQ_NS_COUNT 14
+
+#define RZT2H_ICU_IRQ_S_START (RZT2H_ICU_IRQ_NS_START + \
+ RZT2H_ICU_IRQ_NS_COUNT)
+#define RZT2H_ICU_IRQ_S_COUNT 2
+
+#define RZT2H_ICU_SEI_START (RZT2H_ICU_IRQ_S_START + \
+ RZT2H_ICU_IRQ_S_COUNT)
+#define RZT2H_ICU_SEI_COUNT 1
+
+#define RZT2H_ICU_NUM_IRQ (RZT2H_ICU_INTCPU_NS_COUNT + \
+ RZT2H_ICU_INTCPU_S_COUNT + \
+ RZT2H_ICU_IRQ_NS_COUNT + \
+ RZT2H_ICU_IRQ_S_COUNT + \
+ RZT2H_ICU_SEI_COUNT)
+
+#define RZT2H_ICU_IRQ_IN_RANGE(n, type) \
+ ((n) >= RZT2H_ICU_##type##_START && \
+ (n) < RZT2H_ICU_##type##_START + RZT2H_ICU_##type##_COUNT)
+
+#define RZT2H_ICU_PORTNF_MD 0xc
+#define RZT2H_ICU_PORTNF_MDi_MASK(i) (GENMASK(1, 0) << ((i) * 2))
+#define RZT2H_ICU_PORTNF_MDi_PREP(i, val) (FIELD_PREP(GENMASK(1, 0), val) << ((i) * 2))
+
+#define RZT2H_ICU_MD_LOW_LEVEL 0b00
+#define RZT2H_ICU_MD_FALLING_EDGE 0b01
+#define RZT2H_ICU_MD_RISING_EDGE 0b10
+#define RZT2H_ICU_MD_BOTH_EDGES 0b11
+
+#define RZT2H_ICU_DMACn_RSSELi(n, i) (0x7d0 + 0x18 * (n) + 0x4 * (i))
+#define RZT2H_ICU_DMAC_REQ_SELx_MASK(x) (GENMASK(9, 0) << ((x) * 10))
+#define RZT2H_ICU_DMAC_REQ_SELx_PREP(x, val) (FIELD_PREP(GENMASK(9, 0), val) << ((x) * 10))
+
+struct rzt2h_icu_priv {
+ void __iomem *base_ns;
+ void __iomem *base_s;
+ struct irq_fwspec fwspec[RZT2H_ICU_NUM_IRQ];
+ raw_spinlock_t lock;
+};
+
+void rzt2h_icu_register_dma_req(struct platform_device *icu_dev, u8 dmac_index,
+ u8 dmac_channel, u16 req_no)
+{
+ struct rzt2h_icu_priv *priv = platform_get_drvdata(icu_dev);
+ u8 y, upper;
+ u32 val;
+
+ y = dmac_channel / 3;
+ upper = dmac_channel % 3;
+
+ guard(raw_spinlock_irqsave)(&priv->lock);
+
+ val = readl(priv->base_ns + RZT2H_ICU_DMACn_RSSELi(dmac_index, y));
+ val &= ~RZT2H_ICU_DMAC_REQ_SELx_MASK(upper);
+ val |= RZT2H_ICU_DMAC_REQ_SELx_PREP(upper, req_no);
+ writel(val, priv->base_ns + RZT2H_ICU_DMACn_RSSELi(dmac_index, y));
+}
+EXPORT_SYMBOL_GPL(rzt2h_icu_register_dma_req);
+
+static inline struct rzt2h_icu_priv *irq_data_to_priv(struct irq_data *data)
+{
+ return data->domain->host_data;
+}
+
+static inline int rzt2h_icu_irq_to_offset(struct irq_data *d, void __iomem **base,
+ unsigned int *offset)
+{
+ struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
+ unsigned int hwirq = irqd_to_hwirq(d);
+
+ if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
+ *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
+ *base = priv->base_ns;
+ } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
+ /* SEI follows safety IRQs in registers and in IRQ numbers. */
+ RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
+ *offset = hwirq - RZT2H_ICU_IRQ_S_START;
+ *base = priv->base_s;
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rzt2h_icu_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
+ unsigned int parent_type;
+ unsigned int offset;
+ void __iomem *base;
+ u32 val, md;
+ int ret;
+
+ ret = rzt2h_icu_irq_to_offset(d, &base, &offset);
+ if (ret)
+ return ret;
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_LEVEL_LOW:
+ md = RZT2H_ICU_MD_LOW_LEVEL;
+ parent_type = IRQ_TYPE_LEVEL_HIGH;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ md = RZT2H_ICU_MD_FALLING_EDGE;
+ parent_type = IRQ_TYPE_EDGE_RISING;
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ md = RZT2H_ICU_MD_RISING_EDGE;
+ parent_type = IRQ_TYPE_EDGE_RISING;
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ md = RZT2H_ICU_MD_BOTH_EDGES;
+ parent_type = IRQ_TYPE_EDGE_RISING;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ guard(raw_spinlock)(&priv->lock);
+ val = readl_relaxed(base + RZT2H_ICU_PORTNF_MD);
+ val &= ~RZT2H_ICU_PORTNF_MDi_MASK(offset);
+ val |= RZT2H_ICU_PORTNF_MDi_PREP(offset, md);
+ writel_relaxed(val, base + RZT2H_ICU_PORTNF_MD);
+
+ return irq_chip_set_type_parent(d, parent_type);
+}
+
+static int rzt2h_icu_set_type(struct irq_data *d, unsigned int type)
+{
+ unsigned int hw_irq = irqd_to_hwirq(d);
+
+ /* IRQn and SEI are selectable, others are edge-only. */
+ if (RZT2H_ICU_IRQ_IN_RANGE(hw_irq, IRQ_NS) ||
+ RZT2H_ICU_IRQ_IN_RANGE(hw_irq, IRQ_S) ||
+ RZT2H_ICU_IRQ_IN_RANGE(hw_irq, SEI))
+ return rzt2h_icu_irq_set_type(d, type);
+
+ if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_EDGE_RISING)
+ return -EINVAL;
+
+ return irq_chip_set_type_parent(d, IRQ_TYPE_EDGE_RISING);
+}
+
+static const struct irq_chip rzt2h_icu_chip = {
+ .name = "rzt2h-icu",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_type = rzt2h_icu_set_type,
+ .irq_set_wake = irq_chip_set_wake_parent,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+ .irq_get_irqchip_state = irq_chip_get_parent_state,
+ .irq_set_irqchip_state = irq_chip_set_parent_state,
+ .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SET_TYPE_MASKED |
+ IRQCHIP_SKIP_SET_WAKE,
+};
+
+static int rzt2h_icu_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ struct rzt2h_icu_priv *priv = domain->host_data;
+ irq_hw_number_t hwirq;
+ unsigned int type;
+ int ret;
+
+ ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &rzt2h_icu_chip,
+ NULL);
+ if (ret)
+ return ret;
+
+ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+ &priv->fwspec[hwirq]);
+}
+
+static const struct irq_domain_ops rzt2h_icu_domain_ops = {
+ .alloc = rzt2h_icu_alloc,
+ .free = irq_domain_free_irqs_common,
+ .translate = irq_domain_translate_twocell,
+};
+
+static int rzt2h_icu_parse_interrupts(struct rzt2h_icu_priv *priv,
+ struct device_node *np)
+{
+ struct of_phandle_args map;
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < RZT2H_ICU_NUM_IRQ; i++) {
+ ret = of_irq_parse_one(np, i, &map);
+ if (ret)
+ return ret;
+
+ of_phandle_args_to_fwspec(np, map.args, map.args_count,
+ &priv->fwspec[i]);
+ }
+
+ return 0;
+}
+
+static int rzt2h_icu_init(struct platform_device *pdev,
+ struct device_node *parent)
+{
+ struct irq_domain *irq_domain, *parent_domain;
+ struct device_node *node = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct rzt2h_icu_priv *priv;
+ int ret;
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain)
+ return dev_err_probe(dev, -ENODEV, "cannot find parent domain\n");
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
+ priv->base_ns = devm_of_iomap(dev, dev->of_node, 0, NULL);
+ if (IS_ERR(priv->base_ns))
+ return PTR_ERR(priv->base_ns);
+
+ priv->base_s = devm_of_iomap(dev, dev->of_node, 1, NULL);
+ if (IS_ERR(priv->base_s))
+ return PTR_ERR(priv->base_s);
+
+ ret = rzt2h_icu_parse_interrupts(priv, node);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "cannot parse interrupts: %d\n", ret);
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "devm_pm_runtime_enable failed: %d\n", ret);
+
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "pm_runtime_resume_and_get failed: %d\n", ret);
+
+ raw_spin_lock_init(&priv->lock);
+
+ irq_domain = irq_domain_create_hierarchy(parent_domain, 0, RZT2H_ICU_NUM_IRQ,
+ dev_fwnode(dev),
+ &rzt2h_icu_domain_ops, priv);
+ if (!irq_domain) {
+ pm_runtime_put(dev);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(rzt2h_icu)
+IRQCHIP_MATCH("renesas,r9a09g077-icu", rzt2h_icu_init)
+IRQCHIP_PLATFORM_DRIVER_END(rzt2h_icu)
+MODULE_AUTHOR("Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/T2H ICU Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 340a1ff7e92b..198baf890b14 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -423,6 +423,7 @@ config ARCH_R9A09G057
config ARCH_R9A09G077
bool "ARM64 Platform support for R9A09G077 (RZ/T2H)"
default y if ARCH_RENESAS
+ select RENESAS_RZT2H_ICU
help
This enables support for the Renesas RZ/T2H SoC variants.
diff --git a/include/linux/irqchip/irq-renesas-rzt2h.h b/include/linux/irqchip/irq-renesas-rzt2h.h
new file mode 100644
index 000000000000..1a5bfd461fe4
--- /dev/null
+++ b/include/linux/irqchip/irq-renesas-rzt2h.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Renesas RZ/T2H Interrupt Control Unit (ICU)
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation.
+ */
+
+#ifndef __LINUX_IRQ_RENESAS_RZT2H
+#define __LINUX_IRQ_RENESAS_RZT2H
+
+#include <linux/platform_device.h>
+
+#define RZT2H_ICU_DMAC_REQ_NO_DEFAULT 0x3ff
+
+#ifdef CONFIG_RENESAS_RZT2H_ICU
+void rzt2h_icu_register_dma_req(struct platform_device *icu_dev, u8 dmac_index,
+ u8 dmac_channel, u16 req_no);
+#else
+static inline void rzt2h_icu_register_dma_req(struct platform_device *icu_dev, u8 dmac_index,
+ u8 dmac_channel, u16 req_no) { }
+#endif
+
+#endif /* __LINUX_IRQ_RENESAS_RZT2H */
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] arm64: dts: renesas: r9a09g077: add ICU support
2025-11-21 11:14 [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver Cosmin Tanislav
@ 2025-11-21 11:14 ` Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 4/4] arm64: dts: renesas: r9a09g087: " Cosmin Tanislav
3 siblings, 0 replies; 14+ messages in thread
From: Cosmin Tanislav @ 2025-11-21 11:14 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
The Renesas RZ/T2H (R9A09G077) SoC has an Interrupt Controller (ICU)
block that routes external interrupts to the GIC's SPIs, with the
ability of level-translation, and can also produce software
and aggregate error interrupts.
Add support for it.
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 73 ++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
index f80c6d603eea..0af41287e6a8 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
@@ -862,6 +862,79 @@ cpg: clock-controller@80280000 {
#power-domain-cells = <0>;
};
+ icu: interrupt-controller@802a0000 {
+ compatible = "renesas,r9a09g077-icu";
+ reg = <0 0x802a0000 0 0x10000>,
+ <0 0x812a0000 0 0x50>;
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 7 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 9 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 10 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 11 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 12 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 13 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 15 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 16 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 17 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 18 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 19 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 20 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 21 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 22 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 24 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 27 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 28 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 29 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 31 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 406 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 407 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 408 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 409 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 410 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 411 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 412 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 413 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 414 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 415 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 416 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 417 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 418 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "intcpu0", "intcpu1", "intcpu2",
+ "intcpu3", "intcpu4", "intcpu5",
+ "intcpu6", "intcpu7", "intcpu8",
+ "intcpu9", "intcpu10", "intcpu11",
+ "intcpu12", "intcpu13", "intcpu14",
+ "intcpu15",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "irq8", "irq9", "irq10", "irq11",
+ "irq12", "irq13", "irq14", "irq15",
+ "sei",
+ "ca55-err0", "ca55-err1",
+ "cr520-err0", "cr520-err1",
+ "cr521-err0", "cr521-err1",
+ "peri-err0", "peri-err1",
+ "dsmif-err0", "dsmif-err1",
+ "encif-err0", "encif-err1";
+ clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKM>;
+ power-domains = <&cpg>;
+ };
+
pinctrl: pinctrl@802c0000 {
compatible = "renesas,r9a09g077-pinctrl";
reg = <0 0x802c0000 0 0x10000>,
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] arm64: dts: renesas: r9a09g087: add ICU support
2025-11-21 11:14 [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H Cosmin Tanislav
` (2 preceding siblings ...)
2025-11-21 11:14 ` [PATCH 3/4] arm64: dts: renesas: r9a09g077: add ICU support Cosmin Tanislav
@ 2025-11-21 11:14 ` Cosmin Tanislav
3 siblings, 0 replies; 14+ messages in thread
From: Cosmin Tanislav @ 2025-11-21 11:14 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
The Renesas RZ/N2H (R9A09G087) SoC has an Interrupt Controller (ICU)
block that routes external interrupts to the GIC's SPIs, with the
ability of level-translation, and can also produce software
and aggregate error interrupts.
Add support for it.
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
arch/arm64/boot/dts/renesas/r9a09g087.dtsi | 73 ++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
index f9f49bd3e8b0..6b5693e5c1f9 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
@@ -865,6 +865,79 @@ cpg: clock-controller@80280000 {
#power-domain-cells = <0>;
};
+ icu: interrupt-controller@802a0000 {
+ compatible = "renesas,r9a09g087-icu", "renesas,r9a09g077-icu";
+ reg = <0 0x802a0000 0 0x10000>,
+ <0 0x812a0000 0 0x50>;
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 1 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 2 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 7 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 9 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 10 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 11 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 12 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 13 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 15 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 16 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 17 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 18 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 19 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 20 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 21 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 22 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 23 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 24 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 27 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 28 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 29 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 31 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 406 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 407 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 408 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 409 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 410 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 411 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 412 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 413 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 414 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 415 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 416 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 417 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 418 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "intcpu0", "intcpu1", "intcpu2",
+ "intcpu3", "intcpu4", "intcpu5",
+ "intcpu6", "intcpu7", "intcpu8",
+ "intcpu9", "intcpu10", "intcpu11",
+ "intcpu12", "intcpu13", "intcpu14",
+ "intcpu15",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "irq8", "irq9", "irq10", "irq11",
+ "irq12", "irq13", "irq14", "irq15",
+ "sei",
+ "ca55-err0", "ca55-err1",
+ "cr520-err0", "cr520-err1",
+ "cr521-err0", "cr521-err1",
+ "peri-err0", "peri-err1",
+ "dsmif-err0", "dsmif-err1",
+ "encif-err0", "encif-err1";
+ clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKM>;
+ power-domains = <&cpg>;
+ };
+
pinctrl: pinctrl@802c0000 {
compatible = "renesas,r9a09g087-pinctrl";
reg = <0 0x802c0000 0 0x10000>,
--
2.52.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-21 11:14 ` [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver Cosmin Tanislav
@ 2025-11-22 15:55 ` Thomas Gleixner
2025-11-24 12:50 ` Cosmin-Gabriel Tanislav
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2025-11-22 15:55 UTC (permalink / raw)
To: Cosmin Tanislav, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Cosmin Tanislav
Cc: linux-kernel, devicetree, linux-renesas-soc
On Fri, Nov 21 2025 at 13:14, Cosmin Tanislav wrote:
> +static inline int rzt2h_icu_irq_to_offset(struct irq_data *d, void __iomem **base,
> + unsigned int *offset)
> +{
> + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> +
> + if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
> + *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
> + *base = priv->base_ns;
> + } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
> + /* SEI follows safety IRQs in registers and in IRQ numbers. */
> + RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
This nested commend in the condition is really unreadable.
> + *offset = hwirq - RZT2H_ICU_IRQ_S_START;
> + *base = priv->base_s;
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int rzt2h_icu_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
> + unsigned int parent_type;
> + unsigned int offset;
Combine same data types into one line please.
> + void __iomem *base;
> + u32 val, md;
> + int ret;
> + guard(raw_spinlock)(&priv->lock);
> + val = readl_relaxed(base + RZT2H_ICU_PORTNF_MD);
> + val &= ~RZT2H_ICU_PORTNF_MDi_MASK(offset);
> + val |= RZT2H_ICU_PORTNF_MDi_PREP(offset, md);
> + writel_relaxed(val, base + RZT2H_ICU_PORTNF_MD);
> +
This looks wrong. guard() holds the lock across the set_parent()
call. If you really need that then this needs a comment explaining the
why. Otherwise use scoped_guard().
> + return irq_chip_set_type_parent(d, parent_type);
> +}
> +static const struct irq_chip rzt2h_icu_chip = {
> + .name = "rzt2h-icu",
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_eoi = irq_chip_eoi_parent,
> + .irq_set_type = rzt2h_icu_set_type,
> + .irq_set_wake = irq_chip_set_wake_parent,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> + .irq_retrigger = irq_chip_retrigger_hierarchy,
> + .irq_get_irqchip_state = irq_chip_get_parent_state,
> + .irq_set_irqchip_state = irq_chip_set_parent_state,
> + .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SET_TYPE_MASKED |
> + IRQCHIP_SKIP_SET_WAKE,
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
And please read and follow the rest of the documentation too.
> +};
> +
> +static int rzt2h_icu_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + struct rzt2h_icu_priv *priv = domain->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int type;
> + int ret;
> +
> + ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &rzt2h_icu_chip,
> + NULL);
Get rid of the line breaks all over the place. You have 100 characters.
> + if (ret)
> + return ret;
> +
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> + &priv->fwspec[hwirq]);
> +}
> +static int rzt2h_icu_init(struct platform_device *pdev,
> + struct device_node *parent)
> +{
> + struct irq_domain *irq_domain, *parent_domain;
> + struct device_node *node = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct rzt2h_icu_priv *priv;
> + int ret;
> +
> + parent_domain = irq_find_host(parent);
> + if (!parent_domain)
> + return dev_err_probe(dev, -ENODEV, "cannot find parent domain\n");
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + priv->base_ns = devm_of_iomap(dev, dev->of_node, 0, NULL);
> + if (IS_ERR(priv->base_ns))
> + return PTR_ERR(priv->base_ns);
> +
> + priv->base_s = devm_of_iomap(dev, dev->of_node, 1, NULL);
> + if (IS_ERR(priv->base_s))
> + return PTR_ERR(priv->base_s);
> +
> + ret = rzt2h_icu_parse_interrupts(priv, node);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "cannot parse interrupts: %d\n", ret);
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "devm_pm_runtime_enable failed: %d\n", ret);
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "pm_runtime_resume_and_get failed: %d\n", ret);
> +
> + raw_spin_lock_init(&priv->lock);
> +
> + irq_domain = irq_domain_create_hierarchy(parent_domain, 0, RZT2H_ICU_NUM_IRQ,
> + dev_fwnode(dev),
> + &rzt2h_icu_domain_ops, priv);
> + if (!irq_domain) {
> + pm_runtime_put(dev);
> + return -ENOMEM;
> + }
The mix of 'return $ERR' and 'return dev_err_probe()' is confusing at best.
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
2025-11-21 11:14 ` [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU Cosmin Tanislav
@ 2025-11-23 13:23 ` Krzysztof Kozlowski
2025-11-24 16:25 ` Cosmin-Gabriel Tanislav
0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-23 13:23 UTC (permalink / raw)
To: Cosmin Tanislav, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm
Cc: linux-kernel, devicetree, linux-renesas-soc
On 21/11/2025 12:14, Cosmin Tanislav wrote:
> +properties:
> + compatible:
> + oneOf:
> + - const: renesas,r9a09g077-icu # RZ/T2H
> +
> + - items:
> + - enum:
> + - renesas,r9a09g087-icu # RZ/N2H
> + - const: renesas,r9a09g077-icu
> +
> + '#interrupt-cells':
> + description: The first cell is the SPI number of the interrupt, as per user
> + manual. The second cell is used to specify the flag.
> + const: 2
> +
> + '#address-cells':
> + const: 0
> +
> + interrupt-controller: true
> +
> + reg:
> + items:
> + - description: Non-safety registers (INTCPU0-13, IRQ0-13)
> + - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
reg is always the second property. Please follow DTS coding style.
> +
> + interrupts:
> + items:
> + - description: Software interrupt 0
> + - description: Software interrupt 1
> + - description: Software interrupt 2
> + - description: Software interrupt 3
> + - description: Software interrupt 4
> + - description: Software interrupt 5
> + - description: Software interrupt 6
> + - description: Software interrupt 7
> + - description: Software interrupt 8
> + - description: Software interrupt 9
> + - description: Software interrupt 10
> + - description: Software interrupt 11
> + - description: Software interrupt 12
> + - description: Software interrupt 13
> + - description: Software interrupt 14
> + - description: Software interrupt 15
> + - description: External pin interrupt 0
> + - description: External pin interrupt 1
> + - description: External pin interrupt 2
> + - description: External pin interrupt 3
> + - description: External pin interrupt 4
> + - description: External pin interrupt 5
> + - description: External pin interrupt 6
> + - description: External pin interrupt 7
> + - description: External pin interrupt 8
> + - description: External pin interrupt 9
> + - description: External pin interrupt 10
> + - description: External pin interrupt 11
> + - description: External pin interrupt 12
> + - description: External pin interrupt 13
> + - description: External pin interrupt 14
> + - description: External pin interrupt 15
> + - description: System error interrupt
> + - description: Cortex-A55 error event 0
> + - description: Cortex-A55 error event 1
> + - description: Cortex-R52 CPU 0 error event 0
> + - description: Cortex-R52 CPU 0 error event 1
> + - description: Cortex-R52 CPU 1 error event 0
> + - description: Cortex-R52 CPU 1 error event 1
> + - description: Peripherals error event 0
> + - description: Peripherals error event 1
> + - description: DSMIF error event 0
> + - description: DSMIF error event 1
> + - description: ENCIF error event 0
> + - description: ENCIF error event 1
> +
> + interrupt-names:
> + items:
> + - const: intcpu0
> + - const: intcpu1
> + - const: intcpu2
> + - const: intcpu3
> + - const: intcpu4
> + - const: intcpu5
> + - const: intcpu6
> + - const: intcpu7
> + - const: intcpu8
> + - const: intcpu9
> + - const: intcpu10
> + - const: intcpu11
> + - const: intcpu12
> + - const: intcpu13
> + - const: intcpu14
> + - const: intcpu15
> + - const: irq0
> + - const: irq1
> + - const: irq2
> + - const: irq3
> + - const: irq4
> + - const: irq5
> + - const: irq6
> + - const: irq7
> + - const: irq8
> + - const: irq9
> + - const: irq10
> + - const: irq11
> + - const: irq12
> + - const: irq13
> + - const: irq14
> + - const: irq15
> + - const: sei
> + - const: ca55-err0
> + - const: ca55-err1
> + - const: cr520-err0
> + - const: cr520-err1
> + - const: cr521-err0
> + - const: cr521-err1
> + - const: peri-err0
> + - const: peri-err1
> + - const: dsmif-err0
> + - const: dsmif-err1
> + - const: encif-err0
> + - const: encif-err1
Why all the interrupt names have nothing in common with previous ICU
(renesas,rzv2h-icu.yaml)? These names are supposed to share, not
re-invent every time the name.
Isn't external interrupt the same as GPIO interrupt? How do they differ
for this particular device?
And "Error interrupt to CA55" is "icu-error-ca55", but here THE SAME is
called "ca55-err0"?
No, please start using unified naming, not re-inventing this every time.
Order also is supposed to follow older generation, so bindings share
common parts.
> +
> + clocks:
> + maxItems: 1
> +
> + power-domains:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#interrupt-cells'
> + - '#address-cells'
> + - interrupt-controller
> + - interrupts
> + - interrupt-names
> + - clocks
> + - power-domains
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-22 15:55 ` Thomas Gleixner
@ 2025-11-24 12:50 ` Cosmin-Gabriel Tanislav
2025-11-24 13:49 ` Thomas Gleixner
0 siblings, 1 reply; 14+ messages in thread
From: Cosmin-Gabriel Tanislav @ 2025-11-24 12:50 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Saturday, November 22, 2025 5:56 PM
> To: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Geert Uytterhoeven
> <geert+renesas@glider.be>; magnus.damm <magnus.damm@gmail.com>; Cosmin-Gabriel Tanislav <cosmin-
> gabriel.tanislav.xa@renesas.com>
> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
>
> On Fri, Nov 21 2025 at 13:14, Cosmin Tanislav wrote:
> > +static inline int rzt2h_icu_irq_to_offset(struct irq_data *d, void __iomem **base,
> > + unsigned int *offset)
> > +{
> > + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
> > + unsigned int hwirq = irqd_to_hwirq(d);
> > +
> > + if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
> > + *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
> > + *base = priv->base_ns;
> > + } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
> > + /* SEI follows safety IRQs in registers and in IRQ numbers. */
> > + RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
>
> This nested commend in the condition is really unreadable.
>
Would this read better in your opinion?
/*
* Safety IRQs and SEI use a separate register space from the non-safety IRQs.
* SEI interrupt number follows immediately after the safety IRQs.
*/
if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
*offset = hwirq - RZT2H_ICU_IRQ_NS_START;
*base = priv->base_ns;
} else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
*offset = hwirq - RZT2H_ICU_IRQ_S_START;
*base = priv->base_s;
} else {
return -EINVAL;
}
One more thing, for the above cases where the same macro is used twice
in a condition, is it okay to keep it split across two lines to align
them with each other, or do you want them on a single line up to 100
columns?
> > + *offset = hwirq - RZT2H_ICU_IRQ_S_START;
> > + *base = priv->base_s;
> > + } else {
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rzt2h_icu_irq_set_type(struct irq_data *d, unsigned int type)
> > +{
> > + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
> > + unsigned int parent_type;
> > + unsigned int offset;
>
> Combine same data types into one line please.
>
Ack.
> > + void __iomem *base;
> > + u32 val, md;
> > + int ret;
>
>
> > + guard(raw_spinlock)(&priv->lock);
> > + val = readl_relaxed(base + RZT2H_ICU_PORTNF_MD);
> > + val &= ~RZT2H_ICU_PORTNF_MDi_MASK(offset);
> > + val |= RZT2H_ICU_PORTNF_MDi_PREP(offset, md);
> > + writel_relaxed(val, base + RZT2H_ICU_PORTNF_MD);
> > +
>
> This looks wrong. guard() holds the lock across the set_parent()
> call. If you really need that then this needs a comment explaining the
> why. Otherwise use scoped_guard().
>
Ack.
> > + return irq_chip_set_type_parent(d, parent_type);
> > +}
> > +static const struct irq_chip rzt2h_icu_chip = {
> > + .name = "rzt2h-icu",
> > + .irq_mask = irq_chip_mask_parent,
> > + .irq_unmask = irq_chip_unmask_parent,
> > + .irq_eoi = irq_chip_eoi_parent,
> > + .irq_set_type = rzt2h_icu_set_type,
> > + .irq_set_wake = irq_chip_set_wake_parent,
> > + .irq_set_affinity = irq_chip_set_affinity_parent,
> > + .irq_retrigger = irq_chip_retrigger_hierarchy,
> > + .irq_get_irqchip_state = irq_chip_get_parent_state,
> > + .irq_set_irqchip_state = irq_chip_set_parent_state,
> > + .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SET_TYPE_MASKED |
> > + IRQCHIP_SKIP_SET_WAKE,
>
> https://www.kernel.org/doc/html/latest%25
> 2Fprocess%2Fmaintainer-tip.html%23struct-declarations-and-initializers&data=05%7C02%7Ccosmin-
> gabriel.tanislav.xa%40renesas.com%7C377460d2de2a4899c85f08de29df9604%7C53d82571da1947e49cb4625a166a4a2a
> %7C0%7C0%7C638994237471295714%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiO
> iJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=K%2FjQlj7d2xbeyIrYLSmDoI90CNwyndz%2B0nkU
> j%2Bu6fn0%3D&reserved=0
>
> And please read and follow the rest of the documentation too.
>
Ack.
> > +};
> > +
> > +static int rzt2h_icu_alloc(struct irq_domain *domain, unsigned int virq,
> > + unsigned int nr_irqs, void *arg)
> > +{
> > + struct rzt2h_icu_priv *priv = domain->host_data;
> > + irq_hw_number_t hwirq;
> > + unsigned int type;
> > + int ret;
> > +
> > + ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> > + if (ret)
> > + return ret;
> > +
> > + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &rzt2h_icu_chip,
> > + NULL);
>
> Get rid of the line breaks all over the place. You have 100 characters.
>
Ack.
> > + if (ret)
> > + return ret;
> > +
> > + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> > + &priv->fwspec[hwirq]);
> > +}
>
>
> > +static int rzt2h_icu_init(struct platform_device *pdev,
> > + struct device_node *parent)
> > +{
> > + struct irq_domain *irq_domain, *parent_domain;
> > + struct device_node *node = pdev->dev.of_node;
> > + struct device *dev = &pdev->dev;
> > + struct rzt2h_icu_priv *priv;
> > + int ret;
> > +
> > + parent_domain = irq_find_host(parent);
> > + if (!parent_domain)
> > + return dev_err_probe(dev, -ENODEV, "cannot find parent domain\n");
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, priv);
> > +
> > + priv->base_ns = devm_of_iomap(dev, dev->of_node, 0, NULL);
> > + if (IS_ERR(priv->base_ns))
> > + return PTR_ERR(priv->base_ns);
> > +
> > + priv->base_s = devm_of_iomap(dev, dev->of_node, 1, NULL);
> > + if (IS_ERR(priv->base_s))
> > + return PTR_ERR(priv->base_s);
> > +
> > + ret = rzt2h_icu_parse_interrupts(priv, node);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "cannot parse interrupts: %d\n", ret);
> > +
> > + ret = devm_pm_runtime_enable(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "devm_pm_runtime_enable failed: %d\n", ret);
> > +
> > + ret = pm_runtime_resume_and_get(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "pm_runtime_resume_and_get failed: %d\n", ret);
> > +
> > + raw_spin_lock_init(&priv->lock);
> > +
> > + irq_domain = irq_domain_create_hierarchy(parent_domain, 0, RZT2H_ICU_NUM_IRQ,
> > + dev_fwnode(dev),
> > + &rzt2h_icu_domain_ops, priv);
> > + if (!irq_domain) {
> > + pm_runtime_put(dev);
> > + return -ENOMEM;
> > + }
>
> The mix of 'return $ERR' and 'return dev_err_probe()' is confusing at best.
>
For ENOMEM, dev_err_probe() doesn't really print anything. ENOMEM is
what other drivers seem to use for a NULL irq_domain_create_hierarchy()
result.
Do you want me to use a different error code and switch to
dev_err_probe(), or keep ENOMEM and switch to dev_err_probe() for
uniformity anyway?
What should be done for the devm_kzalloc() failure case at the top of
rzt2h_icu_init()?
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-24 12:50 ` Cosmin-Gabriel Tanislav
@ 2025-11-24 13:49 ` Thomas Gleixner
2025-11-24 15:28 ` Cosmin-Gabriel Tanislav
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2025-11-24 13:49 UTC (permalink / raw)
To: Cosmin-Gabriel Tanislav, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
On Mon, Nov 24 2025 at 12:50, Cosmin-Gabriel Tanislav wrote:
>> -----Original Message-----
>> From: Thomas Gleixner <tglx@linutronix.de>
>> Sent: Saturday, November 22, 2025 5:56 PM
>> To: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>; Rob Herring <robh@kernel.org>;
>> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Geert Uytterhoeven
>> <geert+renesas@glider.be>; magnus.damm <magnus.damm@gmail.com>; Cosmin-Gabriel Tanislav <cosmin-
>> gabriel.tanislav.xa@renesas.com>
>> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org
>> Subject: Re: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
Can you please fix your mail-client not to copy the whole header into
the reply?
>> On Fri, Nov 21 2025 at 13:14, Cosmin Tanislav wrote:
>> > +static inline int rzt2h_icu_irq_to_offset(struct irq_data *d, void __iomem **base,
>> > + unsigned int *offset)
>> > +{
>> > + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
>> > + unsigned int hwirq = irqd_to_hwirq(d);
>> > +
>> > + if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
>> > + *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
>> > + *base = priv->base_ns;
>> > + } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
>> > + /* SEI follows safety IRQs in registers and in IRQ numbers. */
>> > + RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
>>
>> This nested commend in the condition is really unreadable.
>>
>
> Would this read better in your opinion?
>
> /*
> * Safety IRQs and SEI use a separate register space from the non-safety IRQs.
> * SEI interrupt number follows immediately after the safety IRQs.
> */
> if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
> *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
> *base = priv->base_ns;
> } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
> RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
> *offset = hwirq - RZT2H_ICU_IRQ_S_START;
> *base = priv->base_s;
> } else {
> return -EINVAL;
> }
Yes. Way better.
> One more thing, for the above cases where the same macro is used twice
> in a condition, is it okay to keep it split across two lines to align
> them with each other, or do you want them on a single line up to 100
> columns?
Usually single line, but in this case it might be more readable. Up to you.
>> > + if (!irq_domain) {
>> > + pm_runtime_put(dev);
>> > + return -ENOMEM;
>> > + }
>>
>> The mix of 'return $ERR' and 'return dev_err_probe()' is confusing at best.
>>
>
> For ENOMEM, dev_err_probe() doesn't really print anything. ENOMEM is
> what other drivers seem to use for a NULL irq_domain_create_hierarchy()
> result.
That's what I was missing. Now it makes sense.
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-24 13:49 ` Thomas Gleixner
@ 2025-11-24 15:28 ` Cosmin-Gabriel Tanislav
2025-11-24 19:01 ` Thomas Gleixner
0 siblings, 1 reply; 14+ messages in thread
From: Cosmin-Gabriel Tanislav @ 2025-11-24 15:28 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Monday, November 24, 2025 3:49 PM
>
> On Mon, Nov 24 2025 at 12:50, Cosmin-Gabriel Tanislav wrote:
> >> From: Thomas Gleixner <tglx@linutronix.de>
> >> Sent: Saturday, November 22, 2025 5:56 PM
>
> Can you please fix your mail-client not to copy the whole header into
> the reply?
>
Outlook, it's unfixable. I can remove it manually each time if it's
too much noise.
> >> On Fri, Nov 21 2025 at 13:14, Cosmin Tanislav wrote:
> >> > +static inline int rzt2h_icu_irq_to_offset(struct irq_data *d, void __iomem **base,
> >> > + unsigned int *offset)
> >> > +{
> >> > + struct rzt2h_icu_priv *priv = irq_data_to_priv(d);
> >> > + unsigned int hwirq = irqd_to_hwirq(d);
> >> > +
> >> > + if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
> >> > + *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
> >> > + *base = priv->base_ns;
> >> > + } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
> >> > + /* SEI follows safety IRQs in registers and in IRQ numbers. */
> >> > + RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
> >>
> >> This nested commend in the condition is really unreadable.
> >>
> >
> > Would this read better in your opinion?
> >
> > /*
> > * Safety IRQs and SEI use a separate register space from the non-safety IRQs.
> > * SEI interrupt number follows immediately after the safety IRQs.
> > */
> > if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_NS)) {
> > *offset = hwirq - RZT2H_ICU_IRQ_NS_START;
> > *base = priv->base_ns;
> > } else if (RZT2H_ICU_IRQ_IN_RANGE(hwirq, IRQ_S) ||
> > RZT2H_ICU_IRQ_IN_RANGE(hwirq, SEI)) {
> > *offset = hwirq - RZT2H_ICU_IRQ_S_START;
> > *base = priv->base_s;
> > } else {
> > return -EINVAL;
> > }
>
> Yes. Way better.
>
Ack.
> > One more thing, for the above cases where the same macro is used twice
> > in a condition, is it okay to keep it split across two lines to align
> > them with each other, or do you want them on a single line up to 100
> > columns?
>
> Usually single line, but in this case it might be more readable. Up to you.
>
Ack.
> >> > + if (!irq_domain) {
> >> > + pm_runtime_put(dev);
> >> > + return -ENOMEM;
> >> > + }
> >>
> >> The mix of 'return $ERR' and 'return dev_err_probe()' is confusing at best.
> >>
> >
> > For ENOMEM, dev_err_probe() doesn't really print anything. ENOMEM is
> > what other drivers seem to use for a NULL irq_domain_create_hierarchy()
> > result.
>
> That's what I was missing. Now it makes sense.
>
In conclusion, should I keep the bare `return -ENOMEM` in both instances?
Just to make sure the next version is proper.
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
2025-11-23 13:23 ` Krzysztof Kozlowski
@ 2025-11-24 16:25 ` Cosmin-Gabriel Tanislav
2025-11-27 7:19 ` Krzysztof Kozlowski
0 siblings, 1 reply; 14+ messages in thread
From: Cosmin-Gabriel Tanislav @ 2025-11-24 16:25 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Sunday, November 23, 2025 3:24 PM
>
> On 21/11/2025 12:14, Cosmin Tanislav wrote:
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: renesas,r9a09g077-icu # RZ/T2H
> > +
> > + - items:
> > + - enum:
> > + - renesas,r9a09g087-icu # RZ/N2H
> > + - const: renesas,r9a09g077-icu
> > +
> > + '#interrupt-cells':
> > + description: The first cell is the SPI number of the interrupt, as per user
> > + manual. The second cell is used to specify the flag.
> > + const: 2
> > +
> > + '#address-cells':
> > + const: 0
> > +
> > + interrupt-controller: true
> > +
> > + reg:
> > + items:
> > + - description: Non-safety registers (INTCPU0-13, IRQ0-13)
> > + - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
>
> reg is always the second property. Please follow DTS coding style.
>
Ack.
> > +
> > + interrupts:
> > + items:
> > + - description: Software interrupt 0
> > + - description: Software interrupt 1
> > + - description: Software interrupt 2
> > + - description: Software interrupt 3
> > + - description: Software interrupt 4
> > + - description: Software interrupt 5
> > + - description: Software interrupt 6
> > + - description: Software interrupt 7
> > + - description: Software interrupt 8
> > + - description: Software interrupt 9
> > + - description: Software interrupt 10
> > + - description: Software interrupt 11
> > + - description: Software interrupt 12
> > + - description: Software interrupt 13
> > + - description: Software interrupt 14
> > + - description: Software interrupt 15
> > + - description: External pin interrupt 0
> > + - description: External pin interrupt 1
> > + - description: External pin interrupt 2
> > + - description: External pin interrupt 3
> > + - description: External pin interrupt 4
> > + - description: External pin interrupt 5
> > + - description: External pin interrupt 6
> > + - description: External pin interrupt 7
> > + - description: External pin interrupt 8
> > + - description: External pin interrupt 9
> > + - description: External pin interrupt 10
> > + - description: External pin interrupt 11
> > + - description: External pin interrupt 12
> > + - description: External pin interrupt 13
> > + - description: External pin interrupt 14
> > + - description: External pin interrupt 15
> > + - description: System error interrupt
> > + - description: Cortex-A55 error event 0
> > + - description: Cortex-A55 error event 1
> > + - description: Cortex-R52 CPU 0 error event 0
> > + - description: Cortex-R52 CPU 0 error event 1
> > + - description: Cortex-R52 CPU 1 error event 0
> > + - description: Cortex-R52 CPU 1 error event 1
> > + - description: Peripherals error event 0
> > + - description: Peripherals error event 1
> > + - description: DSMIF error event 0
> > + - description: DSMIF error event 1
> > + - description: ENCIF error event 0
> > + - description: ENCIF error event 1
> > +
> > + interrupt-names:
> > + items:
> > + - const: intcpu0
> > + - const: intcpu1
> > + - const: intcpu2
> > + - const: intcpu3
> > + - const: intcpu4
> > + - const: intcpu5
> > + - const: intcpu6
> > + - const: intcpu7
> > + - const: intcpu8
> > + - const: intcpu9
> > + - const: intcpu10
> > + - const: intcpu11
> > + - const: intcpu12
> > + - const: intcpu13
> > + - const: intcpu14
> > + - const: intcpu15
> > + - const: irq0
> > + - const: irq1
> > + - const: irq2
> > + - const: irq3
> > + - const: irq4
> > + - const: irq5
> > + - const: irq6
> > + - const: irq7
> > + - const: irq8
> > + - const: irq9
> > + - const: irq10
> > + - const: irq11
> > + - const: irq12
> > + - const: irq13
> > + - const: irq14
> > + - const: irq15
> > + - const: sei
> > + - const: ca55-err0
> > + - const: ca55-err1
> > + - const: cr520-err0
> > + - const: cr520-err1
> > + - const: cr521-err0
> > + - const: cr521-err1
> > + - const: peri-err0
> > + - const: peri-err1
> > + - const: dsmif-err0
> > + - const: dsmif-err1
> > + - const: encif-err0
> > + - const: encif-err1
>
> Why all the interrupt names have nothing in common with previous ICU
> (renesas,rzv2h-icu.yaml)?
Unfortunately, the functionality is different compared to what was
present on RZ/V2H, hence the different names, descriptions, and order,
which I've taken straight from the User Manual of the SoC.
If the ICUs were similar, I would have tried to reuse the bindings and
drivers, but it would have quickly become too complex for what it's
worth.
> These names are supposed to share, not
> re-invent every time the name.
>
Do you think it is worth diverging from the User Manual to bring the
definition more in line with past SoCs?
The advantage of sticking with the User Manual naming scheme is that
you can easily cross-reference these descriptions with the User Manual
and find what you need, whereas "PORT_IRQ0" / "GPIO interrupt" would
give you no information for RZ/T2H.
> Isn't external interrupt the same as GPIO interrupt? How do they differ
> for this particular device?
>
External pin interrupts on RZ/T2H are more like the PORT_IRQn on RZ/V2H,
since the pin is non-selectable (as opposed to "GPIO interrupt, TINTn"
on RZ/V2H, which has selectable pins). Also, on RZ/T2H, IRQ is a separate
function entirely, once you switch a pin to the IRQ function it is no
longer a GPIO.
> And "Error interrupt to CA55" is "icu-error-ca55", but here THE SAME is
> called "ca55-err0"?
>
Same reason as before, I used the naming scheme from the User Manual.
> No, please start using unified naming, not re-inventing this every time.
> Order also is supposed to follow older generation, so bindings share
> common parts.
>
How do you want me to shuffle the order for it to be more like the older
generation?
I chose the current ordering because it matches the User Manual (and it
coincidentally results in an ascending GIC SPI numbering).
Do you want me to put the software interrupts (intcpuN) after the
external pin interrupts (SEI included)?
Eg:
interrupt-names:
items:
- const: irq0
...
- const: irq15
- const: sei
- const: intcpu0
...
- const: intcpu15
- const: ca55-err0
- const: ca55-err1
- const: cr520-err0
- const: cr520-err1
- const: cr521-err0
- const: cr521-err1
- const: peri-err0
- const: peri-err1
- const: dsmif-err0
- const: dsmif-err1
- const: encif-err0
- const: encif-err1
>
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - '#interrupt-cells'
> > + - '#address-cells'
> > + - interrupt-controller
> > + - interrupts
> > + - interrupt-names
> > + - clocks
> > + - power-domains
>
>
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver
2025-11-24 15:28 ` Cosmin-Gabriel Tanislav
@ 2025-11-24 19:01 ` Thomas Gleixner
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2025-11-24 19:01 UTC (permalink / raw)
To: Cosmin-Gabriel Tanislav, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Geert Uytterhoeven, magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
On Mon, Nov 24 2025 at 15:28, Cosmin-Gabriel Tanislav wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>> Sent: Monday, November 24, 2025 3:49 PM
>>
>> On Mon, Nov 24 2025 at 12:50, Cosmin-Gabriel Tanislav wrote:
>> >> From: Thomas Gleixner <tglx@linutronix.de>
>> >> Sent: Saturday, November 22, 2025 5:56 PM
>>
>> Can you please fix your mail-client not to copy the whole header into
>> the reply?
>
> Outlook, it's unfixable. I can remove it manually each time if it's
> too much noise.
Either that or ask your colleagues how they avoid this nonsense.
>> >> > + if (!irq_domain) {
>> >> > + pm_runtime_put(dev);
>> >> > + return -ENOMEM;
>> >> > + }
>> >>
>> >> The mix of 'return $ERR' and 'return dev_err_probe()' is confusing at best.
>> >>
>> >
>> > For ENOMEM, dev_err_probe() doesn't really print anything. ENOMEM is
>> > what other drivers seem to use for a NULL irq_domain_create_hierarchy()
>> > result.
>>
>> That's what I was missing. Now it makes sense.
>>
> In conclusion, should I keep the bare `return -ENOMEM` in both instances?
> Just to make sure the next version is proper.
Keep the -ENOMEM.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
2025-11-24 16:25 ` Cosmin-Gabriel Tanislav
@ 2025-11-27 7:19 ` Krzysztof Kozlowski
2025-11-27 14:44 ` Cosmin-Gabriel Tanislav
0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 7:19 UTC (permalink / raw)
To: Cosmin-Gabriel Tanislav, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
On 24/11/2025 17:25, Cosmin-Gabriel Tanislav wrote:
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Sunday, November 23, 2025 3:24 PM
>>
>> On 21/11/2025 12:14, Cosmin Tanislav wrote:
>>> +properties:
>>> + compatible:
>>> + oneOf:
>>> + - const: renesas,r9a09g077-icu # RZ/T2H
>>> +
>>> + - items:
>>> + - enum:
>>> + - renesas,r9a09g087-icu # RZ/N2H
>>> + - const: renesas,r9a09g077-icu
>>> +
>>> + '#interrupt-cells':
>>> + description: The first cell is the SPI number of the interrupt, as per user
>>> + manual. The second cell is used to specify the flag.
>>> + const: 2
>>> +
>>> + '#address-cells':
>>> + const: 0
>>> +
>>> + interrupt-controller: true
>>> +
>>> + reg:
>>> + items:
>>> + - description: Non-safety registers (INTCPU0-13, IRQ0-13)
>>> + - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
>>
>> reg is always the second property. Please follow DTS coding style.
>>
>
> Ack.
>
>>> +
>>> + interrupts:
>>> + items:
>>> + - description: Software interrupt 0
>>> + - description: Software interrupt 1
>>> + - description: Software interrupt 2
>>> + - description: Software interrupt 3
>>> + - description: Software interrupt 4
>>> + - description: Software interrupt 5
>>> + - description: Software interrupt 6
>>> + - description: Software interrupt 7
>>> + - description: Software interrupt 8
>>> + - description: Software interrupt 9
>>> + - description: Software interrupt 10
>>> + - description: Software interrupt 11
>>> + - description: Software interrupt 12
>>> + - description: Software interrupt 13
>>> + - description: Software interrupt 14
>>> + - description: Software interrupt 15
>>> + - description: External pin interrupt 0
>>> + - description: External pin interrupt 1
>>> + - description: External pin interrupt 2
>>> + - description: External pin interrupt 3
>>> + - description: External pin interrupt 4
>>> + - description: External pin interrupt 5
>>> + - description: External pin interrupt 6
>>> + - description: External pin interrupt 7
>>> + - description: External pin interrupt 8
>>> + - description: External pin interrupt 9
>>> + - description: External pin interrupt 10
>>> + - description: External pin interrupt 11
>>> + - description: External pin interrupt 12
>>> + - description: External pin interrupt 13
>>> + - description: External pin interrupt 14
>>> + - description: External pin interrupt 15
>>> + - description: System error interrupt
>>> + - description: Cortex-A55 error event 0
>>> + - description: Cortex-A55 error event 1
>>> + - description: Cortex-R52 CPU 0 error event 0
>>> + - description: Cortex-R52 CPU 0 error event 1
>>> + - description: Cortex-R52 CPU 1 error event 0
>>> + - description: Cortex-R52 CPU 1 error event 1
>>> + - description: Peripherals error event 0
>>> + - description: Peripherals error event 1
>>> + - description: DSMIF error event 0
>>> + - description: DSMIF error event 1
>>> + - description: ENCIF error event 0
>>> + - description: ENCIF error event 1
>>> +
>>> + interrupt-names:
>>> + items:
>>> + - const: intcpu0
>>> + - const: intcpu1
>>> + - const: intcpu2
>>> + - const: intcpu3
>>> + - const: intcpu4
>>> + - const: intcpu5
>>> + - const: intcpu6
>>> + - const: intcpu7
>>> + - const: intcpu8
>>> + - const: intcpu9
>>> + - const: intcpu10
>>> + - const: intcpu11
>>> + - const: intcpu12
>>> + - const: intcpu13
>>> + - const: intcpu14
>>> + - const: intcpu15
>>> + - const: irq0
>>> + - const: irq1
>>> + - const: irq2
>>> + - const: irq3
>>> + - const: irq4
>>> + - const: irq5
>>> + - const: irq6
>>> + - const: irq7
>>> + - const: irq8
>>> + - const: irq9
>>> + - const: irq10
>>> + - const: irq11
>>> + - const: irq12
>>> + - const: irq13
>>> + - const: irq14
>>> + - const: irq15
>>> + - const: sei
>>> + - const: ca55-err0
>>> + - const: ca55-err1
>>> + - const: cr520-err0
>>> + - const: cr520-err1
>>> + - const: cr521-err0
>>> + - const: cr521-err1
>>> + - const: peri-err0
>>> + - const: peri-err1
>>> + - const: dsmif-err0
>>> + - const: dsmif-err1
>>> + - const: encif-err0
>>> + - const: encif-err1
>>
>> Why all the interrupt names have nothing in common with previous ICU
>> (renesas,rzv2h-icu.yaml)?
>
> Unfortunately, the functionality is different compared to what was
> present on RZ/V2H, hence the different names, descriptions, and order,
> which I've taken straight from the User Manual of the SoC.
>
> If the ICUs were similar, I would have tried to reuse the bindings and
> drivers, but it would have quickly become too complex for what it's
> worth.
>
>> These names are supposed to share, not
>> re-invent every time the name.
>>
>
> Do you think it is worth diverging from the User Manual to bring the
> definition more in line with past SoCs?
>
> The advantage of sticking with the User Manual naming scheme is that
> you can easily cross-reference these descriptions with the User Manual
> and find what you need, whereas "PORT_IRQ0" / "GPIO interrupt" would
> give you no information for RZ/T2H.
User manuals come with all sorts of different namings, so following it
blindly would made impossible to make any common parts of bindings.
New devices should in general build on top of old ones, so lists like
that can be shared. Of course how much can be shared here is different
question, but person reading previous binding should be able to find
similar things named similarly.
>
>> Isn't external interrupt the same as GPIO interrupt? How do they differ
>> for this particular device?
>>
>
> External pin interrupts on RZ/T2H are more like the PORT_IRQn on RZ/V2H,
> since the pin is non-selectable (as opposed to "GPIO interrupt, TINTn"
> on RZ/V2H, which has selectable pins). Also, on RZ/T2H, IRQ is a separate
> function entirely, once you switch a pin to the IRQ function it is no
> longer a GPIO.
>
>> And "Error interrupt to CA55" is "icu-error-ca55", but here THE SAME is
>> called "ca55-err0"?
>>
>
> Same reason as before, I used the naming scheme from the User Manual.
>
>> No, please start using unified naming, not re-inventing this every time.
>> Order also is supposed to follow older generation, so bindings share
>> common parts.
>>
>
> How do you want me to shuffle the order for it to be more like the older
> generation?
>
> I chose the current ordering because it matches the User Manual (and it
> coincidentally results in an ascending GIC SPI numbering).
>
> Do you want me to put the software interrupts (intcpuN) after the
> external pin interrupts (SEI included)?
>
> Eg:
> interrupt-names:
> items:
> - const: irq0
> ...
> - const: irq15
> - const: sei
> - const: intcpu0
> ...
> - const: intcpu15
> - const: ca55-err0
> - const: ca55-err1
> - const: cr520-err0
> - const: cr520-err1
> - const: cr521-err0
> - const: cr521-err1
> - const: peri-err0
> - const: peri-err1
> - const: dsmif-err0
> - const: dsmif-err1
> - const: encif-err0
> - const: encif-err1
If only the "err" interrupts are similar, then the order indeed does not
matter much, because nothing is in common.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU
2025-11-27 7:19 ` Krzysztof Kozlowski
@ 2025-11-27 14:44 ` Cosmin-Gabriel Tanislav
0 siblings, 0 replies; 14+ messages in thread
From: Cosmin-Gabriel Tanislav @ 2025-11-27 14:44 UTC (permalink / raw)
To: Krzysztof Kozlowski, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
magnus.damm
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, November 27, 2025 9:19 AM
>
> On 24/11/2025 17:25, Cosmin-Gabriel Tanislav wrote:
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Sunday, November 23, 2025 3:24 PM
> >>
> >> On 21/11/2025 12:14, Cosmin Tanislav wrote:
> >>> +properties:
> >>> + compatible:
> >>> + oneOf:
> >>> + - const: renesas,r9a09g077-icu # RZ/T2H
> >>> +
> >>> + - items:
> >>> + - enum:
> >>> + - renesas,r9a09g087-icu # RZ/N2H
> >>> + - const: renesas,r9a09g077-icu
> >>> +
> >>> + '#interrupt-cells':
> >>> + description: The first cell is the SPI number of the interrupt, as per user
> >>> + manual. The second cell is used to specify the flag.
> >>> + const: 2
> >>> +
> >>> + '#address-cells':
> >>> + const: 0
> >>> +
> >>> + interrupt-controller: true
> >>> +
> >>> + reg:
> >>> + items:
> >>> + - description: Non-safety registers (INTCPU0-13, IRQ0-13)
> >>> + - description: Safety registers (INTCPU14-15, IRQ14-15, SEI)
> >>
> >> reg is always the second property. Please follow DTS coding style.
> >>
> >
> > Ack.
> >
> >>> +
> >>> + interrupts:
> >>> + items:
> >>> + - description: Software interrupt 0
> >>> + - description: Software interrupt 1
> >>> + - description: Software interrupt 2
> >>> + - description: Software interrupt 3
> >>> + - description: Software interrupt 4
> >>> + - description: Software interrupt 5
> >>> + - description: Software interrupt 6
> >>> + - description: Software interrupt 7
> >>> + - description: Software interrupt 8
> >>> + - description: Software interrupt 9
> >>> + - description: Software interrupt 10
> >>> + - description: Software interrupt 11
> >>> + - description: Software interrupt 12
> >>> + - description: Software interrupt 13
> >>> + - description: Software interrupt 14
> >>> + - description: Software interrupt 15
> >>> + - description: External pin interrupt 0
> >>> + - description: External pin interrupt 1
> >>> + - description: External pin interrupt 2
> >>> + - description: External pin interrupt 3
> >>> + - description: External pin interrupt 4
> >>> + - description: External pin interrupt 5
> >>> + - description: External pin interrupt 6
> >>> + - description: External pin interrupt 7
> >>> + - description: External pin interrupt 8
> >>> + - description: External pin interrupt 9
> >>> + - description: External pin interrupt 10
> >>> + - description: External pin interrupt 11
> >>> + - description: External pin interrupt 12
> >>> + - description: External pin interrupt 13
> >>> + - description: External pin interrupt 14
> >>> + - description: External pin interrupt 15
> >>> + - description: System error interrupt
> >>> + - description: Cortex-A55 error event 0
> >>> + - description: Cortex-A55 error event 1
> >>> + - description: Cortex-R52 CPU 0 error event 0
> >>> + - description: Cortex-R52 CPU 0 error event 1
> >>> + - description: Cortex-R52 CPU 1 error event 0
> >>> + - description: Cortex-R52 CPU 1 error event 1
> >>> + - description: Peripherals error event 0
> >>> + - description: Peripherals error event 1
> >>> + - description: DSMIF error event 0
> >>> + - description: DSMIF error event 1
> >>> + - description: ENCIF error event 0
> >>> + - description: ENCIF error event 1
> >>> +
> >>> + interrupt-names:
> >>> + items:
> >>> + - const: intcpu0
> >>> + - const: intcpu1
> >>> + - const: intcpu2
> >>> + - const: intcpu3
> >>> + - const: intcpu4
> >>> + - const: intcpu5
> >>> + - const: intcpu6
> >>> + - const: intcpu7
> >>> + - const: intcpu8
> >>> + - const: intcpu9
> >>> + - const: intcpu10
> >>> + - const: intcpu11
> >>> + - const: intcpu12
> >>> + - const: intcpu13
> >>> + - const: intcpu14
> >>> + - const: intcpu15
> >>> + - const: irq0
> >>> + - const: irq1
> >>> + - const: irq2
> >>> + - const: irq3
> >>> + - const: irq4
> >>> + - const: irq5
> >>> + - const: irq6
> >>> + - const: irq7
> >>> + - const: irq8
> >>> + - const: irq9
> >>> + - const: irq10
> >>> + - const: irq11
> >>> + - const: irq12
> >>> + - const: irq13
> >>> + - const: irq14
> >>> + - const: irq15
> >>> + - const: sei
> >>> + - const: ca55-err0
> >>> + - const: ca55-err1
> >>> + - const: cr520-err0
> >>> + - const: cr520-err1
> >>> + - const: cr521-err0
> >>> + - const: cr521-err1
> >>> + - const: peri-err0
> >>> + - const: peri-err1
> >>> + - const: dsmif-err0
> >>> + - const: dsmif-err1
> >>> + - const: encif-err0
> >>> + - const: encif-err1
> >>
> >> Why all the interrupt names have nothing in common with previous ICU
> >> (renesas,rzv2h-icu.yaml)?
> >
> > Unfortunately, the functionality is different compared to what was
> > present on RZ/V2H, hence the different names, descriptions, and order,
> > which I've taken straight from the User Manual of the SoC.
> >
> > If the ICUs were similar, I would have tried to reuse the bindings and
> > drivers, but it would have quickly become too complex for what it's
> > worth.
> >
> >> These names are supposed to share, not
> >> re-invent every time the name.
> >>
> >
> > Do you think it is worth diverging from the User Manual to bring the
> > definition more in line with past SoCs?
> >
> > The advantage of sticking with the User Manual naming scheme is that
> > you can easily cross-reference these descriptions with the User Manual
> > and find what you need, whereas "PORT_IRQ0" / "GPIO interrupt" would
> > give you no information for RZ/T2H.
>
> User manuals come with all sorts of different namings, so following it
> blindly would made impossible to make any common parts of bindings.
>
True, and it makes sense to share them when the blocks are similar and
even more so if they can be made part of the same binding, but it's not
the case here.
> New devices should in general build on top of old ones, so lists like
> that can be shared. Of course how much can be shared here is different
> question, but person reading previous binding should be able to find
> similar things named similarly.
>
In this case, I would keep the naming scheme different, as the ones that
are similarly named on RZ/V2H are different in functionality, it would
only cause confusion if we were to use the RZ/V2H naming scheme.
> >
> >> Isn't external interrupt the same as GPIO interrupt? How do they differ
> >> for this particular device?
> >>
> >
> > External pin interrupts on RZ/T2H are more like the PORT_IRQn on RZ/V2H,
> > since the pin is non-selectable (as opposed to "GPIO interrupt, TINTn"
> > on RZ/V2H, which has selectable pins). Also, on RZ/T2H, IRQ is a separate
> > function entirely, once you switch a pin to the IRQ function it is no
> > longer a GPIO.
> >
> >> And "Error interrupt to CA55" is "icu-error-ca55", but here THE SAME is
> >> called "ca55-err0"?
> >>
> >
> > Same reason as before, I used the naming scheme from the User Manual.
> >
> >> No, please start using unified naming, not re-inventing this every time.
> >> Order also is supposed to follow older generation, so bindings share
> >> common parts.
> >>
> >
> > How do you want me to shuffle the order for it to be more like the older
> > generation?
> >
> > I chose the current ordering because it matches the User Manual (and it
> > coincidentally results in an ascending GIC SPI numbering).
> >
> > Do you want me to put the software interrupts (intcpuN) after the
> > external pin interrupts (SEI included)?
> >
> > Eg:
> > interrupt-names:
> > items:
> > - const: irq0
> > ...
> > - const: irq15
> > - const: sei
> > - const: intcpu0
> > ...
> > - const: intcpu15
> > - const: ca55-err0
> > - const: ca55-err1
> > - const: cr520-err0
> > - const: cr520-err1
> > - const: cr521-err0
> > - const: cr521-err1
> > - const: peri-err0
> > - const: peri-err1
> > - const: dsmif-err0
> > - const: dsmif-err1
> > - const: encif-err0
> > - const: encif-err1
>
> If only the "err" interrupts are similar, then the order indeed does not
> matter much, because nothing is in common.
>
Only the ca55-err0 & ca55-err1 interrupts are similar to icu-error-ca55,
but as you can see RZ/T2H has two of them. I think it is better to keep
the User Manual naming and order here. irqN and intcpuN also match up
nicely with the underlying GIC SPI numbering if we keep the original
order.
Maybe other people can chime in about what they think too, and we can
reach a consensus.
>
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-11-27 14:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-21 11:14 [PATCH 0/4] Add ICU support for RZ/T2H and RZ/N2H Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 1/4] dt-bindings: interrupt-controller: document RZ/{T2H,N2H} ICU Cosmin Tanislav
2025-11-23 13:23 ` Krzysztof Kozlowski
2025-11-24 16:25 ` Cosmin-Gabriel Tanislav
2025-11-27 7:19 ` Krzysztof Kozlowski
2025-11-27 14:44 ` Cosmin-Gabriel Tanislav
2025-11-21 11:14 ` [PATCH 2/4] irqchip: add RZ/{T2H,N2H} Interrupt Controller (ICU) driver Cosmin Tanislav
2025-11-22 15:55 ` Thomas Gleixner
2025-11-24 12:50 ` Cosmin-Gabriel Tanislav
2025-11-24 13:49 ` Thomas Gleixner
2025-11-24 15:28 ` Cosmin-Gabriel Tanislav
2025-11-24 19:01 ` Thomas Gleixner
2025-11-21 11:14 ` [PATCH 3/4] arm64: dts: renesas: r9a09g077: add ICU support Cosmin Tanislav
2025-11-21 11:14 ` [PATCH 4/4] arm64: dts: renesas: r9a09g087: " Cosmin Tanislav
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).