Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()

[Andre Przywara <andre.przywara@arm.com>]

On Tue, 17 Feb 2026 00:09:45 +0800
Chen-Yu Tsai <wens@kernel.org> wrote:

> After commit e623c4303ed1 ("gpiolib: sanitize the return value of
> gpio_chip::get_direction()"), a warning will be printed if the
> gpio driver does not implement this callback.

I am curious how this could slip through? Did the get_direction()
callback become mandatory at one point, but no one noticed that it was
missing for sunxi? It looks like the situation was even worse before that
patch, as it was dereferencing the function pointer without any check?

> Implement it for the sunxi driver. This is simply a matter of reading
> out the mux value from the registers, then checking if it is one of
> the GPIO functions and which direction it is.

Mmh, it feels a bit backwards to resort to the function name *string* for
comparison, when it's always 0 for in and 1 for out (which we actually set
in pinctrl-sunxi-dt.c now). But the mux value for IRQ is different between
SoC generations, and I guess for historic reasons function strings are a
thing in pinctrl, so this is probably the best solution after all:

> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>

FWIW:
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

P.S. Should we have some CC: stable tag here?

> ---
> This is an alternative to James's version. My version does one lookup
> instead of three.
> 
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 51 +++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 0fb057a07dcc..27b2a3e9d78d 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -204,6 +204,32 @@ sunxi_pinctrl_desc_find_function_by_pin(struct sunxi_pinctrl *pctl,
>  	return NULL;
>  }
>  
> +static struct sunxi_desc_function *
> +sunxi_pinctrl_desc_find_function_by_pin_and_mux(struct sunxi_pinctrl *pctl,
> +						const u16 pin_num,
> +						const u8 muxval)
> +{
> +	for (unsigned int i = 0; i < pctl->desc->npins; i++) {
> +		const struct sunxi_desc_pin *pin = pctl->desc->pins + i;
> +		struct sunxi_desc_function *func = pin->functions;
> +
> +		if (pin->pin.number != pin_num)
> +			continue;
> +
> +		if (pin->variant && !(pctl->variant & pin->variant))
> +			continue;
> +
> +		while (func->name) {
> +			if (func->muxval == muxval)
> +				return func;
> +
> +			func++;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +
>  static int sunxi_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
>  {
>  	struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> @@ -930,6 +956,30 @@ static const struct pinmux_ops sunxi_pmx_ops = {
>  	.strict			= true,
>  };
>  
> +static int sunxi_pinctrl_gpio_get_direction(struct gpio_chip *chip,
> +					    unsigned int offset)
> +{
> +	struct sunxi_pinctrl *pctl = gpiochip_get_data(chip);
> +	const struct sunxi_desc_function *func;
> +	u32 pin = offset + chip->base;
> +	u32 reg, shift, mask;
> +	u8 muxval;
> +
> +	sunxi_mux_reg(pctl, offset, &reg, &shift, &mask);
> +
> +	muxval = (readl(pctl->membase + reg) & mask) >> shift;
> +
> +	func = sunxi_pinctrl_desc_find_function_by_pin_and_mux(pctl, pin, muxval);
> +	if (!func)
> +		return -ENODEV;
> +
> +	if (!strcmp(func->name, "gpio_out"))
> +		return GPIO_LINE_DIRECTION_OUT;
> +	if (!strcmp(func->name, "gpio_in") || !strcmp(func->name, "irq"))
> +		return GPIO_LINE_DIRECTION_IN;
> +	return -EINVAL;
> +}
> +
>  static int sunxi_pinctrl_gpio_direction_input(struct gpio_chip *chip,
>  					unsigned offset)
>  {
> @@ -1601,6 +1651,7 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>  	pctl->chip->request = gpiochip_generic_request;
>  	pctl->chip->free = gpiochip_generic_free;
>  	pctl->chip->set_config = gpiochip_generic_config;
> +	pctl->chip->get_direction = sunxi_pinctrl_gpio_get_direction;
>  	pctl->chip->direction_input = sunxi_pinctrl_gpio_direction_input;
>  	pctl->chip->direction_output = sunxi_pinctrl_gpio_direction_output;
>  	pctl->chip->get = sunxi_pinctrl_gpio_get;