RE: [PATCH 2/7] pinctrl: renesas: rzt2h: restore correct pin mode on IRQ free

[Cosmin-Gabriel Tanislav]

> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Tuesday, September 8, 2026 3:16 PM
> 
> Hi Cosmin,
> 
> On Tue, 8 Sept 2026 at 13:33, 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:
> > > > rzt2h_gpio_irq_domain_free() calls rzt2h_pinctrl_set_gpio_en() with
> > > > false leaving the pin in interrupt function instead of returning it to
> > > > GPIO mode.
> > >
> > > OK....
> > >
> > > > Pass true to rzt2h_pinctrl_set_gpio_en() to take the pin out of
> > > > interrupt function after we're done using it as an IRQ.
> > > >
> > > > rzt2h_pinctrl_set_pfc_mode() switches the pin to Hi-Z, losing the
> > > > previous PM value.
> > > >
> > > > Save the PM value before switching to Hi-Z, and restore it after the
> > > > IRQ is freed.
> > >
> > > Why is it a problem if the pin is in Hi-Z after the interrupt has been freed?
> > > Isn't that the right state for an unused pin?
> >
> > It wouldn't be an issue if it was actually unused once the IRQ is freed,
> > but that's not the case, because you can call gpiod_to_irq() on a GPIO,
> > and then request_irq() on the IRQ returned by it. If you then call
> > free_irq(), you only free the IRQ, not the GPIO itself. After that the
> > user of these APIs should still be able to continue using the GPIO.
> 
> OK.
> 
> > If the pin is left in Hi-Z after freeing the IRQ, rzt2h_gpio_get() will
> > see that the pin is not in peripheral mode anymore, it will then see
> > that neither PM_INPUT nor PM_OUTPUT are set, and will return -EINVAL.
> 
> That is fixed by "[PATCH 4/7] pinctrl: renesas: rzt2h: fix reading pin
> value in IRQ function" anyway, right?
> 

Patch 4 fixes get() usage when the pin is in PFC IRQ mode only. That's
particularly useful when using the GPIO character device together with
GPIO_V2_LINE_FLAG_EDGE_BOTH to detect edge transitions, as get() is
used to tell whether the edge was falling or rising
(see edge_irq_thread()).

This patch fixes the state after the IRQ is freed.

Once the IRQ is freed, rzt2h_pin_mode_is_peripheral() will return false,
so the PFC IRQ specific logic is not reached, and the normal GPIO mode
logic will be taken, which, since we didn't restore PM to PM_INPUT or
PM_OUTPUT, will return -EINVAL.

Sorry if this is hard to understand, it took me a while to get how it
works too.

> > > > --- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> > > > +++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> > >
> > > > @@ -170,6 +171,25 @@ static int rzt2h_validate_pin(struct rzt2h_pinctrl *pctrl, unsigned int
> offset)
> > > >         return (pincfg & BIT(pin)) ? 0 : -EINVAL;
> > > >  }
> > > >
> > > > +static u8 rzt2h_pin_read_pm(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin)
> > > > +{
> > > > +       u16 reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
> > > > +
> > > > +       return field_get(PM_PIN_MASK(pin), reg);
> > > > +}
> > > > +
> > > > +static void rzt2h_pin_write_pm(struct rzt2h_pinctrl *pctrl, u8 port, u8 pin, u8 pm)
> > > > +{
> > > > +       u16 reg;
> > > > +
> > > > +       guard(raw_spinlock_irqsave)(&pctrl->lock);
> > > > +
> > > > +       reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
> > > > +       reg &= ~PM_PIN_MASK(pin);
> > > > +       reg |= (u16)pm << (pin * 2);
> > >
> > > General remark: the need for this cast is one reason why function
> > > parameters should use "unsigned int" instead of u8.  The other reason is
> > > that the compiler may add code to mask the parameters to 8 bits,
> > > depending on the CPU architecture.
> >
> > Should I switch the pm parameter to unsigned int for V2 then?
> 
> Yes please.
> 

Will do in V2.