Re: [PATCH v4 3/7] pinctrl: extract pinctrl_generic_to_map() from pinctrl_generic_pins_function_dt_node_to_map()

Conor Dooley
2026-03-26T18:52:17Z

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

On Wed, Mar 25, 2026 at 07:04:12PM -0400, Frank Li wrote:

> diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
> index efb39c6a670331775855efdc8566102b5c6202ef..20a216ae63e91b69985ea4cfcd0b57103c6ca950 100644
> --- a/drivers/pinctrl/pinctrl-generic.c
> +++ b/drivers/pinctrl/pinctrl-generic.c
> @@ -17,29 +17,18 @@
> #include "pinctrl-utils.h"
> #include "pinmux.h"
>
> -static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *pctldev,

> +int
> +pinctrl_generic_to_map(struct pinctrl_dev *pctldev, struct device_node *parent,

Can you drop this stylistic change please? The

> + struct device_node *np, struct pinctrl_map **maps,
> + unsigned int *num_maps, unsigned int *num_reserved_maps,
> + const char **group_names, unsigned int ngroups,
> + const char **functions, unsigned int *pins)
> {
> struct device *dev = pctldev->dev;
> - const char **functions;
> + int npins, ret, reserve = 1;
> + unsigned int num_configs;
> const char *group_name;
> unsigned long *configs;
> - unsigned int num_configs, pin, *pins;
> - int npins, ret, reserve = 1;
> -
> - npins = of_property_count_u32_elems(np, "pins");
> -
> - if (npins < 1) {
> - dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
> - parent, np, npins);
> - return npins;
> - }
>
> group_name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", parent, np);
> if (!group_name)
> @@ -51,22 +40,6 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
> if (!pins)
> return -ENOMEM;

This looks suspect. You've left the pins allocation behind:
pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
if (!pins)
return -ENOMEM;
but pinctrl_generic_pins_function_dt_subnode_to_map() has already
populated this array before calling the function.

Also, this should probably be
Suggested-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

>
> - functions = devm_kcalloc(dev, npins, sizeof(*functions), GFP_KERNEL);
> - if (!functions)
> - return -ENOMEM;
> -
> - for (int i = 0; i < npins; i++) {
> - ret = of_property_read_u32_index(np, "pins", i, &pin);
> - if (ret)
> - return ret;
> -
> - pins[i] = pin;
> -
> - ret = of_property_read_string(np, "function", &functions[i]);
> - if (ret)
> - return ret;
> - }
> -
> ret = pinctrl_utils_reserve_map(pctldev, maps, num_reserved_maps, num_maps, reserve);
> if (ret)
> return ret;
> @@ -103,6 +76,54 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
> return 0;
> };
>
> +static int
> +pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *pctldev,
> + struct device_node *parent,
> + struct device_node *np,
> + struct pinctrl_map **maps,
> + unsigned int *num_maps,
> + unsigned int *num_reserved_maps,
> + const char **group_names,
> + unsigned int ngroups)
> +{
> + struct device *dev = pctldev->dev;
> + unsigned int pin, *pins;
> + const char **functions;
> + int npins, ret;
> +
> + npins = of_property_count_u32_elems(np, "pins");
> +
> + if (npins < 1) {
> + dev_err(dev, "invalid pinctrl group %pOFn.%pOFn %d\n",
> + parent, np, npins);
> + return npins;
> + }
> +
> + pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> + if (!pins)
> + return -ENOMEM;
> +
> + functions = devm_kcalloc(dev, npins, sizeof(*functions), GFP_KERNEL);
> + if (!functions)
> + return -ENOMEM;
> +
> + for (int i = 0; i < npins; i++) {
> + ret = of_property_read_u32_index(np, "pins", i, &pin);
> + if (ret)
> + return ret;
> +
> + pins[i] = pin;
> +
> + ret = of_property_read_string(np, "function", &functions[i]);
> + if (ret)
> + return ret;
> + }
> +
> + return pinctrl_generic_to_map(pctldev, parent, np, maps, num_maps,
> + num_reserved_maps, group_names, ngroups,
> + functions, pins);
> +}
> +
> /*
> * For platforms that do not define groups or functions in the driver, but
> * instead use the devicetree to describe them. This function will, unlike
>
> --
> 2.43.0
>

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