2022-03-17 03:44:14

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 0/5] Renesas RZ/G2L IRQC support

Hi All,

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

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

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

Cheers,
Prabhakar

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

[0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
[email protected]/

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

RFC v3: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
[email protected]/

RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
[email protected]/

RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
[email protected]/


Lad Prabhakar (5):
dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
Controller
irqchip: Add RZ/G2L IA55 Interrupt Controller driver
gpio: gpiolib: Allow free() callback to be overridden
gpio: gpiolib: Add ngirq member to struct gpio_irq_chip
pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO
interrupt

.../renesas,rzg2l-irqc.yaml | 131 +++++
drivers/gpio/gpiolib.c | 13 +-
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzg2l.c | 462 ++++++++++++++++++
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 ++++++++
include/linux/gpio/driver.h | 8 +
7 files changed, 823 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

--
2.17.1


2022-03-17 04:47:33

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 4/5] gpio: gpiolib: Add ngirq member to struct gpio_irq_chip

Supported GPIO IRQs by the chip is not always equal to the number of GPIO
pins. For example on Renesas RZ/G2L SoC where it has GPIO0-122 pins but at
a give point a maximum of only 32 GPIO pins can be used as IRQ lines in
the IRQC domain.

This patch adds ngirq member to struct gpio_irq_chip and passes this as a
size to irq_domain_create_hierarchy()/irq_domain_create_simple() if it is
being set in the driver otherwise fallbacks to using ngpio.

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/gpio/gpiolib.c | 4 ++--
include/linux/gpio/driver.h | 8 ++++++++
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index aede442f610d..8784d11f8a15 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1221,7 +1221,7 @@ static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
gc->irq.domain = irq_domain_create_hierarchy(
gc->irq.parent_domain,
0,
- gc->ngpio,
+ gc->irq.ngirq ?: gc->ngpio,
gc->irq.fwnode,
&gc->irq.child_irq_domain_ops,
gc);
@@ -1564,7 +1564,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
} else {
/* Some drivers provide custom irqdomain ops */
gc->irq.domain = irq_domain_create_simple(fwnode,
- gc->ngpio,
+ gc->irq.ngirq ?: gc->ngpio,
gc->irq.first,
gc->irq.domain_ops ?: &gpiochip_domain_ops,
gc);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index b0728c8ad90c..eada90d59657 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -51,6 +51,14 @@ struct gpio_irq_chip {
*/
const struct irq_domain_ops *domain_ops;

+ /**
+ * @ngirq:
+ *
+ * The number of GPIO IRQ's handled by this IRQ domain; usually is
+ * equal to ngpio.
+ */
+ u16 ngirq;
+
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
/**
* @fwnode:
--
2.17.1

2022-03-17 05:04:36

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt

Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.

GPIO0-GPIO122 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 <[email protected]>
---
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 ccee9c9e2e22..d5bc8b73e514 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -9,8 +9,10 @@
#include <linux/clk.h>
#include <linux/gpio/driver.h>
#include <linux/io.h>
+#include <linux/interrupt.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>
@@ -89,6 +91,7 @@
#define PIN(n) (0x0800 + 0x10 + (n))
#define IOLH(n) (0x1000 + (n) * 8)
#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)
@@ -112,6 +115,10 @@
#define RZG2L_PIN_ID_TO_PORT_OFFSET(id) (RZG2L_PIN_ID_TO_PORT(id) + 0x10)
#define RZG2L_PIN_ID_TO_PIN(id) ((id) % RZG2L_PINS_PER_PORT)

+#define RZG2L_TINT_MAX_INTERRUPT 32
+#define RZG2L_TINT_IRQ_START_INDEX 9
+#define RZG2L_PACK_HWIRQ(t, i) (((t) << 16) | (i))
+
struct rzg2l_dedicated_configs {
const char *name;
u32 config;
@@ -137,6 +144,9 @@ struct rzg2l_pinctrl {

struct gpio_chip gpio_chip;
struct pinctrl_gpio_range gpio_range;
+ DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT);
+ spinlock_t bitmap_lock;
+ unsigned int hwirq[RZG2L_TINT_MAX_INTERRUPT];

spinlock_t lock;
};
@@ -883,6 +893,8 @@ static int rzg2l_gpio_get(struct gpio_chip *chip, unsigned int offset)

static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
{
+ unsigned int virq;
+
pinctrl_gpio_free(chip->base + offset);

/*
@@ -890,6 +902,10 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
* drive the GPIO pin as an output.
*/
rzg2l_gpio_direction_input(chip, offset);
+
+ virq = irq_find_mapping(chip->irq.domain, offset);
+ if (virq)
+ irq_dispose_mapping(virq);
}

static const char * const rzg2l_gpio_names[] = {
@@ -1075,14 +1091,193 @@ static struct rzg2l_dedicated_configs rzg2l_dedicated_pins[] = {
{ "RIIC1_SCL", RZG2L_SINGLE_PIN_PACK(0xe, 3, PIN_CFG_IEN) },
};

+static int rzg2l_gpio_get_gpioint(unsigned int virq)
+{
+ unsigned int gpioint = 0;
+ unsigned int i = 0;
+ u32 port, bit;
+
+ port = virq / 8;
+ bit = virq % 8;
+
+ if (port >= ARRAY_SIZE(rzg2l_gpio_configs))
+ return -EINVAL;
+
+ for (i = 0; i < port; i++)
+ gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
+
+ if (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]))
+ return -EINVAL;
+
+ gpioint += bit;
+
+ return gpioint;
+}
+
+static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct irq_data *d;
+
+ d = irq_domain_get_irq_data(domain, virq);
+ if (d) {
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ unsigned long flags;
+ unsigned int i;
+
+ for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
+ if (pctrl->hwirq[i] == hwirq) {
+ spin_lock_irqsave(&pctrl->bitmap_lock, flags);
+ bitmap_release_region(pctrl->tint_slot, i, get_order(1));
+ spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
+ pctrl->hwirq[i] = 0;
+ break;
+ }
+ }
+ }
+ irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static void rzg2l_gpio_irq_disable(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+ unsigned int hwirq = irqd_to_hwirq(d);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 port;
+ 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);
+ writel(readl(addr) & ~BIT(bit * 8), addr);
+ spin_unlock_irqrestore(&pctrl->lock, flags);
+
+ irq_chip_disable_parent(d);
+}
+
+static void rzg2l_gpio_irq_enable(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+ unsigned int hwirq = irqd_to_hwirq(d);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 port;
+ 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);
+ writel(readl(addr) | BIT(bit * 8), 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)
+{
+ return irq_chip_set_type_parent(d, type);
+}
+
+static void rzg2l_gpio_irqc_eoi(struct irq_data *d)
+{
+ irq_chip_eoi_parent(d);
+}
+
+static struct irq_chip rzg2l_gpio_irqchip = {
+ .name = "rzg2l-gpio",
+ .irq_disable = rzg2l_gpio_irq_disable,
+ .irq_enable = rzg2l_gpio_irq_enable,
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_set_type = rzg2l_gpio_irq_set_type,
+ .irq_eoi = rzg2l_gpio_irqc_eoi,
+};
+
+static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
+{
+ struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
+ unsigned long flags;
+ int gpioint, irq;
+
+ gpioint = rzg2l_gpio_get_gpioint(child);
+ if (gpioint < 0)
+ return gpioint;
+
+ spin_lock_irqsave(&pctrl->bitmap_lock, flags);
+ irq = bitmap_find_free_region(pctrl->tint_slot, RZG2L_TINT_MAX_INTERRUPT, get_order(1));
+ spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
+ if (irq < 0)
+ return -ENOSPC;
+ pctrl->hwirq[irq] = child;
+ irq += RZG2L_TINT_IRQ_START_INDEX;
+
+ /* All these interrupts are level high in the CPU */
+ *parent_type = IRQ_TYPE_LEVEL_HIGH;
+ *parent = RZG2L_PACK_HWIRQ(gpioint, irq);
+ return 0;
+}
+
+static void *rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
+ unsigned int parent_hwirq,
+ unsigned int parent_type)
+{
+ struct irq_fwspec *fwspec;
+
+ fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
+ if (!fwspec)
+ return NULL;
+
+ fwspec->fwnode = chip->irq.parent_domain->fwnode;
+ fwspec->param_count = 2;
+ fwspec->param[0] = parent_hwirq;
+ fwspec->param[1] = parent_type;
+
+ return fwspec;
+}
+
static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
{
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;
+ struct gpio_irq_chip *girq;
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");
@@ -1109,6 +1304,15 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
chip->base = -1;
chip->ngpio = of_args.args[2];

+ girq = &chip->irq;
+ girq->chip = &rzg2l_gpio_irqchip;
+ girq->fwnode = of_node_to_fwnode(np);
+ girq->parent_domain = parent_domain;
+ girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
+ girq->populate_parent_alloc_arg = rzg2l_gpio_populate_parent_fwspec;
+ girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
+ girq->ngirq = RZG2L_TINT_MAX_INTERRUPT;
+
pctrl->gpio_range.id = 0;
pctrl->gpio_range.pin_base = 0;
pctrl->gpio_range.base = 0;
@@ -1224,6 +1428,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
}

