* [RFC PATCH 0/4] Renesas RZ/G2L IRQC support
@ 2021-08-03 17:51 Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Lad Prabhakar @ 2021-08-03 17:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Linus Walleij, Magnus Damm, linux-gpio, devicetree
Cc: linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar
Hi All,
The RZ/G2L Interrupt Controller is a front-end for the GIC found on
Renesas RZ/G2L SoC's with below pins:
- IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
- GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
maximum of only 32 can be mapped to 32 GIC SPI interrupts,
- NMI edge select.
_____________
| GIC |
| ________ |
____________ | | | |
NMI ------------------------------------>| | SPI0-479 | | GIC-600| |
_______ | |------------>| | |
| | | | PPI16-31 | | | |
| | IRQ0-IRQ8 | IRQC |------------>| | |
P0_P48_4 ------>| GPIO |---------------->| | | |________| |
| |GPIOINT0-122 | | | |
| |---------------->| TINT0-31 | | |
|______| |__________| |____________|
The proposed RFC patches, add the IRQ domains in GPIO (pinctrl driver) and the
IRQC driver. The IRQC domain handles the actual SPI interrupt and upon reception
of the interrupt it propagates to the GPIO IRQ domain to handle virq.
Out of GPIOINT0-122 only 32 can be mapped to GIC SPI, this mapping is handled by
the IRQC driver.
GPIO interrupts TINT0-T31 support rising/falling/high/low trigger, support for
both falling/rising edges is handled by the SW by toggling the RISING/FALLING
(we might loose interrupts, I have done limited testing for SD card detection
where interrupt is requested for both rising and falling edge).
Please share your valuable comments on the above implementation.
Cheers,
Prabhakar
Lad Prabhakar (4):
dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
Controller
irqchip: Add RZ/G2L IA55 Interrupt Controller driver
pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO
interrupt
arm64: dts: renesas: r9a07g044: Add IRQC node to SoC DTSI
.../renesas,rzg2l-irqc.yaml | 129 ++++
arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 58 ++
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzg2l.c | 557 ++++++++++++++++++
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 +++++++
drivers/soc/renesas/Kconfig | 1 +
7 files changed, 959 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
@ 2021-08-03 17:51 ` Lad Prabhakar
2021-08-11 12:10 ` Linus Walleij
2021-08-11 18:39 ` Rob Herring
2021-08-03 17:51 ` [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver Lad Prabhakar
` (3 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Lad Prabhakar @ 2021-08-03 17:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Linus Walleij, Magnus Damm, linux-gpio, devicetree
Cc: linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar
Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
.../renesas,rzg2l-irqc.yaml | 129 ++++++++++++++++++
1 file changed, 129 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
new file mode 100644
index 000000000000..66d6a0ebe128
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L Interrupt Controller
+
+maintainers:
+ - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+ - Geert Uytterhoeven <geert+renesas@glider.be>
+
+description: |
+ The RZ/G2L Interrupt Controller is a front-end for the GIC found on Renesas RZ/G2L SoC's
+ - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts,
+ - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts,
+ - NMI edge select.
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - renesas,r9a07g044-irqc # RZ/G2L
+ - const: renesas,rzg2l-irqc
+
+ '#interrupt-cells':
+ const: 2
+
+ '#address-cells':
+ const: 0
+
+ interrupt-controller: true
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description: Specifies the GIC interrupts.
+ maxItems: 41
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: clk
+ - const: pclk
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - '#interrupt-cells'
+ - '#address-cells'
+ - interrupt-controller
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - power-domains
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/r9a07g044-cpg.h>
+
+ irqc: interrupt-controller@110a0000 {
+ compatible = "renesas,r9a07g044-irqc", "renesas,rzg2l-irqc";
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0x110a0000 0x10000>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A07G044_IA55_CLK>,
+ <&cpg CPG_MOD R9A07G044_IA55_PCLK>;
+ clock-names = "clk", "pclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_IA55_RESETN>;
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
@ 2021-08-03 17:51 ` Lad Prabhakar
2021-08-04 11:57 ` Marc Zyngier
2021-08-03 17:51 ` [RFC PATCH 3/4] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt Lad Prabhakar
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Lad Prabhakar @ 2021-08-03 17:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Linus Walleij, Magnus Damm, linux-gpio, devicetree
Cc: linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar
Add a driver for the Renesas RZ/G2L Interrupt Controller.
This supports external pins being used as interrupts. It supports
one line for NMI, 8 external pins and 32 GPIO pins (GPIOINT0-122)
to be used as IRQ lines.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzg2l.c | 557 ++++++++++++++++++++++++++++
drivers/soc/renesas/Kconfig | 1 +
4 files changed, 567 insertions(+)
create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 62543a4eccc0..790c19ad6e39 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -236,6 +236,14 @@ config RENESAS_RZA1_IRQC
Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
to 8 external interrupts with configurable sense select.
+config RENESAS_RZG2L_IRQC
+ bool "Renesas RZ/G2L IRQC support" if COMPILE_TEST
+ select GENERIC_IRQ_CHIP
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Enable support for the Renesas RZ/G2L Interrupt Controller for external
+ devices.
+
config SL28CPLD_INTC
bool "Kontron sl28cpld IRQ controller"
depends on MFD_SL28CPLD=y || COMPILE_TEST
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index f88cbf36a9d2..8017786fbdac 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC) += irq-rda-intc.o
obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
obj-$(CONFIG_RENESAS_RZA1_IRQC) += irq-renesas-rza1.o
+obj-$(CONFIG_RENESAS_RZG2L_IRQC) += irq-renesas-rzg2l.o
obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o
obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
new file mode 100644
index 000000000000..abfaaa8a09af
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -0,0 +1,557 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L IRQC Driver
+ *
+ * Copyright (C) 2020 Renesas Electronics Corporation.
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/clk.h>
+#include <linux/irqdomain.h>
+#include <linux/err.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+
+#define RZG2L_IRQC_IRQ_MAX 41
+#define RZG2L_TINT_MAX_INTERRUPTS 32
+#define RZG2L_TINT_IRQ_START_INDEX 9
+
+#define TSCR 0x20
+#define TITSR0 0x24
+#define TITSR1 0x28
+#define TSSR(n) (0x30 + ((n) * 4))
+#define IRQ_MASK 0x3
+
+#define RISING_EDGE 0
+#define FALLING_EDGE 1
+#define HIGH_LEVEL 2
+#define LOW_LEVEL 3
+
+#define RZG2L_TINT_EDGE_BOTH BIT(4)
+
+#define RZG2L_MAX_PINS_PER_PORT 8
+#define RZG2L_PIN_ID_TO_PORT(id) ((id) / RZG2L_MAX_PINS_PER_PORT)
+#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_MAX_PINS_PER_PORT)
+
+struct irqc_irq {
+ int hw_irq;
+ int requested_irq;
+ struct irqc_priv *p;
+ struct rzg2l_pin_info *pin;
+};
+
+struct rzg2l_pin_info {
+ u32 port;
+ u8 bit;
+ u32 id;
+ u32 trigger_type;
+ unsigned int type;
+ u8 pin_state;
+};
+
+struct irqc_priv {
+ void __iomem *iomem;
+ struct irqc_irq irq[RZG2L_IRQC_IRQ_MAX];
+ DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPTS);
+ unsigned int number_of_irqs;
+ struct device *dev;
+ struct irq_chip_generic *gc;
+ struct irq_chip chip;
+ struct irq_domain *irq_domain;
+ struct irq_domain *parent_domain;
+ unsigned int tint_irq_start;
+ atomic_t wakeup_path;
+ struct clk *clk;
+ struct clk *pclk;
+};
+
+static const struct rzg2l_pin_info gpio_pin_info[] = {
+ { 0, 0, 0 }, { 0, 1, 1 },
+ { 1, 0, 2 }, { 1, 1, 3 },
+ { 2, 0, 4 }, { 2, 1, 5 },
+ { 3, 0, 6 }, { 3, 1, 7 },
+ { 4, 0, 8 }, { 4, 1, 9 },
+ { 5, 0, 10 }, { 5, 1, 11 }, { 5, 2, 12 },
+ { 6, 0, 13 }, { 6, 1, 14 },
+ { 7, 0, 15 }, { 7, 1, 16 }, { 7, 2, 17 },
+ { 8, 0, 18 }, { 8, 1, 19 }, { 8, 2, 20 },
+ { 9, 0, 21 }, { 9, 1, 22 },
+ { 10, 0, 23 }, { 10, 1, 24 },
+ { 11, 0, 25 }, { 11, 1, 26 },
+ { 12, 0, 27 }, { 12, 1, 28 },
+ { 13, 0, 29 }, { 13, 1, 30 }, { 13, 2, 31 },
+ { 14, 0, 32 }, { 14, 1, 33 },
+ { 15, 0, 34 }, { 15, 1, 35 },
+ { 16, 0, 36 }, { 16, 1, 37 },
+ { 17, 0, 38 }, { 17, 1, 39 }, { 17, 2, 40 },
+ { 18, 0, 41 }, { 18, 1, 42 },
+ { 19, 0, 43 }, { 19, 1, 44 },
+ { 20, 0, 45 }, { 20, 1, 46 }, { 20, 2, 47 },
+ { 21, 0, 48 }, { 21, 1, 49 },
+ { 22, 0, 50 }, { 22, 1, 51 },
+ { 23, 0, 52 }, { 23, 1, 53 },
+ { 24, 0, 54 }, { 24, 1, 55 },
+ { 25, 0, 56 }, { 25, 1, 57 },
+ { 26, 0, 58 }, { 26, 1, 59 },
+ { 27, 0, 60 }, { 27, 1, 61 },
+ { 28, 0, 62 }, { 28, 1, 63 },
+ { 29, 0, 64 }, { 29, 1, 65 },
+ { 30, 0, 66 }, { 30, 1, 67 },
+ { 31, 0, 68 }, { 31, 1, 69 },
+ { 32, 0, 70 }, { 32, 1, 71 },
+ { 33, 0, 72 }, { 33, 1, 73 },
+ { 34, 0, 74 }, { 34, 1, 75 },
+ { 35, 0, 76 }, { 35, 1, 77 },
+ { 36, 0, 78 }, { 36, 1, 79 },
+ { 37, 0, 80 }, { 37, 1, 81 }, { 37, 2, 82 },
+ { 38, 0, 83 }, { 38, 1, 84 },
+ { 39, 0, 85 }, { 39, 1, 86 }, { 39, 2, 87 },
+ { 40, 0, 88 }, { 40, 1, 89 }, { 40, 2, 90 },
+ { 41, 0, 91 }, { 41, 1, 92 },
+ { 42, 0, 93 }, { 42, 1, 94 }, { 42, 2, 95 }, { 42, 3, 96 }, { 42, 4, 97 },
+ { 43, 0, 98 }, { 43, 1, 99 }, { 43, 2, 100 }, { 43, 3, 101 },
+ { 44, 0, 102 }, { 44, 1, 103 }, { 44, 2, 104 }, { 44, 3, 105 },
+ { 45, 0, 106 }, { 45, 1, 107 }, { 45, 2, 108 }, { 45, 3, 109 },
+ { 46, 0, 110 }, { 46, 1, 111 }, { 46, 2, 112 }, { 46, 3, 113 },
+ { 47, 0, 114 }, { 47, 1, 115 }, { 47, 2, 116 }, { 47, 3, 117 },
+ { 48, 0, 118 }, { 48, 1, 119 }, { 48, 2, 120 }, { 48, 3, 121 }, { 48, 3, 122 },
+};
+
+static int rzg2l_gpio_irq_validate_id(u32 port, u32 bit)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(gpio_pin_info); i++) {
+ if (gpio_pin_info[i].port == port && gpio_pin_info[i].bit == bit)
+ return gpio_pin_info[i].id;
+ }
+
+ return -EINVAL;
+}
+
+static struct irq_domain *rzg2l_get_pinctrl_domain(struct device *dev)
+{
+ struct device_node *pinctrl_np;
+ struct irq_domain *pinctrl_domain;
+
+ pinctrl_np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-pinctrl");
+ if (!pinctrl_np)
+ return NULL;
+
+ pinctrl_domain = irq_find_host(pinctrl_np);
+ of_node_put(pinctrl_np);
+
+ return pinctrl_domain;
+}
+
+static struct irqc_priv *irq_data_to_priv(struct irq_data *data)
+{
+ return data->domain->host_data;
+}
+
+static void rzg2l_tint_set_edge(struct irqc_priv *priv,
+ u32 bit, unsigned int irq_type)
+{
+ u32 port_bit = bit;
+ u32 reg;
+
+ if (port_bit <= 15) {
+ reg = readl(priv->iomem + TITSR0);
+ reg &= ~(IRQ_MASK << (port_bit * 2));
+ reg |= irq_type << (port_bit * 2);
+ writel(reg, priv->iomem + TITSR0);
+ } else {
+ reg = readl(priv->iomem + TITSR1);
+ port_bit = port_bit / 2;
+ reg &= ~(IRQ_MASK << (port_bit * 2));
+ reg |= irq_type << (port_bit * 2);
+ writel(reg, priv->iomem + TITSR1);
+ }
+}
+
+static int rzg2l_tint_set_type(struct irq_data *d, unsigned int type)
+{
+ struct rzg2l_pin_info *pin_info = d->chip_data;
+ struct irqc_priv *priv = irq_data_to_priv(d);
+ u8 irq_type;
+
+ pin_info->trigger_type = type & GENMASK(15, 0);
+ pin_info->pin_state = ((type & ~GENMASK(15, 0)) >> 15);
+ pin_info->type = pin_info->trigger_type;
+
+ if (pin_info->trigger_type & BIT(0))
+ irq_type = RISING_EDGE;
+ else if (pin_info->trigger_type & BIT(1))
+ irq_type = FALLING_EDGE;
+ else if (pin_info->trigger_type & BIT(2))
+ irq_type = HIGH_LEVEL;
+ else if (pin_info->trigger_type & BIT(3))
+ irq_type = LOW_LEVEL;
+ else if (pin_info->trigger_type & BIT(4))
+ irq_type = pin_info->pin_state ? FALLING_EDGE : RISING_EDGE;
+ else
+ return -EINVAL;
+
+ pin_info->trigger_type = irq_type;
+ rzg2l_tint_set_edge(priv, pin_info->bit, irq_type);
+
+ return 0;
+}
+
+static irqreturn_t rzg2l_irqc_tint_irq_handler(int irq, void *dev_id)
+{
+ struct irq_domain *pctl_irq_domain;
+ struct rzg2l_pin_info *pin_info;
+ struct irqc_irq *i = dev_id;
+ struct irqc_priv *priv = i->p;
+ struct irq_data *irq_data;
+ unsigned int offset;
+ u32 reg;
+
+ offset = irq - priv->irq[RZG2L_TINT_IRQ_START_INDEX].requested_irq;
+ pin_info = priv->irq[offset + RZG2L_TINT_IRQ_START_INDEX].pin;
+ if (!pin_info)
+ return IRQ_NONE;
+
+ irq_data = irq_domain_get_irq_data(priv->parent_domain, i->requested_irq);
+ if (!irq_data)
+ return IRQ_NONE;
+
+ pctl_irq_domain = rzg2l_get_pinctrl_domain(priv->dev);
+ if (!pctl_irq_domain)
+ return IRQ_NONE;
+
+ reg = readl(priv->iomem + TSCR);
+ reg &= ~BIT(offset);
+ writel(reg, priv->iomem + TSCR);
+
+ generic_handle_irq(irq_find_mapping(pctl_irq_domain,
+ ((pin_info->port * 8) + pin_info->bit)));
+
+ if (pin_info->type & BIT(4)) {
+ pin_info->trigger_type = (pin_info->trigger_type == FALLING_EDGE) ?
+ RISING_EDGE : FALLING_EDGE;
+ rzg2l_tint_set_edge(priv, pin_info->bit, pin_info->trigger_type);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void rzg2l_tint_irq_disable(struct irq_data *d)
+{
+ struct rzg2l_pin_info *pin_info = d->chip_data;
+ struct irqc_priv *priv = irq_data_to_priv(d);
+ u32 reg;
+
+ reg = readl(priv->iomem + TSSR(pin_info->id / 4));
+ reg &= ~(GENMASK(7, 0) << (pin_info->id % 4));
+ writel(reg, priv->iomem + TSSR(pin_info->id / 4));
+}
+
+static void rzg2l_tint_irq_enable(struct irq_data *d)
+{
+ struct rzg2l_pin_info *pin_info = d->chip_data;
+ struct irqc_priv *priv = irq_data_to_priv(d);
+ u32 gpioint;
+ u32 reg;
+
+ gpioint = rzg2l_gpio_irq_validate_id(pin_info->port, pin_info->bit);
+
+ reg = readl(priv->iomem + TSSR(pin_info->id / 4));
+ reg |= (BIT(7) | gpioint) << (8 * (pin_info->id % 4));
+ writel(reg, priv->iomem + TSSR(pin_info->id / 4));
+}
+
+static int rzg2l_irqc_domain_translate(struct irq_domain *domain,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ if (WARN_ON(fwspec->param_count < 2))
+ return -EINVAL;
+
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+
+ return 0;
+}
+
+static int rzg2l_irqc_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs,
+ void *arg)
+{
+ struct irqc_priv *priv = domain->host_data;
+ struct irq_fwspec parent_fwspec;
+ struct rzg2l_pin_info *pin_info;
+ struct irq_data *irq_data;
+ irq_hw_number_t hwirq;
+ unsigned int type;
+ int gpioint;
+ u32 port;
+ int irq;
+ int ret;
+ u8 bit;
+
+ ret = rzg2l_irqc_domain_translate(domain, arg, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ port = RZG2L_PIN_ID_TO_PORT(hwirq);
+ bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ case IRQ_TYPE_LEVEL_HIGH:
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ type = IRQ_TYPE_EDGE_RISING;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ type = IRQ_TYPE_LEVEL_HIGH;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ gpioint = rzg2l_gpio_irq_validate_id(port, bit);
+ if (gpioint < 0)
+ return gpioint;
+
+ irq = bitmap_find_free_region(priv->tint_slot, RZG2L_TINT_MAX_INTERRUPTS, get_order(1));
+ if (irq < 0)
+ return -ENOSPC;
+
+ irq_data = irq_domain_get_irq_data(priv->parent_domain,
+ priv->irq[RZG2L_TINT_IRQ_START_INDEX + irq].requested_irq);
+ if (!irq_data) {
+ bitmap_release_region(priv->tint_slot, irq, get_order(1));
+ return -EINVAL;
+ }
+
+ pin_info = kzalloc(sizeof(*pin_info), GFP_KERNEL);
+ if (!pin_info) {
+ bitmap_release_region(priv->tint_slot, irq, get_order(1));
+ return -ENOMEM;
+ }
+
+ priv->irq[RZG2L_TINT_IRQ_START_INDEX + irq].pin = pin_info;
+ pin_info->port = port;
+ pin_info->bit = bit;
+ pin_info->id = irq;
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, irq_data->hwirq,
+ &priv->chip,
+ pin_info);
+ if (ret) {
+ bitmap_release_region(priv->tint_slot, irq, get_order(1));
+ return ret;
+ }
+
+ /* parent is GIC */
+ parent_fwspec.fwnode = domain->parent->fwnode;
+ parent_fwspec.param_count = 3;
+ parent_fwspec.param[0] = 0; /* SPI */
+ parent_fwspec.param[1] = irq_data->hwirq;
+ parent_fwspec.param[2] = type;
+
+ ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
+ if (ret)
+ bitmap_release_region(priv->tint_slot, irq, get_order(1));
+
+ return ret;
+}
+
+static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct rzg2l_pin_info *pin_info;
+ struct irqc_priv *priv;
+ struct irq_data *data;
+
+ /* free the pin_info and release the bitmap */
+ data = irq_domain_get_irq_data(domain, virq);
+ if (data) {
+ priv = irq_data_to_priv(data);
+ pin_info = data->chip_data;
+ bitmap_release_region(priv->tint_slot, pin_info->id, get_order(1));
+ priv->irq[RZG2L_TINT_IRQ_START_INDEX + pin_info->id].pin = NULL;
+ kfree(pin_info);
+ }
+
+ irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
+ .translate = rzg2l_irqc_domain_translate,
+ .alloc = rzg2l_irqc_domain_alloc,
+ .free = rzg2l_irqc_domain_free,
+};
+
+static int irqc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const char *name = dev_name(dev);
+ struct irq_domain *parent_domain;
+ struct device_node *parent_np;
+ struct irqc_priv *p;
+ struct resource *irq;
+ int ret;
+ int k;
+
+ parent_np = of_irq_find_parent(dev->of_node);
+ if (!parent_np)
+ return -ENXIO;
+
+ parent_domain = irq_find_host(parent_np);
+ of_node_put(parent_np);
+ if (!parent_domain)
+ return -EPROBE_DEFER;
+
+ p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ p->parent_domain = parent_domain;
+ p->clk = devm_clk_get(&pdev->dev, "clk");
+ if (IS_ERR(p->clk))
+ return PTR_ERR(p->clk);
+
+ p->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(p->pclk))
+ return PTR_ERR(p->pclk);
+
+ ret = clk_prepare_enable(p->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable main clock, error %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(p->pclk);
+ if (ret) {
+ clk_disable_unprepare(p->clk);
+ dev_err(&pdev->dev, "failed to enable REG access clock, error %d\n", ret);
+ return ret;
+ }
+
+ p->dev = dev;
+ platform_set_drvdata(pdev, p);
+
+ pm_runtime_enable(dev);
+ pm_runtime_get_sync(dev);
+
+ for (k = 0; k < RZG2L_IRQC_IRQ_MAX; k++) {
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
+ if (!irq)
+ break;
+
+ p->irq[k].p = p;
+ p->irq[k].hw_irq = k;
+ p->irq[k].requested_irq = irq->start;
+ }
+
+ p->number_of_irqs = k;
+ if (p->number_of_irqs < 1) {
+ dev_err(dev, "not enough IRQ resources\n");
+ ret = -EINVAL;
+ goto err_runtime_pm_disable;
+ }
+
+ p->iomem = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(p->iomem)) {
+ ret = PTR_ERR(p->iomem);
+ goto err_runtime_pm_disable;
+ }
+
+ p->chip.name = "rzg2l-irqc",
+ p->chip.irq_disable = rzg2l_tint_irq_disable,
+ p->chip.irq_enable = rzg2l_tint_irq_enable,
+ p->chip.irq_set_type = rzg2l_tint_set_type,
+ p->chip.flags = IRQCHIP_MASK_ON_SUSPEND;
+
+ p->irq_domain = irq_domain_create_hierarchy(parent_domain, 0,
+ p->number_of_irqs,
+ of_node_to_fwnode(dev->of_node),
+ &rzg2l_irqc_domain_ops, p);
+ if (!p->irq_domain) {
+ ret = -ENXIO;
+ dev_err(dev, "cannot initialize irq domain\n");
+ goto err_runtime_pm_disable;
+ }
+
+ /* request interrupts one by one for TINT */
+ for (k = RZG2L_TINT_IRQ_START_INDEX; k < RZG2L_IRQC_IRQ_MAX; k++) {
+ if (devm_request_irq(dev, p->irq[k].requested_irq,
+ rzg2l_irqc_tint_irq_handler, IRQF_SHARED, name, &p->irq[k])) {
+ dev_err(dev, "failed to request IRQ\n");
+ ret = -ENOENT;
+ goto err_remove_domain;
+ }
+ }
+
+ dev_info(dev, "driving %d irqs\n", p->number_of_irqs);
+
+ return 0;
+
+err_remove_domain:
+ irq_domain_remove(p->irq_domain);
+err_runtime_pm_disable:
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+ return ret;
+}
+
+static int irqc_remove(struct platform_device *pdev)
+{
+ struct irqc_priv *p = platform_get_drvdata(pdev);
+
+ irq_domain_remove(p->irq_domain);
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ return 0;
+}
+
+static int __maybe_unused irqc_suspend(struct device *dev)
+{
+ struct irqc_priv *p = dev_get_drvdata(dev);
+
+ if (atomic_read(&p->wakeup_path))
+ device_set_wakeup_path(dev);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(irqc_pm_ops, irqc_suspend, NULL);
+
+static const struct of_device_id irqc_dt_ids[] = {
+ { .compatible = "renesas,rzg2l-irqc", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, irqc_dt_ids);
+
+static struct platform_driver irqc_device_driver = {
+ .probe = irqc_probe,
+ .remove = irqc_remove,
+ .driver = {
+ .name = "rzg2l_irqc",
+ .of_match_table = irqc_dt_ids,
+ .pm = &irqc_pm_ops,
+ }
+};
+
+static int __init irqc_init(void)
+{
+ return platform_driver_register(&irqc_device_driver);
+}
+postcore_initcall(irqc_init);
+
+static void __exit irqc_exit(void)
+{
+ platform_driver_unregister(&irqc_device_driver);
+}
+module_exit(irqc_exit);
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 71b44c31b012..b5059774cb45 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -281,6 +281,7 @@ config ARCH_R8A774B1
config ARCH_R9A07G044
bool "ARM64 Platform support for RZ/G2L"
+ select RENESAS_RZG2L_IRQC
help
This enables support for the Renesas RZ/G2L SoC variants.
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 3/4] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver Lad Prabhakar
@ 2021-08-03 17:51 ` Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 4/4] arm64: dts: renesas: r9a07g044: Add IRQC node to SoC DTSI Lad Prabhakar
2021-08-11 12:07 ` [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Linus Walleij
4 siblings, 0 replies; 11+ messages in thread
From: Lad Prabhakar @ 2021-08-03 17:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Linus Walleij, Magnus Damm, linux-gpio, devicetree
Cc: linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar
Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.
GPIO (0-122) pins can be used as IRQ lines but only 32 pins can be
used as IRQ lines at given time. Selection of pins as IRQ lines
is handled by IA55 (which is the IRQC block) which sits in between the
GPIO and GIC.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 ++++++++++++++++++++++++
1 file changed, 205 insertions(+)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 805117b2106e..1c0828206cea 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -11,6 +11,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinctrl.h>
@@ -87,6 +88,7 @@
#define PFC(n) (0x0400 + 0x40 + (n) * 4)
#define PIN(n) (0x0800 + 0x10 + (n))
#define IEN(n) (0x1800 + (n) * 8)
+#define ISEL(n) (0x2C80 + (n) * 8)
#define PWPR (0x3014)
#define SD_CH(n) (0x3000 + (n) * 4)
#define QSPI (0x3008)
@@ -108,6 +110,8 @@
#define RZG2L_PIN_ID_TO_PORT(id) ((id) / RZG2L_PINS_PER_PORT)
#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_PINS_PER_PORT)
+#define RZG2L_TINT_MAX_INTERRUPT 32
+
struct rzg2l_dedicated_configs {
const char *name;
u32 config;
@@ -133,6 +137,8 @@ struct rzg2l_pinctrl {
struct gpio_chip gpio_chip;
struct pinctrl_gpio_range gpio_range;
+ struct irq_chip irq_chip;
+ struct irq_domain *domain;
spinlock_t lock;
};
@@ -782,6 +788,22 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
rzg2l_gpio_direction_input(chip, offset);
}
+static int rzg2l_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+ struct irq_fwspec fwspec;
+
+ fwspec.fwnode = of_node_to_fwnode(chip->parent->of_node);
+ fwspec.param_count = 2;
+ fwspec.param[0] = offset;
+ /*
+ * IRQ_TYPE_NONE is rejected by the parent irq domain. Set LEVEL_HIGH
+ * temporarily. Anyway, ->irq_set_type() will override it later.
+ */
+ fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
+
+ return irq_create_fwspec_mapping(&fwspec);
+}
+
static const char * const rzg2l_gpio_names[] = {
"P0_0", "P0_1", "P0_2", "P0_3", "P0_4", "P0_5", "P0_6", "P0_7",
"P1_0", "P1_1", "P1_2", "P1_3", "P1_4", "P1_5", "P1_6", "P1_7",
@@ -965,14 +987,181 @@ static struct rzg2l_dedicated_configs rzg2l_dedicated_pins[] = {
{ "RIIC1_SCL", RZG2L_SINGLE_PIN_PACK(0xe, 3, PIN_CFG_IEN) },
};
+static int rzg2l_gpio_irq_domain_translate(struct irq_domain *domain,
+ struct irq_fwspec *fwspec,
+ unsigned long *out_hwirq,
+ unsigned int *out_type)
+{
+ if (WARN_ON(fwspec->param_count < 2))
+ return -EINVAL;
+
+ *out_hwirq = fwspec->param[0];
+ *out_type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+
+ return 0;
+}
+
+static int rzg2l_gpio_irq_domain_alloc(struct irq_domain *domain,
+ unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ struct rzg2l_pinctrl *pctrl = domain->host_data;
+ struct irq_fwspec parent_fwspec;
+ irq_hw_number_t hwirq;
+ unsigned int type;
+ int ret;
+
+ if (WARN_ON(nr_irqs != 1))
+ return -EINVAL;
+
+ ret = rzg2l_gpio_irq_domain_translate(domain, arg, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ /* parent is IRQC */
+ parent_fwspec.fwnode = domain->parent->fwnode;
+ parent_fwspec.param_count = 2;
+ parent_fwspec.param[0] = hwirq;
+ parent_fwspec.param[1] = (type == IRQ_TYPE_EDGE_BOTH) ?
+ IRQ_TYPE_EDGE_FALLING : type;
+
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+ &pctrl->irq_chip, pctrl);
+ if (ret)
+ return ret;
+
+ return irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
+}
+
+static const struct irq_domain_ops rzg2l_gpio_irq_domain_ops = {
+ .alloc = rzg2l_gpio_irq_domain_alloc,
+ .free = irq_domain_free_irqs_common,
+ .translate = rzg2l_gpio_irq_domain_translate,
+};
+
+static void rzg2l_gpio_irq_disable(struct irq_data *d)
+{
+ struct rzg2l_pinctrl *pctrl = d->chip_data;
+ int hwirq = irqd_to_hwirq(d);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 port;
+ u32 val;
+ u8 bit;
+
+ port = RZG2L_PIN_ID_TO_PORT(hwirq);
+ bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+ addr = pctrl->base + ISEL(port);
+ if (bit >= 4) {
+ bit -= 4;
+ addr += 4;
+ }
+
+ spin_lock_irqsave(&pctrl->lock, flags);
+ val = readl(addr);
+ val &= ~BIT(bit * 8);
+ writel(val, addr);
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+
+ irq_chip_disable_parent(d);
+}
+
+static void rzg2l_gpio_irq_enable(struct irq_data *d)
+{
+ struct rzg2l_pinctrl *pctrl = d->chip_data;
+ int hwirq = irqd_to_hwirq(d);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 port;
+ u32 val;
+ u8 bit;
+
+ port = RZG2L_PIN_ID_TO_PORT(hwirq);
+ bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+ addr = pctrl->base + ISEL(port);
+ if (bit >= 4) {
+ bit -= 4;
+ addr += 4;
+ }
+
+ spin_lock_irqsave(&pctrl->lock, flags);
+ val = readl(addr);
+ val |= BIT(bit * 8);
+ writel(val, addr);
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+
+ irq_chip_enable_parent(d);
+}
+
+static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct rzg2l_pinctrl *pctrl = d->chip_data;
+ int hwirq = irqd_to_hwirq(d);
+ unsigned int rzg2_irq_type;
+ u32 port;
+ u8 bit;
+
+ port = RZG2L_PIN_ID_TO_PORT(hwirq);
+ bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+ /*
+ * To handle IRQ_TYPE_EDGE_BOTH which is not natively supported
+ * by the HW we split the type and pass the current state of
+ * GPIO PIN BIT(16) to the parent domain and trigger it on
+ * rising/falling edge depending on the PIN state
+ */
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ rzg2_irq_type = BIT(0);
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ rzg2_irq_type = BIT(1);
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ rzg2_irq_type = BIT(2);
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ rzg2_irq_type = BIT(3);
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ rzg2_irq_type = BIT(4);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!!(readb(pctrl->base + PIN(port)) & BIT(bit)))
+ rzg2_irq_type |= BIT(16);
+
+ return irq_chip_set_type_parent(d, rzg2_irq_type);
+}
+
+static void rzg2l_gpio_irqc_eoi(struct irq_data *d)
+{
+}
+
static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
{
+ struct irq_chip *irq_chip = &pctrl->irq_chip;
struct device_node *np = pctrl->dev->of_node;
struct gpio_chip *chip = &pctrl->gpio_chip;
const char *name = dev_name(pctrl->dev);
+ struct irq_domain *parent_domain;
struct of_phandle_args of_args;
+ struct device_node *parent_np;
int ret;
+ parent_np = of_irq_find_parent(np);
+ if (!parent_np)
+ return -ENXIO;
+
+ parent_domain = irq_find_host(parent_np);
+ of_node_put(parent_np);
+ if (!parent_domain)
+ return -EPROBE_DEFER;
+
ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &of_args);
if (ret) {
dev_err(pctrl->dev, "Unable to parse gpio-ranges\n");
@@ -991,6 +1180,7 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
chip->get_direction = rzg2l_gpio_get_direction;
chip->direction_input = rzg2l_gpio_direction_input;
chip->direction_output = rzg2l_gpio_direction_output;
+ chip->to_irq = rzg2l_gpio_to_irq;
chip->get = rzg2l_gpio_get;
chip->set = rzg2l_gpio_set;
chip->label = name;
@@ -999,6 +1189,13 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
chip->base = -1;
chip->ngpio = of_args.args[2];
+ irq_chip->name = chip->label;
+ irq_chip->irq_disable = rzg2l_gpio_irq_disable;
+ irq_chip->irq_enable = rzg2l_gpio_irq_enable;
+ irq_chip->irq_set_type = rzg2l_gpio_irq_set_type;
+ irq_chip->irq_eoi = rzg2l_gpio_irqc_eoi;
+ irq_chip->flags = IRQCHIP_SET_TYPE_MASKED;
+
pctrl->gpio_range.id = 0;
pctrl->gpio_range.pin_base = 0;
pctrl->gpio_range.base = 0;
@@ -1011,6 +1208,14 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
return ret;
}
+ pctrl->domain = irq_domain_create_hierarchy(parent_domain, 0,
+ RZG2L_TINT_MAX_INTERRUPT,
+ of_node_to_fwnode(np),
+ &rzg2l_gpio_irq_domain_ops,
+ pctrl);
+ if (!pctrl->domain)
+ return -ENOMEM;
+
dev_dbg(pctrl->dev, "Registered gpio controller\n");
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH 4/4] arm64: dts: renesas: r9a07g044: Add IRQC node to SoC DTSI
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
` (2 preceding siblings ...)
2021-08-03 17:51 ` [RFC PATCH 3/4] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt Lad Prabhakar
@ 2021-08-03 17:51 ` Lad Prabhakar
2021-08-11 12:07 ` [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Linus Walleij
4 siblings, 0 replies; 11+ messages in thread
From: Lad Prabhakar @ 2021-08-03 17:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Linus Walleij, Magnus Damm, linux-gpio, devicetree
Cc: linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar
Add IRQC node to R9A07G044 (RZ/G2L) SoC DTSI.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 58 ++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 339646843415..c45aa6d2364e 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -95,6 +95,8 @@
pinctrl: pin-controller@11030000 {
compatible = "renesas,r9a07g044-pinctrl";
reg = <0 0x11030000 0 0x10000>;
+ interrupt-parent = <&irqc>;
+ interrupt-controller;
gpio-controller;
#gpio-cells = <2>;
gpio-ranges = <&pinctrl 0 0 392>;
@@ -410,6 +412,62 @@
status = "disabled";
};
+ irqc: interrupt-controller@110a0000 {
+ compatible = "renesas,r9a07g044-irqc",
+ "renesas,rzg2l-irqc";
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0 0x110a0000 0 0x10000>;
+ interrupts =
+ <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A07G044_IA55_CLK>,
+ <&cpg CPG_MOD R9A07G044_IA55_PCLK>;
+ clock-names = "clk", "pclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G044_IA55_RESETN>;
+ };
+
dmac: dma-controller@11820000 {
compatible = "renesas,r9a07g044-dmac",
"renesas,rz-dmac";
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
2021-08-03 17:51 ` [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver Lad Prabhakar
@ 2021-08-04 11:57 ` Marc Zyngier
2021-08-12 8:59 ` Lad, Prabhakar
0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2021-08-04 11:57 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Thomas Gleixner, Rob Herring, Linus Walleij,
Magnus Damm, linux-gpio, devicetree, linux-renesas-soc,
Prabhakar, Biju Das
On Tue, 03 Aug 2021 18:51:07 +0100,
Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Add a driver for the Renesas RZ/G2L Interrupt Controller.
>
> This supports external pins being used as interrupts. It supports
> one line for NMI, 8 external pins and 32 GPIO pins (GPIOINT0-122)
> to be used as IRQ lines.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> drivers/irqchip/Kconfig | 8 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-renesas-rzg2l.c | 557 ++++++++++++++++++++++++++++
> drivers/soc/renesas/Kconfig | 1 +
> 4 files changed, 567 insertions(+)
> create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 62543a4eccc0..790c19ad6e39 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -236,6 +236,14 @@ config RENESAS_RZA1_IRQC
> Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
> to 8 external interrupts with configurable sense select.
>
> +config RENESAS_RZG2L_IRQC
> + bool "Renesas RZ/G2L IRQC support" if COMPILE_TEST
> + select GENERIC_IRQ_CHIP
> + select IRQ_DOMAIN_HIERARCHY
> + help
> + Enable support for the Renesas RZ/G2L Interrupt Controller for external
> + devices.
> +
> config SL28CPLD_INTC
> bool "Kontron sl28cpld IRQ controller"
> depends on MFD_SL28CPLD=y || COMPILE_TEST
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index f88cbf36a9d2..8017786fbdac 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC) += irq-rda-intc.o
> obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
> obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
> obj-$(CONFIG_RENESAS_RZA1_IRQC) += irq-renesas-rza1.o
> +obj-$(CONFIG_RENESAS_RZG2L_IRQC) += irq-renesas-rzg2l.o
> obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
> obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o
> obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> new file mode 100644
> index 000000000000..abfaaa8a09af
> --- /dev/null
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -0,0 +1,557 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G2L IRQC Driver
> + *
> + * Copyright (C) 2020 Renesas Electronics Corporation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/clk.h>
> +#include <linux/irqdomain.h>
> +#include <linux/err.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +
> +#define RZG2L_IRQC_IRQ_MAX 41
> +#define RZG2L_TINT_MAX_INTERRUPTS 32
> +#define RZG2L_TINT_IRQ_START_INDEX 9
> +
> +#define TSCR 0x20
> +#define TITSR0 0x24
> +#define TITSR1 0x28
> +#define TSSR(n) (0x30 + ((n) * 4))
> +#define IRQ_MASK 0x3
> +
> +#define RISING_EDGE 0
> +#define FALLING_EDGE 1
> +#define HIGH_LEVEL 2
> +#define LOW_LEVEL 3
> +
> +#define RZG2L_TINT_EDGE_BOTH BIT(4)
> +
> +#define RZG2L_MAX_PINS_PER_PORT 8
> +#define RZG2L_PIN_ID_TO_PORT(id) ((id) / RZG2L_MAX_PINS_PER_PORT)
> +#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_MAX_PINS_PER_PORT)
> +
> +struct irqc_irq {
> + int hw_irq;
> + int requested_irq;
> + struct irqc_priv *p;
> + struct rzg2l_pin_info *pin;
> +};
> +
> +struct rzg2l_pin_info {
> + u32 port;
> + u8 bit;
> + u32 id;
Given the way you initialise things below, are these types the right
ones?
> + u32 trigger_type;
> + unsigned int type;
> + u8 pin_state;
> +};
> +
> +struct irqc_priv {
> + void __iomem *iomem;
> + struct irqc_irq irq[RZG2L_IRQC_IRQ_MAX];
> + DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPTS);
> + unsigned int number_of_irqs;
> + struct device *dev;
> + struct irq_chip_generic *gc;
> + struct irq_chip chip;
> + struct irq_domain *irq_domain;
> + struct irq_domain *parent_domain;
> + unsigned int tint_irq_start;
> + atomic_t wakeup_path;
> + struct clk *clk;
> + struct clk *pclk;
> +};
> +
> +static const struct rzg2l_pin_info gpio_pin_info[] = {
> + { 0, 0, 0 }, { 0, 1, 1 },
> + { 1, 0, 2 }, { 1, 1, 3 },
> + { 2, 0, 4 }, { 2, 1, 5 },
> + { 3, 0, 6 }, { 3, 1, 7 },
> + { 4, 0, 8 }, { 4, 1, 9 },
> + { 5, 0, 10 }, { 5, 1, 11 }, { 5, 2, 12 },
> + { 6, 0, 13 }, { 6, 1, 14 },
> + { 7, 0, 15 }, { 7, 1, 16 }, { 7, 2, 17 },
> + { 8, 0, 18 }, { 8, 1, 19 }, { 8, 2, 20 },
> + { 9, 0, 21 }, { 9, 1, 22 },
> + { 10, 0, 23 }, { 10, 1, 24 },
> + { 11, 0, 25 }, { 11, 1, 26 },
> + { 12, 0, 27 }, { 12, 1, 28 },
> + { 13, 0, 29 }, { 13, 1, 30 }, { 13, 2, 31 },
> + { 14, 0, 32 }, { 14, 1, 33 },
> + { 15, 0, 34 }, { 15, 1, 35 },
> + { 16, 0, 36 }, { 16, 1, 37 },
> + { 17, 0, 38 }, { 17, 1, 39 }, { 17, 2, 40 },
> + { 18, 0, 41 }, { 18, 1, 42 },
> + { 19, 0, 43 }, { 19, 1, 44 },
> + { 20, 0, 45 }, { 20, 1, 46 }, { 20, 2, 47 },
> + { 21, 0, 48 }, { 21, 1, 49 },
> + { 22, 0, 50 }, { 22, 1, 51 },
> + { 23, 0, 52 }, { 23, 1, 53 },
> + { 24, 0, 54 }, { 24, 1, 55 },
> + { 25, 0, 56 }, { 25, 1, 57 },
> + { 26, 0, 58 }, { 26, 1, 59 },
> + { 27, 0, 60 }, { 27, 1, 61 },
> + { 28, 0, 62 }, { 28, 1, 63 },
> + { 29, 0, 64 }, { 29, 1, 65 },
> + { 30, 0, 66 }, { 30, 1, 67 },
> + { 31, 0, 68 }, { 31, 1, 69 },
> + { 32, 0, 70 }, { 32, 1, 71 },
> + { 33, 0, 72 }, { 33, 1, 73 },
> + { 34, 0, 74 }, { 34, 1, 75 },
> + { 35, 0, 76 }, { 35, 1, 77 },
> + { 36, 0, 78 }, { 36, 1, 79 },
> + { 37, 0, 80 }, { 37, 1, 81 }, { 37, 2, 82 },
> + { 38, 0, 83 }, { 38, 1, 84 },
> + { 39, 0, 85 }, { 39, 1, 86 }, { 39, 2, 87 },
> + { 40, 0, 88 }, { 40, 1, 89 }, { 40, 2, 90 },
> + { 41, 0, 91 }, { 41, 1, 92 },
> + { 42, 0, 93 }, { 42, 1, 94 }, { 42, 2, 95 }, { 42, 3, 96 }, { 42, 4, 97 },
> + { 43, 0, 98 }, { 43, 1, 99 }, { 43, 2, 100 }, { 43, 3, 101 },
> + { 44, 0, 102 }, { 44, 1, 103 }, { 44, 2, 104 }, { 44, 3, 105 },
> + { 45, 0, 106 }, { 45, 1, 107 }, { 45, 2, 108 }, { 45, 3, 109 },
> + { 46, 0, 110 }, { 46, 1, 111 }, { 46, 2, 112 }, { 46, 3, 113 },
> + { 47, 0, 114 }, { 47, 1, 115 }, { 47, 2, 116 }, { 47, 3, 117 },
> + { 48, 0, 118 }, { 48, 1, 119 }, { 48, 2, 120 }, { 48, 3, 121 }, { 48, 3, 122 },
> +};
Is there any reason why this information is not kept in DT instead?
What does it describe exactly?
> +
> +static int rzg2l_gpio_irq_validate_id(u32 port, u32 bit)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(gpio_pin_info); i++) {
> + if (gpio_pin_info[i].port == port && gpio_pin_info[i].bit == bit)
> + return gpio_pin_info[i].id;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static struct irq_domain *rzg2l_get_pinctrl_domain(struct device *dev)
> +{
> + struct device_node *pinctrl_np;
> + struct irq_domain *pinctrl_domain;
> +
> + pinctrl_np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-pinctrl");
> + if (!pinctrl_np)
> + return NULL;
> +
> + pinctrl_domain = irq_find_host(pinctrl_np);
> + of_node_put(pinctrl_np);
> +
> + return pinctrl_domain;
What if you have multiple instances of this pinctrl? The information
should directly come from DT in the form of a phandle.
> +}
> +
> +static struct irqc_priv *irq_data_to_priv(struct irq_data *data)
> +{
> + return data->domain->host_data;
> +}
> +
> +static void rzg2l_tint_set_edge(struct irqc_priv *priv,
> + u32 bit, unsigned int irq_type)
> +{
> + u32 port_bit = bit;
> + u32 reg;
> +
> + if (port_bit <= 15) {
> + reg = readl(priv->iomem + TITSR0);
> + reg &= ~(IRQ_MASK << (port_bit * 2));
> + reg |= irq_type << (port_bit * 2);
> + writel(reg, priv->iomem + TITSR0);
> + } else {
> + reg = readl(priv->iomem + TITSR1);
> + port_bit = port_bit / 2;
> + reg &= ~(IRQ_MASK << (port_bit * 2));
> + reg |= irq_type << (port_bit * 2);
> + writel(reg, priv->iomem + TITSR1);
> + }
This will result in corruption if two interrupts change their type at
the same time. Also, please use relaxed accessors.
> +}
> +
> +static int rzg2l_tint_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct rzg2l_pin_info *pin_info = d->chip_data;
> + struct irqc_priv *priv = irq_data_to_priv(d);
> + u8 irq_type;
> +
> + pin_info->trigger_type = type & GENMASK(15, 0);
> + pin_info->pin_state = ((type & ~GENMASK(15, 0)) >> 15);
This shift is obviously wrong. Also, why are you tracking random
information independently of the core?
> + pin_info->type = pin_info->trigger_type;
More pointless tracking...
> +
> + if (pin_info->trigger_type & BIT(0))
> + irq_type = RISING_EDGE;
> + else if (pin_info->trigger_type & BIT(1))
> + irq_type = FALLING_EDGE;
> + else if (pin_info->trigger_type & BIT(2))
> + irq_type = HIGH_LEVEL;
> + else if (pin_info->trigger_type & BIT(3))
> + irq_type = LOW_LEVEL;
> + else if (pin_info->trigger_type & BIT(4))
> + irq_type = pin_info->pin_state ? FALLING_EDGE : RISING_EDGE;
> + else
> + return -EINVAL;
... considering that this can fail. And please use the macros that
already exist rather than these BIT(x).
> +
> + pin_info->trigger_type = irq_type;
> + rzg2l_tint_set_edge(priv, pin_info->bit, irq_type);
> +
> + return 0;
> +}
> +
> +static irqreturn_t rzg2l_irqc_tint_irq_handler(int irq, void *dev_id)
> +{
> + struct irq_domain *pctl_irq_domain;
> + struct rzg2l_pin_info *pin_info;
> + struct irqc_irq *i = dev_id;
> + struct irqc_priv *priv = i->p;
> + struct irq_data *irq_data;
> + unsigned int offset;
> + u32 reg;
> +
> + offset = irq - priv->irq[RZG2L_TINT_IRQ_START_INDEX].requested_irq;
> + pin_info = priv->irq[offset + RZG2L_TINT_IRQ_START_INDEX].pin;
> + if (!pin_info)
> + return IRQ_NONE;
> +
> + irq_data = irq_domain_get_irq_data(priv->parent_domain, i->requested_irq);
> + if (!irq_data)
> + return IRQ_NONE;
> +
> + pctl_irq_domain = rzg2l_get_pinctrl_domain(priv->dev);
Are you really comfortable with parsing the device tree on each
interrupt you get? This is madness. It also makes no sense to resolve
an interrupt in a foreign irqdomain. Why don't you allocate yours?
> + if (!pctl_irq_domain)
> + return IRQ_NONE;
> +
> + reg = readl(priv->iomem + TSCR);
> + reg &= ~BIT(offset);
> + writel(reg, priv->iomem + TSCR);
> +
> + generic_handle_irq(irq_find_mapping(pctl_irq_domain,
> + ((pin_info->port * 8) + pin_info->bit)));
generic_handle_domain_irq() is your new friend.
> +
> + if (pin_info->type & BIT(4)) {
> + pin_info->trigger_type = (pin_info->trigger_type == FALLING_EDGE) ?
> + RISING_EDGE : FALLING_EDGE;
> + rzg2l_tint_set_edge(priv, pin_info->bit, pin_info->trigger_type);
> + }
This has hardly any chance of working reliably. Yes, plenty of drivers
do that. They are all doomed to fail.
> +
> + return IRQ_HANDLED;
No way. This isn't a standard interrupt handler. This is a mux, with a
number of inputs for each of the *49* outputs. Please implement this
as a chained interrupt controller.
> +}
> +
> +static void rzg2l_tint_irq_disable(struct irq_data *d)
> +{
> + struct rzg2l_pin_info *pin_info = d->chip_data;
> + struct irqc_priv *priv = irq_data_to_priv(d);
> + u32 reg;
> +
> + reg = readl(priv->iomem + TSSR(pin_info->id / 4));
> + reg &= ~(GENMASK(7, 0) << (pin_info->id % 4));
> + writel(reg, priv->iomem + TSSR(pin_info->id / 4));
More buggy RMW, magic numbers all over the shop. Does this act as a
disable (pending state is lost) or as a mask (pending state is
preserved)?
> +}
> +
> +static void rzg2l_tint_irq_enable(struct irq_data *d)
> +{
> + struct rzg2l_pin_info *pin_info = d->chip_data;
> + struct irqc_priv *priv = irq_data_to_priv(d);
> + u32 gpioint;
> + u32 reg;
> +
> + gpioint = rzg2l_gpio_irq_validate_id(pin_info->port, pin_info->bit);
> +
> + reg = readl(priv->iomem + TSSR(pin_info->id / 4));
> + reg |= (BIT(7) | gpioint) << (8 * (pin_info->id % 4));
> + writel(reg, priv->iomem + TSSR(pin_info->id / 4));
On top of the buggy RMW, what happens when
rzg2l_gpio_irq_validate_id() returns -EINVAL?
> +}
> +
> +static int rzg2l_irqc_domain_translate(struct irq_domain *domain,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + if (WARN_ON(fwspec->param_count < 2))
> + return -EINVAL;
> +
> + *hwirq = fwspec->param[0];
> + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> +
> + return 0;
> +}
This is irq_domain_translate_twocell().
> +
> +static int rzg2l_irqc_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs,
> + void *arg)
> +{
> + struct irqc_priv *priv = domain->host_data;
> + struct irq_fwspec parent_fwspec;
> + struct rzg2l_pin_info *pin_info;
> + struct irq_data *irq_data;
> + irq_hw_number_t hwirq;
> + unsigned int type;
> + int gpioint;
> + u32 port;
> + int irq;
> + int ret;
> + u8 bit;
> +
> + ret = rzg2l_irqc_domain_translate(domain, arg, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + port = RZG2L_PIN_ID_TO_PORT(hwirq);
> + bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + case IRQ_TYPE_LEVEL_HIGH:
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + type = IRQ_TYPE_EDGE_RISING;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + type = IRQ_TYPE_LEVEL_HIGH;
Really? How is that handled?
I stopped reading at this stage. This needs to be redesigned from the
ground up as an interrupt controller instead of an interrupt handler,
have proper locking, data structures that are not redundant with that
of the core code, and be written in a way that is self documenting.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 0/4] Renesas RZ/G2L IRQC support
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
` (3 preceding siblings ...)
2021-08-03 17:51 ` [RFC PATCH 4/4] arm64: dts: renesas: r9a07g044: Add IRQC node to SoC DTSI Lad Prabhakar
@ 2021-08-11 12:07 ` Linus Walleij
4 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2021-08-11 12:07 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Magnus Damm, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas, Prabhakar, Biju Das
On Tue, Aug 3, 2021 at 7:51 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> The RZ/G2L Interrupt Controller is a front-end for the GIC found on
> Renesas RZ/G2L SoC's with below pins:
> - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> - GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
> maximum of only 32 can be mapped to 32 GIC SPI interrupts,
This looks good to me but I count on Geert to do final review, merge and
send pull requests for everything Renesas.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
@ 2021-08-11 12:10 ` Linus Walleij
2021-08-12 9:05 ` Lad, Prabhakar
2021-08-11 18:39 ` Rob Herring
1 sibling, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2021-08-11 12:10 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Rob Herring,
Magnus Damm, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas, Prabhakar, Biju Das
On Tue, Aug 3, 2021 at 7:51 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> +description: |
> + The RZ/G2L Interrupt Controller is a front-end for the GIC found on Renesas RZ/G2L SoC's
> + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts,
> + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts,
> + - NMI edge select.
Not that we don't have weird documentation but what on earth is an
"NMI edge"???
I know about rising and falling edges, and I know about non-maskable
interrupts. But NMI edge? Maybe expand this to explain what it is?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
2021-08-11 12:10 ` Linus Walleij
@ 2021-08-11 18:39 ` Rob Herring
1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-08-11 18:39 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier, Linus Walleij,
Magnus Damm, linux-gpio, devicetree, linux-renesas-soc,
Prabhakar, Biju Das
On Tue, Aug 03, 2021 at 06:51:06PM +0100, Lad Prabhakar wrote:
> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../renesas,rzg2l-irqc.yaml | 129 ++++++++++++++++++
> 1 file changed, 129 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> new file mode 100644
> index 000000000000..66d6a0ebe128
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L Interrupt Controller
> +
> +maintainers:
> + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> + - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description: |
> + The RZ/G2L Interrupt Controller is a front-end for the GIC found on Renesas RZ/G2L SoC's
missing 'with below pins:"?
> + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts,
> + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts,
> + - NMI edge select.
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r9a07g044-irqc # RZ/G2L
> + - const: renesas,rzg2l-irqc
> +
> + '#interrupt-cells':
> + const: 2
> +
> + '#address-cells':
> + const: 0
> +
> + interrupt-controller: true
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description: Specifies the GIC interrupts.
> + maxItems: 41
> +
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: clk
> + - const: pclk
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - '#interrupt-cells'
> + - '#address-cells'
> + - interrupt-controller
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - power-domains
> + - resets
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/r9a07g044-cpg.h>
> +
> + irqc: interrupt-controller@110a0000 {
> + compatible = "renesas,r9a07g044-irqc", "renesas,rzg2l-irqc";
> + #interrupt-cells = <2>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0x110a0000 0x10000>;
> + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD R9A07G044_IA55_CLK>,
> + <&cpg CPG_MOD R9A07G044_IA55_PCLK>;
> + clock-names = "clk", "pclk";
> + power-domains = <&cpg>;
> + resets = <&cpg R9A07G044_IA55_RESETN>;
> + };
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
2021-08-04 11:57 ` Marc Zyngier
@ 2021-08-12 8:59 ` Lad, Prabhakar
0 siblings, 0 replies; 11+ messages in thread
From: Lad, Prabhakar @ 2021-08-12 8:59 UTC (permalink / raw)
To: Marc Zyngier
Cc: Lad Prabhakar, Geert Uytterhoeven, Thomas Gleixner, Rob Herring,
Linus Walleij, Magnus Damm, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas, Biju Das
Hi Marc,
Thank you for the review.
On Wed, Aug 4, 2021 at 12:57 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 03 Aug 2021 18:51:07 +0100,
> Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> >
> > This supports external pins being used as interrupts. It supports
> > one line for NMI, 8 external pins and 32 GPIO pins (GPIOINT0-122)
> > to be used as IRQ lines.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > drivers/irqchip/Kconfig | 8 +
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-renesas-rzg2l.c | 557 ++++++++++++++++++++++++++++
> > drivers/soc/renesas/Kconfig | 1 +
> > 4 files changed, 567 insertions(+)
> > create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 62543a4eccc0..790c19ad6e39 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -236,6 +236,14 @@ config RENESAS_RZA1_IRQC
> > Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
> > to 8 external interrupts with configurable sense select.
> >
> > +config RENESAS_RZG2L_IRQC
> > + bool "Renesas RZ/G2L IRQC support" if COMPILE_TEST
> > + select GENERIC_IRQ_CHIP
> > + select IRQ_DOMAIN_HIERARCHY
> > + help
> > + Enable support for the Renesas RZ/G2L Interrupt Controller for external
> > + devices.
> > +
> > config SL28CPLD_INTC
> > bool "Kontron sl28cpld IRQ controller"
> > depends on MFD_SL28CPLD=y || COMPILE_TEST
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index f88cbf36a9d2..8017786fbdac 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC) += irq-rda-intc.o
> > obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
> > obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
> > obj-$(CONFIG_RENESAS_RZA1_IRQC) += irq-renesas-rza1.o
> > +obj-$(CONFIG_RENESAS_RZG2L_IRQC) += irq-renesas-rzg2l.o
> > obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
> > obj-$(CONFIG_ARCH_NSPIRE) += irq-zevio.o
> > obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o
> > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> > new file mode 100644
> > index 000000000000..abfaaa8a09af
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -0,0 +1,557 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G2L IRQC Driver
> > + *
> > + * Copyright (C) 2020 Renesas Electronics Corporation.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/ioport.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/clk.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/err.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/slab.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#define RZG2L_IRQC_IRQ_MAX 41
> > +#define RZG2L_TINT_MAX_INTERRUPTS 32
> > +#define RZG2L_TINT_IRQ_START_INDEX 9
> > +
> > +#define TSCR 0x20
> > +#define TITSR0 0x24
> > +#define TITSR1 0x28
> > +#define TSSR(n) (0x30 + ((n) * 4))
> > +#define IRQ_MASK 0x3
> > +
> > +#define RISING_EDGE 0
> > +#define FALLING_EDGE 1
> > +#define HIGH_LEVEL 2
> > +#define LOW_LEVEL 3
> > +
> > +#define RZG2L_TINT_EDGE_BOTH BIT(4)
> > +
> > +#define RZG2L_MAX_PINS_PER_PORT 8
> > +#define RZG2L_PIN_ID_TO_PORT(id) ((id) / RZG2L_MAX_PINS_PER_PORT)
> > +#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_MAX_PINS_PER_PORT)
> > +
> > +struct irqc_irq {
> > + int hw_irq;
> > + int requested_irq;
> > + struct irqc_priv *p;
> > + struct rzg2l_pin_info *pin;
> > +};
> > +
> > +struct rzg2l_pin_info {
> > + u32 port;
> > + u8 bit;
> > + u32 id;
>
> Given the way you initialise things below, are these types the right
> ones?
>
Yes they are correct, port ranges from 0-48, bit ranges from 0-7 and
id ranges from 0-122
> > + u32 trigger_type;
> > + unsigned int type;
> > + u8 pin_state;
> > +};
> > +
> > +struct irqc_priv {
> > + void __iomem *iomem;
> > + struct irqc_irq irq[RZG2L_IRQC_IRQ_MAX];
> > + DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPTS);
> > + unsigned int number_of_irqs;
> > + struct device *dev;
> > + struct irq_chip_generic *gc;
> > + struct irq_chip chip;
> > + struct irq_domain *irq_domain;
> > + struct irq_domain *parent_domain;
> > + unsigned int tint_irq_start;
> > + atomic_t wakeup_path;
> > + struct clk *clk;
> > + struct clk *pclk;
> > +};
> > +
> > +static const struct rzg2l_pin_info gpio_pin_info[] = {
> > + { 0, 0, 0 }, { 0, 1, 1 },
> > + { 1, 0, 2 }, { 1, 1, 3 },
> > + { 2, 0, 4 }, { 2, 1, 5 },
> > + { 3, 0, 6 }, { 3, 1, 7 },
> > + { 4, 0, 8 }, { 4, 1, 9 },
> > + { 5, 0, 10 }, { 5, 1, 11 }, { 5, 2, 12 },
> > + { 6, 0, 13 }, { 6, 1, 14 },
> > + { 7, 0, 15 }, { 7, 1, 16 }, { 7, 2, 17 },
> > + { 8, 0, 18 }, { 8, 1, 19 }, { 8, 2, 20 },
> > + { 9, 0, 21 }, { 9, 1, 22 },
> > + { 10, 0, 23 }, { 10, 1, 24 },
> > + { 11, 0, 25 }, { 11, 1, 26 },
> > + { 12, 0, 27 }, { 12, 1, 28 },
> > + { 13, 0, 29 }, { 13, 1, 30 }, { 13, 2, 31 },
> > + { 14, 0, 32 }, { 14, 1, 33 },
> > + { 15, 0, 34 }, { 15, 1, 35 },
> > + { 16, 0, 36 }, { 16, 1, 37 },
> > + { 17, 0, 38 }, { 17, 1, 39 }, { 17, 2, 40 },
> > + { 18, 0, 41 }, { 18, 1, 42 },
> > + { 19, 0, 43 }, { 19, 1, 44 },
> > + { 20, 0, 45 }, { 20, 1, 46 }, { 20, 2, 47 },
> > + { 21, 0, 48 }, { 21, 1, 49 },
> > + { 22, 0, 50 }, { 22, 1, 51 },
> > + { 23, 0, 52 }, { 23, 1, 53 },
> > + { 24, 0, 54 }, { 24, 1, 55 },
> > + { 25, 0, 56 }, { 25, 1, 57 },
> > + { 26, 0, 58 }, { 26, 1, 59 },
> > + { 27, 0, 60 }, { 27, 1, 61 },
> > + { 28, 0, 62 }, { 28, 1, 63 },
> > + { 29, 0, 64 }, { 29, 1, 65 },
> > + { 30, 0, 66 }, { 30, 1, 67 },
> > + { 31, 0, 68 }, { 31, 1, 69 },
> > + { 32, 0, 70 }, { 32, 1, 71 },
> > + { 33, 0, 72 }, { 33, 1, 73 },
> > + { 34, 0, 74 }, { 34, 1, 75 },
> > + { 35, 0, 76 }, { 35, 1, 77 },
> > + { 36, 0, 78 }, { 36, 1, 79 },
> > + { 37, 0, 80 }, { 37, 1, 81 }, { 37, 2, 82 },
> > + { 38, 0, 83 }, { 38, 1, 84 },
> > + { 39, 0, 85 }, { 39, 1, 86 }, { 39, 2, 87 },
> > + { 40, 0, 88 }, { 40, 1, 89 }, { 40, 2, 90 },
> > + { 41, 0, 91 }, { 41, 1, 92 },
> > + { 42, 0, 93 }, { 42, 1, 94 }, { 42, 2, 95 }, { 42, 3, 96 }, { 42, 4, 97 },
> > + { 43, 0, 98 }, { 43, 1, 99 }, { 43, 2, 100 }, { 43, 3, 101 },
> > + { 44, 0, 102 }, { 44, 1, 103 }, { 44, 2, 104 }, { 44, 3, 105 },
> > + { 45, 0, 106 }, { 45, 1, 107 }, { 45, 2, 108 }, { 45, 3, 109 },
> > + { 46, 0, 110 }, { 46, 1, 111 }, { 46, 2, 112 }, { 46, 3, 113 },
> > + { 47, 0, 114 }, { 47, 1, 115 }, { 47, 2, 116 }, { 47, 3, 117 },
> > + { 48, 0, 118 }, { 48, 1, 119 }, { 48, 2, 120 }, { 48, 3, 121 }, { 48, 3, 122 },
> > +};
>
> Is there any reason why this information is not kept in DT instead?
> What does it describe exactly?
>
I will check if this can be moved to DT or handled in pinctrl driver.
This describes the GPIO pins which can be used as an interrupt.
> > +
> > +static int rzg2l_gpio_irq_validate_id(u32 port, u32 bit)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(gpio_pin_info); i++) {
> > + if (gpio_pin_info[i].port == port && gpio_pin_info[i].bit == bit)
> > + return gpio_pin_info[i].id;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static struct irq_domain *rzg2l_get_pinctrl_domain(struct device *dev)
> > +{
> > + struct device_node *pinctrl_np;
> > + struct irq_domain *pinctrl_domain;
> > +
> > + pinctrl_np = of_find_compatible_node(NULL, NULL, "renesas,r9a07g044-pinctrl");
> > + if (!pinctrl_np)
> > + return NULL;
> > +
> > + pinctrl_domain = irq_find_host(pinctrl_np);
> > + of_node_put(pinctrl_np);
> > +
> > + return pinctrl_domain;
>
> What if you have multiple instances of this pinctrl? The information
> should directly come from DT in the form of a phandle.
>
agreed, said that this will go away when implemented as an interrupt controller.
> > +}
> > +
> > +static struct irqc_priv *irq_data_to_priv(struct irq_data *data)
> > +{
> > + return data->domain->host_data;
> > +}
> > +
> > +static void rzg2l_tint_set_edge(struct irqc_priv *priv,
> > + u32 bit, unsigned int irq_type)
> > +{
> > + u32 port_bit = bit;
> > + u32 reg;
> > +
> > + if (port_bit <= 15) {
> > + reg = readl(priv->iomem + TITSR0);
> > + reg &= ~(IRQ_MASK << (port_bit * 2));
> > + reg |= irq_type << (port_bit * 2);
> > + writel(reg, priv->iomem + TITSR0);
> > + } else {
> > + reg = readl(priv->iomem + TITSR1);
> > + port_bit = port_bit / 2;
> > + reg &= ~(IRQ_MASK << (port_bit * 2));
> > + reg |= irq_type << (port_bit * 2);
> > + writel(reg, priv->iomem + TITSR1);
> > + }
>
> This will result in corruption if two interrupts change their type at
> the same time. Also, please use relaxed accessors.
>
agreed.
> > +}
> > +
> > +static int rzg2l_tint_set_type(struct irq_data *d, unsigned int type)
> > +{
> > + struct rzg2l_pin_info *pin_info = d->chip_data;
> > + struct irqc_priv *priv = irq_data_to_priv(d);
> > + u8 irq_type;
> > +
> > + pin_info->trigger_type = type & GENMASK(15, 0);
> > + pin_info->pin_state = ((type & ~GENMASK(15, 0)) >> 15);
>
> This shift is obviously wrong. Also, why are you tracking random
> information independently of the core?
>
To handle IRQ_TYPE_EDGE_BOTH which is not natively supported by the HW
we split the type and pass the current state of GPIO PIN BIT(16) to
the parent irqc domain and trigger it on rising/falling edge depending
on the PIN state.
> > + pin_info->type = pin_info->trigger_type;
>
> More pointless tracking...
>
> > +
> > + if (pin_info->trigger_type & BIT(0))
> > + irq_type = RISING_EDGE;
> > + else if (pin_info->trigger_type & BIT(1))
> > + irq_type = FALLING_EDGE;
> > + else if (pin_info->trigger_type & BIT(2))
> > + irq_type = HIGH_LEVEL;
> > + else if (pin_info->trigger_type & BIT(3))
> > + irq_type = LOW_LEVEL;
> > + else if (pin_info->trigger_type & BIT(4))
> > + irq_type = pin_info->pin_state ? FALLING_EDGE : RISING_EDGE;
> > + else
> > + return -EINVAL;
>
> ... considering that this can fail. And please use the macros that
> already exist rather than these BIT(x).
>
sure will do.
> > +
> > + pin_info->trigger_type = irq_type;
> > + rzg2l_tint_set_edge(priv, pin_info->bit, irq_type);
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t rzg2l_irqc_tint_irq_handler(int irq, void *dev_id)
> > +{
> > + struct irq_domain *pctl_irq_domain;
> > + struct rzg2l_pin_info *pin_info;
> > + struct irqc_irq *i = dev_id;
> > + struct irqc_priv *priv = i->p;
> > + struct irq_data *irq_data;
> > + unsigned int offset;
> > + u32 reg;
> > +
> > + offset = irq - priv->irq[RZG2L_TINT_IRQ_START_INDEX].requested_irq;
> > + pin_info = priv->irq[offset + RZG2L_TINT_IRQ_START_INDEX].pin;
> > + if (!pin_info)
> > + return IRQ_NONE;
> > +
> > + irq_data = irq_domain_get_irq_data(priv->parent_domain, i->requested_irq);
> > + if (!irq_data)
> > + return IRQ_NONE;
> > +
> > + pctl_irq_domain = rzg2l_get_pinctrl_domain(priv->dev);
>
> Are you really comfortable with parsing the device tree on each
> interrupt you get? This is madness. It also makes no sense to resolve
> an interrupt in a foreign irqdomain. Why don't you allocate yours?
>
Making this as an interrupt controller, this will go away.
> > + if (!pctl_irq_domain)
> > + return IRQ_NONE;
> > +
> > + reg = readl(priv->iomem + TSCR);
> > + reg &= ~BIT(offset);
> > + writel(reg, priv->iomem + TSCR);
> > +
> > + generic_handle_irq(irq_find_mapping(pctl_irq_domain,
> > + ((pin_info->port * 8) + pin_info->bit)));
>
> generic_handle_domain_irq() is your new friend.
>
Ok will switch to generic_handle_domain_irq().
> > +
> > + if (pin_info->type & BIT(4)) {
> > + pin_info->trigger_type = (pin_info->trigger_type == FALLING_EDGE) ?
> > + RISING_EDGE : FALLING_EDGE;
> > + rzg2l_tint_set_edge(priv, pin_info->bit, pin_info->trigger_type);
> > + }
>
> This has hardly any chance of working reliably. Yes, plenty of drivers
> do that. They are all doomed to fail.
>
Agreed, for the first version will drop both edge support.
> > +
> > + return IRQ_HANDLED;
>
> No way. This isn't a standard interrupt handler. This is a mux, with a
> number of inputs for each of the *49* outputs. Please implement this
> as a chained interrupt controller.
>
Ok will implement this as a chained interrupt controller.
> > +}
> > +
> > +static void rzg2l_tint_irq_disable(struct irq_data *d)
> > +{
> > + struct rzg2l_pin_info *pin_info = d->chip_data;
> > + struct irqc_priv *priv = irq_data_to_priv(d);
> > + u32 reg;
> > +
> > + reg = readl(priv->iomem + TSSR(pin_info->id / 4));
> > + reg &= ~(GENMASK(7, 0) << (pin_info->id % 4));
> > + writel(reg, priv->iomem + TSSR(pin_info->id / 4));
>
> More buggy RMW, magic numbers all over the shop. Does this act as a
> disable (pending state is lost) or as a mask (pending state is
> preserved)?
>
it acts as a mask, the pending state is preserved.
> > +}
> > +
> > +static void rzg2l_tint_irq_enable(struct irq_data *d)
> > +{
> > + struct rzg2l_pin_info *pin_info = d->chip_data;
> > + struct irqc_priv *priv = irq_data_to_priv(d);
> > + u32 gpioint;
> > + u32 reg;
> > +
> > + gpioint = rzg2l_gpio_irq_validate_id(pin_info->port, pin_info->bit);
> > +
> > + reg = readl(priv->iomem + TSSR(pin_info->id / 4));
> > + reg |= (BIT(7) | gpioint) << (8 * (pin_info->id % 4));
> > + writel(reg, priv->iomem + TSSR(pin_info->id / 4));
>
> On top of the buggy RMW, what happens when
> rzg2l_gpio_irq_validate_id() returns -EINVAL?
>
will use rmw, in rzg2l_tint_irq_enable() callback -EINVAL case wont
happen as this would have been already caught in alloc() callback. But
to keep it consistent will add a check for -EINVAL.
> > +}
> > +
> > +static int rzg2l_irqc_domain_translate(struct irq_domain *domain,
> > + struct irq_fwspec *fwspec,
> > + unsigned long *hwirq,
> > + unsigned int *type)
> > +{
> > + if (WARN_ON(fwspec->param_count < 2))
> > + return -EINVAL;
> > +
> > + *hwirq = fwspec->param[0];
> > + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> > +
> > + return 0;
> > +}
>
> This is irq_domain_translate_twocell().
>
agreed.
> > +
> > +static int rzg2l_irqc_domain_alloc(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs,
> > + void *arg)
> > +{
> > + struct irqc_priv *priv = domain->host_data;
> > + struct irq_fwspec parent_fwspec;
> > + struct rzg2l_pin_info *pin_info;
> > + struct irq_data *irq_data;
> > + irq_hw_number_t hwirq;
> > + unsigned int type;
> > + int gpioint;
> > + u32 port;
> > + int irq;
> > + int ret;
> > + u8 bit;
> > +
> > + ret = rzg2l_irqc_domain_translate(domain, arg, &hwirq, &type);
> > + if (ret)
> > + return ret;
> > +
> > + port = RZG2L_PIN_ID_TO_PORT(hwirq);
> > + bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> > +
> > + switch (type) {
> > + case IRQ_TYPE_EDGE_RISING:
> > + case IRQ_TYPE_LEVEL_HIGH:
> > + break;
> > + case IRQ_TYPE_EDGE_FALLING:
> > + type = IRQ_TYPE_EDGE_RISING;
> > + break;
> > + case IRQ_TYPE_LEVEL_LOW:
> > + type = IRQ_TYPE_LEVEL_HIGH;
>
> Really? How is that handled?
>
my bad this can be set to IRQ_TYPE_ eihter of one and the actual type
will be handled in the irq_set_type() callback.
> I stopped reading at this stage. This needs to be redesigned from the
> ground up as an interrupt controller instead of an interrupt handler,
> have proper locking, data structures that are not redundant with that
> of the core code, and be written in a way that is self documenting.
>
Thank you for the feedback, I'll re-design this as an interrupt controller.
Cheers,
Prabhakar
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
2021-08-11 12:10 ` Linus Walleij
@ 2021-08-12 9:05 ` Lad, Prabhakar
0 siblings, 0 replies; 11+ messages in thread
From: Lad, Prabhakar @ 2021-08-12 9:05 UTC (permalink / raw)
To: Linus Walleij
Cc: Lad Prabhakar, Geert Uytterhoeven, Thomas Gleixner, Marc Zyngier,
Rob Herring, Magnus Damm, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas, Biju Das
Hi Linus,
Thank you for the review.
On Wed, Aug 11, 2021 at 1:10 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Aug 3, 2021 at 7:51 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> > +description: |
> > + The RZ/G2L Interrupt Controller is a front-end for the GIC found on Renesas RZ/G2L SoC's
> > + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts,
> > + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts,
> > + - NMI edge select.
>
> Not that we don't have weird documentation but what on earth is an
> "NMI edge"???
>
On this SoC NMI is not treated as an NMI exception, the irqc has bits
to select the NMI interrupt (Rising/Falling-edge) detection.
> I know about rising and falling edges, and I know about non-maskable
> interrupts. But NMI edge? Maybe expand this to explain what it is?
>
sure will add more details on the above.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-12 9:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03 17:51 [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 1/4] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller Lad Prabhakar
2021-08-11 12:10 ` Linus Walleij
2021-08-12 9:05 ` Lad, Prabhakar
2021-08-11 18:39 ` Rob Herring
2021-08-03 17:51 ` [RFC PATCH 2/4] irqchip: Add RZ/G2L IA55 Interrupt Controller driver Lad Prabhakar
2021-08-04 11:57 ` Marc Zyngier
2021-08-12 8:59 ` Lad, Prabhakar
2021-08-03 17:51 ` [RFC PATCH 3/4] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt Lad Prabhakar
2021-08-03 17:51 ` [RFC PATCH 4/4] arm64: dts: renesas: r9a07g044: Add IRQC node to SoC DTSI Lad Prabhakar
2021-08-11 12:07 ` [RFC PATCH 0/4] Renesas RZ/G2L IRQC support Linus Walleij
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.