From: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/pinctrl/Kconfig | 6 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinconf.h | 7 ++
drivers/pinctrl/pinctrl-generic.c | 186 ++++++++++++++++++++++++++++++
4 files changed, 200 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-generic.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 4ec2bb7f67cf..b43d4d7d25a8 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -25,6 +25,12 @@ config GENERIC_PINCONF
bool
select PINCONF
+config GENERIC_PINCTRL_BELLS_AND_WHISTLES
+ bool
+ depends on GENERIC_PINCONF
+ depends on GENERIC_PINCTRL_GROUPS
+ depends on GENERIC_PINMUX_FUNCTIONS
+
config DEBUG_PINCTRL
bool "Debug PINCTRL calls"
depends on DEBUG_KERNEL
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index ea4e890766e1..ea679ca84d85 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -7,6 +7,7 @@ obj-y += core.o pinctrl-utils.o
obj-$(CONFIG_PINMUX) += pinmux.o
obj-$(CONFIG_PINCONF) += pinconf.o
obj-$(CONFIG_GENERIC_PINCONF) += pinconf-generic.o
+obj-$(CONFIG_GENERIC_PINCTRL_BELLS_AND_WHISTLES) += pinctrl-generic.o
obj-$(CONFIG_OF) += devicetree.o
obj-$(CONFIG_PINCTRL_AMD) += pinctrl-amd.o
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index e1ae71610526..127da9a1e30e 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -160,3 +160,10 @@ pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
return -ENOTSUPP;
}
#endif
+
+#if defined(CONFIG_GENERIC_PINCTRL_BELLS_AND_WHISTLES) && defined (CONFIG_OF)
+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);
+#endif
diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
new file mode 100644
index 000000000000..08f21e25e1eb
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "generic pinconfig core: " fmt
+
+#include <linux/array_size.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "core.h"
+#include "pinconf.h"
+#include "pinctrl-utils.h"
+#include "pinmux.h"
+
+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;
+ const char **functions;
+ 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)
+ return -ENOMEM;
+
+ group_names[ngroups] = group_name;
+
+ 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;
+ }
+
+ 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_generic_add_group(pctldev, group_name, pins, npins, functions);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to add group %s: %d\n",
+ group_name, ret);
+
+ ret = pinconf_generic_parse_dt_config(np, pctldev, &configs, &num_configs);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to parse pin config of group %s\n",
+ group_name);
+
+ if (num_configs == 0)
+ return 0;
+
+ ret = pinctrl_utils_reserve_map(pctldev, maps, num_reserved_maps, num_maps, reserve);
+ if (ret)
+ return ret;
+
+ ret = pinctrl_utils_add_map_configs(pctldev, maps, num_reserved_maps, num_maps, group_name, configs,
+ num_configs, PIN_MAP_TYPE_CONFIGS_GROUP);
+ kfree(configs);
+ if (ret)
+ return ret;
+
+ 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)
+{
+ 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);
--
2.51.0Hi Conor, sorry for being slow in reviews! On Thu, Nov 27, 2025 at 11:58 AM Conor Dooley <conor@kernel.org> wrote: > +config GENERIC_PINCTRL_BELLS_AND_WHISTLES Interesting name :D A bit like GENERIC_PINCTRL_LOCK_STOCK_AND_BARREL. Have you considered simply GENERIC_PINCTRL? > +obj-$(CONFIG_GENERIC_PINCTRL_BELLS_AND_WHISTLES) += pinctrl-generic.o especially since the file is named like so... > +/* > + * 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) All code looks fine. There is just the philosophical question whether groups and functions should really be in the device tree, as they can obviously be statically defined and associated with the compatible. I got so much pressure to do it this way because so many driver authors really wanted to keep this in the device tree (usually because it saves memory in the kernel) that I eventually caved in, and I have also been criticized for being to lenient on this because the compatible should suffice. For me this is all fine, and with you submitting this I suppose even the DT maintainers think this is fine to keep groups and functions in the device tree, so there it is. I can merge this when it's out of RFC. Yours, Linus Walleij
On Fri, Dec 26, 2025 at 10:29:31AM +0100, Linus Walleij wrote: > Hi Conor, > > sorry for being slow in reviews! > > On Thu, Nov 27, 2025 at 11:58 AM Conor Dooley <conor@kernel.org> wrote: > > > +config GENERIC_PINCTRL_BELLS_AND_WHISTLES > > Interesting name :D > > A bit like GENERIC_PINCTRL_LOCK_STOCK_AND_BARREL. > > Have you considered simply GENERIC_PINCTRL? Sure, I really didn't know if that was too "core" sounding for something with almost nothing in it. > > +obj-$(CONFIG_GENERIC_PINCTRL_BELLS_AND_WHISTLES) += pinctrl-generic.o > > especially since the file is named like so... > > > +/* > > + * 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) > > All code looks fine. > > There is just the philosophical question whether groups and functions should > really be in the device tree, as they can obviously be statically defined and > associated with the compatible. > > I got so much pressure to do it this way because so many driver authors really > wanted to keep this in the device tree (usually because it saves memory in the > kernel) that I eventually caved in, and I have also been criticized for being to > lenient on this because the compatible should suffice. > > For me this is all fine, and with you submitting this I suppose even the DT > maintainers think this is fine to keep groups and functions in the device > tree, so there it is. I think that you're mostly right though. There's a lot of situations where the pinmux-node properties are used because they're easier than putting functions and groups structs into a devicetree. They're boring and annoying to write, when a pinmux property can do the job for you, particularly on bigger devices where there may be lots of them. My personal take would be that it comes down to complexity, and the number of possible groups/functions that a pin can belong to. If that number is very high then it makes sense I think to put it in dt, but if there's only a small number than spelling them out in the driver I think makes the most sense. The pinmux property on the other hand, were I asked to review it as a new property, I would reject. I'd probably push for not using pinmux, as you did with my drivers, more. I think finding good examples to copy from of it being done that way was why I didn't try. The docs that explain about statically defined functions and groups, IIRC, kinda come across as being a mach-foo thing, rather than intended for use with dt, until you pointed out an example and I was able to link the two. Based on what I've done for this series, I know that what that property seeks to do can be done using pins and functions, rather than mixing both up together into a single property - even worse, it's even a single cell! I only used it myself in the earlier versions cos everyone else does it and the infra existed to use it already. I've not looked at every user, but every user I looked at used it to hold the function, not some additional mux setting. > I can merge this when it's out of RFC. Sweet. I'll hopefully get it out in the next day or so.
© 2016 - 2026 Red Hat, Inc.