spin_lock_init(&pctrl->lock);
+ spin_lock_init(&pctrl->bitmap_lock);

platform_set_drvdata(pdev, pctrl);

--
2.17.1

2022-03-17 05:42:45

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 3/5] gpio: gpiolib: Allow free() callback to be overridden

Allow free() callback to be overridden from irq_domain_ops for
hierarchical chips.

This allows drivers to free any resources which are allocated during
populate_parent_alloc_arg().

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/gpio/gpiolib.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index defb7c464b87..aede442f610d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1187,15 +1187,18 @@ static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
ops->activate = gpiochip_irq_domain_activate;
ops->deactivate = gpiochip_irq_domain_deactivate;
ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
- ops->free = irq_domain_free_irqs_common;

/*
- * We only allow overriding the translate() function for
+ * We only allow overriding the translate() and free() function for
* hierarchical chips, and this should only be done if the user
- * really need something other than 1:1 translation.
+ * really need something other than 1:1 translation for translate()
+ * callback and free if user wants to free up any resources which
+ * were allocated during callbacks for example populate_parent_alloc_arg.
*/
if (!ops->translate)
ops->translate = gpiochip_hierarchy_irq_domain_translate;
+ if (!ops->free)
+ ops->free = irq_domain_free_irqs_common;
}

static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
--
2.17.1

2022-03-17 06:21:57

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

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

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

Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-rzg2l.c | 462 ++++++++++++++++++++++++++++
3 files changed, 471 insertions(+)
create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7038957f4a77..afc5999f4955 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
to 8 external interrupts with configurable sense select.

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

2022-03-17 06:36:00

by Prabhakar Mahadev Lad

[permalink] [raw]
Subject: [RFC PATCH v4 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

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

Signed-off-by: Lad Prabhakar <[email protected]>
---
.../renesas,rzg2l-irqc.yaml | 131 ++++++++++++++++++
1 file changed, 131 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
new file mode 100644
index 000000000000..a14492ec9235
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
+
+maintainers:
+ - Lad Prabhakar <[email protected]>
+ - Geert Uytterhoeven <[email protected]>
+
+description: |
+ IA55 performs various interrupt controls including synchronization for the external
+ interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
+ interrupts output by each IP. And it notifies the interrupt to the GIC
+ - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
+ - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
+ - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
+ stand-up edge detection interrupts)
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - renesas,r9a07g044-irqc # RZ/G2L
+ - const: renesas,rzg2l-irqc
+
+ '#interrupt-cells':
+ const: 2
+
+ '#address-cells':
+ const: 0
+
+ interrupt-controller: true
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 41
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: clk
+ - const: pclk
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - '#interrupt-cells'
+ - '#address-cells'
+ - interrupt-controller
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - power-domains
+ - resets
+
+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

2022-03-17 11:05:00

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/5] Renesas RZ/G2L IRQC support

On Thu, Mar 17, 2022 at 5:43 AM Lad Prabhakar
<[email protected]> wrote:
>
> Hi All,
>
> The RZ/G2L Interrupt Controller is a front-end for the GIC found on
> Renesas RZ/G2L SoC's with below pins:
> - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> - GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
> maximum of only 32 can be mapped to 32 GIC SPI interrupts,
> - NMI edge select.
>
> _____________
> | GIC |
> | ________ |
> ____________ | | | |
> NMI ------------------------------------>| | SPI0-479 | | GIC-600| |
> _______ | |------------>| | |
> | | | | PPI16-31 | | | |
> | | IRQ0-IRQ7 | IRQC |------------>| | |
> P0_P48_4 ------>| GPIO |---------------->| | | |________| |
> | |GPIOINT0-122 | | | |
> | |---------------->| TINT0-31 | | |
> |______| |__________| |____________|
>
> The proposed RFC patches adds hierarchical IRQ domain one in IRQC driver and other in

