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

[Geert Uytterhoeven]

Hi Cosmin,

On Tue, 8 Sept 2026 at 14:23, Cosmin-Gabriel Tanislav
<cosmin-gabriel.tanislav.xa@xxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > 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>

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

Thanks, that's the part I was missing.  Please document in the patch
description that irq_domain_free_irqs_common() is set up automatically.

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