RE: [PATCH 3/7] pinctrl: renesas: rzt2h: setup IRQ in .irq_{request,release}_resources()

[Cosmin-Gabriel Tanislav]

> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Tuesday, September 8, 2026 12:41 PM
> 
> Hi Cosmin,
> 
> On Mon, 17 Aug 2026 at 20:58, Cosmin Tanislav
> <cosmin-gabriel.tanislav.xa@xxxxxxxxxxx> wrote:
> > When exporting a GPIO using the deprecated sysfs GPIO API,
> > gpio_is_visible() calls gpiod_to_irq() to determine whether a GPIO is
> > IRQ-capable, which ends up using
> > struct gpio_irq_chip::child_to_parent_hwirq().
> >
> > When using the GPIO character device, it is possible to request a line
> > and to dynamically switch between using it as an IRQ or as a GPIO, by
> > updating the edge detector.
> >
> > The current struct gpio_irq_chip::child_to_parent_hwirq() implementation
> > makes the assumption that it will only be called with the intention of
> > setting up the IRQ permanently, and that it will only be released when
> > the IRQ domain is freed.
> >
> > Since we now know that this is not the case, take the following steps in
> > order to properly handle that case.
> >
> > Implement struct irq_chip::irq_{request,release}_resources() based on
> > the default GPIOCHIP_IRQ_RESOURCE_HELPERS implementations.
> >
> > Move the IRQ setup and release logic inside them.
> >
> > Keep the mapping logic inside ::child_to_parent_hwirq() to be able to
> > provide a proper HW IRQ number, but do not actually setup the IRQ.
> >
> > Remove rzt2h_gpio_irq_domain_free() as its only purpose was to free
> > the acquired IRQs.
> >
> > Cc: stable@xxxxxxxxxx
> > Fixes: 829dde3369a9 ("pinctrl: renesas: rzt2h: Add GPIO IRQ chip to handle interrupts")
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@xxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> > @@ -1062,18 +1047,21 @@ static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> >
> >         rzt2h_pinctrl_set_pfc_mode(pctrl, port, pin, PFC_FUNC_INTERRUPT);
> >
> > -       *parent = parent_irq;
> > -       *parent_type = child_type;
> > +       ret = gpiochip_lock_as_irq(gc, hwirq);
> > +       if (ret) {
> > +               clear_bit(parent_irq - RZT2H_INTERRUPTS_START, pctrl->used_irqs);
> 
> irq_idx
> 

Ack.

> > +               rzt2h_pin_write_pm(pctrl, port, pin, pctrl->saved_pm[irq_idx]);
> > +               rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, true);
> > +               return ret;
> > +       }
> >
> >         return 0;
> >  }
> >
> > -static void rzt2h_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> > -                                      unsigned int nr_irqs)
> > +static void rzt2h_gpio_irq_release_resources(struct irq_data *d)
> >  {
> > -       struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> >         struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > -       struct rzt2h_pinctrl *pctrl = container_of(gc, struct rzt2h_pinctrl, gpio_chip);
> > +       struct rzt2h_pinctrl *pctrl = gpiochip_get_data(gc);
> >         irq_hw_number_t hwirq = irqd_to_hwirq(d);
> >         u8 port = RZT2H_PIN_ID_TO_PORT(hwirq);
> >         u8 pin = RZT2H_PIN_ID_TO_PIN(hwirq);
> > @@ -1089,7 +1077,39 @@ static void rzt2h_gpio_irq_domain_free(struct irq_domain *domain, unsigned
> int v
> >                 rzt2h_pinctrl_set_gpio_en(pctrl, port, pin, true);
> >         }
> >
> > -       irq_domain_free_irqs_common(domain, virq, nr_irqs);
> 
> Why is this no longer needed?
> 

We moved from using struct gpio_irq_chip::child_irq_domain_ops::free to
using struct irq_chip::irq_release_resources, so free is no longer set.

devm_gpiochip_add_data() ->
devm_gpiochip_add_data_with_key() ->
gpiochip_add_data_with_key() ->
gpiochip_add_irqchip() ->
gpiochip_hierarchy_create_domain() ->
gpiochip_hierarchy_setup_domain_ops() ->

	if (!ops->free)
		ops->free = irq_domain_free_irqs_common;

It will end up being populated automatically, let me know if you think
we should set it explicitly.

> > +       gpiochip_unlock_as_irq(gc, hwirq);
> > +}
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> 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