Re: [PATCH v4 2/2] pinctrl: sunxi: fix gpiochip_lock_as_irq() failure when pinmux is unknown

[Andre Przywara]

Hi Michal,

thanks for the very quick turnaround!

On 3/19/26 17:44, Chen-Yu Tsai wrote:
> On Fri, Mar 20, 2026 at 12:10 AM Michal Piekos
> <michal.piekos@xxxxxxxxxxxxx> wrote:
> > Fixes kernel hang during boot due to inability to set up IRQ on AXP313a.
> >
> > The issue is caused by gpiochip_lock_as_irq() which is failing when gpio
> > is in unitialized state.
>
>          ^ uninitialized
>
> > Solution is to set pinmux to GPIO INPUT in
> > sunxi_pinctrl_irq_request_resources() if it wasn't initialized
> > earlier.
> >
> > Tested on Orange Pi Zero 3.
> >
> > Fixes: 01e10d0272b9 ("pinctrl: sunxi: Implement gpiochip::get_direction()")
> > Signed-off-by: Michal Piekos <michal.piekos@xxxxxxxxxxxxx>
> > ---
> >   drivers/pinctrl/sunxi/pinctrl-sunxi.c | 19 +++++++++++++++++++
> >   drivers/pinctrl/sunxi/pinctrl-sunxi.h |  2 ++
> >   2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > index 685b79fc0bf8..321ee97f5745 100644
> > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > @@ -1092,13 +1092,32 @@ static int sunxi_pinctrl_irq_request_resources(struct irq_data *d)
> >   {
> >          struct sunxi_pinctrl *pctl = irq_data_get_irq_chip_data(d);
> >          struct sunxi_desc_function *func;
> > +       unsigned int offset;
> > +       u32 reg, shift, mask;
> > +       u8 muxval;
> >          int ret;
> > +       bool is_new_layout;
> > +       bool is_reset_mux;
> >
> >          func = sunxi_pinctrl_desc_find_function_by_pin(pctl,
> >                                          pctl->irq_array[d->hwirq], "irq");
> >          if (!func)
> >                  return -EINVAL;
> >
> > +       offset = pctl->irq_array[d->hwirq] - pctl->desc->pin_base;
> > +       sunxi_mux_reg(pctl, offset, &reg, &shift, &mask);
> > +       muxval = (readl(pctl->membase + reg) & mask) >> shift;
> > +
> > +       /* Change muxing to GPIO INPUT mode if at reset value */
> > +       is_new_layout = pctl->flags & SUNXI_PINCTRL_NEW_REG_LAYOUT;
>
> You would want
>
>      !!(pctl->flags & SUNXI_PINCTRL_NEW_REG_LAYOUT)
>
> here to normalize it to a bool value. This is quite common in kernel code.

The whole expression above is a bit cumbersome (though correct). And 
since is_reset_mux isn't really needed, I'd suggest something like:

	u8 disabled_mux;
....
	if (pctl->flags & SUNXI_PINCTRL_NEW_REG_LAYOUT)
		disabled_mux = SUN4I_FUNC_DISABLED_NEW;
	else
		disabled_mux = SUN4I_FUNC_DISABLED_OLD;
	if (muxval == disabled_mux)
....

But yeah, in general this was what I had in mind.

Cheers,
Andre


	
> > +       is_reset_mux = (!is_new_layout && muxval == SUN4I_FUNC_DISABLED_OLD) ||
> > +                      (is_new_layout && muxval == SUN4I_FUNC_DISABLED_NEW);
> > +
> > +       if (is_reset_mux) {
> > +               sunxi_pmx_set(pctl->pctl_dev, pctl->irq_array[d->hwirq],
> > +                             SUN4I_FUNC_INPUT);
> > +       }
>
> Nit: the curly braces aren't needed.
>
> > +
> >          ret = gpiochip_lock_as_irq(pctl->chip,
> >                          pctl->irq_array[d->hwirq] - pctl->desc->pin_base);
>
> Nit: you probably want to replace this with "offset" as well.
>
>
> Just minor issues, otherwise
>
> Reviewed-by: Chen-Yu Tsai <wens@xxxxxxxxxx>
>
> >          if (ret) {
> > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > index 22bffac1c3f0..0daf7600e2fb 100644
> > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> > @@ -86,6 +86,8 @@
> >
> >   #define SUN4I_FUNC_INPUT       0
> >   #define SUN4I_FUNC_IRQ         6
> > +#define SUN4I_FUNC_DISABLED_OLD 7
> > +#define SUN4I_FUNC_DISABLED_NEW 15
> >
> >   #define SUNXI_PINCTRL_VARIANT_MASK     GENMASK(7, 0)
> >   #define SUNXI_PINCTRL_NEW_REG_LAYOUT   BIT(8)
> >
> > --
> > 2.43.0