add
domain, one
another

> pinctrl driver. Upon interrupt requests map the interrupt to GIC. Out of GPIOINT0-122
> only 32 can be mapped to GIC SPI, this mapping is handled by the pinctrl and IRQC driver.

What I want to know now is whether it is going to collide with Marc's
series about GPIO IRQ chip constification?

--
With Best Regards,
Andy Shevchenko

2022-03-17 12:49:36

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH v4 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

On 17/03/2022 02:24, Lad Prabhakar wrote:
> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> .../renesas,rzg2l-irqc.yaml | 131 ++++++++++++++++++
> 1 file changed, 131 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> new file mode 100644
> index 000000000000..a14492ec9235
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> +
> +maintainers:
> + - Lad Prabhakar <[email protected]>
> + - Geert Uytterhoeven <[email protected]>
> +
> +description: |
> + IA55 performs various interrupt controls including synchronization for the external
> + interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
> + interrupts output by each IP. And it notifies the interrupt to the GIC
> + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> + - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> + stand-up edge detection interrupts)
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r9a07g044-irqc # RZ/G2L
> + - const: renesas,rzg2l-irqc
> +
> + '#interrupt-cells':
> + const: 2
> +
> + '#address-cells':
> + const: 0
> +
> + interrupt-controller: true
> +
> + reg:
> + maxItems: 1> +
> + interrupts:
> + maxItems: 41
> +
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: clk
> + - const: pclk
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - '#interrupt-cells'
> + - '#address-cells'
> + - interrupt-controller
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - power-domains
> + - resets
> +
> +additionalProperties: false

This should be rather unevaluatedProperties and remove
interrupt-controller:true from properties.

> +
> +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>;

Put the reg after compatible in DTS code. The ordering of properties in
bindings is a bit odd, but I wasn't following order by myself, so cannot
complain. :)



Best regards,
Krzysztof

2022-03-17 12:58:39

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/5] Renesas RZ/G2L IRQC support

On Thu, 17 Mar 2022 08:46:14 +0000,
Andy Shevchenko <[email protected]> wrote:
>
> On Thu, Mar 17, 2022 at 5:43 AM Lad Prabhakar
> <[email protected]> wrote:
> >
> > 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.
> >
> What I want to know now is whether it is going to collide with Marc's
> series about GPIO IRQ chip constification?

Probably, but the current scheme will still be alive for some time
(you'll need a couple of cycles to sort out all the drivers).

Worse case, this can be fixed at merge time.

M.

--
Without deviation from the norm, progress is not possible.

2022-03-17 16:14:29

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/5] Renesas RZ/G2L IRQC support

Hi Marc,

On Thu, Mar 17, 2022 at 9:23 AM Marc Zyngier <[email protected]> wrote:
>
> On Thu, 17 Mar 2022 08:46:14 +0000,
> Andy Shevchenko <[email protected]> wrote:
> >
> > On Thu, Mar 17, 2022 at 5:43 AM Lad Prabhakar
> > <[email protected]> wrote:
> > >
> > > 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.
> > >
> > What I want to know now is whether it is going to collide with Marc's
> > series about GPIO IRQ chip constification?
>
> Probably, but the current scheme will still be alive for some time
> (you'll need a couple of cycles to sort out all the drivers).
>
Ouch, thanks for letting me know. BTW there are a couple of changes to
GPIO core which you have to review (this was missed in the previous
version).

Cheers,
Prabhakar

> Worse case, this can be fixed at merge time.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.

2022-03-17 16:33:22

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v4 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

Hi Krzysztof,

Thank you for the review.

On Thu, Mar 17, 2022 at 9:44 AM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 17/03/2022 02:24, Lad Prabhakar wrote:
> > Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > .../renesas,rzg2l-irqc.yaml | 131 ++++++++++++++++++
> > 1 file changed, 131 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > new file mode 100644
> > index 000000000000..a14492ec9235
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> > @@ -0,0 +1,131 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> > +
> > +maintainers:
> > + - Lad Prabhakar <[email protected]>
> > + - Geert Uytterhoeven <[email protected]>
> > +
> > +description: |
> > + IA55 performs various interrupt controls including synchronization for the external
> > + interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
> > + interrupts output by each IP. And it notifies the interrupt to the GIC
> > + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> > + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> > + - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> > + stand-up edge detection interrupts)
> > +
> > +allOf:
> > + - $ref: /schemas/interrupt-controller.yaml#
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - renesas,r9a07g044-irqc # RZ/G2L
> > + - const: renesas,rzg2l-irqc
> > +
> > + '#interrupt-cells':
> > + const: 2
> > +
> > + '#address-cells':
> > + const: 0
> > +
> > + interrupt-controller: true
> > +
> > + reg:
> > + maxItems: 1> +
> > + interrupts:
> > + maxItems: 41
> > +
> > + clocks:
> > + maxItems: 2
> > +
> > + clock-names:
> > + items:
> > + - const: clk
> > + - const: pclk
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - '#interrupt-cells'
> > + - '#address-cells'
> > + - interrupt-controller
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - power-domains
> > + - resets
> > +
> > +additionalProperties: false
>
> This should be rather unevaluatedProperties and remove
> interrupt-controller:true from properties.
>
Ok will do.

> > +
> > +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>;
>
> Put the reg after compatible in DTS code. The ordering of properties in
> bindings is a bit odd, but I wasn't following order by myself, so cannot
> complain. :)
>
Ok will fix that in next version.

Cheers,
Prabhakar

2022-03-17 18:14:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH v4 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller

