[RFC v1 0/4] generic pinmux dt_node_to_map implementation
Conor Dooley
conor at kernel.org
Thu May 14 12:40:22 PDT 2026
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,
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/8c42f9d1/attachment.sig>
More information about the linux-riscv
mailing list