Re: [PATCH v2 03/22] pinctrl: pinctrl-generic: Make the "function" property optional

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3019 bytes --]

On Thu, May 14, 2026 at 04:11:59AM -0700, Changhuang Liang wrote:
> Some pinctrl subnodes only need to configure pin properties (e.g.,
> power-source, bias, drive strength) without assigning any mux function.
> 
> Currently, the driver requires a valid "function" property for all
> pinctrl subnodes. This forces the addition of dummy or redundant
> "function" entries when only pin configuration is needed.
> 
> Example use case:
> gpios-configs {
>     config {
>         pins = <0 1 2 3>;
>         power-source = <0>;
>     };
> };
> 
> Make the "function" property optional. If it is missing, skip adding
> the mux map and only process the pin configuration.

I looked through the series though and all controllers appear
to have pins and functions, is it the case that gpio is the default for
these pins, so you are omitting the functions property when you are
using the pin in gpio mode? Saying that the functions property is
"redudant" makes it seem like this might be the case?

I've got some feedback here, but I can't really provide it without
knowing the answer to that question.

Cheers,
Conor.

> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/pinctrl/pinctrl-generic.c | 37 ++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
> index efb39c6a6703..c7dd0924aa0e 100644
> --- a/drivers/pinctrl/pinctrl-generic.c
> +++ b/drivers/pinctrl/pinctrl-generic.c
> @@ -62,19 +62,36 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
>  
>  		pins[i] = pin;
>  
> -		ret = of_property_read_string(np, "function", &functions[i]);
> -		if (ret)
> -			return ret;
> +		if (functions) {
> +			ret = of_property_read_string(np, "function", &functions[i]);
> +			if (ret < 0) {
> +				/* EINVAL = missing, which is fine since it's optional */
> +				if (ret != -EINVAL) {
> +					dev_err(dev, "%pOF: could not parse property function\n",
> +						np);
> +					return ret;
> +				}
> +
> +				devm_kfree(dev, functions);
> +				functions = NULL;
> +
> +				/* Continue parsing all pins */
> +				continue;
> +			}
> +		}
>  	}
>  
> -	ret = pinctrl_utils_reserve_map(pctldev, maps, num_reserved_maps, num_maps, reserve);
> -	if (ret)
> -		return ret;
> +	if (functions) {
> +		ret = pinctrl_utils_reserve_map(pctldev, maps, num_reserved_maps,
> +						num_maps, reserve);
> +		if (ret)
> +			return ret;
>  
> -	ret = pinctrl_utils_add_map_mux(pctldev, maps, num_reserved_maps, num_maps, group_name,
> -					parent->name);
> -	if (ret < 0)
> -		return ret;
> +		ret = pinctrl_utils_add_map_mux(pctldev, maps, num_reserved_maps,
> +						num_maps, group_name, parent->name);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = pinctrl_generic_add_group(pctldev, group_name, pins, npins, functions);
>  	if (ret < 0)
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]