On 17/03/2022 12:55, Lad, Prabhakar wrote:
> Hi Krzysztof,
>
> Thank you for the review.
>
> On Thu, Mar 17, 2022 at 9:44 AM Krzysztof Kozlowski <[email protected]> wrote:
>>
>> On 17/03/2022 02:24, Lad Prabhakar wrote:
>>> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
>>>
>>> Signed-off-by: Lad Prabhakar <[email protected]>
>>> ---
>>> .../renesas,rzg2l-irqc.yaml | 131 ++++++++++++++++++
>>> 1 file changed, 131 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>>> new file mode 100644
>>> index 000000000000..a14492ec9235
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
>>> @@ -0,0 +1,131 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
>>> +
>>> +maintainers:
>>> + - Lad Prabhakar <[email protected]>
>>> + - Geert Uytterhoeven <[email protected]>
>>> +
>>> +description: |
>>> + IA55 performs various interrupt controls including synchronization for the external
>>> + interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
>>> + interrupts output by each IP. And it notifies the interrupt to the GIC
>>> + - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
>>> + - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
>>> + - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
>>> + stand-up edge detection interrupts)
>>> +
>>> +allOf:
>>> + - $ref: /schemas/interrupt-controller.yaml#
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - renesas,r9a07g044-irqc # RZ/G2L
>>> + - const: renesas,rzg2l-irqc
>>> +
>>> + '#interrupt-cells':
>>> + const: 2
>>> +
>>> + '#address-cells':
>>> + const: 0
>>> +
>>> + interrupt-controller: true
>>> +
>>> + reg:
>>> + maxItems: 1> +
>>> + interrupts:
>>> + maxItems: 41
>>> +
>>> + clocks:
>>> + maxItems: 2
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: clk
>>> + - const: pclk
>>> +
>>> + power-domains:
>>> + maxItems: 1
>>> +
>>> + resets:
>>> + maxItems: 1
>>> +
>>> +required:
>>> + - compatible
>>> + - '#interrupt-cells'
>>> + - '#address-cells'
>>> + - interrupt-controller
>>> + - reg
>>> + - interrupts
>>> + - clocks
>>> + - clock-names
>>> + - power-domains
>>> + - resets
>>> +
>>> +additionalProperties: false
>>
>> This should be rather unevaluatedProperties and remove
>> interrupt-controller:true from properties.
>>
> Ok will do.

After some tests it seems interrupt-controller:true is required in
properties, so only change unevaluatedProperties.


Best regards,
Krzysztof

2022-03-17 20:16:01

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v4 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

On Thu, 17 Mar 2022 01:24:01 +0000,
Lad Prabhakar <[email protected]> wrote:
>
> Add a driver for the Renesas RZ/G2L Interrupt Controller.
>
> This supports external pins being used as interrupts. It supports
> one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> to be used as IRQ lines.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/irqchip/Kconfig | 8 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-renesas-rzg2l.c | 462 ++++++++++++++++++++++++++++
> 3 files changed, 471 insertions(+)
> create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 7038957f4a77..afc5999f4955 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
> Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
> to 8 external interrupts with configurable sense select.
>
> +config RENESAS_RZG2L_IRQC
> + bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
> + select GENERIC_IRQ_CHIP
> + select IRQ_DOMAIN_HIERARCHY
> + help
> + Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
> + for external devices.
> +
> config SL28CPLD_INTC
> bool "Kontron sl28cpld IRQ controller"
> depends on MFD_SL28CPLD=y || COMPILE_TEST
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index c1f611cbfbf8..b536e514a7c9 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..be9741e88bd7
> --- /dev/null
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -0,0 +1,462 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G2L IRQC Driver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation.
> + *
> + * Author: Lad Prabhakar <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#define IRQC_IRQ_START 1
> +#define IRQC_IRQ_COUNT 8
> +#define IRQC_TINT_START 9
> +#define IRQC_TINT_COUNT 32
> +#define IRQC_NUM_IRQ 41
> +
> +#define ISCR 0x10
> +#define IITSR 0x14
> +#define TSCR 0x20
> +#define TITSR0 0x24
> +#define TITSR1 0x28
> +#define TITSR0_MAX_INT 16
> +#define TITSEL_WIDTH 0x2
> +#define TSSR(n) (0x30 + ((n) * 4))
> +#define TIEN BIT(7)
> +#define TSSEL_SHIFT(n) (8 * (n))
> +#define TSSEL_MASK GENMASK(7, 0)
> +#define IRQ_MASK 0x3
> +
> +#define TSSR_OFFSET(n) ((n) % 4)
> +#define TSSR_INDEX(n) ((n) / 4)
> +
> +#define TITSR_TITSEL_EDGE_RISING 0
> +#define TITSR_TITSEL_EDGE_FALLING 1
> +#define TITSR_TITSEL_LEVEL_HIGH 2
> +#define TITSR_TITSEL_LEVEL_LOW 3
> +
> +#define IITSR_IITSEL(n, sense) ((sense) << ((n) * 2))
> +#define IITSR_IITSEL_LEVEL_LOW 0
> +#define IITSR_IITSEL_EDGE_FALLING 1
> +#define IITSR_IITSEL_EDGE_RISING 2
> +#define IITSR_IITSEL_EDGE_BOTH 3
> +#define IITSR_IITSEL_MASK(n) IITSR_IITSEL((n), 3)
> +
> +#define TINT_EXTRACT_HWIRQ(x) ((x) & ~GENMASK(31, 16))
> +#define TINT_EXTRACT_GPIOINT(x) ((x) >> 16)
> +
> +struct rzg2l_irqc_priv {
> + void __iomem *base;
> + struct of_phandle_args map[IRQC_NUM_IRQ];
> + raw_spinlock_t lock;
> +};
> +
> +struct rzg2l_irqc_chip_data {
> + int tint;
> +};
> +
> +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
> +{
> + return data->domain->host_data;
> +}
> +
> +static void rzg2l_irq_eoi(struct irq_data *d)
> +{
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> + u16 bit = BIT(hw_irq);
> + unsigned long flags;
> + u32 reg;
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);

Why the irqsave flavour? Is there any case where EOI isn't performed
in a context that has interrupts already disabled?

