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

Chen-Yu Tsai posted 1 patch 2 weeks, 2 days ago
There is a newer version of this series
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 51 +++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
[PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Chen-Yu Tsai 2 weeks, 2 days ago
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.

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.

Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
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;
-- 
2.47.3
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Linus Walleij 1 week, 1 day ago
On Mon, Feb 16, 2026 at 5:09 PM 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.
>
> 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.
>
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>

Patch applied for fixes since everyone likes this patch and
everyone is unhappy with the warning.

Yours,
Linus Walleij
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Chen-Yu Tsai 1 week, 1 day ago
Hi Linus,

On Tue, Feb 24, 2026 at 4:43 PM Linus Walleij <linusw@kernel.org> wrote:
>
> On Mon, Feb 16, 2026 at 5:09 PM 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.
> >
> > 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.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
>
> Patch applied for fixes since everyone likes this patch and
> everyone is unhappy with the warning.

I was about to send v2 with an updated commit message but didn't get around
to sending it out. Would you consider dropping this one?

Sorry about the delay.


ChenYu
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Bartosz Golaszewski 2 weeks, 1 day ago
On Mon, 16 Feb 2026 17:09:45 +0100, Chen-Yu Tsai <wens@kernel.org> said:
> 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.
>
> 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.
>
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Andre Przywara 2 weeks, 1 day ago
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;
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Chen-Yu Tsai 2 weeks, 1 day ago
On Wed, Feb 18, 2026 at 12:32 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> 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?

Digging deeper, my attribution is incorrect. The actual commit that
triggers the warning is 471e998c0e31 ("gpiolib: remove redundant callback
check"), in which a check of .get_direction before calling it was removed.
So I believe this just landed for the next release.

Thanks for spotting that. I'll send a v2 to add that information.

> > 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:

Yeah, I was a bit undecided, but using the raw values for in / out and then
having a string comparison for irq made things even weirder.

> > Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
>
> FWIW:
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>


Thanks!
Re: [PATCH] pinctrl: sunxi: Implement gpiochip::get_direction()
Posted by Jernej Škrabec 2 weeks, 2 days ago
Dne ponedeljek, 16. februar 2026 ob 17:09:45 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> 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.
> 
> 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.
> 
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej