pinctrl: sunxi: Implement function_is_gpio

Message ID 20260517171405.3697469-1-paulk@sys-base.io (mailing list archive)
State New
Headers
Series pinctrl: sunxi: Implement function_is_gpio |

Commit Message

Paul Kocialkowski May 17, 2026, 5:14 p.m. UTC
The function_is_gpio pinmux op allows the core to find out whether a
GPIO can be safely requested from a pinctrl property and requested as a
GPIO at the same time.

This is especially useful to request a GPIO with a particular drive
strength, which would otherwise not be possible.

Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
  

Comments

Andre Przywara May 25, 2026, 9:38 p.m. UTC | #1
On Sun, 17 May 2026 19:14:05 +0200
Paul Kocialkowski <paulk@sys-base.io> wrote:

Hi Paul,

> The function_is_gpio pinmux op allows the core to find out whether a
> GPIO can be safely requested from a pinctrl property and requested as a
> GPIO at the same time.
> 
> This is especially useful to request a GPIO with a particular drive
> strength, which would otherwise not be possible.

That looks a easy enough solution, but:

> Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index d3042e0c9712..6162f2d86723 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -821,6 +821,17 @@ static int sunxi_pmx_get_func_groups(struct pinctrl_dev *pctldev,
>  	return 0;
>  }
>  
> +static bool sunxi_pmx_function_is_gpio(struct pinctrl_dev *pctldev,
> +				       unsigned function)
> +{
> +	struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	if (!strncmp(pctl->functions[function].name, "gpio", 4))

I wonder if this condition is a bit too relaxed? There could be
some (theoretical) function just starting with gpio, but not being mux
0 or 1. So should we check for gpio_in or gpio_out, explicitly? Or at
least use (strcmp(name, "gpio_", 5)? Or maybe even better for the mux
value directly? Is "function" an indicator of this, or does this rely
on the two GPIO functions being always listed first, at least so far?

And what about the IRQ function? Isn't that some GPIO as well, or does
that not count for the purpose of the function_is_gpio() callback?

Cheers,
Andre

> +		return true;
> +
> +	return false;
> +}
> +
>  static void sunxi_pmx_set(struct pinctrl_dev *pctldev,
>  				 unsigned pin,
>  				 u8 config)
> @@ -952,6 +963,7 @@ static const struct pinmux_ops sunxi_pmx_ops = {
>  	.get_functions_count	= sunxi_pmx_get_funcs_cnt,
>  	.get_function_name	= sunxi_pmx_get_func_name,
>  	.get_function_groups	= sunxi_pmx_get_func_groups,
> +	.function_is_gpio	= sunxi_pmx_function_is_gpio,
>  	.set_mux		= sunxi_pmx_set_mux,
>  	.gpio_set_direction	= sunxi_pmx_gpio_set_direction,
>  	.request		= sunxi_pmx_request,
  
Linus Walleij May 26, 2026, 9:34 a.m. UTC | #2
On Mon, May 25, 2026 at 11:39 PM Andre Przywara <andre.przywara@arm.com> wrote:

> And what about the IRQ function? Isn't that some GPIO as well, or does
> that not count for the purpose of the function_is_gpio() callback?

The purpose of the function is to tell the GPIO subsystem if a pin
is muxed into GPIO mode, and by that we mean a driver using
struct gpio_chip, so we can approve a request of gpiod_get*().

The struct irq_chip is often orthogonal and I think it would usually
"just work", and a struct irq_chip unrelated to a struct gpio_chip
(i.e. there is no way to actually drive or listen to the lines) is
something completely separate from struct gpio_chip.

Yours,
Linus Walleij
  
Paul Kocialkowski May 27, 2026, 8:08 a.m. UTC | #3
Hi Andrew,

On Mon 25 May 26, 23:38, Andre Przywara wrote:
> On Sun, 17 May 2026 19:14:05 +0200
> Paul Kocialkowski <paulk@sys-base.io> wrote:
> 
> Hi Paul,
> 
> > The function_is_gpio pinmux op allows the core to find out whether a
> > GPIO can be safely requested from a pinctrl property and requested as a
> > GPIO at the same time.
> > 
> > This is especially useful to request a GPIO with a particular drive
> > strength, which would otherwise not be possible.
> 
> That looks a easy enough solution, but:
> 
> > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > ---
> >  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > index d3042e0c9712..6162f2d86723 100644
> > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > @@ -821,6 +821,17 @@ static int sunxi_pmx_get_func_groups(struct pinctrl_dev *pctldev,
> >  	return 0;
> >  }
> >  
> > +static bool sunxi_pmx_function_is_gpio(struct pinctrl_dev *pctldev,
> > +				       unsigned function)
> > +{
> > +	struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	if (!strncmp(pctl->functions[function].name, "gpio", 4))
> 
> I wonder if this condition is a bit too relaxed? There could be
> some (theoretical) function just starting with gpio, but not being mux
> 0 or 1.

Yes maybe this is a bit too relaxed.

> So should we check for gpio_in or gpio_out, explicitly? Or at
> least use (strcmp(name, "gpio_", 5)? Or maybe even better for the mux
> value directly? Is "function" an indicator of this, or does this rely
> on the two GPIO functions being always listed first, at least so far?

I considered mathcing the function to the mux index, but we can't really rely
on Allwinner keeping GPIO to mux 0/1 either so I think the string comparison
is still the best approach.

But I am totally fine with having an explicit check against
"gpio_in" || "gpio_out".

And to answer your question it's not safe to use the function as mux value,
even though the values are the same in practice. The muxval field of
struct sunxi_desc_function should be used instead and that can be looked up by
sunxi_pinctrl_desc_find_function_by_name.

All the best,

Paul

> And what about the IRQ function? Isn't that some GPIO as well, or does
> that not count for the purpose of the function_is_gpio() callback?
> 
> Cheers,
> Andre
> 
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static void sunxi_pmx_set(struct pinctrl_dev *pctldev,
> >  				 unsigned pin,
> >  				 u8 config)
> > @@ -952,6 +963,7 @@ static const struct pinmux_ops sunxi_pmx_ops = {
> >  	.get_functions_count	= sunxi_pmx_get_funcs_cnt,
> >  	.get_function_name	= sunxi_pmx_get_func_name,
> >  	.get_function_groups	= sunxi_pmx_get_func_groups,
> > +	.function_is_gpio	= sunxi_pmx_function_is_gpio,
> >  	.set_mux		= sunxi_pmx_set_mux,
> >  	.gpio_set_direction	= sunxi_pmx_gpio_set_direction,
> >  	.request		= sunxi_pmx_request,
>
  

Patch

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index d3042e0c9712..6162f2d86723 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -821,6 +821,17 @@  static int sunxi_pmx_get_func_groups(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static bool sunxi_pmx_function_is_gpio(struct pinctrl_dev *pctldev,
+				       unsigned function)
+{
+	struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	if (!strncmp(pctl->functions[function].name, "gpio", 4))
+		return true;
+
+	return false;
+}
+
 static void sunxi_pmx_set(struct pinctrl_dev *pctldev,
 				 unsigned pin,
 				 u8 config)
@@ -952,6 +963,7 @@  static const struct pinmux_ops sunxi_pmx_ops = {
 	.get_functions_count	= sunxi_pmx_get_funcs_cnt,
 	.get_function_name	= sunxi_pmx_get_func_name,
 	.get_function_groups	= sunxi_pmx_get_func_groups,
+	.function_is_gpio	= sunxi_pmx_function_is_gpio,
 	.set_mux		= sunxi_pmx_set_mux,
 	.gpio_set_direction	= sunxi_pmx_gpio_set_direction,
 	.request		= sunxi_pmx_request,