> + reg = readl_relaxed(priv->base + ISCR);
> + if (reg & bit)
> + writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
> + priv->base + ISCR);
> + raw_spin_unlock_irqrestore(&priv->lock, flags);
> + irq_chip_eoi_parent(d);
> +}
> +
> +static void rzg2l_tint_eoi(struct irq_data *d)
> +{
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + unsigned int hw_irq = irqd_to_hwirq(d);
> + u32 bit = BIT(hw_irq - IRQC_TINT_START);
> + unsigned long flags;
> + u32 reg;
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);
> + reg = readl_relaxed(priv->base + TSCR);
> + if (reg & bit)
> + writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
> + priv->base + TSCR);
> + raw_spin_unlock_irqrestore(&priv->lock, flags);
> + irq_chip_eoi_parent(d);
> +}
> +
> +static void rzg2l_irqc_eoi(struct irq_data *d)
> +{
> + unsigned int hw_irq = irqd_to_hwirq(d);
> +
> + if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> + return rzg2l_irq_eoi(d);

return function_returning_void()? No, please.

> + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> + return rzg2l_tint_eoi(d);

Move the locking and the call to irq_chip_eoi_parent() here, as there
is no need to duplicate any of this.

> +}
> +
> +static void rzg2l_irqc_irq_disable(struct irq_data *d)
> +{
> + unsigned int hw_irq = irqd_to_hwirq(d);
> +
> + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + u32 offset = hw_irq - IRQC_TINT_START;
> + u32 tssr_offset = TSSR_OFFSET(offset);
> + u8 tssr_index = TSSR_INDEX(offset);
> + unsigned long flags;
> + u32 reg;
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);
> + reg = readl_relaxed(priv->base + TSSR(tssr_index));
> + reg &= ~(TSSEL_MASK << tssr_offset);
> + writel_relaxed(reg, priv->base + TSSR(tssr_index));
> + raw_spin_unlock_irqrestore(&priv->lock, flags);
> + }
> + irq_chip_disable_parent(d);
> +}
> +
> +static void rzg2l_irqc_irq_enable(struct irq_data *d)
> +{
> + unsigned int hw_irq = irqd_to_hwirq(d);
> +
> + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> + u32 offset = hw_irq - IRQC_TINT_START;
> + u32 tssr_offset = TSSR_OFFSET(offset);
> + u8 tssr_index = TSSR_INDEX(offset);
> + unsigned long flags;
> + u32 reg;
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);
> + reg = readl_relaxed(priv->base + TSSR(tssr_index));
> + reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
> + writel_relaxed(reg, priv->base + TSSR(tssr_index));
> + raw_spin_unlock_irqrestore(&priv->lock, flags);
> + }
> + irq_chip_enable_parent(d);
> +}
> +
> +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + unsigned long flags;
> + u16 sense, tmp;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sense = IITSR_IITSEL_LEVEL_LOW;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + sense = IITSR_IITSEL_EDGE_FALLING;
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING:
> + sense = IITSR_IITSEL_EDGE_RISING;
> + break;
> +
> + case IRQ_TYPE_EDGE_BOTH:
> + sense = IITSR_IITSEL_EDGE_BOTH;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);

This already happens in an irq disabled context.

> + tmp = readl_relaxed(priv->base + IITSR);
> + tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> + tmp |= IITSR_IITSEL(hw_irq, sense);
> + writel_relaxed(tmp, priv->base + IITSR);
> + raw_spin_unlock_irqrestore(&priv->lock, flags);

What about the parent irqchip? If this is a hierarchical setup, it
definitely should be told.

> +
> + return 0;
> +}
> +
> +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
> +{
> + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> + u32 titseln = hwirq - IRQC_TINT_START;
> + unsigned long flags;
> + u32 offset;
> + u8 sense;
> + u32 reg;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_EDGE_RISING:
> + sense = TITSR_TITSEL_EDGE_RISING;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + sense = TITSR_TITSEL_EDGE_FALLING;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + if (titseln < TITSR0_MAX_INT) {
> + offset = TITSR0;
> + } else {
> + titseln /= TITSEL_WIDTH;
> + offset = TITSR1;
> + }
> +
> + raw_spin_lock_irqsave(&priv->lock, flags);
> + reg = readl_relaxed(priv->base + offset);
> + reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> + reg |= sense << (titseln * TITSEL_WIDTH);
> + writel_relaxed(reg, priv->base + offset);
> + raw_spin_unlock_irqrestore(&priv->lock, flags);
> +
> + return 0;
> +}
> +
> +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
> +{
> + unsigned int hw_irq = irqd_to_hwirq(d);
> +
> + if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> + return rzg2l_irq_set_type(d, type);
> + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> + return rzg2l_tint_set_edge(d, type);
> +
> + return -EINVAL;
> +}
> +
> +static struct irq_chip irqc_chip = {
> + .name = "rzg2l-irqc",
> + .irq_eoi = rzg2l_irqc_eoi,
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_disable = rzg2l_irqc_irq_disable,
> + .irq_enable = rzg2l_irqc_irq_enable,

So this looks a bit odd. irq_mask only calls the parent and does nothing
locally, while irq_disable does something locally and calls into the
parent. If the parent is a GIC, this is turned into a mask (GIC has no
notion of disabled).

If that's the flow you expect, good. But please check this is the case.

> + .irq_get_irqchip_state = irq_chip_get_parent_state,
> + .irq_set_irqchip_state = irq_chip_set_parent_state,
> + .irq_retrigger = irq_chip_retrigger_hierarchy,
> + .irq_set_type = rzg2l_irqc_set_type,
> + .flags = IRQCHIP_MASK_ON_SUSPEND |
> + IRQCHIP_SET_TYPE_MASKED |
> + IRQCHIP_SKIP_SET_WAKE,
> +};
> +
> +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + struct rzg2l_irqc_priv *priv = domain->host_data;
> + struct rzg2l_irqc_chip_data *chip_data = NULL;
> + struct irq_fwspec spec;
> + irq_hw_number_t hwirq;
> + int tint = -EINVAL;
> + unsigned int type;
> + unsigned int i;
> + int ret;
> +
> + ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + /*
> + * For TINIT interrupts ie where pinctrl driver is child of irqc domain
> + * the hwirq and TINT are encoded in fwspec->param[0].
> + * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and TINT
> + * from 16-31 bits. TINIT from the pinctrl driver needs to be programmed
> + * in IRQC registers to enable a given gpio pin as interrupt.
> + */
> + if (hwirq > IRQC_IRQ_COUNT) {
> + tint = TINT_EXTRACT_GPIOINT(hwirq);
> + hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> + }
> +
> + if (hwirq > (IRQC_NUM_IRQ - 1))
> + return -EINVAL;
> +
> + if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq > (IRQC_NUM_IRQ - 1)))
> + return -EINVAL;
> +
> + chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> + if (!chip_data)
> + return -ENOMEM;
> + chip_data->tint = tint;
> +
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
> + chip_data);
> + if (ret) {
> + kfree(chip_data);
> + return ret;
> + }
> +
> + spec.fwnode = domain->parent->fwnode;
> + spec.param_count = priv->map[hwirq].args_count;
> + for (i = 0; i < spec.param_count; i++)
> + spec.param[i] = priv->map[hwirq].args[i];
> +
> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> + if (ret)
> + kfree(chip_data);
> +
> + return ret;
> +}
> +
> +static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *d;
> +
> + d = irq_domain_get_irq_data(domain, virq);
> + if (d) {
> + struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> +
> + kfree(chip_data);
> + }
> + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +}
> +
> +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> + .alloc = rzg2l_irqc_alloc,
> + .free = rzg2l_irqc_domain_free,
> + .translate = irq_domain_translate_twocell,
> +};
> +
> +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> + struct device_node *np,
> + struct device_node *parent)
> +{
> + unsigned int len, j;
> + const __be32 *range;
> +
> + range = of_get_property(np, "interrupts", &len);
> + if (!range)
> + return -EINVAL;
> +
> + for (len /= sizeof(*range), j = 0; len >= 3; len -= 3) {
> + if (j >= IRQC_NUM_IRQ)
> + return -EINVAL;
> +
> + priv->map[j].args[0] = be32_to_cpu(*range++);
> + priv->map[j].args[1] = be32_to_cpu(*range++);
> + priv->map[j].args[2] = be32_to_cpu(*range++);
> + priv->map[j].args_count = 3;
> + j++;
> + }

I'm confused. Why do you have to invent a new interrupt parser? What
is wrong with of_irq_parse_one() populating the of_phandle_args array?
I expected to see something like:

static void rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
struct device_node *np)
{
int i;

for (i = 0; i < IRQC_NUM_IRQ; i++)
of_irq_parse_one(np, i, &priv->map[i]);
}

and that's it.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2022-03-18 16:56:49

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v4 3/5] gpio: gpiolib: Allow free() callback to be overridden

Hi Linus,

On Thu, Mar 17, 2022 at 1:24 AM Lad Prabhakar
<[email protected]> wrote:
>
> Allow free() callback to be overridden from irq_domain_ops for
> hierarchical chips.
>
> This allows drivers to free any resources which are allocated during
> populate_parent_alloc_arg().
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/gpio/gpiolib.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
Sorry to ping early, this patch and 4/5 hasn't been reviewed at all in
previous versions. So I wanted to get your feedback before I post an
non -RFC version of the series.

Cheers,
Prabhakar

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index defb7c464b87..aede442f610d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1187,15 +1187,18 @@ static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
> ops->activate = gpiochip_irq_domain_activate;
> ops->deactivate = gpiochip_irq_domain_deactivate;
> ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
> - ops->free = irq_domain_free_irqs_common;
>
> /*
> - * We only allow overriding the translate() function for
> + * We only allow overriding the translate() and free() function for
> * hierarchical chips, and this should only be done if the user
> - * really need something other than 1:1 translation.
> + * really need something other than 1:1 translation for translate()
> + * callback and free if user wants to free up any resources which
> + * were allocated during callbacks for example populate_parent_alloc_arg.
> */
> if (!ops->translate)
> ops->translate = gpiochip_hierarchy_irq_domain_translate;
> + if (!ops->free)
> + ops->free = irq_domain_free_irqs_common;
> }
>
> static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
> --
> 2.17.1
>

2022-03-18 19:24:35

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v4 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

