From: Lad Prabhakar <[email protected]>
Hi All,
This patch series does the following:
* Adds IRQC support to the RZ/G2UL SoC.
* Includes a fix for pinctrl driver when using GPIO pins as interrupts
* Adds PHY interrupt support for ETH{0/1}
Reason for sending it as RFC, as I am introducing new compatible string for
RZ/G2UL SoC as there are some differences when compared to RZ/Five:
- G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have
PLIC (chained interrupt domain) -> RISCV INTC
- On the RZ/Five we have additional registers for IRQC block
- On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
Cheers,
Prabhakar
Lad Prabhakar (5):
dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document
RZ/G2UL SoC
pinctrl: renesas: rzg2l: Fix configuring the GPIO pins as interrupts
arm64: dts: renesas: r9a07g043[u]: Add IRQC node
arm64: dts: renesas: r9a07g043[u]: Update pinctrl node to handle GPIO
interrupts
arm64: dts: renesas: rzg2ul-smarc-som: Add PHY interrupt support for
ETH{0/1}
.../renesas,rzg2l-irqc.yaml | 1 +
arch/arm64/boot/dts/renesas/r9a07g043.dtsi | 10 ++++
arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 54 +++++++++++++++++++
.../boot/dts/renesas/rzg2ul-smarc-som.dtsi | 11 +++-
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 17 +++---
5 files changed, 84 insertions(+), 9 deletions(-)
--
2.25.1
From: Lad Prabhakar <[email protected]>
The PHY interrupt (INT_N) pin is connected to IRQ2 and IRQ7 for ETH0 and
ETH1 respectively.
Signed-off-by: Lad Prabhakar <[email protected]>
---
arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
index 931efc07d6fb..49ecd33aeeb8 100644
--- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
@@ -6,6 +6,7 @@
*/
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irqc-rzg2l.h>
#include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
/ {
@@ -77,6 +78,8 @@ phy0: ethernet-phy@7 {
compatible = "ethernet-phy-id0022.1640",
"ethernet-phy-ieee802.3-c22";
reg = <7>;
+ interrupt-parent = <&irqc>;
+ interrupts = <RZG2L_IRQ2 IRQ_TYPE_LEVEL_LOW>;
rxc-skew-psec = <2400>;
txc-skew-psec = <2400>;
rxdv-skew-psec = <0>;
@@ -104,6 +107,8 @@ phy1: ethernet-phy@7 {
compatible = "ethernet-phy-id0022.1640",
"ethernet-phy-ieee802.3-c22";
reg = <7>;
+ interrupt-parent = <&irqc>;
+ interrupts = <RZG2L_IRQ7 IRQ_TYPE_LEVEL_LOW>;
rxc-skew-psec = <2400>;
txc-skew-psec = <2400>;
rxdv-skew-psec = <0>;
@@ -151,7 +156,8 @@ eth0_pins: eth0 {
<RZG2L_PORT_PINMUX(3, 2, 1)>, /* ET0_RXD0 */
<RZG2L_PORT_PINMUX(3, 3, 1)>, /* ET0_RXD1 */
<RZG2L_PORT_PINMUX(4, 0, 1)>, /* ET0_RXD2 */
- <RZG2L_PORT_PINMUX(4, 1, 1)>; /* ET0_RXD3 */
+ <RZG2L_PORT_PINMUX(4, 1, 1)>, /* ET0_RXD3 */
+ <RZG2L_PORT_PINMUX(5, 1, 7)>; /* IRQ2 */
};
eth1_pins: eth1 {
@@ -169,7 +175,8 @@ eth1_pins: eth1 {
<RZG2L_PORT_PINMUX(9, 1, 1)>, /* ET1_RXD0 */
<RZG2L_PORT_PINMUX(9, 2, 1)>, /* ET1_RXD1 */
<RZG2L_PORT_PINMUX(9, 3, 1)>, /* ET1_RXD2 */
- <RZG2L_PORT_PINMUX(10, 0, 1)>; /* ET1_RXD3 */
+ <RZG2L_PORT_PINMUX(10, 0, 1)>, /* ET1_RXD3 */
+ <RZG2L_PORT_PINMUX(18, 5, 1)>; /* IRQ7 */
};
sdhi0_emmc_pins: sd0emmc {
--
2.25.1
From: Lad Prabhakar <[email protected]>
Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
identical to one found on the RZ/G2L SoC. No driver changes are
required as generic compatible string "renesas,rzg2l-irqc" will be
used as a fallback.
Signed-off-by: Lad Prabhakar <[email protected]>
---
Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
- G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
domain) -> RISCV INTC
- On the RZ/Five we have additional registers for IRQC block
- On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
---
.../bindings/interrupt-controller/renesas,rzg2l-irqc.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
index 33b90e975e33..8f3678a82ba4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -26,6 +26,7 @@ properties:
compatible:
items:
- enum:
+ - renesas,r9a07g043u-irqc # RZ/G2UL
- renesas,r9a07g044-irqc # RZ/G2{L,LC}
- renesas,r9a07g054-irqc # RZ/V2L
- const: renesas,rzg2l-irqc
--
2.25.1
From: Lad Prabhakar <[email protected]>
Add IRQC node to R9A07G043 (RZ/G2UL) SoC DTSI.
Signed-off-by: Lad Prabhakar <[email protected]>
---
Note,
- clocks and resets are differnt when compared to RZ/Five hence its added
in r9a07g043u.dtsi
- We have additional interrupt on RZ/Five hence interrupts are added in
r9a07g043u.dtsi
- clock-names is also added in r9a07g043u.dtsi to avoid dtbs_check warning
---
arch/arm64/boot/dts/renesas/r9a07g043.dtsi | 8 ++++
arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 50 +++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
index 3f7d451b1199..44b9bc6294be 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
@@ -538,6 +538,14 @@ pinctrl: pinctrl@11030000 {
<&cpg R9A07G043_GPIO_SPARE_RESETN>;
};
+ irqc: interrupt-controller@110a0000 {
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0 0x110a0000 0 0x10000>;
+ power-domains = <&cpg>;
+ };
+
dmac: dma-controller@11820000 {
compatible = "renesas,r9a07g043-dmac",
"renesas,rz-dmac";
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
index b8bf06b51235..7a8ed7ae253b 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
@@ -48,6 +48,56 @@ timer {
};
};
+&irqc {
+ compatible = "renesas,r9a07g043u-irqc",
+ "renesas,rzg2l-irqc";
+ interrupts = <SOC_PERIPHERAL_IRQ(0) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(1) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(2) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(3) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(4) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(5) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(6) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(7) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(8) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(444) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(445) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(446) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(447) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(448) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(449) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(450) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(451) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(452) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(453) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(454) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(455) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(456) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(457) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(458) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(459) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(460) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(461) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(462) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(463) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(464) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(465) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(466) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(467) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(468) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(469) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(470) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(471) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(472) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(473) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(474) IRQ_TYPE_LEVEL_HIGH>,
+ <SOC_PERIPHERAL_IRQ(475) IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A07G043_IA55_CLK>,
+ <&cpg CPG_MOD R9A07G043_IA55_PCLK>;
+ clock-names = "clk", "pclk";
+ resets = <&cpg R9A07G043_IA55_RESETN>;
+};
+
&soc {
interrupt-parent = <&gic>;
--
2.25.1
From: Lad Prabhakar <[email protected]>
On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L and also
the pin configs are completely different. This patch makes sure we use the
appropriate pin configs for each SoC (which is passed as part of the OF
data) while configuring the GPIO pin as interrupts instead of using
rzg2l_gpio_configs[] for all the SoCs.
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index a43824fd9505..dcc495baa678 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -127,6 +127,7 @@ struct rzg2l_dedicated_configs {
struct rzg2l_pinctrl_data {
const char * const *port_pins;
const u32 *port_pin_configs;
+ unsigned int n_port_pin_configs;
struct rzg2l_dedicated_configs *dedicated_pins;
unsigned int n_port_pins;
unsigned int n_dedicated_pins;
@@ -1122,7 +1123,7 @@ static struct {
}
};
-static int rzg2l_gpio_get_gpioint(unsigned int virq)
+static int rzg2l_gpio_get_gpioint(unsigned int virq, const struct rzg2l_pinctrl_data *data)
{
unsigned int gpioint;
unsigned int i;
@@ -1131,13 +1132,13 @@ static int rzg2l_gpio_get_gpioint(unsigned int virq)
port = virq / 8;
bit = virq % 8;
- if (port >= ARRAY_SIZE(rzg2l_gpio_configs) ||
- bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
+ if (port >= data->n_port_pin_configs ||
+ bit >= RZG2L_GPIO_PORT_GET_PINCNT(data->port_pin_configs[port]))
return -EINVAL;
gpioint = bit;
for (i = 0; i < port; i++)
- gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
+ gpioint += RZG2L_GPIO_PORT_GET_PINCNT(data->port_pin_configs[i]);
return gpioint;
}
@@ -1237,7 +1238,7 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
unsigned long flags;
int gpioint, irq;
- gpioint = rzg2l_gpio_get_gpioint(child);
+ gpioint = rzg2l_gpio_get_gpioint(child, pctrl->data);
if (gpioint < 0)
return gpioint;
@@ -1311,8 +1312,8 @@ static void rzg2l_init_irq_valid_mask(struct gpio_chip *gc,
port = offset / 8;
bit = offset % 8;
- if (port >= ARRAY_SIZE(rzg2l_gpio_configs) ||
- bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
+ if (port >= pctrl->data->n_port_pin_configs ||
+ bit >= RZG2L_GPIO_PORT_GET_PINCNT(pctrl->data->port_pin_configs[port]))
clear_bit(offset, valid_mask);
}
}
@@ -1517,6 +1518,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
static struct rzg2l_pinctrl_data r9a07g043_data = {
.port_pins = rzg2l_gpio_names,
.port_pin_configs = r9a07g043_gpio_configs,
+ .n_port_pin_configs = ARRAY_SIZE(r9a07g043_gpio_configs),
.dedicated_pins = rzg2l_dedicated_pins.common,
.n_port_pins = ARRAY_SIZE(r9a07g043_gpio_configs) * RZG2L_PINS_PER_PORT,
.n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common),
@@ -1525,6 +1527,7 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
static struct rzg2l_pinctrl_data r9a07g044_data = {
.port_pins = rzg2l_gpio_names,
.port_pin_configs = rzg2l_gpio_configs,
+ .n_port_pin_configs = ARRAY_SIZE(rzg2l_gpio_configs),
.dedicated_pins = rzg2l_dedicated_pins.common,
.n_port_pins = ARRAY_SIZE(rzg2l_gpio_names),
.n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
--
2.25.1
From: Lad Prabhakar <[email protected]>
Add required properties in pinctrl node to handle GPIO interrupts.
Note as IRQC is not enabled in RZ/Five the phandle for interrupt-parent
is added in RZ/G2UL specific dtsi so that RZ/Five pinctrl driver
continues without waiting for IRQC to probe.
Signed-off-by: Lad Prabhakar <[email protected]>
---
arch/arm64/boot/dts/renesas/r9a07g043.dtsi | 2 ++
arch/arm64/boot/dts/renesas/r9a07g043u.dtsi | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
index 44b9bc6294be..afb1abaa8b9a 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
@@ -531,6 +531,8 @@ pinctrl: pinctrl@11030000 {
gpio-controller;
#gpio-cells = <2>;
gpio-ranges = <&pinctrl 0 0 152>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
clocks = <&cpg CPG_MOD R9A07G043_GPIO_HCLK>;
power-domains = <&cpg>;
resets = <&cpg R9A07G043_GPIO_RSTN>,
diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
index 7a8ed7ae253b..65e7b029361e 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
@@ -98,6 +98,10 @@ &irqc {
resets = <&cpg R9A07G043_IA55_RESETN>;
};
+&pinctrl {
+ interrupt-parent = <&irqc>;
+};
+
&soc {
interrupt-parent = <&gic>;
--
2.25.1
On 07/11/2022 18:53, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> identical to one found on the RZ/G2L SoC. No driver changes are
> required as generic compatible string "renesas,rzg2l-irqc" will be
> used as a fallback.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Prabhakar,
> Subject: [PATCH RFC 2/5] pinctrl: renesas: rzg2l: Fix configuring the GPIO
> pins as interrupts
>
> From: Lad Prabhakar <[email protected]>
>
> On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L and also
> the pin configs are completely different. This patch makes sure we use the
> appropriate pin configs for each SoC (which is passed as part of the OF
> data) while configuring the GPIO pin as interrupts instead of using
> rzg2l_gpio_configs[] for all the SoCs.
>
Looks like you are missing fixes tag.
Fixes: db2e5f21a48ed ("pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt")
As we have already pinctrl support for RZ/G2UL [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pinctrl/renesas/pinctrl-rzg2l.c?h=v6.1-rc4&id=bfc69bdbaad141ac408e6de86b7e0d771c8e3ccb
Cheers,
Biju
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/pinctrl/renesas/pinctrl-rzg2l.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index a43824fd9505..dcc495baa678 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -127,6 +127,7 @@ struct rzg2l_dedicated_configs { struct
> rzg2l_pinctrl_data {
> const char * const *port_pins;
> const u32 *port_pin_configs;
> + unsigned int n_port_pin_configs;
> struct rzg2l_dedicated_configs *dedicated_pins;
> unsigned int n_port_pins;
> unsigned int n_dedicated_pins;
> @@ -1122,7 +1123,7 @@ static struct {
> }
> };
>
> -static int rzg2l_gpio_get_gpioint(unsigned int virq)
> +static int rzg2l_gpio_get_gpioint(unsigned int virq, const struct
> +rzg2l_pinctrl_data *data)
> {
> unsigned int gpioint;
> unsigned int i;
> @@ -1131,13 +1132,13 @@ static int rzg2l_gpio_get_gpioint(unsigned int virq)
> port = virq / 8;
> bit = virq % 8;
>
> - if (port >= ARRAY_SIZE(rzg2l_gpio_configs) ||
> - bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
> + if (port >= data->n_port_pin_configs ||
> + bit >= RZG2L_GPIO_PORT_GET_PINCNT(data->port_pin_configs[port]))
> return -EINVAL;
>
> gpioint = bit;
> for (i = 0; i < port; i++)
> - gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
> + gpioint += RZG2L_GPIO_PORT_GET_PINCNT(data->port_pin_configs[i]);
>
> return gpioint;
> }
> @@ -1237,7 +1238,7 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct
> gpio_chip *gc,
> unsigned long flags;
> int gpioint, irq;
>
> - gpioint = rzg2l_gpio_get_gpioint(child);
> + gpioint = rzg2l_gpio_get_gpioint(child, pctrl->data);
> if (gpioint < 0)
> return gpioint;
>
> @@ -1311,8 +1312,8 @@ static void rzg2l_init_irq_valid_mask(struct gpio_chip
> *gc,
> port = offset / 8;
> bit = offset % 8;
>
> - if (port >= ARRAY_SIZE(rzg2l_gpio_configs) ||
> - bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
> + if (port >= pctrl->data->n_port_pin_configs ||
> + bit >=
> +RZG2L_GPIO_PORT_GET_PINCNT(pctrl->data->port_pin_configs[port]))
> clear_bit(offset, valid_mask);
> }
> }
> @@ -1517,6 +1518,7 @@ static int rzg2l_pinctrl_probe(struct platform_device
> *pdev) static struct rzg2l_pinctrl_data r9a07g043_data = {
> .port_pins = rzg2l_gpio_names,
> .port_pin_configs = r9a07g043_gpio_configs,
> + .n_port_pin_configs = ARRAY_SIZE(r9a07g043_gpio_configs),
> .dedicated_pins = rzg2l_dedicated_pins.common,
> .n_port_pins = ARRAY_SIZE(r9a07g043_gpio_configs) *
> RZG2L_PINS_PER_PORT,
> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common),
> @@ -1525,6 +1527,7 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> static struct rzg2l_pinctrl_data r9a07g044_data = {
> .port_pins = rzg2l_gpio_names,
> .port_pin_configs = rzg2l_gpio_configs,
> + .n_port_pin_configs = ARRAY_SIZE(rzg2l_gpio_configs),
> .dedicated_pins = rzg2l_dedicated_pins.common,
> .n_port_pins = ARRAY_SIZE(rzg2l_gpio_names),
> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> --
> 2.25.1
> -----Original Message-----
> From: Lad, Prabhakar <[email protected]>
> Sent: 08 November 2022 09:10
> To: Biju Das <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Marc Zyngier <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Geert Uytterhoeven
> <[email protected]>; Magnus Damm <[email protected]>; Linus Walleij
> <[email protected]>; [email protected]; linux-renesas-
> [email protected]; [email protected]; linux-
> [email protected]; Prabhakar Mahadev Lad <prabhakar.mahadev-
> [email protected]>
> Subject: Re: [PATCH RFC 2/5] pinctrl: renesas: rzg2l: Fix configuring the
> GPIO pins as interrupts
>
> Hi Biju,
>
> On Tue, Nov 8, 2022 at 7:14 AM Biju Das <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> >
> > > Subject: [PATCH RFC 2/5] pinctrl: renesas: rzg2l: Fix configuring
> > > the GPIO pins as interrupts
> > >
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L
> > > and also the pin configs are completely different. This patch makes
> > > sure we use the appropriate pin configs for each SoC (which is
> > > passed as part of the OF
> > > data) while configuring the GPIO pin as interrupts instead of using
> > > rzg2l_gpio_configs[] for all the SoCs.
> > >
> >
> > Looks like you are missing fixes tag.
> > Fixes: db2e5f21a48ed ("pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain
> > to handle GPIO interrupt")
> >
> I did think about but then I realised this fixes the GPIO IRQ functions only
> and we didn't support IRQC and GPIO interrupts up until now so I hadn't added
> the fixes tag.
Yep that is true, even though we have pinctrl support for both RZ/G2L and RZ/G2UL.
Interrupt support added only for RZ/G2L at that time. Maybe change to reflect
RZ/G2UL GPIO interrupt support.
Cheers,
Biju
Hi Biju,
On Tue, Nov 8, 2022 at 7:14 AM Biju Das <[email protected]> wrote:
>
> Hi Prabhakar,
>
>
> > Subject: [PATCH RFC 2/5] pinctrl: renesas: rzg2l: Fix configuring the GPIO
> > pins as interrupts
> >
> > From: Lad Prabhakar <[email protected]>
> >
> > On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L and also
> > the pin configs are completely different. This patch makes sure we use the
> > appropriate pin configs for each SoC (which is passed as part of the OF
> > data) while configuring the GPIO pin as interrupts instead of using
> > rzg2l_gpio_configs[] for all the SoCs.
> >
>
> Looks like you are missing fixes tag.
> Fixes: db2e5f21a48ed ("pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt")
>
I did think about but then I realised this fixes the GPIO IRQ
functions only and we didn't support IRQC and GPIO interrupts up until
now so I hadn't added the fixes tag.
Cheers,
Prabhakar
Hi Prabhakar,
On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Add IRQC node to R9A07G043 (RZ/G2UL) SoC DTSI.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Thanks for your patch!
> ---
> Note,
> - clocks and resets are differnt when compared to RZ/Five hence its added
> in r9a07g043u.dtsi
> - We have additional interrupt on RZ/Five hence interrupts are added in
> r9a07g043u.dtsi
Which additional interrupts?
Do you already have the r9a06g043f-variant ready, so we can compare?
> - clock-names is also added in r9a07g043u.dtsi to avoid dtbs_check warning
Why does this warn?
> --- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> @@ -538,6 +538,14 @@ pinctrl: pinctrl@11030000 {
> <&cpg R9A07G043_GPIO_SPARE_RESETN>;
> };
>
> + irqc: interrupt-controller@110a0000 {
> + #interrupt-cells = <2>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0 0x110a0000 0 0x10000>;
The size of the "reg" property will be 0x20000 on RZ/Five, to cover the
extra [NIT]MSK registers, so I think this belongs in r9a07g043u.dtsi, too.
> + power-domains = <&cpg>;
> + };
> +
> dmac: dma-controller@11820000 {
> compatible = "renesas,r9a07g043-dmac",
> "renesas,rz-dmac";
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Thank you for the review.
On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > identical to one found on the RZ/G2L SoC. No driver changes are
> > required as generic compatible string "renesas,rzg2l-irqc" will be
> > used as a fallback.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > ---
> > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > domain) -> RISCV INTC
>
> I think this difference is purely a software difference, and abstracted
> in DTS through the interrupt hierarchy.
> Does it have any impact on the bindings?
>
For now I dont know for sure, as I havent started looking into it yet.
> > - On the RZ/Five we have additional registers for IRQC block
>
> Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> warranting separate compatible values.
>
\o/
> > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
>
> Can you please elaborate? I may have missed something, but to me it
> looks like that is exactly the same on RZ/G2UL and on RZ/Five.
>
I completely missed rz/g2ul had this interrupt too.
Cheers,
Prabhakar
Hi Prabhakar,
On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> identical to one found on the RZ/G2L SoC. No driver changes are
> required as generic compatible string "renesas,rzg2l-irqc" will be
> used as a fallback.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Thanks for your patch!
> ---
> Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> domain) -> RISCV INTC
I think this difference is purely a software difference, and abstracted
in DTS through the interrupt hierarchy.
Does it have any impact on the bindings?
> - On the RZ/Five we have additional registers for IRQC block
Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
warranting separate compatible values.
> - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
Can you please elaborate? I may have missed something, but to me it
looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -26,6 +26,7 @@ properties:
> compatible:
> items:
> - enum:
> + - renesas,r9a07g043u-irqc # RZ/G2UL
> - renesas,r9a07g044-irqc # RZ/G2{L,LC}
> - renesas,r9a07g054-irqc # RZ/V2L
> - const: renesas,rzg2l-irqc
Reviewed-by: Geert Uytterhoeven <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Prabhakar,
On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L and also
> the pin configs are completely different. This patch makes sure we use the
> appropriate pin configs for each SoC (which is passed as part of the OF
> data) while configuring the GPIO pin as interrupts instead of using
> rzg2l_gpio_configs[] for all the SoCs.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <[email protected]>
But I do think there is room for improvement...
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -127,6 +127,7 @@ struct rzg2l_dedicated_configs {
> struct rzg2l_pinctrl_data {
> const char * const *port_pins;
> const u32 *port_pin_configs;
> + unsigned int n_port_pin_configs;
n_ports?
> struct rzg2l_dedicated_configs *dedicated_pins;
> unsigned int n_port_pins;
n_port_pins is now always n_port_pin_configs * RZG2L_PINS_PER_PORT,
right?
> unsigned int n_dedicated_pins;
> @@ -1517,6 +1518,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
> static struct rzg2l_pinctrl_data r9a07g043_data = {
> .port_pins = rzg2l_gpio_names,
> .port_pin_configs = r9a07g043_gpio_configs,
> + .n_port_pin_configs = ARRAY_SIZE(r9a07g043_gpio_configs),
> .dedicated_pins = rzg2l_dedicated_pins.common,
> .n_port_pins = ARRAY_SIZE(r9a07g043_gpio_configs) * RZG2L_PINS_PER_PORT,
> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common),
> @@ -1525,6 +1527,7 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> static struct rzg2l_pinctrl_data r9a07g044_data = {
> .port_pins = rzg2l_gpio_names,
.port_pins is always rzg2l_gpio_names
> .port_pin_configs = rzg2l_gpio_configs,
> + .n_port_pin_configs = ARRAY_SIZE(rzg2l_gpio_configs),
> .dedicated_pins = rzg2l_dedicated_pins.common,
> .n_port_pins = ARRAY_SIZE(rzg2l_gpio_names),
I think this should have become
ARRAY_SIZE(rzg2l_gpio_configs) * RZG2L_PINS_PER_PORT)
when support for r9a07g043 was introduced.
To avoid overflows when adding support for more SoCs, you can add a
bunch of checks like
BUILD_BUG_ON(ARRAY_SIZE(r9a07g043_gpio_configs) *
RZG2L_PINS_PER_PORT > ARRAY_SIZE(rzg2l_gpio_names))
BUILD_BUG_ON(ARRAY_SIZE(rzg2l_gpio_configs) * RZG2L_PINS_PER_PORT
> ARRAY_SIZE(rzg2l_gpio_names))
> .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Prabhakar,
On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> Add required properties in pinctrl node to handle GPIO interrupts.
>
> Note as IRQC is not enabled in RZ/Five the phandle for interrupt-parent
> is added in RZ/G2UL specific dtsi so that RZ/Five pinctrl driver
> continues without waiting for IRQC to probe.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
Thanks for your patch!
> --- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> @@ -531,6 +531,8 @@ pinctrl: pinctrl@11030000 {
> gpio-controller;
> #gpio-cells = <2>;
> gpio-ranges = <&pinctrl 0 0 152>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> clocks = <&cpg CPG_MOD R9A07G043_GPIO_HCLK>;
> power-domains = <&cpg>;
> resets = <&cpg R9A07G043_GPIO_RSTN>,
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> index 7a8ed7ae253b..65e7b029361e 100644
> --- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> @@ -98,6 +98,10 @@ &irqc {
> resets = <&cpg R9A07G043_IA55_RESETN>;
> };
>
> +&pinctrl {
> + interrupt-parent = <&irqc>;
> +};
Do you plan to move it back to the common r9a07g043.dtsi later?
Perhaps it makes sense to move the full irqc node to r9a07g043[uf].dtsi?
There is not that much common left, even the compatible value differs.
We don't keep the few common properties of the cpu0 node in
r9a07g043.dtsi neither.
> +
> &soc {
> interrupt-parent = <&gic>;
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Thank you for the review.
On Thu, Nov 17, 2022 at 11:09 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > On the RZ/G2UL SoC we have less number of pins compared to RZ/G2L and also
> > the pin configs are completely different. This patch makes sure we use the
> > appropriate pin configs for each SoC (which is passed as part of the OF
> > data) while configuring the GPIO pin as interrupts instead of using
> > rzg2l_gpio_configs[] for all the SoCs.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> But I do think there is room for improvement...
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -127,6 +127,7 @@ struct rzg2l_dedicated_configs {
> > struct rzg2l_pinctrl_data {
> > const char * const *port_pins;
> > const u32 *port_pin_configs;
> > + unsigned int n_port_pin_configs;
>
> n_ports?
>
Ok I will rename it to n_ports.
> > struct rzg2l_dedicated_configs *dedicated_pins;
> > unsigned int n_port_pins;
>
> n_port_pins is now always n_port_pin_configs * RZG2L_PINS_PER_PORT,
> right?
>
Yes, that's right. So are you suggesting to drop it and use it runtime instead?
> > unsigned int n_dedicated_pins;
>
> > @@ -1517,6 +1518,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
> > static struct rzg2l_pinctrl_data r9a07g043_data = {
> > .port_pins = rzg2l_gpio_names,
> > .port_pin_configs = r9a07g043_gpio_configs,
> > + .n_port_pin_configs = ARRAY_SIZE(r9a07g043_gpio_configs),
> > .dedicated_pins = rzg2l_dedicated_pins.common,
> > .n_port_pins = ARRAY_SIZE(r9a07g043_gpio_configs) * RZG2L_PINS_PER_PORT,
> > .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common),
> > @@ -1525,6 +1527,7 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> > static struct rzg2l_pinctrl_data r9a07g044_data = {
> > .port_pins = rzg2l_gpio_names,
>
> .port_pins is always rzg2l_gpio_names
>
Yes to avoid the huge array to be duplicated for other SoCs but bound
checking is done by n_port_pins.
> > .port_pin_configs = rzg2l_gpio_configs,
> > + .n_port_pin_configs = ARRAY_SIZE(rzg2l_gpio_configs),
> > .dedicated_pins = rzg2l_dedicated_pins.common,
> > .n_port_pins = ARRAY_SIZE(rzg2l_gpio_names),
>
> I think this should have become
> ARRAY_SIZE(rzg2l_gpio_configs) * RZG2L_PINS_PER_PORT)
> when support for r9a07g043 was introduced.
>
Agreed, I will update it as part of v2.
> To avoid overflows when adding support for more SoCs, you can add a
> bunch of checks like
>
> BUILD_BUG_ON(ARRAY_SIZE(r9a07g043_gpio_configs) *
> RZG2L_PINS_PER_PORT > ARRAY_SIZE(rzg2l_gpio_names))
> BUILD_BUG_ON(ARRAY_SIZE(rzg2l_gpio_configs) * RZG2L_PINS_PER_PORT
> > ARRAY_SIZE(rzg2l_gpio_names))
>
OK, I'll add those checks in the probe as a separate patch.
Cheers,
Prabhakar
Hi Geert,
Thank you for the review.
On Thu, Nov 17, 2022 at 11:13 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Add IRQC node to R9A07G043 (RZ/G2UL) SoC DTSI.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > ---
> > Note,
> > - clocks and resets are differnt when compared to RZ/Five hence its added
> > in r9a07g043u.dtsi
> > - We have additional interrupt on RZ/Five hence interrupts are added in
> > r9a07g043u.dtsi
>
> Which additional interrupts?
> Do you already have the r9a06g043f-variant ready, so we can compare?
>
I missed this RZ/G2UL has this interrupt too. So ignore this point.
> > - clock-names is also added in r9a07g043u.dtsi to avoid dtbs_check warning
>
> Why does this warn?
>
If we add the clock-names property in the base dtsi we get a
dtbs_check warning "clocks is a dependency of clock-names" for RZ/Five
as we haven't added the clocks property to it.
> > --- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> > @@ -538,6 +538,14 @@ pinctrl: pinctrl@11030000 {
> > <&cpg R9A07G043_GPIO_SPARE_RESETN>;
> > };
> >
> > + irqc: interrupt-controller@110a0000 {
> > + #interrupt-cells = <2>;
> > + #address-cells = <0>;
> > + interrupt-controller;
> > + reg = <0 0x110a0000 0 0x10000>;
>
> The size of the "reg" property will be 0x20000 on RZ/Five, to cover the
> extra [NIT]MSK registers, so I think this belongs in r9a07g043u.dtsi, too.
>
Agreed.
Cheers,
Prabhakar
Hi Geert,
Thank you for the review.
On Thu, Nov 17, 2022 at 11:20 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Add required properties in pinctrl node to handle GPIO interrupts.
> >
> > Note as IRQC is not enabled in RZ/Five the phandle for interrupt-parent
> > is added in RZ/G2UL specific dtsi so that RZ/Five pinctrl driver
> > continues without waiting for IRQC to probe.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g043.dtsi
> > @@ -531,6 +531,8 @@ pinctrl: pinctrl@11030000 {
> > gpio-controller;
> > #gpio-cells = <2>;
> > gpio-ranges = <&pinctrl 0 0 152>;
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > clocks = <&cpg CPG_MOD R9A07G043_GPIO_HCLK>;
> > power-domains = <&cpg>;
> > resets = <&cpg R9A07G043_GPIO_RSTN>,
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > index 7a8ed7ae253b..65e7b029361e 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g043u.dtsi
> > @@ -98,6 +98,10 @@ &irqc {
> > resets = <&cpg R9A07G043_IA55_RESETN>;
> > };
> >
> > +&pinctrl {
> > + interrupt-parent = <&irqc>;
> > +};
>
> Do you plan to move it back to the common r9a07g043.dtsi later?
Yes we should be doing that.
> Perhaps it makes sense to move the full irqc node to r9a07g043[uf].dtsi?
> There is not that much common left, even the compatible value differs.
> We don't keep the few common properties of the cpu0 node in
> r9a07g043.dtsi neither.
>
Agreed, I will move it in the next version.
Cheers,
Prabhakar
Hi Geert,
On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > identical to one found on the RZ/G2L SoC. No driver changes are
> > required as generic compatible string "renesas,rzg2l-irqc" will be
> > used as a fallback.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > ---
> > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > domain) -> RISCV INTC
>
> I think this difference is purely a software difference, and abstracted
> in DTS through the interrupt hierarchy.
> Does it have any impact on the bindings?
>
> > - On the RZ/Five we have additional registers for IRQC block
>
> Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> warranting separate compatible values.
>
> > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
>
> Can you please elaborate? I may have missed something, but to me it
> looks like that is exactly the same on RZ/G2UL and on RZ/Five.
>
Now that we have to update the binding doc with the BUS_ERR_INT too,
do you think it would make sense to add interrupt-names too?
BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
register a handler for it).
Cheers,
Prabhakar
Hi Geert,
On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
<[email protected]> wrote:
>
> Hi Geert,
>
> On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > used as a fallback.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> >
> > Thanks for your patch!
> >
> > > ---
> > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > domain) -> RISCV INTC
> >
> > I think this difference is purely a software difference, and abstracted
> > in DTS through the interrupt hierarchy.
> > Does it have any impact on the bindings?
> >
> > > - On the RZ/Five we have additional registers for IRQC block
> >
> > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > warranting separate compatible values.
> >
> > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> >
> > Can you please elaborate? I may have missed something, but to me it
> > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> >
> Now that we have to update the binding doc with the BUS_ERR_INT too,
> do you think it would make sense to add interrupt-names too?
>
> BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> register a handler for it).
>
Gentle ping.
Cheers,
Prabhakar
Hi Prabhakar,
On Mon, Dec 19, 2022 at 1:57 PM Lad, Prabhakar
<[email protected]> wrote:
> On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> > <[email protected]> wrote:
> > > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > > > From: Lad Prabhakar <[email protected]>
> > > >
> > > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > > used as a fallback.
> > > >
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > > domain) -> RISCV INTC
> > >
> > > I think this difference is purely a software difference, and abstracted
> > > in DTS through the interrupt hierarchy.
> > > Does it have any impact on the bindings?
> > >
> > > > - On the RZ/Five we have additional registers for IRQC block
> > >
> > > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > > warranting separate compatible values.
> > >
> > > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> > >
> > > Can you please elaborate? I may have missed something, but to me it
> > > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> > >
> > Now that we have to update the binding doc with the BUS_ERR_INT too,
> > do you think it would make sense to add interrupt-names too?
> Gentle ping.
Thanks for the ping, I had missed you were waiting on input from me.
Sorry for that...
As there are three different groups of parent interrupts, adding
interrupt-names makes sense. However, as this binding is already
in active use since v6.1, you probably need to keep on supporting the
ack of interrupt-names. Or do you think there are no real users yet,
and we can drop support for that?
> > BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> > register a handler for it).
Do you mean you will need a fourth parent type for that?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Mon, Dec 19, 2022 at 1:50 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Dec 19, 2022 at 1:57 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
> > <[email protected]> wrote:
> > > On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> > > <[email protected]> wrote:
> > > > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > > > > From: Lad Prabhakar <[email protected]>
> > > > >
> > > > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > > > used as a fallback.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <[email protected]>
>
> > > > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > > > domain) -> RISCV INTC
> > > >
> > > > I think this difference is purely a software difference, and abstracted
> > > > in DTS through the interrupt hierarchy.
> > > > Does it have any impact on the bindings?
> > > >
> > > > > - On the RZ/Five we have additional registers for IRQC block
> > > >
> > > > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > > > warranting separate compatible values.
> > > >
> > > > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> > > >
> > > > Can you please elaborate? I may have missed something, but to me it
> > > > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> > > >
> > > Now that we have to update the binding doc with the BUS_ERR_INT too,
> > > do you think it would make sense to add interrupt-names too?
>
> > Gentle ping.
>
> Thanks for the ping, I had missed you were waiting on input from me.
> Sorry for that...
>
No worries.
> As there are three different groups of parent interrupts, adding
> interrupt-names makes sense.
Ok.
> However, as this binding is already in active use since v6.1, you
> probably need to keep on supporting the
> ack of interrupt-names. Or do you think there are no real users yet,
> and we can drop support for that?
>
Sorry can you please elaborate on "ack of interrupt-names".
So moving forward the driver will first check for interrupt-names
property and if that exists it will map the IRQ0-7 and GPIO-TINIT
interrupts (based on the names it will create a hierarchy domain) and
for the NMI and BUS_ERR_INT we request the IRQ numbers and register
the IRQ handler in IRQC driver itself.
And for backward compatibility we parse the IRQ numbers based on
indexes i.e. 0 = NMI, 1-8 = IRQ 0-7 and 9-41 GPIO TINT interrupts.
> > > BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> > > register a handler for it).
>
> Do you mean you will need a fourth parent type for that?
>
No something like what we have for NMI we can add something similar
below for bus error interrupts:
interrupts = ....
<GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
interrupt-names = ....,
"bus-error-int";
As the registers to handle the NMI and BUS_ERR_INT are present on the
IRQC block, the interrupt handler will have to be registered by the
IRQC block itself by requesting the IRQ. So we will have to skip
mapping of BUS_ERR_INT as we do for the NMI case. Does that make
sense?
Cheers,
Prabhakar
Hi Prabhakar,
On Mon, Dec 19, 2022 at 3:26 PM Lad, Prabhakar
<[email protected]> wrote:
> On Mon, Dec 19, 2022 at 1:50 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Mon, Dec 19, 2022 at 1:57 PM Lad, Prabhakar
> > <[email protected]> wrote:
> > > On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
> > > <[email protected]> wrote:
> > > > On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> > > > <[email protected]> wrote:
> > > > > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > > > > > From: Lad Prabhakar <[email protected]>
> > > > > >
> > > > > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > > > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > > > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > > > > used as a fallback.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> >
> > > > > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > > > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > > > > domain) -> RISCV INTC
> > > > >
> > > > > I think this difference is purely a software difference, and abstracted
> > > > > in DTS through the interrupt hierarchy.
> > > > > Does it have any impact on the bindings?
> > > > >
> > > > > > - On the RZ/Five we have additional registers for IRQC block
> > > > >
> > > > > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > > > > warranting separate compatible values.
> > > > >
> > > > > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> > > > >
> > > > > Can you please elaborate? I may have missed something, but to me it
> > > > > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> > > > >
> > > > Now that we have to update the binding doc with the BUS_ERR_INT too,
> > > > do you think it would make sense to add interrupt-names too?
> >
> > > Gentle ping.
> >
> > Thanks for the ping, I had missed you were waiting on input from me.
> > Sorry for that...
> >
> No worries.
>
> > As there are three different groups of parent interrupts, adding
> > interrupt-names makes sense.
> Ok.
>
> > However, as this binding is already in active use since v6.1, you
> > probably need to keep on supporting the
> > ack of interrupt-names. Or do you think there are no real users yet,
> > and we can drop support for that?
> >
> Sorry can you please elaborate on "ack of interrupt-names".
Oops, s/ack/lack/. I.e. what you described below.
> So moving forward the driver will first check for interrupt-names
> property and if that exists it will map the IRQ0-7 and GPIO-TINIT
> interrupts (based on the names it will create a hierarchy domain) and
> for the NMI and BUS_ERR_INT we request the IRQ numbers and register
> the IRQ handler in IRQC driver itself.
>
> And for backward compatibility we parse the IRQ numbers based on
> indexes i.e. 0 = NMI, 1-8 = IRQ 0-7 and 9-41 GPIO TINT interrupts.
Exactly.
> > > > BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> > > > register a handler for it).
> >
> > Do you mean you will need a fourth parent type for that?
> >
> No something like what we have for NMI we can add something similar
> below for bus error interrupts:
> interrupts = ....
> <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
> interrupt-names = ....,
> "bus-error-int";
Hence a fourth name?
1. legacy index 0 -> "nmi"
2. legacy indices 1-8 -> "irq%u" (0-7)
3. legacy indices 9-41 -> "tint%u" (0-31)
4. (not supported) -> "bus-error-int" (or "bus-err"?)
> As the registers to handle the NMI and BUS_ERR_INT are present on the
> IRQC block, the interrupt handler will have to be registered by the
> IRQC block itself by requesting the IRQ. So we will have to skip
> mapping of BUS_ERR_INT as we do for the NMI case. Does that make
> sense?
OK.
BTW, that means RZG2L_NMI from <dt-bindings/interrupt-controller/irqc-rzg2l.h>
will never be used?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Mon, Dec 19, 2022 at 2:47 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Dec 19, 2022 at 3:26 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Mon, Dec 19, 2022 at 1:50 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Mon, Dec 19, 2022 at 1:57 PM Lad, Prabhakar
> > > <[email protected]> wrote:
> > > > On Fri, Nov 18, 2022 at 12:29 PM Lad, Prabhakar
> > > > <[email protected]> wrote:
> > > > > On Thu, Nov 17, 2022 at 10:54 AM Geert Uytterhoeven
> > > > > <[email protected]> wrote:
> > > > > > On Mon, Nov 7, 2022 at 6:53 PM Prabhakar <[email protected]> wrote:
> > > > > > > From: Lad Prabhakar <[email protected]>
> > > > > > >
> > > > > > > Document RZ/G2UL (R9A07G043) IRQC bindings. The RZ/G2UL IRQC block is
> > > > > > > identical to one found on the RZ/G2L SoC. No driver changes are
> > > > > > > required as generic compatible string "renesas,rzg2l-irqc" will be
> > > > > > > used as a fallback.
> > > > > > >
> > > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > >
> > > > > > > Note, renesas,r9a07g043u-irqc is added we have slight difference's compared to RZ/Five
> > > > > > > - G2UL IRQCHIP (hierarchical IRQ domain) -> GIC where as on RZ/Five we have PLIC (chained interrupt
> > > > > > > domain) -> RISCV INTC
> > > > > >
> > > > > > I think this difference is purely a software difference, and abstracted
> > > > > > in DTS through the interrupt hierarchy.
> > > > > > Does it have any impact on the bindings?
> > > > > >
> > > > > > > - On the RZ/Five we have additional registers for IRQC block
> > > > > >
> > > > > > Indeed, the NMI/IRQ/TINT "Interruput" Mask Control Registers, thus
> > > > > > warranting separate compatible values.
> > > > > >
> > > > > > > - On the RZ/Five we have BUS_ERR_INT which needs to be handled by IRQC
> > > > > >
> > > > > > Can you please elaborate? I may have missed something, but to me it
> > > > > > looks like that is exactly the same on RZ/G2UL and on RZ/Five.
> > > > > >
> > > > > Now that we have to update the binding doc with the BUS_ERR_INT too,
> > > > > do you think it would make sense to add interrupt-names too?
> > >
> > > > Gentle ping.
> > >
> > > Thanks for the ping, I had missed you were waiting on input from me.
> > > Sorry for that...
> > >
> > No worries.
> >
> > > As there are three different groups of parent interrupts, adding
> > > interrupt-names makes sense.
> > Ok.
> >
> > > However, as this binding is already in active use since v6.1, you
> > > probably need to keep on supporting the
> > > ack of interrupt-names. Or do you think there are no real users yet,
> > > and we can drop support for that?
> > >
> > Sorry can you please elaborate on "ack of interrupt-names".
>
> Oops, s/ack/lack/. I.e. what you described below.
>
Got that.
> > So moving forward the driver will first check for interrupt-names
> > property and if that exists it will map the IRQ0-7 and GPIO-TINIT
> > interrupts (based on the names it will create a hierarchy domain) and
> > for the NMI and BUS_ERR_INT we request the IRQ numbers and register
> > the IRQ handler in IRQC driver itself.
> >
> > And for backward compatibility we parse the IRQ numbers based on
> > indexes i.e. 0 = NMI, 1-8 = IRQ 0-7 and 9-41 GPIO TINT interrupts.
>
> Exactly.
>
> > > > > BUS_ERR_INT will have to be handled IRQC itself (i.e. IRQC will
> > > > > register a handler for it).
> > >
> > > Do you mean you will need a fourth parent type for that?
> > >
> > No something like what we have for NMI we can add something similar
> > below for bus error interrupts:
> > interrupts = ....
> > <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
> > interrupt-names = ....,
> > "bus-error-int";
>
> Hence a fourth name?
>
Agreed.
> 1. legacy index 0 -> "nmi"
> 2. legacy indices 1-8 -> "irq%u" (0-7)
> 3. legacy indices 9-41 -> "tint%u" (0-31)
> 4. (not supported) -> "bus-error-int" (or "bus-err"?)
>
"bus-err" I think based on previous experience ;)
While I am at it I'll expand the interrupts property with descriptions.
> > As the registers to handle the NMI and BUS_ERR_INT are present on the
> > IRQC block, the interrupt handler will have to be registered by the
> > IRQC block itself by requesting the IRQ. So we will have to skip
> > mapping of BUS_ERR_INT as we do for the NMI case. Does that make
> > sense?
>
> OK.
>
> BTW, that means RZG2L_NMI from <dt-bindings/interrupt-controller/irqc-rzg2l.h>
> will never be used?
>
Agreed, that needs to be dropped.
Cheers,
Prabhakar