* [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit
@ 2024-09-17 17:32 Fabrizio Castro
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Michael Turquette, Stephen Boyd, Philipp Zabel,
Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, Lad Prabhakar, Marc Zyngier,
linux-kernel, devicetree, linux-renesas-soc, linux-gpio,
linux-clk, Chris Paterson, Biju Das
Dear All,
This series adds whatever is required for supporting NMI, IRQ, and
TINT interrupts to the Renesas RZ/V2H SoC.
Thanks,
Fab
Fabrizio Castro (6):
dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
pinctrl: renesas: rzg2l: Remove RZG2L_TINT_IRQ_START_INDEX
dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt
Controller
clk: renesas: r9a09g057: Add clock and reset entries for ICU
irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver
arm64: dts: renesas: r9a09g057: Add ICU node
.../renesas,rzv2h-icu.yaml | 278 ++++++++++
.../pinctrl/renesas,rzg2l-pinctrl.yaml | 4 +
arch/arm64/boot/dts/renesas/r9a09g057.dtsi | 88 +++
drivers/clk/renesas/r9a09g057-cpg.c | 2 +
drivers/irqchip/Kconfig | 7 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzv2h.c | 517 ++++++++++++++++++
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 8 +-
drivers/soc/renesas/Kconfig | 1 +
.../interrupt-controller/icu-rzv2h.h | 48 ++
10 files changed, 952 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
create mode 100644 drivers/irqchip/irq-renesas-rzv2h.c
create mode 100644 include/dt-bindings/interrupt-controller/icu-rzv2h.h
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
2024-09-17 18:31 ` Rob Herring (Arm)
2024-09-17 22:14 ` Rob Herring
2024-09-17 17:32 ` [PATCH 2/6] pinctrl: renesas: rzg2l: Remove RZG2L_TINT_IRQ_START_INDEX Fabrizio Castro
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, Lad Prabhakar, Marc Zyngier,
linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Chris Paterson, Biju Das
All the platforms from the renesas,rzg2l-pinctrl.yaml binding
actually require the interrupt-parent property. Add it.
Fixes: 35c37efd1273 ("dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Document the properties to handle GPIO IRQ")
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
.../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
index 56d90c8e1fa3..10f724e87ae7 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
@@ -51,6 +51,8 @@ properties:
interrupt-controller: true
+ interrupt-parent: true
+
'#interrupt-cells':
const: 2
description:
@@ -155,6 +157,7 @@ required:
- '#gpio-cells'
- gpio-ranges
- interrupt-controller
+ - interrupt-parent
- '#interrupt-cells'
- clocks
- power-domains
@@ -174,6 +177,7 @@ examples:
gpio-ranges = <&pinctrl 0 0 392>;
interrupt-controller;
#interrupt-cells = <2>;
+ interrupt-parent = <&irqc>;
clocks = <&cpg CPG_MOD R9A07G044_GPIO_HCLK>;
resets = <&cpg R9A07G044_GPIO_RSTN>,
<&cpg R9A07G044_GPIO_PORT_RESETN>,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/6] pinctrl: renesas: rzg2l: Remove RZG2L_TINT_IRQ_START_INDEX
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
2024-09-17 17:32 ` [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller Fabrizio Castro
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Linus Walleij, Geert Uytterhoeven
Cc: Fabrizio Castro, linux-renesas-soc, linux-gpio, linux-kernel,
Chris Paterson, Biju Das, Lad Prabhakar
The RZ/V2H(P) has 16 IRQ interrupts, while every other platforms
has 8, and this affects the start index of TINT interrupts
(1 + 16 = 17, rather than 1 + 8 = 9).
Macro RZG2L_TINT_IRQ_START_INDEX cannot work anymore, replace
it with a new member within struct rzg2l_hwcfg.
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 5a403915fed2..0aba75dce229 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -168,7 +168,6 @@
#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_PINS_PER_PORT)
#define RZG2L_TINT_MAX_INTERRUPT 32
-#define RZG2L_TINT_IRQ_START_INDEX 9
#define RZG2L_PACK_HWIRQ(t, i) (((t) << 16) | (i))
/* Custom pinconf parameters */
@@ -251,6 +250,7 @@ enum rzg2l_iolh_index {
* @func_base: base number for port function (see register PFC)
* @oen_max_pin: the maximum pin number supporting output enable
* @oen_max_port: the maximum port number supporting output enable
+ * @tint_start_index: the start index for the TINT interrupts
*/
struct rzg2l_hwcfg {
const struct rzg2l_register_offsets regs;
@@ -262,6 +262,7 @@ struct rzg2l_hwcfg {
u8 func_base;
u8 oen_max_pin;
u8 oen_max_port;
+ unsigned int tint_start_index;
};
struct rzg2l_dedicated_configs {
@@ -2379,7 +2380,7 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
rzg2l_gpio_irq_endisable(pctrl, child, true);
pctrl->hwirq[irq] = child;
- irq += RZG2L_TINT_IRQ_START_INDEX;
+ irq += pctrl->data->hwcfg->tint_start_index;
/* All these interrupts are level high in the CPU */
*parent_type = IRQ_TYPE_LEVEL_HIGH;
@@ -3035,6 +3036,7 @@ static const struct rzg2l_hwcfg rzg2l_hwcfg = {
},
.iolh_groupb_oi = { 100, 66, 50, 33, },
.oen_max_pin = 0,
+ .tint_start_index = 9,
};
static const struct rzg2l_hwcfg rzg3s_hwcfg = {
@@ -3067,12 +3069,14 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = {
.func_base = 1,
.oen_max_pin = 1, /* Pin 1 of P0 and P7 is the maximum OEN pin. */
.oen_max_port = 7, /* P7_1 is the maximum OEN port. */
+ .tint_start_index = 9,
};
static const struct rzg2l_hwcfg rzv2h_hwcfg = {
.regs = {
.pwpr = 0x3c04,
},
+ .tint_start_index = 17,
};
static struct rzg2l_pinctrl_data r9a07g043_data = {
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
2024-09-17 17:32 ` [PATCH 2/6] pinctrl: renesas: rzg2l: Remove RZG2L_TINT_IRQ_START_INDEX Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
2024-09-18 17:28 ` Rob Herring
2024-09-17 17:32 ` [PATCH 4/6] clk: renesas: r9a09g057: Add clock and reset entries for ICU Fabrizio Castro
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, linux-kernel, devicetree,
linux-renesas-soc, Chris Paterson, Biju Das, Lad Prabhakar
Add DT bindings for the Renesas RZ/V2H(P) Interrupt Controller.
Also add macros for the NMI and IRQ0-15 interrupts which map the
SPI0-16 interrupts on the RZ/V2H(P) SoC so that they can be
used in the first cell of the interrupt specifiers.
For the second cell of the interrupt specifier, since NMI, IRQn
and TINTn support different types of interrupts between themselves,
add helper macros to make it easier for the user to work out what's
available.
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
.../renesas,rzv2h-icu.yaml | 278 ++++++++++++++++++
.../interrupt-controller/icu-rzv2h.h | 48 +++
2 files changed, 326 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
create mode 100644 include/dt-bindings/interrupt-controller/icu-rzv2h.h
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
new file mode 100644
index 000000000000..28f5b2f30c31
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
@@ -0,0 +1,278 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzv2h-icu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2H(P) Interrupt Control Unit
+
+maintainers:
+ - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+ - Geert Uytterhoeven <geert+renesas@glider.be>
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+description: |
+ The Interrupt Control Unit (ICU) handles external interrupts (NMI, IRQ, and
+ TINT), error interrupts, DMAC requests, GPT interrupts, and internal
+ interrupts.
+ It notifies GIC of the relevant interrupts.
+
+properties:
+ compatible:
+ const: renesas,r9a09g057-icu # RZ/V2H(P)
+
+ '#interrupt-cells':
+ description: The first cell should contain a macro RZV2H_{NMI,IRQn} from
+ file include/dt-bindings/interrupt-controller/icu-rzv2h.h,
+ and the second cell is used to specify the flag. Helper macros
+ for the second cell can also be found in the same header file.
+ const: 2
+
+ '#address-cells':
+ const: 0
+
+ interrupt-controller: true
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 58
+ items:
+ - description: NMI interrupt
+ - description: IRQ0 interrupt
+ - description: IRQ1 interrupt
+ - description: IRQ2 interrupt
+ - description: IRQ3 interrupt
+ - description: IRQ4 interrupt
+ - description: IRQ5 interrupt
+ - description: IRQ6 interrupt
+ - description: IRQ7 interrupt
+ - description: IRQ8 interrupt
+ - description: IRQ9 interrupt
+ - description: IRQ10 interrupt
+ - description: IRQ11 interrupt
+ - description: IRQ12 interrupt
+ - description: IRQ13 interrupt
+ - description: IRQ14 interrupt
+ - description: IRQ15 interrupt
+ - description: GPIO interrupt, TINT0
+ - description: GPIO interrupt, TINT1
+ - description: GPIO interrupt, TINT2
+ - description: GPIO interrupt, TINT3
+ - description: GPIO interrupt, TINT4
+ - description: GPIO interrupt, TINT5
+ - description: GPIO interrupt, TINT6
+ - description: GPIO interrupt, TINT7
+ - description: GPIO interrupt, TINT8
+ - description: GPIO interrupt, TINT9
+ - description: GPIO interrupt, TINT10
+ - description: GPIO interrupt, TINT11
+ - description: GPIO interrupt, TINT12
+ - description: GPIO interrupt, TINT13
+ - description: GPIO interrupt, TINT14
+ - description: GPIO interrupt, TINT15
+ - description: GPIO interrupt, TINT16
+ - description: GPIO interrupt, TINT17
+ - description: GPIO interrupt, TINT18
+ - description: GPIO interrupt, TINT19
+ - description: GPIO interrupt, TINT20
+ - description: GPIO interrupt, TINT21
+ - description: GPIO interrupt, TINT22
+ - description: GPIO interrupt, TINT23
+ - description: GPIO interrupt, TINT24
+ - description: GPIO interrupt, TINT25
+ - description: GPIO interrupt, TINT26
+ - description: GPIO interrupt, TINT27
+ - description: GPIO interrupt, TINT28
+ - description: GPIO interrupt, TINT29
+ - description: GPIO interrupt, TINT30
+ - description: GPIO interrupt, TINT31
+ - description: Software interrupt, INTA55_0
+ - description: Software interrupt, INTA55_1
+ - description: Software interrupt, INTA55_2
+ - description: Software interrupt, INTA55_3
+ - description: Error interrupt to CA55
+ - description: GTCCRA compare match/input capture (U0)
+ - description: GTCCRB compare match/input capture (U0)
+ - description: GTCCRA compare match/input capture (U1)
+ - description: GTCCRB compare match/input capture (U1)
+
+ interrupt-names:
+ minItems: 58
+ items:
+ - const: nmi
+ - const: irq0
+ - const: irq1
+ - const: irq2
+ - const: irq3
+ - const: irq4
+ - const: irq5
+ - const: irq6
+ - const: irq7
+ - const: irq8
+ - const: irq9
+ - const: irq10
+ - const: irq11
+ - const: irq12
+ - const: irq13
+ - const: irq14
+ - const: irq15
+ - const: tint0
+ - const: tint1
+ - const: tint2
+ - const: tint3
+ - const: tint4
+ - const: tint5
+ - const: tint6
+ - const: tint7
+ - const: tint8
+ - const: tint9
+ - const: tint10
+ - const: tint11
+ - const: tint12
+ - const: tint13
+ - const: tint14
+ - const: tint15
+ - const: tint16
+ - const: tint17
+ - const: tint18
+ - const: tint19
+ - const: tint20
+ - const: tint21
+ - const: tint22
+ - const: tint23
+ - const: tint24
+ - const: tint25
+ - const: tint26
+ - const: tint27
+ - const: tint28
+ - const: tint29
+ - const: tint30
+ - const: tint31
+ - const: int-ca55-0
+ - const: int-ca55-1
+ - const: int-ca55-2
+ - const: int-ca55-3
+ - const: icu-error-ca55
+ - const: gpt-u0-gtciada
+ - const: gpt-u0-gtciadb
+ - const: gpt-u1-gtciada
+ - const: gpt-u1-gtciadb
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - '#interrupt-cells'
+ - '#address-cells'
+ - interrupt-controller
+ - interrupts
+ - interrupt-names
+ - clocks
+ - power-domains
+ - resets
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+ icu: interrupt-controller@10400000 {
+ compatible = "renesas,r9a09g057-icu";
+ reg = <0x10400000 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 9 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 427 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 430 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 432 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 443 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 262 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 266 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>;
+ interrupt-names = "nmi",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "irq8", "irq9", "irq10", "irq11",
+ "irq12", "irq13", "irq14", "irq15",
+ "tint0", "tint1", "tint2", "tint3",
+ "tint4", "tint5", "tint6", "tint7",
+ "tint8", "tint9", "tint10", "tint11",
+ "tint12", "tint13", "tint14", "tint15",
+ "tint16", "tint17", "tint18", "tint19",
+ "tint20", "tint21", "tint22", "tint23",
+ "tint24", "tint25", "tint26", "tint27",
+ "tint28", "tint29", "tint30", "tint31",
+ "int-ca55-0", "int-ca55-1",
+ "int-ca55-2", "int-ca55-3",
+ "icu-error-ca55",
+ "gpt-u0-gtciada", "gpt-u0-gtciadb",
+ "gpt-u1-gtciada", "gpt-u1-gtciadb";
+ clocks = <&cpg CPG_MOD 0x5>;
+ power-domains = <&cpg>;
+ resets = <&cpg 0x36>;
+ };
diff --git a/include/dt-bindings/interrupt-controller/icu-rzv2h.h b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
new file mode 100644
index 000000000000..5d1e7bb256cd
--- /dev/null
+++ b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * This header provides constants for Renesas RZ/V2H(P) SoC ICU bindings.
+ *
+ * Copyright (C) 2024 Renesas Electronics Corp.
+ *
+ */
+
+#ifndef __DT_BINDINGS_ICU_RZV2H_H
+#define __DT_BINDINGS_ICU_RZV2H_H
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/* NMI maps to SPI0 */
+#define RZV2H_NMI 0
+
+/* IRQ0-15 map to SPI1-16 */
+#define RZV2H_IRQ0 1
+#define RZV2H_IRQ1 2
+#define RZV2H_IRQ2 3
+#define RZV2H_IRQ3 4
+#define RZV2H_IRQ4 5
+#define RZV2H_IRQ5 6
+#define RZV2H_IRQ6 7
+#define RZV2H_IRQ7 8
+#define RZV2H_IRQ8 9
+#define RZV2H_IRQ9 10
+#define RZV2H_IRQ10 11
+#define RZV2H_IRQ11 12
+#define RZV2H_IRQ12 13
+#define RZV2H_IRQ13 14
+#define RZV2H_IRQ14 15
+#define RZV2H_IRQ15 16
+
+#define RZV2H_NMI_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
+#define RZV2H_NMI_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
+
+#define RZV2H_IRQ_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
+#define RZV2H_IRQ_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
+#define RZV2H_IRQ_TYPE_EDGE_BOTH IRQ_TYPE_EDGE_BOTH
+#define RZV2H_IRQ_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
+
+#define RZV2H_TINT_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
+#define RZV2H_TINT_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
+#define RZV2H_TINT_TYPE_LEVEL_HIGH IRQ_TYPE_LEVEL_HIGH
+#define RZV2H_TINT_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
+
+#endif /* __DT_BINDINGS_ICU_RZV2H_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/6] clk: renesas: r9a09g057: Add clock and reset entries for ICU
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
` (2 preceding siblings ...)
2024-09-17 17:32 ` [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
2024-09-17 17:32 ` [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver Fabrizio Castro
2024-09-17 17:32 ` [PATCH 6/6] arm64: dts: renesas: r9a09g057: Add ICU node Fabrizio Castro
5 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Geert Uytterhoeven
Cc: Fabrizio Castro, linux-renesas-soc, linux-clk, linux-kernel,
Chris Paterson, Biju Das, Lad Prabhakar
Add clock and reset entries for the Renesas RZ/V2H(P) ICU IP block.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
drivers/clk/renesas/r9a09g057-cpg.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/renesas/r9a09g057-cpg.c b/drivers/clk/renesas/r9a09g057-cpg.c
index 3ee32db5c0af..b82fee006d65 100644
--- a/drivers/clk/renesas/r9a09g057-cpg.c
+++ b/drivers/clk/renesas/r9a09g057-cpg.c
@@ -78,6 +78,7 @@ static const struct cpg_core_clk r9a09g057_core_clks[] __initconst = {
};
static const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = {
+ DEF_MOD_CRITICAL("icu_0_pclk_i", CLK_PLLCM33_DIV16, 0, 5, 0, 5),
DEF_MOD("gtm_0_pclk", CLK_PLLCM33_DIV16, 4, 3, 2, 3),
DEF_MOD("gtm_1_pclk", CLK_PLLCM33_DIV16, 4, 4, 2, 4),
DEF_MOD("gtm_2_pclk", CLK_PLLCLN_DIV16, 4, 5, 2, 5),
@@ -119,6 +120,7 @@ static const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = {
};
static const struct rzv2h_reset r9a09g057_resets[] __initconst = {
+ DEF_RST(3, 6, 1, 7), /* ICU_0_PRESETN_I */
DEF_RST(6, 13, 2, 30), /* GTM_0_PRESETZ */
DEF_RST(6, 14, 2, 31), /* GTM_1_PRESETZ */
DEF_RST(6, 15, 3, 0), /* GTM_2_PRESETZ */
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
` (3 preceding siblings ...)
2024-09-17 17:32 ` [PATCH 4/6] clk: renesas: r9a09g057: Add clock and reset entries for ICU Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
2024-10-02 10:51 ` Thomas Gleixner
2024-09-17 17:32 ` [PATCH 6/6] arm64: dts: renesas: r9a09g057: Add ICU node Fabrizio Castro
5 siblings, 1 reply; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Thomas Gleixner, Philipp Zabel, Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, linux-kernel, linux-renesas-soc,
Chris Paterson, Biju Das, Lad Prabhakar
Add driver for the Renesas RZ/V2H(P) Interrupt Control Unit (ICU).
This driver supports the external interrupts NMI, IRQn, and TINTn.
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
drivers/irqchip/Kconfig | 7 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzv2h.c | 517 ++++++++++++++++++++++++++++
drivers/soc/renesas/Kconfig | 1 +
4 files changed, 526 insertions(+)
create mode 100644 drivers/irqchip/irq-renesas-rzv2h.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 341cd9ca5a05..006a47a86ed5 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -265,6 +265,13 @@ config RENESAS_RZG2L_IRQC
Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
for external devices.
+config RENESAS_RZV2H_ICU
+ bool "Renesas RZ/V2H(P) ICU support" if COMPILE_TEST
+ select GENERIC_IRQ_CHIP
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Enable support for the Renesas RZ/V2H(P) Interrupt Control Unit (ICU)
+
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 e3679ec2b9f7..94ecaebff37f 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -51,6 +51,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_RZV2H_ICU) += irq-renesas-rzv2h.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-rzv2h.c b/drivers/irqchip/irq-renesas-rzv2h.c
new file mode 100644
index 000000000000..07a6ac20565c
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-rzv2h.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/V2H(P) ICU Driver
+ *
+ * Based on irq-renesas-rzg2l.c
+ *
+ * Copyright (C) 2024 Renesas Electronics Corporation.
+ *
+ * Author: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
+ */
+
+#include <linux/bitfield.h>
+#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/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+#include <linux/syscore_ops.h>
+
+/* DT "interrupts" indexes */
+#define ICU_IRQ_START 1
+#define ICU_IRQ_COUNT 16
+#define ICU_TINT_START (ICU_IRQ_START + ICU_IRQ_COUNT)
+#define ICU_TINT_COUNT 32
+#define ICU_NUM_IRQ (ICU_TINT_START + ICU_TINT_COUNT)
+
+/* Registers */
+#define ICU_NSCNT 0x00
+#define ICU_NSCLR 0x04
+#define ICU_NITSR 0x08
+#define ICU_ISCTR 0x10
+#define ICU_ISCLR 0x14
+#define ICU_IITSR 0x18
+#define ICU_TSCTR 0x20
+#define ICU_TSCLR 0x24
+#define ICU_TITSR(k) (0x28 + (k) * 4)
+#define ICU_TSSR(k) (0x30 + (k) * 4)
+
+/* NMI */
+#define ICU_NMI_EDGE_FALLING 0
+#define ICU_NMI_EDGE_RISING 1
+
+#define ICU_NSCNT_NSTAT BIT(0)
+#define ICU_NSCNT_NSTAT_DETECTED 1
+
+#define ICU_NSCLR_NCLR BIT(0)
+
+/* IRQ */
+#define ICU_IRQ_LEVEL_LOW 0
+#define ICU_IRQ_EDGE_FALLING 1
+#define ICU_IRQ_EDGE_RISING 2
+#define ICU_IRQ_EDGE_BOTH 3
+
+#define ICU_IITSR_IITSEL_PREP(iitsel, n) ((iitsel) << ((n) * 2))
+#define ICU_IITSR_IITSEL_GET(iitsr, n) (((iitsr) >> ((n) * 2)) & 0x03)
+#define ICU_IITSR_IITSEL_MASK(n) ICU_IITSR_IITSEL_PREP(0x03, n)
+
+/* TINT */
+#define ICU_TINT_EDGE_RISING 0
+#define ICU_TINT_EDGE_FALLING 1
+#define ICU_TINT_LEVEL_HIGH 2
+#define ICU_TINT_LEVEL_LOW 3
+
+#define ICU_TSSR_K(tint_nr) ((tint_nr) / 4)
+#define ICU_TSSR_TSSEL_N(tint_nr) ((tint_nr) % 4)
+#define ICU_TSSR_TSSEL_PREP(tssel, n) ((tssel) << ((n) * 8))
+#define ICU_TSSR_TSSEL_MASK(n) ICU_TSSR_TSSEL_PREP(0x7F, n)
+#define ICU_TSSR_TIEN(n) (BIT(7) << ((n) * 8))
+
+#define ICU_TITSR_K(tint_nr) ((tint_nr) / 16)
+#define ICU_TITSR_TITSEL_N(tint_nr) ((tint_nr) % 16)
+#define ICU_TITSR_TITSEL_PREP(titsel, n) ICU_IITSR_IITSEL_PREP(titsel, n)
+#define ICU_TITSR_TITSEL_MASK(n) ICU_IITSR_IITSEL_MASK(n)
+#define ICU_TITSR_TITSEL_GET(titsr, n) ICU_IITSR_IITSEL_GET(titsr, n)
+
+#define ICU_TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
+#define ICU_TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
+#define ICU_PB5_TINT 0x55
+
+/**
+ * struct rzv2h_icu_priv - Interrupt Control Unit controller private data
+ * structure.
+ * @base: Controller's base address
+ * @irqchip: Pointer to struct irq_chip
+ * @fwspec: IRQ firmware specific data
+ * @lock: Lock to serialize access to hardware registers
+ */
+struct rzv2h_icu_priv {
+ void __iomem *base;
+ const struct irq_chip *irqchip;
+ struct irq_fwspec fwspec[ICU_NUM_IRQ];
+ raw_spinlock_t lock;
+};
+
+static inline struct rzv2h_icu_priv *irq_data_to_priv(struct irq_data *data)
+{
+ return data->domain->host_data;
+}
+
+static void rzv2h_clear_nmi_int(struct rzv2h_icu_priv *priv)
+{
+ u32 nscnt = readl_relaxed(priv->base + ICU_NSCNT);
+
+ if ((nscnt & ICU_NSCNT_NSTAT) == ICU_NSCNT_NSTAT_DETECTED)
+ writel_relaxed(ICU_NSCLR_NCLR, priv->base + ICU_NSCLR);
+}
+
+static void rzv2h_clear_irq_int(struct rzv2h_icu_priv *priv, unsigned int hwirq)
+{
+ unsigned int irq_nr = hwirq - ICU_IRQ_START;
+ u32 isctr, iitsr, iitsel;
+ u32 bit = BIT(irq_nr);
+
+ isctr = readl_relaxed(priv->base + ICU_ISCTR);
+ iitsr = readl_relaxed(priv->base + ICU_IITSR);
+ iitsel = ICU_IITSR_IITSEL_GET(iitsr, irq_nr);
+
+ /*
+ * When level sensing is used, the interrupt flag gets automatically
+ * cleared when the interrupt signal is de-asserted by the source of
+ * the interrupt request, therefore clear the interrupt only for edge
+ * triggered interrupts.
+ */
+ if ((isctr & bit) && (iitsel != ICU_IRQ_LEVEL_LOW))
+ writel_relaxed(bit, priv->base + ICU_ISCLR);
+}
+
+static void rzv2h_clear_tint_int(struct rzv2h_icu_priv *priv,
+ unsigned int hwirq)
+{
+ unsigned int tint_nr = hwirq - ICU_TINT_START;
+ int titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
+ u32 tsctr, titsr, titsel;
+ u32 bit = BIT(tint_nr);
+ int k = tint_nr / 16;
+
+ tsctr = readl_relaxed(priv->base + ICU_TSCTR);
+ titsr = readl_relaxed(priv->base + ICU_TITSR(k));
+ titsel = ICU_TITSR_TITSEL_GET(titsr, titsel_n);
+
+ /*
+ * Writing 1 to the corresponding flag from register ICU_TSCTR only
+ * has effect if TSTATn = 1b and if it's a rising edge or a falling
+ * edge interrupt.
+ */
+ if ((tsctr & bit) && ((titsel == ICU_TINT_EDGE_RISING) ||
+ (titsel == ICU_TINT_EDGE_FALLING)))
+ writel_relaxed(bit, priv->base + ICU_TSCLR);
+}
+
+static void rzv2h_icu_eoi(struct irq_data *d)
+{
+ struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
+ unsigned int hw_irq = irqd_to_hwirq(d);
+
+ raw_spin_lock(&priv->lock);
+ if (hw_irq >= ICU_TINT_START)
+ rzv2h_clear_tint_int(priv, hw_irq);
+ else if (hw_irq >= ICU_IRQ_START)
+ rzv2h_clear_irq_int(priv, hw_irq);
+ else
+ rzv2h_clear_nmi_int(priv);
+ raw_spin_unlock(&priv->lock);
+
+ irq_chip_eoi_parent(d);
+}
+
+static void rzv2h_tint_irq_endisable(struct irq_data *d, bool enable)
+{
+ struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
+ unsigned int hw_irq = irqd_to_hwirq(d);
+ u32 tint_nr, tssel_n, k, tssr;
+
+ if (hw_irq < ICU_TINT_START)
+ return;
+
+ tint_nr = hw_irq - ICU_TINT_START;
+ k = ICU_TSSR_K(tint_nr);
+ tssel_n = ICU_TSSR_TSSEL_N(tint_nr);
+
+ raw_spin_lock(&priv->lock);
+ tssr = readl_relaxed(priv->base + ICU_TSSR(k));
+ if (enable)
+ tssr |= ICU_TSSR_TIEN(tssel_n);
+ else
+ tssr &= ~ICU_TSSR_TIEN(tssel_n);
+ writel_relaxed(tssr, priv->base + ICU_TSSR(k));
+ raw_spin_unlock(&priv->lock);
+}
+
+static void rzv2h_icu_irq_disable(struct irq_data *d)
+{
+ irq_chip_disable_parent(d);
+ rzv2h_tint_irq_endisable(d, false);
+}
+
+static void rzv2h_icu_irq_enable(struct irq_data *d)
+{
+ rzv2h_tint_irq_endisable(d, true);
+ irq_chip_enable_parent(d);
+}
+
+static int rzv2h_nmi_set_type(struct irq_data *d, unsigned int type)
+{
+ struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
+ u32 sense;
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_EDGE_FALLING:
+ sense = ICU_NMI_EDGE_FALLING;
+ break;
+
+ case IRQ_TYPE_EDGE_RISING:
+ sense = ICU_NMI_EDGE_RISING;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ writel_relaxed(sense, priv->base + ICU_NITSR);
+
+ return 0;
+}
+
+static int rzv2h_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
+ unsigned int hwirq = irqd_to_hwirq(d);
+ u32 irq_nr = hwirq - ICU_IRQ_START;
+ u32 iitsr, sense;
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sense = ICU_IRQ_LEVEL_LOW;
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ sense = ICU_IRQ_EDGE_FALLING;
+ break;
+
+ case IRQ_TYPE_EDGE_RISING:
+ sense = ICU_IRQ_EDGE_RISING;
+ break;
+
+ case IRQ_TYPE_EDGE_BOTH:
+ sense = ICU_IRQ_EDGE_BOTH;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ raw_spin_lock(&priv->lock);
+ iitsr = readl_relaxed(priv->base + ICU_IITSR);
+ iitsr &= ~ICU_IITSR_IITSEL_MASK(irq_nr);
+ iitsr |= ICU_IITSR_IITSEL_PREP(sense, irq_nr);
+ rzv2h_clear_irq_int(priv, hwirq);
+ writel_relaxed(iitsr, priv->base + ICU_IITSR);
+ raw_spin_unlock(&priv->lock);
+
+ return 0;
+}
+
+static int rzv2h_tint_set_type(struct irq_data *d, unsigned int type)
+{
+ u32 titsr, titsr_k, titsel_n, tien;
+ struct rzv2h_icu_priv *priv;
+ u32 tssr, tssr_k, tssel_n;
+ unsigned int hwirq;
+ u32 tint, sense;
+ int tint_nr;
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sense = ICU_TINT_LEVEL_LOW;
+ break;
+
+ case IRQ_TYPE_LEVEL_HIGH:
+ sense = ICU_TINT_LEVEL_HIGH;
+ break;
+
+ case IRQ_TYPE_EDGE_RISING:
+ sense = ICU_TINT_EDGE_RISING;
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ sense = ICU_TINT_EDGE_FALLING;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ tint = (u32)(uintptr_t)irq_data_get_irq_chip_data(d);
+ if (tint > ICU_PB5_TINT)
+ return -EINVAL;
+
+ priv = irq_data_to_priv(d);
+ hwirq = irqd_to_hwirq(d);
+
+ tint_nr = hwirq - ICU_TINT_START;
+
+ tssr_k = ICU_TSSR_K(tint_nr);
+ tssel_n = ICU_TSSR_TSSEL_N(tint_nr);
+
+ titsr_k = ICU_TITSR_K(tint_nr);
+ titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
+ tien = ICU_TSSR_TIEN(titsel_n);
+
+ raw_spin_lock(&priv->lock);
+
+ tssr = readl_relaxed(priv->base + ICU_TSSR(tssr_k));
+ tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n) | tien);
+ tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n);
+
+ writel_relaxed(tssr, priv->base + ICU_TSSR(tssr_k));
+
+ titsr = readl_relaxed(priv->base + ICU_TITSR(titsr_k));
+ titsr &= ~ICU_TITSR_TITSEL_MASK(titsel_n);
+ titsr |= ICU_TITSR_TITSEL_PREP(sense, titsel_n);
+
+ writel_relaxed(titsr, priv->base + ICU_TITSR(titsr_k));
+
+ rzv2h_clear_tint_int(priv, hwirq);
+
+ writel_relaxed(tssr | tien, priv->base + ICU_TSSR(tssr_k));
+
+ raw_spin_unlock(&priv->lock);
+
+ return 0;
+}
+
+static int rzv2h_icu_set_type(struct irq_data *d, unsigned int type)
+{
+ unsigned int hw_irq = irqd_to_hwirq(d);
+ int ret;
+
+ if (hw_irq >= ICU_TINT_START)
+ ret = rzv2h_tint_set_type(d, type);
+ else if (hw_irq >= ICU_IRQ_START)
+ ret = rzv2h_irq_set_type(d, type);
+ else
+ ret = rzv2h_nmi_set_type(d, type);
+
+ if (ret)
+ return ret;
+
+ return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH);
+}
+
+static const struct irq_chip rzv2h_icu_chip = {
+ .name = "rzv2h-icu",
+ .irq_eoi = rzv2h_icu_eoi,
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_disable = rzv2h_icu_irq_disable,
+ .irq_enable = rzv2h_icu_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 = rzv2h_icu_set_type,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
+};
+
+static int rzv2h_icu_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ struct rzv2h_icu_priv *priv = domain->host_data;
+ unsigned long tint = 0;
+ irq_hw_number_t hwirq;
+ unsigned int type;
+ int ret;
+
+ ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ /*
+ * For TINT interrupts the hwirq and TINT are encoded in
+ * fwspec->param[0].
+ * hwirq is embedded in bits 0-15.
+ * TINT is embedded in bits 16-31.
+ */
+ if (hwirq >= ICU_TINT_START) {
+ tint = ICU_TINT_EXTRACT_GPIOINT(hwirq);
+ hwirq = ICU_TINT_EXTRACT_HWIRQ(hwirq);
+
+ if (hwirq < ICU_TINT_START)
+ return -EINVAL;
+ }
+
+ if (hwirq > (ICU_NUM_IRQ - 1))
+ return -EINVAL;
+
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, priv->irqchip,
+ (void *)(uintptr_t)tint);
+ if (ret)
+ return ret;
+
+ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+ &priv->fwspec[hwirq]);
+}
+
+static const struct irq_domain_ops rzv2h_icu_domain_ops = {
+ .alloc = rzv2h_icu_alloc,
+ .free = irq_domain_free_irqs_common,
+ .translate = irq_domain_translate_twocell,
+};
+
+static int rzv2h_icu_parse_interrupts(struct rzv2h_icu_priv *priv,
+ struct device_node *np)
+{
+ struct of_phandle_args map;
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < 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 rzv2h_icu_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct irq_domain *irq_domain, *parent_domain;
+ struct rzv2h_icu_priv *rzv2h_icu_data;
+ struct platform_device *pdev;
+ struct reset_control *resetn;
+ int ret;
+
+ pdev = of_find_device_by_node(node);
+ if (!pdev)
+ return -ENODEV;
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain) {
+ dev_err(&pdev->dev, "cannot find parent domain\n");
+ return -ENODEV;
+ }
+
+ rzv2h_icu_data = devm_kzalloc(&pdev->dev, sizeof(*rzv2h_icu_data),
+ GFP_KERNEL);
+ if (!rzv2h_icu_data)
+ return -ENOMEM;
+
+ rzv2h_icu_data->irqchip = &rzv2h_icu_chip;
+
+ rzv2h_icu_data->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0,
+ NULL);
+ if (IS_ERR(rzv2h_icu_data->base))
+ return PTR_ERR(rzv2h_icu_data->base);
+
+ ret = rzv2h_icu_parse_interrupts(rzv2h_icu_data, node);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot parse interrupts: %d\n", ret);
+ return ret;
+ }
+
+ resetn = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(resetn))
+ return PTR_ERR(resetn);
+
+ ret = reset_control_deassert(resetn);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to deassert resetn pin, %d\n", ret);
+ return ret;
+ }
+
+ pm_runtime_enable(&pdev->dev);
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "pm_runtime_resume_and_get failed: %d\n",
+ ret);
+ goto pm_disable;
+ }
+
+ raw_spin_lock_init(&rzv2h_icu_data->lock);
+
+ irq_domain = irq_domain_add_hierarchy(parent_domain, 0, ICU_NUM_IRQ,
+ node, &rzv2h_icu_domain_ops,
+ rzv2h_icu_data);
+ if (!irq_domain) {
+ dev_err(&pdev->dev, "failed to add irq domain\n");
+ ret = -ENOMEM;
+ goto pm_put;
+ }
+
+ return 0;
+
+pm_put:
+ pm_runtime_put(&pdev->dev);
+pm_disable:
+ pm_runtime_disable(&pdev->dev);
+ reset_control_assert(resetn);
+
+ return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(rzv2h_icu)
+IRQCHIP_MATCH("renesas,r9a09g057-icu", rzv2h_icu_init)
+IRQCHIP_PLATFORM_DRIVER_END(rzv2h_icu)
+MODULE_AUTHOR("Fabrizio Castro <fabrizio.castro.jz@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/V2H(P) ICU Driver");
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 5d94c3f31494..9f7fe02310b9 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -347,6 +347,7 @@ config ARCH_R9A09G011
config ARCH_R9A09G057
bool "ARM64 Platform support for RZ/V2H(P)"
+ select RENESAS_RZV2H_ICU
help
This enables support for the Renesas RZ/V2H(P) SoC variants.
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/6] arm64: dts: renesas: r9a09g057: Add ICU node
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
` (4 preceding siblings ...)
2024-09-17 17:32 ` [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver Fabrizio Castro
@ 2024-09-17 17:32 ` Fabrizio Castro
5 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-17 17:32 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, linux-renesas-soc, devicetree,
linux-kernel, Chris Paterson, Biju Das, Lad Prabhakar
Add node for the Interrupt Control Unit IP found on the Renesas
RZ/V2H(P) SoC, and modify the pinctrl node as its interrupt parent
is the ICU node.
Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
arch/arm64/boot/dts/renesas/r9a09g057.dtsi | 88 ++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
index 1ad5a1b6917f..72d54aa68e37 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
@@ -90,6 +90,93 @@ soc: soc {
#size-cells = <2>;
ranges;
+ icu: interrupt-controller@10400000 {
+ compatible = "renesas,r9a09g057-icu";
+ reg = <0 0x10400000 0 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 9 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 427 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 430 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 432 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 443 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 262 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 266 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>;
+ interrupt-names = "nmi",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "irq8", "irq9", "irq10", "irq11",
+ "irq12", "irq13", "irq14", "irq15",
+ "tint0", "tint1", "tint2", "tint3",
+ "tint4", "tint5", "tint6", "tint7",
+ "tint8", "tint9", "tint10", "tint11",
+ "tint12", "tint13", "tint14", "tint15",
+ "tint16", "tint17", "tint18", "tint19",
+ "tint20", "tint21", "tint22", "tint23",
+ "tint24", "tint25", "tint26", "tint27",
+ "tint28", "tint29", "tint30", "tint31",
+ "int-ca55-0", "int-ca55-1",
+ "int-ca55-2", "int-ca55-3",
+ "icu-error-ca55",
+ "gpt-u0-gtciada", "gpt-u0-gtciadb",
+ "gpt-u1-gtciada", "gpt-u1-gtciadb";
+ clocks = <&cpg CPG_MOD 0x5>;
+ power-domains = <&cpg>;
+ resets = <&cpg 0x36>;
+ };
+
pinctrl: pinctrl@10410000 {
compatible = "renesas,r9a09g057-pinctrl";
reg = <0 0x10410000 0 0x10000>;
@@ -99,6 +186,7 @@ pinctrl: pinctrl@10410000 {
gpio-ranges = <&pinctrl 0 0 96>;
#interrupt-cells = <2>;
interrupt-controller;
+ interrupt-parent = <&icu>;
power-domains = <&cpg>;
resets = <&cpg 0xa5>, <&cpg 0xa6>;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
@ 2024-09-17 18:31 ` Rob Herring (Arm)
2024-09-17 22:14 ` Rob Herring
1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring (Arm) @ 2024-09-17 18:31 UTC (permalink / raw)
To: Fabrizio Castro
Cc: devicetree, Marc Zyngier, Magnus Damm, linux-renesas-soc,
Krzysztof Kozlowski, Biju Das, linux-kernel, Lad Prabhakar,
Geert Uytterhoeven, linux-gpio, Conor Dooley, Linus Walleij,
Chris Paterson
On Tue, 17 Sep 2024 18:32:44 +0100, Fabrizio Castro wrote:
> All the platforms from the renesas,rzg2l-pinctrl.yaml binding
> actually require the interrupt-parent property. Add it.
>
> Fixes: 35c37efd1273 ("dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Document the properties to handle GPIO IRQ")
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> .../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.example.dtb: pinctrl@11030000: 'interrupt-parent' is a required property
from schema $id: http://devicetree.org/schemas/pinctrl/renesas,rzg2l-pinctrl.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240917173249.158920-2-fabrizio.castro.jz@renesas.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
2024-09-17 18:31 ` Rob Herring (Arm)
@ 2024-09-17 22:14 ` Rob Herring
2024-09-18 9:27 ` Fabrizio Castro
1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2024-09-17 22:14 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Linus Walleij, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Lad Prabhakar, Marc Zyngier,
linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Chris Paterson, Biju Das
On Tue, Sep 17, 2024 at 06:32:44PM +0100, Fabrizio Castro wrote:
> All the platforms from the renesas,rzg2l-pinctrl.yaml binding
> actually require the interrupt-parent property. Add it.
But they don't require it. It *never* is required. If interrupt-parent
is not found in a node, the parent will be checked.
The check failure is because the example extraction has to play with
interrupt-parent to make interrupt parsing work.
>
> Fixes: 35c37efd1273 ("dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Document the properties to handle GPIO IRQ")
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> .../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
2024-09-17 22:14 ` Rob Herring
@ 2024-09-18 9:27 ` Fabrizio Castro
0 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-18 9:27 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Prabhakar Mahadev Lad,
Marc Zyngier, linux-renesas-soc@vger.kernel.org,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Chris Paterson, Biju Das
Hi Rob,
Thank you for your reply.
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, September 17, 2024 11:15 PM
> To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent
>
> On Tue, Sep 17, 2024 at 06:32:44PM +0100, Fabrizio Castro wrote:
> > All the platforms from the renesas,rzg2l-pinctrl.yaml binding actually
> > require the interrupt-parent property. Add it.
>
> But they don't require it. It *never* is required. If interrupt-parent is not found in a node, the
> parent will be checked.
Indeed.
I am dropping this patch.
>
> The check failure is because the example extraction has to play with interrupt-parent to make interrupt
> parsing work.
Gotcha.
Thank you for the explanation.
Cheers,
Fab
>
> >
> > Fixes: 35c37efd1273 ("dt-bindings: pinctrl: renesas,rzg2l-pinctrl:
> > Document the properties to handle GPIO IRQ")
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > ---
> > .../devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller
2024-09-17 17:32 ` [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller Fabrizio Castro
@ 2024-09-18 17:28 ` Rob Herring
2024-09-18 18:15 ` Fabrizio Castro
0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2024-09-18 17:28 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Thomas Gleixner, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, linux-kernel, devicetree,
linux-renesas-soc, Chris Paterson, Biju Das, Lad Prabhakar
On Tue, Sep 17, 2024 at 06:32:46PM +0100, Fabrizio Castro wrote:
> Add DT bindings for the Renesas RZ/V2H(P) Interrupt Controller.
>
> Also add macros for the NMI and IRQ0-15 interrupts which map the
> SPI0-16 interrupts on the RZ/V2H(P) SoC so that they can be
> used in the first cell of the interrupt specifiers.
>
> For the second cell of the interrupt specifier, since NMI, IRQn
> and TINTn support different types of interrupts between themselves,
> add helper macros to make it easier for the user to work out what's
> available.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> .../renesas,rzv2h-icu.yaml | 278 ++++++++++++++++++
> .../interrupt-controller/icu-rzv2h.h | 48 +++
> 2 files changed, 326 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
> create mode 100644 include/dt-bindings/interrupt-controller/icu-rzv2h.h
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
> new file mode 100644
> index 000000000000..28f5b2f30c31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-icu.yaml
> @@ -0,0 +1,278 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzv2h-icu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/V2H(P) Interrupt Control Unit
> +
> +maintainers:
> + - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> + - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +description: |
Don't need '|' if no formatting.
> + The Interrupt Control Unit (ICU) handles external interrupts (NMI, IRQ, and
> + TINT), error interrupts, DMAC requests, GPT interrupts, and internal
> + interrupts.
> + It notifies GIC of the relevant interrupts.
If you want paragraphs, then put a blank line in between.
What this interrupt controller is attached to is really outside the
scope of this binding, so maybe just drop it.
> +
> +properties:
> + compatible:
> + const: renesas,r9a09g057-icu # RZ/V2H(P)
> +
> + '#interrupt-cells':
> + description: The first cell should contain a macro RZV2H_{NMI,IRQn} from
> + file include/dt-bindings/interrupt-controller/icu-rzv2h.h,
> + and the second cell is used to specify the flag. Helper macros
> + for the second cell can also be found in the same header file.
Ident by 2 more than 'description'.
> + const: 2
> +
> + '#address-cells':
> + const: 0
> +
> + interrupt-controller: true
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 58
> + items:
> + - description: NMI interrupt
> + - description: IRQ0 interrupt
> + - description: IRQ1 interrupt
> + - description: IRQ2 interrupt
> + - description: IRQ3 interrupt
> + - description: IRQ4 interrupt
> + - description: IRQ5 interrupt
> + - description: IRQ6 interrupt
> + - description: IRQ7 interrupt
> + - description: IRQ8 interrupt
> + - description: IRQ9 interrupt
> + - description: IRQ10 interrupt
> + - description: IRQ11 interrupt
> + - description: IRQ12 interrupt
> + - description: IRQ13 interrupt
> + - description: IRQ14 interrupt
> + - description: IRQ15 interrupt
> + - description: GPIO interrupt, TINT0
> + - description: GPIO interrupt, TINT1
> + - description: GPIO interrupt, TINT2
> + - description: GPIO interrupt, TINT3
> + - description: GPIO interrupt, TINT4
> + - description: GPIO interrupt, TINT5
> + - description: GPIO interrupt, TINT6
> + - description: GPIO interrupt, TINT7
> + - description: GPIO interrupt, TINT8
> + - description: GPIO interrupt, TINT9
> + - description: GPIO interrupt, TINT10
> + - description: GPIO interrupt, TINT11
> + - description: GPIO interrupt, TINT12
> + - description: GPIO interrupt, TINT13
> + - description: GPIO interrupt, TINT14
> + - description: GPIO interrupt, TINT15
> + - description: GPIO interrupt, TINT16
> + - description: GPIO interrupt, TINT17
> + - description: GPIO interrupt, TINT18
> + - description: GPIO interrupt, TINT19
> + - description: GPIO interrupt, TINT20
> + - description: GPIO interrupt, TINT21
> + - description: GPIO interrupt, TINT22
> + - description: GPIO interrupt, TINT23
> + - description: GPIO interrupt, TINT24
> + - description: GPIO interrupt, TINT25
> + - description: GPIO interrupt, TINT26
> + - description: GPIO interrupt, TINT27
> + - description: GPIO interrupt, TINT28
> + - description: GPIO interrupt, TINT29
> + - description: GPIO interrupt, TINT30
> + - description: GPIO interrupt, TINT31
> + - description: Software interrupt, INTA55_0
> + - description: Software interrupt, INTA55_1
> + - description: Software interrupt, INTA55_2
> + - description: Software interrupt, INTA55_3
> + - description: Error interrupt to CA55
> + - description: GTCCRA compare match/input capture (U0)
> + - description: GTCCRB compare match/input capture (U0)
> + - description: GTCCRA compare match/input capture (U1)
> + - description: GTCCRB compare match/input capture (U1)
> +
> + interrupt-names:
> + minItems: 58
> + items:
> + - const: nmi
> + - const: irq0
> + - const: irq1
> + - const: irq2
> + - const: irq3
> + - const: irq4
> + - const: irq5
> + - const: irq6
> + - const: irq7
> + - const: irq8
> + - const: irq9
> + - const: irq10
> + - const: irq11
> + - const: irq12
> + - const: irq13
> + - const: irq14
> + - const: irq15
> + - const: tint0
> + - const: tint1
> + - const: tint2
> + - const: tint3
> + - const: tint4
> + - const: tint5
> + - const: tint6
> + - const: tint7
> + - const: tint8
> + - const: tint9
> + - const: tint10
> + - const: tint11
> + - const: tint12
> + - const: tint13
> + - const: tint14
> + - const: tint15
> + - const: tint16
> + - const: tint17
> + - const: tint18
> + - const: tint19
> + - const: tint20
> + - const: tint21
> + - const: tint22
> + - const: tint23
> + - const: tint24
> + - const: tint25
> + - const: tint26
> + - const: tint27
> + - const: tint28
> + - const: tint29
> + - const: tint30
> + - const: tint31
> + - const: int-ca55-0
> + - const: int-ca55-1
> + - const: int-ca55-2
> + - const: int-ca55-3
> + - const: icu-error-ca55
> + - const: gpt-u0-gtciada
> + - const: gpt-u0-gtciadb
> + - const: gpt-u1-gtciada
> + - const: gpt-u1-gtciadb
> +
> + clocks:
> + maxItems: 1
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - '#interrupt-cells'
> + - '#address-cells'
> + - interrupt-controller
> + - interrupts
> + - interrupt-names
> + - clocks
> + - power-domains
> + - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> + icu: interrupt-controller@10400000 {
> + compatible = "renesas,r9a09g057-icu";
> + reg = <0x10400000 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 9 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 427 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 430 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 432 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 443 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 262 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 266 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>;
> + interrupt-names = "nmi",
> + "irq0", "irq1", "irq2", "irq3",
> + "irq4", "irq5", "irq6", "irq7",
> + "irq8", "irq9", "irq10", "irq11",
> + "irq12", "irq13", "irq14", "irq15",
> + "tint0", "tint1", "tint2", "tint3",
> + "tint4", "tint5", "tint6", "tint7",
> + "tint8", "tint9", "tint10", "tint11",
> + "tint12", "tint13", "tint14", "tint15",
> + "tint16", "tint17", "tint18", "tint19",
> + "tint20", "tint21", "tint22", "tint23",
> + "tint24", "tint25", "tint26", "tint27",
> + "tint28", "tint29", "tint30", "tint31",
> + "int-ca55-0", "int-ca55-1",
> + "int-ca55-2", "int-ca55-3",
> + "icu-error-ca55",
> + "gpt-u0-gtciada", "gpt-u0-gtciadb",
> + "gpt-u1-gtciada", "gpt-u1-gtciadb";
> + clocks = <&cpg CPG_MOD 0x5>;
> + power-domains = <&cpg>;
> + resets = <&cpg 0x36>;
> + };
> diff --git a/include/dt-bindings/interrupt-controller/icu-rzv2h.h b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
> new file mode 100644
> index 000000000000..5d1e7bb256cd
> --- /dev/null
> +++ b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * This header provides constants for Renesas RZ/V2H(P) SoC ICU bindings.
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_ICU_RZV2H_H
> +#define __DT_BINDINGS_ICU_RZV2H_H
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/* NMI maps to SPI0 */
> +#define RZV2H_NMI 0
> +
> +/* IRQ0-15 map to SPI1-16 */
> +#define RZV2H_IRQ0 1
> +#define RZV2H_IRQ1 2
> +#define RZV2H_IRQ2 3
> +#define RZV2H_IRQ3 4
> +#define RZV2H_IRQ4 5
> +#define RZV2H_IRQ5 6
> +#define RZV2H_IRQ6 7
> +#define RZV2H_IRQ7 8
> +#define RZV2H_IRQ8 9
> +#define RZV2H_IRQ9 10
> +#define RZV2H_IRQ10 11
> +#define RZV2H_IRQ11 12
> +#define RZV2H_IRQ12 13
> +#define RZV2H_IRQ13 14
> +#define RZV2H_IRQ14 15
> +#define RZV2H_IRQ15 16
#define FOO_N N (or N+1 in this case) is pretty useless.
Also, we generally don't do defines unless we've made up the numbering
(like clock indices). That's generally never the case for IRQs because
they are defined in SoC reference manuals. Note the lack of headers in
this directory.
> +
> +#define RZV2H_NMI_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> +#define RZV2H_NMI_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> +
> +#define RZV2H_IRQ_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> +#define RZV2H_IRQ_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> +#define RZV2H_IRQ_TYPE_EDGE_BOTH IRQ_TYPE_EDGE_BOTH
> +#define RZV2H_IRQ_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
> +
> +#define RZV2H_TINT_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> +#define RZV2H_TINT_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> +#define RZV2H_TINT_TYPE_LEVEL_HIGH IRQ_TYPE_LEVEL_HIGH
> +#define RZV2H_TINT_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
Humm, that's a big no. Use the existing defines directly.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller
2024-09-18 17:28 ` Rob Herring
@ 2024-09-18 18:15 ` Fabrizio Castro
0 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-09-18 18:15 UTC (permalink / raw)
To: Rob Herring
Cc: Thomas Gleixner, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad
Hi Rob,
thank you for your reply.
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, September 18, 2024 6:28 PM
> To: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller
>
> On Tue, Sep 17, 2024 at 06:32:46PM +0100, Fabrizio Castro wrote:
> > Add DT bindings for the Renesas RZ/V2H(P) Interrupt Controller.
> >
> > Also add macros for the NMI and IRQ0-15 interrupts which map the
> > SPI0-16 interrupts on the RZ/V2H(P) SoC so that they can be used in
> > the first cell of the interrupt specifiers.
> >
> > For the second cell of the interrupt specifier, since NMI, IRQn and
> > TINTn support different types of interrupts between themselves, add
> > helper macros to make it easier for the user to work out what's
> > available.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > ---
> > .../renesas,rzv2h-icu.yaml | 278 ++++++++++++++++++
> > .../interrupt-controller/icu-rzv2h.h | 48 +++
> > 2 files changed, 326 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h-i
> > cu.yaml create mode 100644
> > include/dt-bindings/interrupt-controller/icu-rzv2h.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h
> > -icu.yaml
> > b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzv2h
> > -icu.yaml
> > new file mode 100644
> > index 000000000000..28f5b2f30c31
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,r
> > +++ zv2h-icu.yaml
> > @@ -0,0 +1,278 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +http://devicetree.org/schemas/interrupt-controller/renesas,rzv2h-icu.
> > +yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/V2H(P) Interrupt Control Unit
> > +
> > +maintainers:
> > + - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> > + - Geert Uytterhoeven <geert+renesas@glider.be>
> > +
> > +allOf:
> > + - $ref: /schemas/interrupt-controller.yaml#
> > +
> > +description: |
>
> Don't need '|' if no formatting.
I will take it out in v2.
>
> > + The Interrupt Control Unit (ICU) handles external interrupts (NMI,
> > + IRQ, and TINT), error interrupts, DMAC requests, GPT interrupts,
> > + and internal interrupts.
> > + It notifies GIC of the relevant interrupts.
>
> If you want paragraphs, then put a blank line in between.
>
> What this interrupt controller is attached to is really outside the scope of this binding, so maybe
> just drop it.
I'll drop it.
>
> > +
> > +properties:
> > + compatible:
> > + const: renesas,r9a09g057-icu # RZ/V2H(P)
> > +
> > + '#interrupt-cells':
> > + description: The first cell should contain a macro RZV2H_{NMI,IRQn} from
> > + file include/dt-bindings/interrupt-controller/icu-rzv2h.h,
> > + and the second cell is used to specify the flag. Helper macros
> > + for the second cell can also be found in the same header file.
>
> Ident by 2 more than 'description'.
Will do, and I will amend the description as the header file is most likely going to be dropped.
>
> > + const: 2
> > +
> > + '#address-cells':
> > + const: 0
> > +
> > + interrupt-controller: true
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 58
> > + items:
> > + - description: NMI interrupt
> > + - description: IRQ0 interrupt
> > + - description: IRQ1 interrupt
> > + - description: IRQ2 interrupt
> > + - description: IRQ3 interrupt
> > + - description: IRQ4 interrupt
> > + - description: IRQ5 interrupt
> > + - description: IRQ6 interrupt
> > + - description: IRQ7 interrupt
> > + - description: IRQ8 interrupt
> > + - description: IRQ9 interrupt
> > + - description: IRQ10 interrupt
> > + - description: IRQ11 interrupt
> > + - description: IRQ12 interrupt
> > + - description: IRQ13 interrupt
> > + - description: IRQ14 interrupt
> > + - description: IRQ15 interrupt
> > + - description: GPIO interrupt, TINT0
> > + - description: GPIO interrupt, TINT1
> > + - description: GPIO interrupt, TINT2
> > + - description: GPIO interrupt, TINT3
> > + - description: GPIO interrupt, TINT4
> > + - description: GPIO interrupt, TINT5
> > + - description: GPIO interrupt, TINT6
> > + - description: GPIO interrupt, TINT7
> > + - description: GPIO interrupt, TINT8
> > + - description: GPIO interrupt, TINT9
> > + - description: GPIO interrupt, TINT10
> > + - description: GPIO interrupt, TINT11
> > + - description: GPIO interrupt, TINT12
> > + - description: GPIO interrupt, TINT13
> > + - description: GPIO interrupt, TINT14
> > + - description: GPIO interrupt, TINT15
> > + - description: GPIO interrupt, TINT16
> > + - description: GPIO interrupt, TINT17
> > + - description: GPIO interrupt, TINT18
> > + - description: GPIO interrupt, TINT19
> > + - description: GPIO interrupt, TINT20
> > + - description: GPIO interrupt, TINT21
> > + - description: GPIO interrupt, TINT22
> > + - description: GPIO interrupt, TINT23
> > + - description: GPIO interrupt, TINT24
> > + - description: GPIO interrupt, TINT25
> > + - description: GPIO interrupt, TINT26
> > + - description: GPIO interrupt, TINT27
> > + - description: GPIO interrupt, TINT28
> > + - description: GPIO interrupt, TINT29
> > + - description: GPIO interrupt, TINT30
> > + - description: GPIO interrupt, TINT31
> > + - description: Software interrupt, INTA55_0
> > + - description: Software interrupt, INTA55_1
> > + - description: Software interrupt, INTA55_2
> > + - description: Software interrupt, INTA55_3
> > + - description: Error interrupt to CA55
> > + - description: GTCCRA compare match/input capture (U0)
> > + - description: GTCCRB compare match/input capture (U0)
> > + - description: GTCCRA compare match/input capture (U1)
> > + - description: GTCCRB compare match/input capture (U1)
> > +
> > + interrupt-names:
> > + minItems: 58
> > + items:
> > + - const: nmi
> > + - const: irq0
> > + - const: irq1
> > + - const: irq2
> > + - const: irq3
> > + - const: irq4
> > + - const: irq5
> > + - const: irq6
> > + - const: irq7
> > + - const: irq8
> > + - const: irq9
> > + - const: irq10
> > + - const: irq11
> > + - const: irq12
> > + - const: irq13
> > + - const: irq14
> > + - const: irq15
> > + - const: tint0
> > + - const: tint1
> > + - const: tint2
> > + - const: tint3
> > + - const: tint4
> > + - const: tint5
> > + - const: tint6
> > + - const: tint7
> > + - const: tint8
> > + - const: tint9
> > + - const: tint10
> > + - const: tint11
> > + - const: tint12
> > + - const: tint13
> > + - const: tint14
> > + - const: tint15
> > + - const: tint16
> > + - const: tint17
> > + - const: tint18
> > + - const: tint19
> > + - const: tint20
> > + - const: tint21
> > + - const: tint22
> > + - const: tint23
> > + - const: tint24
> > + - const: tint25
> > + - const: tint26
> > + - const: tint27
> > + - const: tint28
> > + - const: tint29
> > + - const: tint30
> > + - const: tint31
> > + - const: int-ca55-0
> > + - const: int-ca55-1
> > + - const: int-ca55-2
> > + - const: int-ca55-3
> > + - const: icu-error-ca55
> > + - const: gpt-u0-gtciada
> > + - const: gpt-u0-gtciadb
> > + - const: gpt-u1-gtciada
> > + - const: gpt-u1-gtciadb
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - '#interrupt-cells'
> > + - '#address-cells'
> > + - interrupt-controller
> > + - interrupts
> > + - interrupt-names
> > + - clocks
> > + - power-domains
> > + - resets
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/renesas-cpg-mssr.h>
> > +
> > + icu: interrupt-controller@10400000 {
> > + compatible = "renesas,r9a09g057-icu";
> > + reg = <0x10400000 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 9 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 427 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 430 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 432 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 443 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 262 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 263 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 265 IRQ_TYPE_EDGE_RISING>,
> > + <GIC_SPI 266 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>;
> > + interrupt-names = "nmi",
> > + "irq0", "irq1", "irq2", "irq3",
> > + "irq4", "irq5", "irq6", "irq7",
> > + "irq8", "irq9", "irq10", "irq11",
> > + "irq12", "irq13", "irq14", "irq15",
> > + "tint0", "tint1", "tint2", "tint3",
> > + "tint4", "tint5", "tint6", "tint7",
> > + "tint8", "tint9", "tint10", "tint11",
> > + "tint12", "tint13", "tint14", "tint15",
> > + "tint16", "tint17", "tint18", "tint19",
> > + "tint20", "tint21", "tint22", "tint23",
> > + "tint24", "tint25", "tint26", "tint27",
> > + "tint28", "tint29", "tint30", "tint31",
> > + "int-ca55-0", "int-ca55-1",
> > + "int-ca55-2", "int-ca55-3",
> > + "icu-error-ca55",
> > + "gpt-u0-gtciada", "gpt-u0-gtciadb",
> > + "gpt-u1-gtciada", "gpt-u1-gtciadb";
> > + clocks = <&cpg CPG_MOD 0x5>;
> > + power-domains = <&cpg>;
> > + resets = <&cpg 0x36>;
> > + };
> > diff --git a/include/dt-bindings/interrupt-controller/icu-rzv2h.h
> > b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
> > new file mode 100644
> > index 000000000000..5d1e7bb256cd
> > --- /dev/null
> > +++ b/include/dt-bindings/interrupt-controller/icu-rzv2h.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * This header provides constants for Renesas RZ/V2H(P) SoC ICU bindings.
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + *
> > + */
> > +
> > +#ifndef __DT_BINDINGS_ICU_RZV2H_H
> > +#define __DT_BINDINGS_ICU_RZV2H_H
> > +
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +/* NMI maps to SPI0 */
> > +#define RZV2H_NMI 0
> > +
> > +/* IRQ0-15 map to SPI1-16 */
> > +#define RZV2H_IRQ0 1
> > +#define RZV2H_IRQ1 2
> > +#define RZV2H_IRQ2 3
> > +#define RZV2H_IRQ3 4
> > +#define RZV2H_IRQ4 5
> > +#define RZV2H_IRQ5 6
> > +#define RZV2H_IRQ6 7
> > +#define RZV2H_IRQ7 8
> > +#define RZV2H_IRQ8 9
> > +#define RZV2H_IRQ9 10
> > +#define RZV2H_IRQ10 11
> > +#define RZV2H_IRQ11 12
> > +#define RZV2H_IRQ12 13
> > +#define RZV2H_IRQ13 14
> > +#define RZV2H_IRQ14 15
> > +#define RZV2H_IRQ15 16
>
> #define FOO_N N (or N+1 in this case) is pretty useless.
>
> Also, we generally don't do defines unless we've made up the numbering (like clock indices). That's
> generally never the case for IRQs because they are defined in SoC reference manuals. Note the lack of
> headers in this directory.
>
> > +
> > +#define RZV2H_NMI_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> > +#define RZV2H_NMI_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> > +
> > +#define RZV2H_IRQ_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> > +#define RZV2H_IRQ_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> > +#define RZV2H_IRQ_TYPE_EDGE_BOTH IRQ_TYPE_EDGE_BOTH
> > +#define RZV2H_IRQ_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
> > +
> > +#define RZV2H_TINT_TYPE_EDGE_RISING IRQ_TYPE_EDGE_RISING
> > +#define RZV2H_TINT_TYPE_EDGE_FALLING IRQ_TYPE_EDGE_FALLING
> > +#define RZV2H_TINT_TYPE_LEVEL_HIGH IRQ_TYPE_LEVEL_HIGH
> > +#define RZV2H_TINT_TYPE_LEVEL_LOW IRQ_TYPE_LEVEL_LOW
>
> Humm, that's a big no. Use the existing defines directly.
I think I am just going to drop this header file completely in v2.
Thanks for your feedback.
Cheers,
Fab
>
> Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver
2024-09-17 17:32 ` [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver Fabrizio Castro
@ 2024-10-02 10:51 ` Thomas Gleixner
2024-10-09 17:44 ` Fabrizio Castro
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2024-10-02 10:51 UTC (permalink / raw)
To: Fabrizio Castro, Philipp Zabel, Geert Uytterhoeven
Cc: Fabrizio Castro, Magnus Damm, linux-kernel, linux-renesas-soc,
Chris Paterson, Biju Das, Lad Prabhakar
On Tue, Sep 17 2024 at 18:32, Fabrizio Castro wrote:
> +
> +/* DT "interrupts" indexes */
> +#define ICU_IRQ_START 1
> +#define ICU_IRQ_COUNT 16
> +#define ICU_TINT_START (ICU_IRQ_START + ICU_IRQ_COUNT)
> +#define ICU_TINT_COUNT 32
> +#define ICU_NUM_IRQ (ICU_TINT_START + ICU_TINT_COUNT)
> +
> +/* Registers */
> +#define ICU_NSCNT 0x00
> +#define ICU_NSCLR 0x04
> +#define ICU_NITSR 0x08
> +#define ICU_ISCTR 0x10
> +#define ICU_ISCLR 0x14
> +#define ICU_IITSR 0x18
> +#define ICU_TSCTR 0x20
> +#define ICU_TSCLR 0x24
> +#define ICU_TITSR(k) (0x28 + (k) * 4)
> +#define ICU_TSSR(k) (0x30 + (k) * 4)
> +
> +/* NMI */
> +#define ICU_NMI_EDGE_FALLING 0
> +#define ICU_NMI_EDGE_RISING 1
> +
> +#define ICU_NSCNT_NSTAT BIT(0)
> +#define ICU_NSCNT_NSTAT_DETECTED 1
> +
> +#define ICU_NSCLR_NCLR BIT(0)
> +
> +/* IRQ */
> +#define ICU_IRQ_LEVEL_LOW 0
> +#define ICU_IRQ_EDGE_FALLING 1
> +#define ICU_IRQ_EDGE_RISING 2
> +#define ICU_IRQ_EDGE_BOTH 3
> +
> +#define ICU_IITSR_IITSEL_PREP(iitsel, n) ((iitsel) << ((n) * 2))
> +#define ICU_IITSR_IITSEL_GET(iitsr, n) (((iitsr) >> ((n) * 2)) & 0x03)
> +#define ICU_IITSR_IITSEL_MASK(n) ICU_IITSR_IITSEL_PREP(0x03, n)
> +
> +/* TINT */
> +#define ICU_TINT_EDGE_RISING 0
> +#define ICU_TINT_EDGE_FALLING 1
> +#define ICU_TINT_LEVEL_HIGH 2
> +#define ICU_TINT_LEVEL_LOW 3
> +
> +#define ICU_TSSR_K(tint_nr) ((tint_nr) / 4)
> +#define ICU_TSSR_TSSEL_N(tint_nr) ((tint_nr) % 4)
> +#define ICU_TSSR_TSSEL_PREP(tssel, n) ((tssel) << ((n) * 8))
> +#define ICU_TSSR_TSSEL_MASK(n) ICU_TSSR_TSSEL_PREP(0x7F, n)
> +#define ICU_TSSR_TIEN(n) (BIT(7) << ((n) * 8))
> +
> +#define ICU_TITSR_K(tint_nr) ((tint_nr) / 16)
> +#define ICU_TITSR_TITSEL_N(tint_nr) ((tint_nr) % 16)
> +#define ICU_TITSR_TITSEL_PREP(titsel, n) ICU_IITSR_IITSEL_PREP(titsel, n)
> +#define ICU_TITSR_TITSEL_MASK(n) ICU_IITSR_IITSEL_MASK(n)
> +#define ICU_TITSR_TITSEL_GET(titsr, n) ICU_IITSR_IITSEL_GET(titsr, n)
> +
> +#define ICU_TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
> +#define ICU_TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
> +#define ICU_PB5_TINT 0x55
> +
> +/**
> + * struct rzv2h_icu_priv - Interrupt Control Unit controller private data
> + * structure.
If you need a line break, then please format it so:
* struct rzv2h_icu_priv - Interrupt Control Unit controller private data
* structure.
This makes it readable.
> +static void rzv2h_clear_nmi_int(struct rzv2h_icu_priv *priv)
> +{
> + u32 nscnt = readl_relaxed(priv->base + ICU_NSCNT);
> +
> + if ((nscnt & ICU_NSCNT_NSTAT) == ICU_NSCNT_NSTAT_DETECTED)
> + writel_relaxed(ICU_NSCLR_NCLR, priv->base + ICU_NSCLR);
> +}
> +
> +static void rzv2h_clear_irq_int(struct rzv2h_icu_priv *priv, unsigned int hwirq)
> +{
> + unsigned int irq_nr = hwirq - ICU_IRQ_START;
> + u32 isctr, iitsr, iitsel;
> + u32 bit = BIT(irq_nr);
> +
> + isctr = readl_relaxed(priv->base + ICU_ISCTR);
> + iitsr = readl_relaxed(priv->base + ICU_IITSR);
> + iitsel = ICU_IITSR_IITSEL_GET(iitsr, irq_nr);
Not that I care about the performance of your device, but why do you
need to read back the type configuration. It's known and cached in
irq_data, no?
Also this is invoked from eoi(), so why would the bit not be set when
the interrupt is type edge and has fired? It should be set which means
the ISCTR read is pointless too. Unless I'm missing something,
> +static void rzv2h_clear_tint_int(struct rzv2h_icu_priv *priv,
> + unsigned int hwirq)
No line break required.
> +{
> + unsigned int tint_nr = hwirq - ICU_TINT_START;
> + int titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
> + u32 tsctr, titsr, titsel;
> + u32 bit = BIT(tint_nr);
> + int k = tint_nr / 16;
> +
> + tsctr = readl_relaxed(priv->base + ICU_TSCTR);
> + titsr = readl_relaxed(priv->base + ICU_TITSR(k));
> + titsel = ICU_TITSR_TITSEL_GET(titsr, titsel_n);
Same comment.
> +static void rzv2h_icu_eoi(struct irq_data *d)
> +{
> + struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
> + unsigned int hw_irq = irqd_to_hwirq(d);
> +
> + raw_spin_lock(&priv->lock);
scoped_guard(raw_spinlock, ....) {
> + if (hw_irq >= ICU_TINT_START)
> + rzv2h_clear_tint_int(priv, hw_irq);
> + else if (hw_irq >= ICU_IRQ_START)
> + rzv2h_clear_irq_int(priv, hw_irq);
> + else
> + rzv2h_clear_nmi_int(priv);
Huch. Is NMI a real NMI or just some unmaskable regular interrupt?
If it's a real NMI, then you _cannot_ take the spinlock here.
> + raw_spin_unlock(&priv->lock);
> +
> + irq_chip_eoi_parent(d);
> +}
> +
> +static void rzv2h_tint_irq_endisable(struct irq_data *d, bool enable)
> +{
> + struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
> + unsigned int hw_irq = irqd_to_hwirq(d);
> + u32 tint_nr, tssel_n, k, tssr;
> +
> + if (hw_irq < ICU_TINT_START)
> + return;
> +
> + tint_nr = hw_irq - ICU_TINT_START;
> + k = ICU_TSSR_K(tint_nr);
> + tssel_n = ICU_TSSR_TSSEL_N(tint_nr);
> +
> + raw_spin_lock(&priv->lock);
guard()
> + tssr = readl_relaxed(priv->base + ICU_TSSR(k));
> + if (enable)
> + tssr |= ICU_TSSR_TIEN(tssel_n);
> + else
> + tssr &= ~ICU_TSSR_TIEN(tssel_n);
> + writel_relaxed(tssr, priv->base + ICU_TSSR(k));
> + raw_spin_unlock(&priv->lock);
> +}
> + raw_spin_lock(&priv->lock);
guard()
> +static const struct irq_domain_ops rzv2h_icu_domain_ops = {
> + .alloc = rzv2h_icu_alloc,
> + .free = irq_domain_free_irqs_common,
> + .translate = irq_domain_translate_twocell,
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
> +};
> +
> +static int rzv2h_icu_parse_interrupts(struct rzv2h_icu_priv *priv,
> + struct device_node *np)
Please get rid of the line breaks. You have 100 characters.
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver
2024-10-02 10:51 ` Thomas Gleixner
@ 2024-10-09 17:44 ` Fabrizio Castro
0 siblings, 0 replies; 14+ messages in thread
From: Fabrizio Castro @ 2024-10-09 17:44 UTC (permalink / raw)
To: Thomas Gleixner, Philipp Zabel, Geert Uytterhoeven
Cc: Magnus Damm, linux-kernel@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad
Hi Thomas,
thanks for your feedback!
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Wednesday, October 2, 2024 11:51 AM
> Subject: Re: [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver
>
> On Tue, Sep 17 2024 at 18:32, Fabrizio Castro wrote:
> > +
> > +/* DT "interrupts" indexes */
> > +#define ICU_IRQ_START 1
> > +#define ICU_IRQ_COUNT 16
> > +#define ICU_TINT_START (ICU_IRQ_START + ICU_IRQ_COUNT)
> > +#define ICU_TINT_COUNT 32
> > +#define ICU_NUM_IRQ (ICU_TINT_START + ICU_TINT_COUNT)
> > +
> > +/* Registers */
> > +#define ICU_NSCNT 0x00
> > +#define ICU_NSCLR 0x04
> > +#define ICU_NITSR 0x08
> > +#define ICU_ISCTR 0x10
> > +#define ICU_ISCLR 0x14
> > +#define ICU_IITSR 0x18
> > +#define ICU_TSCTR 0x20
> > +#define ICU_TSCLR 0x24
> > +#define ICU_TITSR(k) (0x28 + (k) * 4)
> > +#define ICU_TSSR(k) (0x30 + (k) * 4)
> > +
> > +/* NMI */
> > +#define ICU_NMI_EDGE_FALLING 0
> > +#define ICU_NMI_EDGE_RISING 1
> > +
> > +#define ICU_NSCNT_NSTAT BIT(0)
> > +#define ICU_NSCNT_NSTAT_DETECTED 1
> > +
> > +#define ICU_NSCLR_NCLR BIT(0)
> > +
> > +/* IRQ */
> > +#define ICU_IRQ_LEVEL_LOW 0
> > +#define ICU_IRQ_EDGE_FALLING 1
> > +#define ICU_IRQ_EDGE_RISING 2
> > +#define ICU_IRQ_EDGE_BOTH 3
> > +
> > +#define ICU_IITSR_IITSEL_PREP(iitsel, n) ((iitsel) << ((n) * 2))
> > +#define ICU_IITSR_IITSEL_GET(iitsr, n) (((iitsr) >> ((n) * 2)) & 0x03)
> > +#define ICU_IITSR_IITSEL_MASK(n) ICU_IITSR_IITSEL_PREP(0x03, n)
> > +
> > +/* TINT */
> > +#define ICU_TINT_EDGE_RISING 0
> > +#define ICU_TINT_EDGE_FALLING 1
> > +#define ICU_TINT_LEVEL_HIGH 2
> > +#define ICU_TINT_LEVEL_LOW 3
> > +
> > +#define ICU_TSSR_K(tint_nr) ((tint_nr) / 4)
> > +#define ICU_TSSR_TSSEL_N(tint_nr) ((tint_nr) % 4)
> > +#define ICU_TSSR_TSSEL_PREP(tssel, n) ((tssel) << ((n) * 8))
> > +#define ICU_TSSR_TSSEL_MASK(n) ICU_TSSR_TSSEL_PREP(0x7F, n)
> > +#define ICU_TSSR_TIEN(n) (BIT(7) << ((n) * 8))
> > +
> > +#define ICU_TITSR_K(tint_nr) ((tint_nr) / 16)
> > +#define ICU_TITSR_TITSEL_N(tint_nr) ((tint_nr) % 16)
> > +#define ICU_TITSR_TITSEL_PREP(titsel, n) ICU_IITSR_IITSEL_PREP(titsel, n)
> > +#define ICU_TITSR_TITSEL_MASK(n) ICU_IITSR_IITSEL_MASK(n)
> > +#define ICU_TITSR_TITSEL_GET(titsr, n) ICU_IITSR_IITSEL_GET(titsr, n)
> > +
> > +#define ICU_TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
> > +#define ICU_TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
> > +#define ICU_PB5_TINT 0x55
> > +
> > +/**
> > + * struct rzv2h_icu_priv - Interrupt Control Unit controller private data
> > + * structure.
>
> If you need a line break, then please format it so:
>
> * struct rzv2h_icu_priv - Interrupt Control Unit controller private data
> * structure.
Since I have 100 chars, I'll just get rid of this line break.
>
> This makes it readable.
>
> > +static void rzv2h_clear_nmi_int(struct rzv2h_icu_priv *priv)
> > +{
> > + u32 nscnt = readl_relaxed(priv->base + ICU_NSCNT);
> > +
> > + if ((nscnt & ICU_NSCNT_NSTAT) == ICU_NSCNT_NSTAT_DETECTED)
> > + writel_relaxed(ICU_NSCLR_NCLR, priv->base + ICU_NSCLR);
> > +}
> > +
> > +static void rzv2h_clear_irq_int(struct rzv2h_icu_priv *priv, unsigned int hwirq)
> > +{
> > + unsigned int irq_nr = hwirq - ICU_IRQ_START;
> > + u32 isctr, iitsr, iitsel;
> > + u32 bit = BIT(irq_nr);
> > +
> > + isctr = readl_relaxed(priv->base + ICU_ISCTR);
> > + iitsr = readl_relaxed(priv->base + ICU_IITSR);
> > + iitsel = ICU_IITSR_IITSEL_GET(iitsr, irq_nr);
>
> Not that I care about the performance of your device, but why do you
> need to read back the type configuration. It's known and cached in
> irq_data, no?
>
> Also this is invoked from eoi(), so why would the bit not be set when
> the interrupt is type edge and has fired? It should be set which means
> the ISCTR read is pointless too. Unless I'm missing something,
Both rzv2h_clear_irq_int and rzv2h_clear_tint_int are also called from
the irq_set_type path, that's why all the checks.
As you pointed out, no need for all those checks from within the eoi
path, therefore I'll implement whatever is required straight in
rzv2h_icu_eoi, to improve on performance.
>
> > +static void rzv2h_clear_tint_int(struct rzv2h_icu_priv *priv,
> > + unsigned int hwirq)
>
> No line break required.
Agreed.
>
> > +{
> > + unsigned int tint_nr = hwirq - ICU_TINT_START;
> > + int titsel_n = ICU_TITSR_TITSEL_N(tint_nr);
> > + u32 tsctr, titsr, titsel;
> > + u32 bit = BIT(tint_nr);
> > + int k = tint_nr / 16;
> > +
> > + tsctr = readl_relaxed(priv->base + ICU_TSCTR);
> > + titsr = readl_relaxed(priv->base + ICU_TITSR(k));
> > + titsel = ICU_TITSR_TITSEL_GET(titsr, titsel_n);
>
> Same comment.
Agreed.
>
> > +static void rzv2h_icu_eoi(struct irq_data *d)
> > +{
> > + struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > + raw_spin_lock(&priv->lock);
>
> scoped_guard(raw_spinlock, ....) {
Good point!
>
> > + if (hw_irq >= ICU_TINT_START)
> > + rzv2h_clear_tint_int(priv, hw_irq);
> > + else if (hw_irq >= ICU_IRQ_START)
> > + rzv2h_clear_irq_int(priv, hw_irq);
> > + else
> > + rzv2h_clear_nmi_int(priv);
>
> Huch. Is NMI a real NMI or just some unmaskable regular interrupt?
>
> If it's a real NMI, then you _cannot_ take the spinlock here.
It's not a real NMI, as it's mapped to SPI 0.
>
>
> > + raw_spin_unlock(&priv->lock);
> > +
> > + irq_chip_eoi_parent(d);
> > +}
> > +
> > +static void rzv2h_tint_irq_endisable(struct irq_data *d, bool enable)
> > +{
> > + struct rzv2h_icu_priv *priv = irq_data_to_priv(d);
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > + u32 tint_nr, tssel_n, k, tssr;
> > +
> > + if (hw_irq < ICU_TINT_START)
> > + return;
> > +
> > + tint_nr = hw_irq - ICU_TINT_START;
> > + k = ICU_TSSR_K(tint_nr);
> > + tssel_n = ICU_TSSR_TSSEL_N(tint_nr);
> > +
> > + raw_spin_lock(&priv->lock);
>
> guard()
Agreed.
>
> > + tssr = readl_relaxed(priv->base + ICU_TSSR(k));
> > + if (enable)
> > + tssr |= ICU_TSSR_TIEN(tssel_n);
> > + else
> > + tssr &= ~ICU_TSSR_TIEN(tssel_n);
> > + writel_relaxed(tssr, priv->base + ICU_TSSR(k));
> > + raw_spin_unlock(&priv->lock);
> > +}
>
> > + raw_spin_lock(&priv->lock);
>
> guard()
Agreed.
>
> > +static const struct irq_domain_ops rzv2h_icu_domain_ops = {
> > + .alloc = rzv2h_icu_alloc,
> > + .free = irq_domain_free_irqs_common,
> > + .translate = irq_domain_translate_twocell,
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Will fix.
>
> > +};
> > +
> > +static int rzv2h_icu_parse_interrupts(struct rzv2h_icu_priv *priv,
> > + struct device_node *np)
>
> Please get rid of the line breaks. You have 100 characters.
Thanks for pointing this out, I'll go through the whole file adjust accordingly.
I'll send a new version soon.
Thanks,
Fab
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-10-09 17:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-17 17:32 [PATCH 0/6] Add support for the RZ/V2H Interrupt Control Unit Fabrizio Castro
2024-09-17 17:32 ` [PATCH 1/6] dt-bindings: pinctrl: renesas: rzg2l-pinctrl: Add interrupt-parent Fabrizio Castro
2024-09-17 18:31 ` Rob Herring (Arm)
2024-09-17 22:14 ` Rob Herring
2024-09-18 9:27 ` Fabrizio Castro
2024-09-17 17:32 ` [PATCH 2/6] pinctrl: renesas: rzg2l: Remove RZG2L_TINT_IRQ_START_INDEX Fabrizio Castro
2024-09-17 17:32 ` [PATCH 3/6] dt-bindings: interrupt-controller: Add Renesas RZ/V2H(P) Interrupt Controller Fabrizio Castro
2024-09-18 17:28 ` Rob Herring
2024-09-18 18:15 ` Fabrizio Castro
2024-09-17 17:32 ` [PATCH 4/6] clk: renesas: r9a09g057: Add clock and reset entries for ICU Fabrizio Castro
2024-09-17 17:32 ` [PATCH 5/6] irqchip: Add RZ/V2H(P) Interrupt Control Unit (ICU) driver Fabrizio Castro
2024-10-02 10:51 ` Thomas Gleixner
2024-10-09 17:44 ` Fabrizio Castro
2024-09-17 17:32 ` [PATCH 6/6] arm64: dts: renesas: r9a09g057: Add ICU node Fabrizio Castro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.