[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+V-a8syBvN7czRA7tq0TJSmUcBzmgjLrFmizHD6Ycp5kLXJWw@mail.gmail.com>
Date: Mon, 9 May 2022 08:48:15 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Biju Das <biju.das.jz@...renesas.com>
Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Philipp Zabel <p.zabel@...gutronix.de>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to
handle GPIO interrupt
Hi Biju,
Thank you for the review.
On Mon, May 9, 2022 at 7:49 AM Biju Das <biju.das.jz@...renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH v2 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 <prabhakar.mahadev-lad.rj@...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 a48cac55152c..275dfec74329 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[] = { @@ -1104,14 +1120,193 @@
> > static struct {
> > }
> > };
> >
> > +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;
>
> May be combine this statement to above with
>
> || (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
> return -EINVAL;
>
The reason I have kept it outside the loop is that I'll have to check
it only once at the end of the loop instead of repeating the check
every time in the loop.
Cheers,
Prabhakar
> Cheers,
> BIju
>
> > +
> > + 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"); @@ -
> > 1138,6 +1333,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;
> > @@ -1253,6 +1457,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.25.1
>
Powered by blists - more mailing lists