[RFC v1 0/4] generic pinmux dt_node_to_map implementation
Conor Dooley
conor at kernel.org
Thu May 14 12:42:55 PDT 2026
On Thu, May 14, 2026 at 08:40:26PM +0100, Conor Dooley wrote:
> On Thu, May 14, 2026 at 07:57:46PM +0100, Conor Dooley wrote:
> > On Mon, May 11, 2026 at 10:23:16PM +0200, Linus Walleij wrote:
> > > On Wed, May 6, 2026 at 11:58 AM Conor Dooley <conor at kernel.org> wrote:
> > >
> > > > Whipped this up last week, at to a first glance it appears to work,
> > > > although the spacemit platform I've used to implement this has very
> > > > limited in-tree use of pinctrl so it is hard to be sure.
> > >
> > > I like it, if it wasn't RFC I would merge it.
> >
> > Half the reason that it is RFC is that I knew dlan wanted to take a look
> > but told me they weren't available, so I said I'd send it on the list in
> > the interim.
> >
> > > > What I don't love though is how similar the functions
> > > > pinctrl_generic_pins_function_dt_node_to_map() and
> > > > pinctrl_generic_pinmux_dt_node_to_map() are - essentially identical
> > > > other than which function they in turn call.
> > >
> > > Hm we can maybe think of something more descriptive
> > > to the first one?
> >
> > I think the name is actually okay, it was the similarity of the code
> > that I don't like. There's a fair bit of duplication.
> >
> > > I think the new function is very much to the point. That's what
> > > it does. pinctrl_generic_pins_function_dt_node_to_map() could
> > > perhaps be names something that make it evident what is
> > > special about it. Not that I have a good idea.
> > >
> > > > Basically, I wanna know if you think that that is acceptable,
> > >
> > > Looks Good To Me (TM) no-one else is helping out with pin
> > > control core work so I'm happy for everything I get.
> >
> > Right, well I'll go clean it up I suppose. I might send a rfc v2 with an
> > extra patch that tries to get rid of some of the code duplication and
> > you can tell me which version you prefer?
>
> Actually, no need for rfc v2, can just paste here:
>
> diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
> index d1365acfd1f8c..9759b0186bcc2 100644
> --- a/drivers/pinctrl/pinctrl-generic.c
> +++ b/drivers/pinctrl/pinctrl-generic.c
> @@ -119,92 +119,6 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
> functions, pins, npins);
> }
>
> -/*
> - * For platforms that do not define groups or functions in the driver, but
> - * instead use the devicetree to describe them. This function will, unlike
> - * pinconf_generic_dt_node_to_map() etc which rely on driver defined groups
> - * and functions, create them in addition to parsing pinconf properties and
> - * adding mappings.
> - */
> -int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
> - struct device_node *np,
> - struct pinctrl_map **maps,
> - unsigned int *num_maps)
> -{
> - struct device *dev = pctldev->dev;
> - struct device_node *child_np;
> - const char **group_names;
> - unsigned int num_reserved_maps = 0;
> - int ngroups = 0;
> - int ret;
> -
> - *maps = NULL;
> - *num_maps = 0;
> -
> - /*
> - * Check if this is actually the pins node, or a parent containing
> - * multiple pins nodes.
> - */
> - if (!of_property_present(np, "pins"))
> - goto parent;
> -
> - group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
> - if (!group_names)
> - return -ENOMEM;
> -
> - ret = pinctrl_generic_pins_function_dt_subnode_to_map(pctldev, np, np,
> - maps, num_maps,
> - &num_reserved_maps,
> - group_names,
> - ngroups);
> - if (ret) {
> - pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> - return dev_err_probe(dev, ret, "error figuring out mappings for %s\n", np->name);
> - }
> -
> - ret = pinmux_generic_add_function(pctldev, np->name, group_names, 1, NULL);
> - if (ret < 0) {
> - pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> - return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
> - }
> -
> - return 0;
> -
> -parent:
> - for_each_available_child_of_node(np, child_np)
> - ngroups += 1;
> -
> - group_names = devm_kcalloc(dev, ngroups, sizeof(*group_names), GFP_KERNEL);
> - if (!group_names)
> - return -ENOMEM;
> -
> - ngroups = 0;
> - for_each_available_child_of_node_scoped(np, child_np) {
> - ret = pinctrl_generic_pins_function_dt_subnode_to_map(pctldev, np, child_np,
> - maps, num_maps,
> - &num_reserved_maps,
> - group_names,
> - ngroups);
> - if (ret) {
> - pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> - return dev_err_probe(dev, ret, "error figuring out mappings for %s\n",
> - np->name);
> - }
> -
> - ngroups++;
> - }
> -
> - ret = pinmux_generic_add_function(pctldev, np->name, group_names, ngroups, NULL);
> - if (ret < 0) {
> - pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> - return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(pinctrl_generic_pins_function_dt_node_to_map);
> -
> -
> static int pinctrl_generic_pinmux_dt_subnode_to_map(struct pinctrl_dev *pctldev,
> struct device_node *parent,
> struct device_node *np,
> @@ -250,20 +164,19 @@ static int pinctrl_generic_pinmux_dt_subnode_to_map(struct pinctrl_dev *pctldev,
> muxes, pins, npins);
> }
>
> -/*
> - * For platforms that do not define groups or functions in the driver, but
> - * instead use the devicetree to describe them. This function will, unlike
> - * pinconf_generic_dt_node_to_map() etc which rely on driver defined groups
> - * and functions, create them in addition to parsing pinconf properties and
> - * adding mappings.
> - *
> - * It assumes that the upper 16 bits of the pinmux items contain the pin
> - * and the lower 16 the mux setting.
> - */
> -int pinctrl_generic_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> +static int pinctrl_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
This function name conflicts with another helper in the pinctrl
core, so I would have to rename it, but that's not needed for proof of
concept of course.
> struct device_node *np,
> struct pinctrl_map **maps,
> - unsigned int *num_maps)
> + unsigned int *num_maps,
> + int (dt_subnode_to_map)(
> + struct pinctrl_dev *,
> + struct device_node *,
> + struct device_node *,
> + struct pinctrl_map **,
> + unsigned int *,
> + unsigned int *,
> + const char **,
> + unsigned int))
> {
> struct device *dev = pctldev->dev;
> struct device_node *child_np;
> @@ -276,21 +189,18 @@ int pinctrl_generic_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> *num_maps = 0;
>
> /*
> - * Check if this is actually the pinmux node, or a parent containing
> - * multiple pinmux nodes.
> + * Check if this is actually the pins node, or a parent containing
> + * multiple pins nodes.
> */
> - if (!of_property_present(np, "pinmux"))
> + if (!of_property_present(np, "pins"))
> goto parent;
>
> group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
> if (!group_names)
> return -ENOMEM;
>
> - ret = pinctrl_generic_pinmux_dt_subnode_to_map(pctldev, np, np,
> - maps, num_maps,
> - &num_reserved_maps,
> - group_names,
> - ngroups);
> + ret = dt_subnode_to_map(pctldev, np, np, maps, num_maps,
> + &num_reserved_maps, group_names, ngroups);
> if (ret) {
> pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> return dev_err_probe(dev, ret, "error figuring out mappings for %s\n", np->name);
> @@ -314,11 +224,8 @@ int pinctrl_generic_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> ngroups = 0;
> for_each_available_child_of_node_scoped(np, child_np) {
> - ret = pinctrl_generic_pinmux_dt_subnode_to_map(pctldev, np, child_np,
> - maps, num_maps,
> - &num_reserved_maps,
> - group_names,
> - ngroups);
> + ret = dt_subnode_to_map(pctldev, np, child_np, maps, num_maps,
> + &num_reserved_maps, group_names, ngroups);
> if (ret) {
> pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> return dev_err_probe(dev, ret, "error figuring out mappings for %s\n",
> @@ -336,4 +243,40 @@ int pinctrl_generic_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> return 0;
> }
> +
> +/*
> + * For platforms that do not define groups or functions in the driver, but
> + * instead use the devicetree to describe them. This function will, unlike
> + * pinconf_generic_dt_node_to_map() etc which rely on driver defined groups
> + * and functions, create them in addition to parsing pinconf properties and
> + * adding mappings.
> + */
> +int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
> + struct device_node *np,
> + struct pinctrl_map **maps,
> + unsigned int *num_maps)
> +{
> + return pinctrl_generic_dt_node_to_map(pctldev, np, maps, num_maps,
> + &pinctrl_generic_pins_function_dt_subnode_to_map);
> +}
> +EXPORT_SYMBOL_GPL(pinctrl_generic_pins_function_dt_node_to_map);
> +
> +/*
> + * For platforms that do not define groups or functions in the driver, but
> + * instead use the devicetree to describe them. This function will, unlike
> + * pinconf_generic_dt_node_to_map() etc which rely on driver defined groups
> + * and functions, create them in addition to parsing pinconf properties and
> + * adding mappings.
> + *
> + * It assumes that the upper 16 bits of the pinmux items contain the pin
> + * and the lower 16 the mux setting.
> + */
> +int pinctrl_generic_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> + struct device_node *np,
> + struct pinctrl_map **maps,
> + unsigned int *num_maps)
> +{
> + return pinctrl_generic_dt_node_to_map(pctldev, np, maps, num_maps,
> + &pinctrl_generic_pinmux_dt_subnode_to_map);
> +};
> EXPORT_SYMBOL_GPL(pinctrl_generic_pinmux_dt_node_to_map);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20260514/3cedfe47/attachment.sig>
More information about the linux-riscv
mailing list