On Fri, 18 Mar 2022 14:59:41 +0000,
"Lad, Prabhakar" <[email protected]> wrote:
>
> Hi Marc,
>
> Thank you for the review.
>
> On Thu, Mar 17, 2022 at 4:13 PM Marc Zyngier <[email protected]> wrote:
> >
> > On Thu, 17 Mar 2022 01:24:01 +0000,
> > Lad Prabhakar <[email protected]> wrote:
> > >
> > > +static struct irq_chip irqc_chip = {
> > > + .name = "rzg2l-irqc",
> > > + .irq_eoi = rzg2l_irqc_eoi,
> > > + .irq_mask = irq_chip_mask_parent,
> > > + .irq_unmask = irq_chip_unmask_parent,
> > > + .irq_disable = rzg2l_irqc_irq_disable,
> > > + .irq_enable = rzg2l_irqc_irq_enable,
> >
> > So this looks a bit odd. irq_mask only calls the parent and does nothing
> > locally, while irq_disable does something locally and calls into the
> > parent. If the parent is a GIC, this is turned into a mask (GIC has no
> > notion of disabled).
> >
> My understanding for enable callback is one time call during irq setup
> and for the disable callback it will be called during irq shutdown.
> During enable/disable callback we config the required registers.
> For mask callback this will be called when an interrupt occurs and for
> unmask we want to re-enable the interrupt. Since there are no specific
> registers to mask/unmask on RZ/G2L the callbacks point to
> irq_chip_mask_parent/irq_chip_unmask_parent.
>
> I could move all the code from enable/disable callbacks to mask/unmask
> callbacks and drop setting irq_enable/irq_disable completely. Please
> let me know what should be the correct approach.

I'm OK with your current setup, but I just wanted to check that this
was your understanding as well.

M.

--
Without deviation from the norm, progress is not possible.

2022-03-19 05:53:07

by Prabhakar

[permalink] [raw]
Subject: Re: [RFC PATCH v4 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver

Hi Marc,

Thank you for the review.

On Thu, Mar 17, 2022 at 4:13 PM Marc Zyngier <[email protected]> wrote:
>
> On Thu, 17 Mar 2022 01:24:01 +0000,
> Lad Prabhakar <[email protected]> wrote:
> >
> > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> >
> > This supports external pins being used as interrupts. It supports
> > one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
> > to be used as IRQ lines.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > drivers/irqchip/Kconfig | 8 +
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-renesas-rzg2l.c | 462 ++++++++++++++++++++++++++++
> > 3 files changed, 471 insertions(+)
> > create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 7038957f4a77..afc5999f4955 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
> > Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
> > to 8 external interrupts with configurable sense select.
> >
> > +config RENESAS_RZG2L_IRQC
> > + bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
> > + select GENERIC_IRQ_CHIP
> > + select IRQ_DOMAIN_HIERARCHY
> > + help
> > + Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
> > + for external devices.
> > +
> > config SL28CPLD_INTC
> > bool "Kontron sl28cpld IRQ controller"
> > depends on MFD_SL28CPLD=y || COMPILE_TEST
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index c1f611cbfbf8..b536e514a7c9 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..be9741e88bd7
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -0,0 +1,462 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G2L IRQC Driver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > + *
> > + * Author: Lad Prabhakar <[email protected]>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of_address.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define IRQC_IRQ_START 1
> > +#define IRQC_IRQ_COUNT 8
> > +#define IRQC_TINT_START 9
> > +#define IRQC_TINT_COUNT 32
> > +#define IRQC_NUM_IRQ 41
> > +
> > +#define ISCR 0x10
> > +#define IITSR 0x14
> > +#define TSCR 0x20
> > +#define TITSR0 0x24
> > +#define TITSR1 0x28
> > +#define TITSR0_MAX_INT 16
> > +#define TITSEL_WIDTH 0x2
> > +#define TSSR(n) (0x30 + ((n) * 4))
> > +#define TIEN BIT(7)
> > +#define TSSEL_SHIFT(n) (8 * (n))
> > +#define TSSEL_MASK GENMASK(7, 0)
> > +#define IRQ_MASK 0x3
> > +
> > +#define TSSR_OFFSET(n) ((n) % 4)
> > +#define TSSR_INDEX(n) ((n) / 4)
> > +
> > +#define TITSR_TITSEL_EDGE_RISING 0
> > +#define TITSR_TITSEL_EDGE_FALLING 1
> > +#define TITSR_TITSEL_LEVEL_HIGH 2
> > +#define TITSR_TITSEL_LEVEL_LOW 3
> > +
> > +#define IITSR_IITSEL(n, sense) ((sense) << ((n) * 2))
> > +#define IITSR_IITSEL_LEVEL_LOW 0
> > +#define IITSR_IITSEL_EDGE_FALLING 1
> > +#define IITSR_IITSEL_EDGE_RISING 2
> > +#define IITSR_IITSEL_EDGE_BOTH 3
> > +#define IITSR_IITSEL_MASK(n) IITSR_IITSEL((n), 3)
> > +
> > +#define TINT_EXTRACT_HWIRQ(x) ((x) & ~GENMASK(31, 16))
> > +#define TINT_EXTRACT_GPIOINT(x) ((x) >> 16)
> > +
> > +struct rzg2l_irqc_priv {
> > + void __iomem *base;
> > + struct of_phandle_args map[IRQC_NUM_IRQ];
> > + raw_spinlock_t lock;
> > +};
> > +
> > +struct rzg2l_irqc_chip_data {
> > + int tint;
> > +};
> > +
> > +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
> > +{
> > + return data->domain->host_data;
> > +}
> > +
> > +static void rzg2l_irq_eoi(struct irq_data *d)
> > +{
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > + u16 bit = BIT(hw_irq);
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
>
> Why the irqsave flavour? Is there any case where EOI isn't performed
> in a context that has interrupts already disabled?
>
Agreed this callback will be called with IRQ's disabled, so we can use
raw_spin_lock() instead.

> > + reg = readl_relaxed(priv->base + ISCR);
> > + if (reg & bit)
> > + writel_relaxed(GENMASK(IRQC_IRQ_COUNT - 1, 0) & ~bit,
> > + priv->base + ISCR);
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
> > + irq_chip_eoi_parent(d);
> > +}
> > +
> > +static void rzg2l_tint_eoi(struct irq_data *d)
> > +{
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > + u32 bit = BIT(hw_irq - IRQC_TINT_START);
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
> > + reg = readl_relaxed(priv->base + TSCR);
> > + if (reg & bit)
> > + writel_relaxed(GENMASK(IRQC_TINT_COUNT - 1, 0) & ~bit,
> > + priv->base + TSCR);
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
> > + irq_chip_eoi_parent(d);
> > +}
> > +
> > +static void rzg2l_irqc_eoi(struct irq_data *d)
> > +{
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > + if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > + return rzg2l_irq_eoi(d);
>
> return function_returning_void()? No, please.
>
Agreed will drop that.

> > + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > + return rzg2l_tint_eoi(d);
>
> Move the locking and the call to irq_chip_eoi_parent() here, as there
> is no need to duplicate any of this.
>
OK, will move the locking and irq_chip_eoi_parent() here.

> > +}
> > +
> > +static void rzg2l_irqc_irq_disable(struct irq_data *d)
> > +{
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + u32 offset = hw_irq - IRQC_TINT_START;
> > + u32 tssr_offset = TSSR_OFFSET(offset);
> > + u8 tssr_index = TSSR_INDEX(offset);
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
> > + reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > + reg &= ~(TSSEL_MASK << tssr_offset);
> > + writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
> > + }
> > + irq_chip_disable_parent(d);
> > +}
> > +
> > +static void rzg2l_irqc_irq_enable(struct irq_data *d)
> > +{
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > + u32 offset = hw_irq - IRQC_TINT_START;
> > + u32 tssr_offset = TSSR_OFFSET(offset);
> > + u8 tssr_index = TSSR_INDEX(offset);
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
> > + reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > + reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
> > + writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
> > + }
> > + irq_chip_enable_parent(d);
> > +}
> > +
> > +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type)
> > +{
> > + unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + unsigned long flags;
> > + u16 sense, tmp;
> > +
> > + switch (type & IRQ_TYPE_SENSE_MASK) {
> > + case IRQ_TYPE_LEVEL_LOW:
> > + sense = IITSR_IITSEL_LEVEL_LOW;
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_FALLING:
> > + sense = IITSR_IITSEL_EDGE_FALLING;
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_RISING:
> > + sense = IITSR_IITSEL_EDGE_RISING;
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_BOTH:
> > + sense = IITSR_IITSEL_EDGE_BOTH;
> > + break;
> > +
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
>
> This already happens in an irq disabled context.
>
> > + tmp = readl_relaxed(priv->base + IITSR);
> > + tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> > + tmp |= IITSR_IITSEL(hw_irq, sense);
> > + writel_relaxed(tmp, priv->base + IITSR);
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
>
> What about the parent irqchip? If this is a hierarchical setup, it
> definitely should be told.
>
Ok, I will add a call for it.

> > +
> > + return 0;
> > +}
> > +
> > +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
> > +{
> > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > + unsigned int hwirq = irqd_to_hwirq(d);
> > + u32 titseln = hwirq - IRQC_TINT_START;
> > + unsigned long flags;
> > + u32 offset;
> > + u8 sense;
> > + u32 reg;
> > +
> > + switch (type & IRQ_TYPE_SENSE_MASK) {
> > + case IRQ_TYPE_EDGE_RISING:
> > + sense = TITSR_TITSEL_EDGE_RISING;
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_FALLING:
> > + sense = TITSR_TITSEL_EDGE_FALLING;
> > + break;
> > +
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (titseln < TITSR0_MAX_INT) {
> > + offset = TITSR0;
> > + } else {
> > + titseln /= TITSEL_WIDTH;
> > + offset = TITSR1;
> > + }
> > +
> > + raw_spin_lock_irqsave(&priv->lock, flags);
> > + reg = readl_relaxed(priv->base + offset);
> > + reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> > + reg |= sense << (titseln * TITSEL_WIDTH);
> > + writel_relaxed(reg, priv->base + offset);
> > + raw_spin_unlock_irqrestore(&priv->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
> > +{
> > + unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > + if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > + return rzg2l_irq_set_type(d, type);
> > + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > + return rzg2l_tint_set_edge(d, type);
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static struct irq_chip irqc_chip = {
> > + .name = "rzg2l-irqc",
> > + .irq_eoi = rzg2l_irqc_eoi,
> > + .irq_mask = irq_chip_mask_parent,
> > + .irq_unmask = irq_chip_unmask_parent,
> > + .irq_disable = rzg2l_irqc_irq_disable,
> > + .irq_enable = rzg2l_irqc_irq_enable,
>
> So this looks a bit odd. irq_mask only calls the parent and does nothing
> locally, while irq_disable does something locally and calls into the
> parent. If the parent is a GIC, this is turned into a mask (GIC has no
> notion of disabled).
>
My understanding for enable callback is one time call during irq setup
and for the disable callback it will be called during irq shutdown.
During enable/disable callback we config the required registers.
For mask callback this will be called when an interrupt occurs and for
unmask we want to re-enable the interrupt. Since there are no specific
registers to mask/unmask on RZ/G2L the callbacks point to
irq_chip_mask_parent/irq_chip_unmask_parent.

I could move all the code from enable/disable callbacks to mask/unmask
callbacks and drop setting irq_enable/irq_disable completely. Please
let me know what should be the correct approach.

> If that's the flow you expect, good. But please check this is the case.
>
> > + .irq_get_irqchip_state = irq_chip_get_parent_state,
> > + .irq_set_irqchip_state = irq_chip_set_parent_state,
> > + .irq_retrigger = irq_chip_retrigger_hierarchy,
> > + .irq_set_type = rzg2l_irqc_set_type,
> > + .flags = IRQCHIP_MASK_ON_SUSPEND |
> > + IRQCHIP_SET_TYPE_MASKED |
> > + IRQCHIP_SKIP_SET_WAKE,
> > +};
> > +
> > +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> > + unsigned int nr_irqs, void *arg)
> > +{
> > + struct rzg2l_irqc_priv *priv = domain->host_data;
> > + struct rzg2l_irqc_chip_data *chip_data = NULL;
> > + struct irq_fwspec spec;
> > + irq_hw_number_t hwirq;
> > + int tint = -EINVAL;
> > + unsigned int type;
> > + unsigned int i;
> > + int ret;
> > +
> > + ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * For TINIT interrupts ie where pinctrl driver is child of irqc domain
> > + * the hwirq and TINT are encoded in fwspec->param[0].
> > + * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and TINT
> > + * from 16-31 bits. TINIT from the pinctrl driver needs to be programmed
> > + * in IRQC registers to enable a given gpio pin as interrupt.
> > + */
> > + if (hwirq > IRQC_IRQ_COUNT) {
> > + tint = TINT_EXTRACT_GPIOINT(hwirq);
> > + hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> > + }
> > +
> > + if (hwirq > (IRQC_NUM_IRQ - 1))
> > + return -EINVAL;
> > +
> > + if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq > (IRQC_NUM_IRQ - 1)))
> > + return -EINVAL;
> > +
> > + chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> > + if (!chip_data)
> > + return -ENOMEM;
> > + chip_data->tint = tint;
> > +
> > + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
> > + chip_data);
> > + if (ret) {
> > + kfree(chip_data);
> > + return ret;
> > + }
> > +
> > + spec.fwnode = domain->parent->fwnode;
> > + spec.param_count = priv->map[hwirq].args_count;
> > + for (i = 0; i < spec.param_count; i++)
> > + spec.param[i] = priv->map[hwirq].args[i];
> > +
> > + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> > + if (ret)
> > + kfree(chip_data);
> > +
> > + return ret;
> > +}
> > +
> > +static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int virq,
> > + unsigned int nr_irqs)
> > +{
> > + struct irq_data *d;
> > +
> > + d = irq_domain_get_irq_data(domain, virq);
> > + if (d) {
> > + struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > +
> > + kfree(chip_data);
> > + }
> > + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> > +}
> > +
> > +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> > + .alloc = rzg2l_irqc_alloc,
> > + .free = rzg2l_irqc_domain_free,
> > + .translate = irq_domain_translate_twocell,
> > +};
> > +
> > +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> > + struct device_node *np,
> > + struct device_node *parent)
> > +{
> > + unsigned int len, j;
> > + const __be32 *range;
> > +
> > + range = of_get_property(np, "interrupts", &len);
> > + if (!range)
> > + return -EINVAL;
> > +
> > + for (len /= sizeof(*range), j = 0; len >= 3; len -= 3) {
> > + if (j >= IRQC_NUM_IRQ)
> > + return -EINVAL;
> > +
> > + priv->map[j].args[0] = be32_to_cpu(*range++);
> > + priv->map[j].args[1] = be32_to_cpu(*range++);
> > + priv->map[j].args[2] = be32_to_cpu(*range++);
> > + priv->map[j].args_count = 3;
> > + j++;
> > + }
>
> I'm confused. Why do you have to invent a new interrupt parser? What
> is wrong with of_irq_parse_one() populating the of_phandle_args array?
> I expected to see something like:
>
> static void rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> struct device_node *np)
> {
> int i;
>
> for (i = 0; i < IRQC_NUM_IRQ; i++)
> of_irq_parse_one(np, i, &priv->map[i]);
> }
>
> and that's it.
>
Agreed of_irq_parse_one() already does this (and ofcourse I need to
check the return value of of_irq_parse_one()).

Cheers,
Prabhakar

> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.