lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