| Message ID | 20220511183210.5248-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| Headers | show |
| Series | Renesas RZ/G2L IRQC support | expand |
> -----Original Message----- > From: Biju Das > Sent: 12 May 2022 18:59 > To: Lad, Prabhakar <prabhakar.csengg@gmail.com> > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert > Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > <linus.walleij@linaro.org>; Thomas Gleixner <tglx@linutronix.de>; Marc > Zyngier <maz@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Bartosz Golaszewski > <brgl@bgdev.pl>; Philipp Zabel <p.zabel@pengutronix.de>; linux- > gpio@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas- > soc@vger.kernel.org; devicetree@vger.kernel.org; Phil Edworthy > <phil.edworthy@renesas.com> > Subject: RE: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain > to handle GPIO interrupt > > Hi Prabhakar, > > > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ > > domain to handle GPIO interrupt > > > > Hi Biju, > > > > Thank you for the review. > > > > On Thu, May 12, 2022 at 6:35 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > > > > Hi Prabhakar, > > > > > > Thanks for the patch. > > > > > > > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Subject: [PATCH v3 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. > > > > > > Do we need to update bindings with interrupt-cells on [1] like [2] > > > as it > > act as parent for GPIO interrupts? > > > > > Yes interrupt-controller and interrupt-parent needs to be added. I'm > > wondering if "interrupt-cells" is not required. If the pin is an > > interrupt it will be passed as an GPIO. > > It is same as external interrupt case right? > > For eg:- Ethernet PHY case, > > interrupt-parent = <&irqc>; > interrupts = <3 IRQ_TYPE_LEVEL_LOW>; > > if you use GPIO, it will be like this right? > > interrupt-parent = <&pinctrl>; > interrupts = <RZG2L_GPIO(1, 0) IRQ_TYPE_LEVEL_LOW>; FYI, Previously, I have tested ADV HPD interrupt with below changes while investigating [1] interrupt-parent = <&pinctrl>; interrupts = <RZG2L_GPIO(2, 1) IRQ_TYPE_EDGE_FALLING>; [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220512&id=04b19d32213654e54ec819b6ac033360f1551902 > > Cheers, > Biju > > > > > > > > > > @Geert - your thoughts ? > > > > Cheers, > > Prabhakar
Hi Prabhakar, On Thu, May 12, 2022 at 7:36 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Thu, May 12, 2022 at 8:39 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > 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@bp.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. > Are you suggesting the callback to avoid looping through all the GPIO pins? gpiochip_irqchip_remove() does: /* Remove all IRQ mappings and delete the domain */ if (gc->irq.domain) { unsigned int irq; for (offset = 0; offset < gc->ngpio; offset++) { if (!gpiochip_irqchip_irq_valid(gc, offset)) continue; irq = irq_find_mapping(gc->irq.domain, offset); irq_dispose_mapping(irq); } irq_domain_remove(gc->irq.domain); } The main thing is not about avoiding to loop through all GPIO pins, but to avoid irq_{find,dispose}_mapping() doing the wrong thing. The loop is over all GPIO offsets, while not all of them are mapped to valid interrupts. Does the above work correctly? > > 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? > Yep that's correct. > > > 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. > > > The mask initialization is a one time process and that is during > adding the GPIO chip. At this stage we won't be knowing what will be > the valid GPIO pins used as interrupts. Maybe the core needs to > implement a callback which lands in the GPIO controller driver to tell > if the gpio irq line is valid. This way we can handle dynamic > interrupts. Upon closer look, I think the mask is a red herring, and we don't need it. But we do need to handle the (possible) mismatch between GPIO offset (index) and IRQ offset in the above code. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
On Fri, May 13, 2022 at 7:12 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > > -----Original Message----- > > From: Biju Das > > Sent: 12 May 2022 18:59 > > To: Lad, Prabhakar <prabhakar.csengg@gmail.com> > > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert > > Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > > <linus.walleij@linaro.org>; Thomas Gleixner <tglx@linutronix.de>; Marc > > Zyngier <maz@kernel.org>; Rob Herring <robh+dt@kernel.org>; Krzysztof > > Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Bartosz Golaszewski > > <brgl@bgdev.pl>; Philipp Zabel <p.zabel@pengutronix.de>; linux- > > gpio@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas- > > soc@vger.kernel.org; devicetree@vger.kernel.org; Phil Edworthy > > <phil.edworthy@renesas.com> > > Subject: RE: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain > > to handle GPIO interrupt > > > > Hi Prabhakar, > > > > > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ > > > domain to handle GPIO interrupt > > > > > > Hi Biju, > > > > > > Thank you for the review. > > > > > > On Thu, May 12, 2022 at 6:35 AM Biju Das <biju.das.jz@bp.renesas.com> > > > wrote: > > > > > > > > Hi Prabhakar, > > > > > > > > Thanks for the patch. > > > > > > > > > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > Subject: [PATCH v3 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. > > > > > > > > Do we need to update bindings with interrupt-cells on [1] like [2] > > > > as it > > > act as parent for GPIO interrupts? > > > > > > > Yes interrupt-controller and interrupt-parent needs to be added. I'm > > > wondering if "interrupt-cells" is not required. If the pin is an > > > interrupt it will be passed as an GPIO. > > > > It is same as external interrupt case right? > > > > For eg:- Ethernet PHY case, > > > > interrupt-parent = <&irqc>; > > interrupts = <3 IRQ_TYPE_LEVEL_LOW>; > > > > if you use GPIO, it will be like this right? > > > > interrupt-parent = <&pinctrl>; > > interrupts = <RZG2L_GPIO(1, 0) IRQ_TYPE_LEVEL_LOW>; > > FYI, > > Previously, I have tested ADV HPD interrupt with below changes while investigating [1] > > interrupt-parent = <&pinctrl>; > interrupts = <RZG2L_GPIO(2, 1) IRQ_TYPE_EDGE_FALLING>; > Right, #interrupt-cells=<2> , where the first cell is the GPIO pin and the second cell is the flag. Cheers, Prabhakar
Hi Geert, On Fri, May 13, 2022 at 7:53 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Thu, May 12, 2022 at 7:36 PM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Thu, May 12, 2022 at 8:39 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > 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@bp.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. > > Are you suggesting the callback to avoid looping through all the GPIO pins? > > gpiochip_irqchip_remove() does: > > /* Remove all IRQ mappings and delete the domain */ > if (gc->irq.domain) { > unsigned int irq; > > for (offset = 0; offset < gc->ngpio; offset++) { > if (!gpiochip_irqchip_irq_valid(gc, offset)) > continue; > > irq = irq_find_mapping(gc->irq.domain, offset); > irq_dispose_mapping(irq); > } > > irq_domain_remove(gc->irq.domain); > > } > > The main thing is not about avoiding to loop through all GPIO pins, > but to avoid irq_{find,dispose}_mapping() doing the wrong thing. So in our case if we don't implement valid masks, that would mean all the pins are valid. irq_find_mapping() would return 0 if no mapping is found to the corresponding offset and irq_dispose_mapping() would simply return back without doing anything if virq == 0.(In this patch rzg2l_gpio_free() does call irq_{find,dispose}_mapping()) > The loop is over all GPIO offsets, while not all of them are mapped > to valid interrupts. Does the above work correctly? > I haven't tested unloading the pinctrl driver which should call gpiochip_irqchip_remove() (we don't have remove call back for pinctrl driver) > > > 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? > > Yep that's correct. > > > > > 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. > > > > > The mask initialization is a one time process and that is during > > adding the GPIO chip. At this stage we won't be knowing what will be > > the valid GPIO pins used as interrupts. Maybe the core needs to > > implement a callback which lands in the GPIO controller driver to tell > > if the gpio irq line is valid. This way we can handle dynamic > > interrupts. > > Upon closer look, I think the mask is a red herring, and we don't > need it. Agreed. > But we do need to handle the (possible) mismatch between GPIO > offset (index) and IRQ offset in the above code. > Agreed, do you see any possibility of the mismatch I have missed? Cheers, Prabhakar
Hi Prabhakar, On Fri, May 13, 2022 at 3:56 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Fri, May 13, 2022 at 7:53 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Thu, May 12, 2022 at 7:36 PM Lad, Prabhakar > > <prabhakar.csengg@gmail.com> wrote: > > > On Thu, May 12, 2022 at 8:39 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > 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@bp.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. > > > Are you suggesting the callback to avoid looping through all the GPIO pins? > > > > gpiochip_irqchip_remove() does: > > > > /* Remove all IRQ mappings and delete the domain */ > > if (gc->irq.domain) { > > unsigned int irq; > > > > for (offset = 0; offset < gc->ngpio; offset++) { > > if (!gpiochip_irqchip_irq_valid(gc, offset)) > > continue; > > > > irq = irq_find_mapping(gc->irq.domain, offset); > > irq_dispose_mapping(irq); > > } > > > > irq_domain_remove(gc->irq.domain); > > > > } > > > > The main thing is not about avoiding to loop through all GPIO pins, > > but to avoid irq_{find,dispose}_mapping() doing the wrong thing. > So in our case if we don't implement valid masks, that would mean all > the pins are valid. irq_find_mapping() would return 0 if no mapping is > found to the corresponding offset and irq_dispose_mapping() would > simply return back without doing anything if virq == 0.(In this patch > rzg2l_gpio_free() does call irq_{find,dispose}_mapping()) But "offset" is a number from the GPIO offset space (0-122), while irq_find_mapping() expects a number from the domain's IRQ space, which is only 0-31? > > The loop is over all GPIO offsets, while not all of them are mapped > > to valid interrupts. Does the above work correctly? > > > I haven't tested unloading the pinctrl driver which should call > gpiochip_irqchip_remove() (we don't have remove call back for pinctrl > driver) > > > > > 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? > > > Yep that's correct. > > > > > > > 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. > > > > > > > The mask initialization is a one time process and that is during > > > adding the GPIO chip. At this stage we won't be knowing what will be > > > the valid GPIO pins used as interrupts. Maybe the core needs to > > > implement a callback which lands in the GPIO controller driver to tell > > > if the gpio irq line is valid. This way we can handle dynamic > > > interrupts. > > > > Upon closer look, I think the mask is a red herring, and we don't > > need it. > Agreed. > > > But we do need to handle the (possible) mismatch between GPIO > > offset (index) and IRQ offset in the above code. > > > Agreed, do you see any possibility of the mismatch I have missed? gpiochip_to_irq(): if (irq_domain_is_hierarchy(domain)) { struct irq_fwspec spec; spec.fwnode = domain->fwnode; spec.param_count = 2; spec.param[0] = gc->irq.child_offset_to_irq(gc, offset); spec.param[1] = IRQ_TYPE_NONE; return irq_create_fwspec_mapping(&spec); } Same here: in the absence of a child_offset_to_irq() callback, the default gpiochip_child_offset_to_irq_noop() will be used, assuming an identity mapping between GPIO numbers and IRQ numbers. So perhaps 1. you need to provide a child_offset_to_irq() callback, 2. gpiochip_irqchip_remove() needs to apply the child_offset_to_irq() mapping too? 3. you do need the mask, or let child_offset_to_irq() an error code, to avoid irq_{find,dispose}_mapping() handling non-existent irqs? Or am I missing something? I guess this is easy to verify by adding some debug prints to the code. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
Hi Geert On Fri, May 13, 2022 at 3:29 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, May 13, 2022 at 3:56 PM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Fri, May 13, 2022 at 7:53 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Thu, May 12, 2022 at 7:36 PM Lad, Prabhakar > > > <prabhakar.csengg@gmail.com> wrote: > > > > On Thu, May 12, 2022 at 8:39 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > On Wed, May 11, 2022 at 8:32 PM Lad Prabhakar > > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > > 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@bp.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. > > > > Are you suggesting the callback to avoid looping through all the GPIO pins? > > > > > > gpiochip_irqchip_remove() does: > > > > > > /* Remove all IRQ mappings and delete the domain */ > > > if (gc->irq.domain) { > > > unsigned int irq; > > > > > > for (offset = 0; offset < gc->ngpio; offset++) { > > > if (!gpiochip_irqchip_irq_valid(gc, offset)) > > > continue; > > > > > > irq = irq_find_mapping(gc->irq.domain, offset); > > > irq_dispose_mapping(irq); > > > } > > > > > > irq_domain_remove(gc->irq.domain); > > > > > > } > > > > > > The main thing is not about avoiding to loop through all GPIO pins, > > > but to avoid irq_{find,dispose}_mapping() doing the wrong thing. > > So in our case if we don't implement valid masks, that would mean all > > the pins are valid. irq_find_mapping() would return 0 if no mapping is > > found to the corresponding offset and irq_dispose_mapping() would > > simply return back without doing anything if virq == 0.(In this patch > > rzg2l_gpio_free() does call irq_{find,dispose}_mapping()) > > But "offset" is a number from the GPIO offset space (0-122), while The "offset" reported by kernel is 120-511: root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 120-511, parent: platform/11030000.pinctrl, 11030000.pinctrl: gpio-120 (P0_0 ) gpio-121 (P0_1 ) gpio-122 (P0_2 ) gpio-123 (P0_3 ) gpio-124 (P0_4 ) ..... gpio-507 (P48_3 ) gpio-508 (P48_4 ) gpio-509 (P48_5 ) gpio-510 (P48_6 ) gpio-511 (P48_7 ) > irq_find_mapping() expects a number from the domain's IRQ space, > which is only 0-31? > Nope, let me demonstrate with an example, I have configured the gpio pins as GPIO keys in DTS: + keyboard { + compatible = "gpio-keys"; + status = "okay"; + + key-1 { + gpios = <&pinctrl RZG2L_GPIO(43, 0) GPIO_ACTIVE_HIGH>; + linux,code = <KEY_1>; + linux,input-type = <EV_KEY>; + wakeup-source; + label = "SW1"; + }; + + key-2 { + gpios = <&pinctrl RZG2L_GPIO(41, 0) GPIO_ACTIVE_HIGH>; + linux,code = <KEY_2>; + linux,input-type = <EV_KEY>; + wakeup-source; + label = "SW2"; + }; + + key-3 { + gpios = <&pinctrl RZG2L_GPIO(43, 1) GPIO_ACTIVE_HIGH>; + linux,code = <KEY_3>; + linux,input-type = <EV_KEY>; + wakeup-source; + label = "SW3"; + }; + }; root@smarc-rzg2l:~# cat /proc/interrupts | grep SW root@smarc-rzg2l:~# root@smarc-rzg2l:~# insmod gpio_keys.ko [ 925.002720] input: keyboard as /devices/platform/keyboard/input/input3 root@smarc-rzg2l:~# cat /proc/interrupts | grep SW 82: 0 0 11030000.pinctrl 344 Edge SW1 83: 0 0 11030000.pinctrl 328 Edge SW2 84: 0 0 11030000.pinctrl 345 Edge SW3 root@smarc-rzg2l:~# In here 82/83/84 are virq and 344/328/345 are hwirq, which can be confirmed from sysfs file: root@smarc-rzg2l:~# cat /sys/kernel/debug/irq/irqs/82 handler: handle_fasteoi_irq device: (null) status: 0x00000001 istate: 0x00000000 ddepth: 0 wdepth: 0 dstate: 0x13400201 IRQ_TYPE_EDGE_RISING IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_DEFAULT_TRIGGER_SET IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 0-1 effectiv: domain: :soc:pinctrl@11030000 hwirq: 0x158 chip: 11030000.pinctrl flags: 0x800 IRQCHIP_IMMUTABLE parent: domain: :soc:interrupt-controller@110a0000 hwirq: 0x9 chip: rzg2l-irqc flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE parent: domain: :soc:interrupt-controller@11900000-1 hwirq: 0x1dc chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE root@smarc-rzg2l:~# root@smarc-rzg2l:~# root@smarc-rzg2l:~# cat /sys/kernel/debug/irq/irqs/83 handler: handle_fasteoi_irq device: (null) status: 0x00000001 istate: 0x00000000 ddepth: 0 wdepth: 0 dstate: 0x13400201 IRQ_TYPE_EDGE_RISING IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_DEFAULT_TRIGGER_SET IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 0-1 effectiv: domain: :soc:pinctrl@11030000 hwirq: 0x148 chip: 11030000.pinctrl flags: 0x800 IRQCHIP_IMMUTABLE parent: domain: :soc:interrupt-controller@110a0000 hwirq: 0xa chip: rzg2l-irqc flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE parent: domain: :soc:interrupt-controller@11900000-1 hwirq: 0x1dd chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE root@smarc-rzg2l:~# root@smarc-rzg2l:~# cat /sys/kernel/debug/irq/irqs/84 handler: handle_fasteoi_irq device: (null) status: 0x00000001 istate: 0x00000000 ddepth: 0 wdepth: 0 dstate: 0x13400201 IRQ_TYPE_EDGE_RISING IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_DEFAULT_TRIGGER_SET IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 0-1 effectiv: domain: :soc:pinctrl@11030000 hwirq: 0x159 chip: 11030000.pinctrl flags: 0x800 IRQCHIP_IMMUTABLE parent: domain: :soc:interrupt-controller@110a0000 hwirq: 0xb chip: rzg2l-irqc flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE parent: domain: :soc:interrupt-controller@11900000-1 hwirq: 0x1de chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE root@smarc-rzg2l:~# root@smarc-rzg2l:~# root@smarc-rzg2l:~# root@smarc-rzg2l:~# rmmod gpio_keys.ko [ 1143.037314] rzg2l_gpio_free offset:345 virq:84 [ 1143.042488] rzg2l_gpio_free offset:328 virq:83 [ 1143.048700] rzg2l_gpio_free offset:344 virq:82 root@smarc-rzg2l:~# root@smarc-rzg2l:~# I have added print in gpio_free callback where irq_{find,dispose}_mapping()) prints the correct value above. > > > The loop is over all GPIO offsets, while not all of them are mapped > > > to valid interrupts. Does the above work correctly? > > > > > I haven't tested unloading the pinctrl driver which should call > > gpiochip_irqchip_remove() (we don't have remove call back for pinctrl > > driver) > > > > > > > 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? > > > > Yep that's correct. > > > > > > > > > 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. > > > > > > > > > The mask initialization is a one time process and that is during > > > > adding the GPIO chip. At this stage we won't be knowing what will be > > > > the valid GPIO pins used as interrupts. Maybe the core needs to > > > > implement a callback which lands in the GPIO controller driver to tell > > > > if the gpio irq line is valid. This way we can handle dynamic > > > > interrupts. > > > > > > Upon closer look, I think the mask is a red herring, and we don't > > > need it. > > Agreed. > > > > > But we do need to handle the (possible) mismatch between GPIO > > > offset (index) and IRQ offset in the above code. > > > > > Agreed, do you see any possibility of the mismatch I have missed? > > gpiochip_to_irq(): > > if (irq_domain_is_hierarchy(domain)) { > struct irq_fwspec spec; > > spec.fwnode = domain->fwnode; > spec.param_count = 2; > spec.param[0] = gc->irq.child_offset_to_irq(gc, offset); > spec.param[1] = IRQ_TYPE_NONE; > > return irq_create_fwspec_mapping(&spec); > } > > Same here: in the absence of a child_offset_to_irq() callback, > the default gpiochip_child_offset_to_irq_noop() will be used, > assuming an identity mapping between GPIO numbers and IRQ > numbers. > Agreed, gpiochip_child_offset_to_irq_noop will return the "offset", but irq_create_fwspec_mapping() in gpiochip_to_irq() will return the virq number which will not be equal to the offset. I added the below change in gpio_keys.c where it calls gpiod_to_irq() -> to_irq() and the below is the log: --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -589,6 +589,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev, button->gpio, error); return error; } + dev_err(dev,"%s gpiod_to_irq() = (irq) %d\n", __func__, irq); + bdata->irq = irq; } root@smarc-rzg2l:~# insmod gpio_keys.ko [ 54.288678] gpio-keys keyboard: gpio_keys_setup_key gpiod_to_irq() = (irq) 82 [ 54.297230] gpio-keys keyboard: gpio_keys_setup_key gpiod_to_irq() = (irq) 83 [ 54.311256] gpio-keys keyboard: gpio_keys_setup_key gpiod_to_irq() = (irq) 84 [ 54.332560] input: keyboard as /devices/platform/keyboard/input/input0 root@smarc-rzg2l:~# > So perhaps > 1. you need to provide a child_offset_to_irq() callback, > 2. gpiochip_irqchip_remove() needs to apply the child_offset_to_irq() > mapping too? > 3. you do need the mask, or let child_offset_to_irq() an error code, > to avoid irq_{find,dispose}_mapping() handling non-existent irqs? >
Hi Prabhakar, Thanks for the example. > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain > to handle GPIO interrupt > > > But "offset" is a number from the GPIO offset space (0-122), while > > The "offset" reported by kernel is 120-511: > > root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio > gpiochip0: GPIOs 120-511, parent: platform/11030000.pinctrl, > 11030000.pinctrl: > gpio-120 (P0_0 ) > gpio-121 (P0_1 ) > gpio-122 (P0_2 ) > gpio-123 (P0_3 ) > gpio-124 (P0_4 ) > ..... > gpio-507 (P48_3 ) > gpio-508 (P48_4 ) > gpio-509 (P48_5 ) > gpio-510 (P48_6 ) > gpio-511 (P48_7 ) > > > irq_find_mapping() expects a number from the domain's IRQ space, which > > is only 0-31? > > > Nope, let me demonstrate with an example, I have configured the gpio pins > as GPIO keys in DTS: > > + keyboard { > + compatible = "gpio-keys"; > + status = "okay"; > + > + key-1 { > + gpios = <&pinctrl RZG2L_GPIO(43, 0) > GPIO_ACTIVE_HIGH>; > + linux,code = <KEY_1>; > + linux,input-type = <EV_KEY>; > + wakeup-source; > + label = "SW1"; > + }; > + > + key-2 { > + gpios = <&pinctrl RZG2L_GPIO(41, 0) > GPIO_ACTIVE_HIGH>; > + linux,code = <KEY_2>; > + linux,input-type = <EV_KEY>; > + wakeup-source; > + label = "SW2"; > + }; > + > + key-3 { > + gpios = <&pinctrl RZG2L_GPIO(43, 1) > GPIO_ACTIVE_HIGH>; > + linux,code = <KEY_3>; > + linux,input-type = <EV_KEY>; > + wakeup-source; > + label = "SW3"; > + }; > + }; > > root@smarc-rzg2l:~# cat /proc/interrupts | grep SW root@smarc-rzg2l:~# > root@smarc-rzg2l:~# insmod gpio_keys.ko [ 925.002720] input: keyboard as > /devices/platform/keyboard/input/input3 > root@smarc-rzg2l:~# cat /proc/interrupts | grep SW > 82: 0 0 11030000.pinctrl 344 Edge SW1 > 83: 0 0 11030000.pinctrl 328 Edge SW2 > 84: 0 0 11030000.pinctrl 345 Edge SW3 > root@smarc-rzg2l:~# > > In here 82/83/84 are virq and 344/328/345 are hwirq, which can be confirmed > from sysfs file: From your example, Looks like I believe from interrupt statistics point of view, cat /proc/interrupts should report actual gpioint number (0->122) corresponding to pin index for SW1, SW2 and SW3 ?? May be another mapping required for pinindex to gpioint to get proper statistics?? From usage point, another point is, who will track gpioint statistics, pinctrl driver or framework?? Example Use case:- create gpioint0-30 which will fill tint0-tint30. Then insmod gpioint corresponding to SW1 and trigger 1 interrupt and check cat /proc/interrupts for tint31 and SW1 Then rmmode gpioint corresponding to SW1 and insmod SW2 and trigger 5 interrupts and check cat /proc/interrupts for tint31 and SW2 Then rmmode gpioint corresponding to SW2 and insmod SW3 and trigger 7 interrupts and check cat /proc/interrupts for tint31 and SW3 Then rmmode gpioint corresponding to SW3 and insmod SW1 and check cat /proc/interrupts for tint31 and SW1 Then rmmode gpioint corresponding to SW1 and insmod SW2 and check cat /proc/interrupts for tint31 and SW2 Tint31 should report 13 interrupts gpioint corresponding to SW1 should report 1 interrupt gpioint corresponding to SW2 should report 5 interrupts gpioint corresponding to SW3 should report 7 interrupts Cheers, Biju
Hi Marc, Thanks for the feedback. > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain > to handle GPIO interrupt > > On Sun, 15 May 2022 06:13:22 +0100, > Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > Hi Prabhakar, > > > > Thanks for the example. > > > > > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ > > > domain to handle GPIO interrupt > > > > > > > But "offset" is a number from the GPIO offset space (0-122), while > > > > > > The "offset" reported by kernel is 120-511: > > > > > > root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio > > > gpiochip0: GPIOs 120-511, parent: platform/11030000.pinctrl, > > > 11030000.pinctrl: > > > gpio-120 (P0_0 ) > > > gpio-121 (P0_1 ) > > > gpio-122 (P0_2 ) > > > gpio-123 (P0_3 ) > > > gpio-124 (P0_4 ) > > > ..... > > > gpio-507 (P48_3 ) > > > gpio-508 (P48_4 ) > > > gpio-509 (P48_5 ) > > > gpio-510 (P48_6 ) > > > gpio-511 (P48_7 ) > > > > > > > irq_find_mapping() expects a number from the domain's IRQ space, > > > > which is only 0-31? > > > > > > > Nope, let me demonstrate with an example, I have configured the gpio > > > pins as GPIO keys in DTS: > > > > > > + keyboard { > > > + compatible = "gpio-keys"; > > > + status = "okay"; > > > + > > > + key-1 { > > > + gpios = <&pinctrl RZG2L_GPIO(43, 0) > > > GPIO_ACTIVE_HIGH>; > > > + linux,code = <KEY_1>; > > > + linux,input-type = <EV_KEY>; > > > + wakeup-source; > > > + label = "SW1"; > > > + }; > > > + > > > + key-2 { > > > + gpios = <&pinctrl RZG2L_GPIO(41, 0) > > > GPIO_ACTIVE_HIGH>; > > > + linux,code = <KEY_2>; > > > + linux,input-type = <EV_KEY>; > > > + wakeup-source; > > > + label = "SW2"; > > > + }; > > > + > > > + key-3 { > > > + gpios = <&pinctrl RZG2L_GPIO(43, 1) > > > GPIO_ACTIVE_HIGH>; > > > + linux,code = <KEY_3>; > > > + linux,input-type = <EV_KEY>; > > > + wakeup-source; > > > + label = "SW3"; > > > + }; > > > + }; > > > > > > root@smarc-rzg2l:~# cat /proc/interrupts | grep SW > > > root@smarc-rzg2l:~# root@smarc-rzg2l:~# insmod gpio_keys.ko [ > > > 925.002720] input: keyboard as > > > /devices/platform/keyboard/input/input3 > > > root@smarc-rzg2l:~# cat /proc/interrupts | grep SW > > > 82: 0 0 11030000.pinctrl 344 Edge SW1 > > > 83: 0 0 11030000.pinctrl 328 Edge SW2 > > > 84: 0 0 11030000.pinctrl 345 Edge SW3 > > > root@smarc-rzg2l:~# > > > > > > In here 82/83/84 are virq and 344/328/345 are hwirq, which can be > > > confirmed from sysfs file: > > > > From your example, Looks like > > > > I believe from interrupt statistics point of view, cat > > /proc/interrupts should report actual gpioint number (0->122) > > corresponding to pin index for SW1, SW2 and SW3 ?? > > No. There is no need for such userspace-visible behaviour. Userspace has no > business tracking those. The required information is in debugfs, and that > more than enough. Ok, So far I used cat /proc/interrupts for debugging, since I don't need to enable DEBUG config for Enabling Debugfs for irq. This Debugfs irq is new info to me. Our hardware manual has below info for usb-phy irq 2H0_OBINT 126(InterruptID) SPI 94 IRQ 94 Level cat /proc/interrupts matches with GICV3 Interrupt ID/ type in the HW manual 113: 0 0 GICv3 126 Level 11c50200.usb-phy Debugfs is also showing similar info like hwirq and interrupt type. But I don't know which field corresponds to number of interrupts? root@smarc-rzg2l:~# cat /sys/kernel/debug/irq/irqs/113 handler: handle_fasteoi_irq device: (null) status: 0x00000104 istate: 0x00000000 ddepth: 0 wdepth: 0 dstate: 0x13402204 IRQ_TYPE_LEVEL_HIGH IRQD_LEVEL IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_SINGLE_TARGET IRQD_DEFAULT_TRIGGER_SET IRQD_HANDLE_ENFORCE_IRQCTX node: 0 affinity: 0-1 effectiv: 0 domain: :soc:interrupt-controller@11900000-1 hwirq: 0x7e chip: GICv3 flags: 0x15 IRQCHIP_SET_TYPE_MASKED IRQCHIP_MASK_ON_SUSPEND IRQCHIP_SKIP_SET_WAKE Now coming to current case, Currently GPIO INT 0-122(123 interrupts) corresponding to 120-511(291 interrupts) with same invalid lines.
Hi Marc, .org>; Phil Edworthy > <phil.edworthy@renesas.com> > Subject: Re: [PATCH v3 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ > domain to handle GPIO interrupt > > On Mon, 16 May 2022 08:20:47 +0100, > Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > > > > > > > I believe from interrupt statistics point of view, cat > > > > /proc/interrupts should report actual gpioint number (0->122) > > > > corresponding to pin index for SW1, SW2 and SW3 ?? > > > > > > No. There is no need for such userspace-visible behaviour. Userspace > > > has no business tracking those. The required information is in > > > debugfs, and that more than enough. > > > > Ok, So far I used cat /proc/interrupts for debugging, since I don't > > need to enable DEBUG config for Enabling Debugfs for irq. This Debugfs > > irq is new info to me. > > > > Our hardware manual has below info for usb-phy irq > > 2H0_OBINT 126(InterruptID) SPI 94 IRQ 94 Level > > > > cat /proc/interrupts matches with GICV3 Interrupt ID/ type in the HW > manual > > 113: 0 0 GICv3 126 Level 11c50200.usb-phy > > > > Debugfs is also showing similar info like hwirq and interrupt type. > > But I don't know which field corresponds to number of interrupts? > > > > root@smarc-rzg2l:~# cat /sys/kernel/debug/irq/irqs/113 > > handler: handle_fasteoi_irq > > device: (null) > > status: 0x00000104 > > istate: 0x00000000 > > ddepth: 0 > > wdepth: 0 > > dstate: 0x13402204 > > IRQ_TYPE_LEVEL_HIGH > > IRQD_LEVEL > > IRQD_ACTIVATED > > IRQD_IRQ_STARTED > > IRQD_SINGLE_TARGET > > IRQD_DEFAULT_TRIGGER_SET > > IRQD_HANDLE_ENFORCE_IRQCTX > > node: 0 > > affinity: 0-1 > > effectiv: 0 > > domain: :soc:interrupt-controller@11900000-1 > > hwirq: 0x7e > > 0x7e = 126 = 94 - 32 -> SPI94. > > What else do you need? OK, similar to GIC, I thought for gpio interrupts, The hwirq should match with gpiointN mentioned in hwmanual. That is all. Any way it is minor thing, it may be not at all needed. Please ignore this. Eg:-for gpioint0, it should be root@smarc-rzg2l:~# cat /proc/interrupts | grep SW 82: 0 0 11030000.pinctrl 0 Edge XXX Not like root@smarc-rzg2l:~# cat /proc/interrupts | grep SW 82: 0 0 11030000.pinctrl 120 Edge XXX Cheers, Biju > > > chip: GICv3 > > flags: 0x15 > > IRQCHIP_SET_TYPE_MASKED > > IRQCHIP_MASK_ON_SUSPEND > > IRQCHIP_SKIP_SET_WAKE > > > > Now coming to current case, > > > > Currently GPIO INT 0-122(123 interrupts) corresponding to > > 120-511(291 interrupts) with same invalid lines. > > > > From a debugging point, If user has put same irq name for gpioints(cat > > /proc/interrupts case), then how do we distinguish these interrupts?? > > (using hwirq??) > > Yes. > > > > > For using Debugfs, Do we need to first execute cat /proc/interrupts to > > get virq and from there we need to use virq to get statistics, right? > > It depends what you want to do. /sys/kernel/debug/irq/irqs/ has the exact > same information. The only thing /proc/interrupts has that debugfs doesn't > is the per-CPU accounting of delivered interrupts.
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 patches add hierarchical IRQ domain, one in IRQC driver and another 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 v2->v3: * Updated description for interrupts-cells property in patch #1 * Included RB tag from Geert for binding patch * Fixed review comments pointed by Geert, Biju and Sergei. Changes for v1->v2: * Included RB tag from Rob * Fixed review comments pointed by Geert * included GPIO driver changes Changes for RFCV4 -> V1: * Used unevaluatedProperties. * Altered the sequence of reg property * Set the parent type * Used raw_spin_lock() instead of raw_spin_lock_irqsave() * Simplified parsing IRQ map. * Will send the GPIO and pinctrl changes as part of separate series Changes for RFC 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/ 20220316200633.28974-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/ Changes for RFC 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 v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/ 20210921193028.13099-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/ RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/ 20210803175109.1729-1-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/ 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 | 134 ++++++ drivers/gpio/gpiolib.c | 13 +- drivers/irqchip/Kconfig | 8 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-renesas-rzg2l.c | 444 ++++++++++++++++++ drivers/pinctrl/renesas/pinctrl-rzg2l.c | 202 ++++++++ include/linux/gpio/driver.h | 8 + 7 files changed, 805 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c