linux-kernel - Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt

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]

Date:   Thu, 12 May 2022 09:39:13 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Phil Edworthy <phil.edworthy@...esas.com>,
        Biju Das <biju.das.jz@...renesas.com>,
        Prabhakar <prabhakar.csengg@...il.com>
Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to
 handle GPIO interrupt

Hi Prabhakar,

On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@...renesas.com> wrote:
> Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.

domain

> 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

at a given time

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

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c

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

I think you need to provide a .init_valid_mask() callback, as
gpiochip_irqchip_remove() relies on that for destroying interrupts.
However, the mask will need to be dynamic, as GPIO interrupts can be
mapped and unmapped to one of the 32 available interrupts dynamically,
right?
I'm not sure if that can be done easily: if gpiochip_irqchip_irq_valid()
is ever called too early, before the mapping is done, it would fail.

>         pctrl->gpio_range.id = 0;
>         pctrl->gpio_range.pin_base = 0;
>         pctrl->gpio_range.base = 0;


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds