Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow
passing private data and skip_npins to pinmux_generic_add_function().
The 'skip_npins' to skip parse pins in dts because on boards MUX control
chip switch the whole group together, so needn't handle each pins.
Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper
calling the new helper with a NULL argument to preserve backward
compatibility.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v3
- new patch
---
drivers/pinctrl/pinconf.h | 21 ++++++++++++++++----
drivers/pinctrl/pinctrl-generic.c | 41 +++++++++++++++++++++++----------------
2 files changed, 41 insertions(+), 21 deletions(-)
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index 2880adef476e68950ffdd540ea42cdee6a16ec27..d6d22c088a144de0180314826b8cb8c528290970 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -162,10 +162,23 @@ pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
#endif
#if defined(CONFIG_GENERIC_PINCTRL) && 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);
+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,
+ void *func_data,
+ bool skip_npins);
+static inline 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_pins_function_dt_node_to_map(pctldev, np, maps,
+ num_maps, NULL,
+ false);
+}
+
#else
static inline int
pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index efb39c6a670331775855efdc8566102b5c6202ef..dc575fbcc7103bebded49d8268764531f349a8c9 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -24,21 +24,24 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
unsigned int *num_maps,
unsigned int *num_reserved_maps,
const char **group_names,
- unsigned int ngroups)
+ unsigned int ngroups,
+ bool skip_npins)
{
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;
+ int npins = 0, ret, reserve = 1;
- npins = of_property_count_u32_elems(np, "pins");
+ if (!skip_npins) {
+ 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;
+ 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);
@@ -110,10 +113,12 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
* 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)
+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,
+ void *func_data,
+ bool skip_npins)
{
struct device *dev = pctldev->dev;
struct device_node *child_np;
@@ -129,7 +134,7 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
* Check if this is actually the pins node, or a parent containing
* multiple pins nodes.
*/
- if (!of_property_present(np, "pins"))
+ if (!of_property_present(np, "pins") && !skip_npins)
goto parent;
group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
@@ -140,13 +145,14 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
maps, num_maps,
&num_reserved_maps,
group_names,
- ngroups);
+ ngroups,
+ skip_npins);
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);
+ ret = pinmux_generic_add_function(pctldev, np->name, group_names, 1, func_data);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *maps, *num_maps);
return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
@@ -168,7 +174,8 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
maps, num_maps,
&num_reserved_maps,
group_names,
- ngroups);
+ ngroups,
+ skip_npins);
if (ret) {
pinctrl_utils_free_map(pctldev, *maps, *num_maps);
return dev_err_probe(dev, ret, "error figuring out mappings for %s\n",
@@ -178,7 +185,7 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
ngroups++;
}
- ret = pinmux_generic_add_function(pctldev, np->name, group_names, ngroups, NULL);
+ ret = pinmux_generic_add_function(pctldev, np->name, group_names, ngroups, func_data);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *maps, *num_maps);
return dev_err_probe(dev, ret, "error adding function %s\n", np->name);
@@ -186,4 +193,4 @@ int pinctrl_generic_pins_function_dt_node_to_map(struct pinctrl_dev *pctldev,
return 0;
}
-EXPORT_SYMBOL_GPL(pinctrl_generic_pins_function_dt_node_to_map);
+EXPORT_SYMBOL_GPL(__pinctrl_generic_pins_function_dt_node_to_map);
--
2.43.0On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote: > Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow > passing private data and skip_npins to pinmux_generic_add_function(). > > The 'skip_npins' to skip parse pins in dts because on boards MUX control > chip switch the whole group together, so needn't handle each pins. > > Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper > calling the new helper with a NULL argument to preserve backward > compatibility. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> Ad attested by several reviews, the pinctrl subsystem maintainer strongly dislikes any use of __double_underscore_function_names(). The reason I dislike it is because it is ambiguous. For example there are __compiler_intrinsics such as __iomem and all the stuff from <linux/compiler_types.h>. Then there are __non_atomics such as __set_bit(). This means __inner_function() just adds to this mess and creates a big confusion for the mind. That said: in this case you're just adding a parameter, just add the parameter and change all of the in-tree users to pass false or whatever you need, these is just one (1) in-tree user anyway. Yours, Linus Walleij
On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > On Wed, Mar 11, 2026 at 8:08 PM Frank Li <Frank.Li@nxp.com> wrote: > > > Introduce __pinctrl_generic_pins_function_dt_node_to_map() to allow > > passing private data and skip_npins to pinmux_generic_add_function(). > > > > The 'skip_npins' to skip parse pins in dts because on boards MUX control > > chip switch the whole group together, so needn't handle each pins. > > > > Keep pinctrl_generic_pins_function_dt_node_to_map() as a wrapper > > calling the new helper with a NULL argument to preserve backward > > compatibility. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > Ad attested by several reviews, the pinctrl subsystem maintainer > strongly dislikes any use of __double_underscore_function_names(). > > The reason I dislike it is because it is ambiguous. > > For example there are __compiler_intrinsics such as > __iomem and all the stuff from <linux/compiler_types.h>. > > Then there are __non_atomics such as __set_bit(). > > This means __inner_function() just adds to this mess and creates > a big confusion for the mind. > > That said: in this case you're just adding a parameter, just add > the parameter and change all of the in-tree users to pass false > or whatever you need, these is just one (1) in-tree user anyway. pinctrl_generic_pins_function_dt_node_to_map() directly feed to .dt_node_to_map() callback, add parameter will impact too much. If don't like __funciton_name(), can we use pinctrl_generic_pins_function_dt_node_to_map_ext() or other name Frank > > Yours, > Linus Walleij
On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote: > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > > That said: in this case you're just adding a parameter, just add > > the parameter and change all of the in-tree users to pass false > > or whatever you need, these is just one (1) in-tree user anyway. > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to > .dt_node_to_map() callback, add parameter will impact too much. Why do you say that. It already has many parameters, one more or less doesn't matter. It's not like this call is performance-critical. Just change the users. Yours, Linus Walleij
On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote: > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote: > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > > > > That said: in this case you're just adding a parameter, just add > > > the parameter and change all of the in-tree users to pass false > > > or whatever you need, these is just one (1) in-tree user anyway. > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to > > .dt_node_to_map() callback, add parameter will impact too much. > > Why do you say that. It already has many parameters, one more > or less doesn't matter. It's not like this call is performance-critical. > Just change the users. In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c, .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map; pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s declear. So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map() Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in pinconf.h. If add two parameter in .dt_node_to_map(), need change all functions, which .dt_node_to_map = xxx_to_map(). and OF core part. Frank > > Yours, > Linus Walleij
On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote: > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote: > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote: > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > > > > > > That said: in this case you're just adding a parameter, just add > > > > the parameter and change all of the in-tree users to pass false > > > > or whatever you need, these is just one (1) in-tree user anyway. > > > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to > > > .dt_node_to_map() callback, add parameter will impact too much. > > > > Why do you say that. It already has many parameters, one more > > or less doesn't matter. It's not like this call is performance-critical. > > Just change the users. > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c, > .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map; > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s > declear. > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map() > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in > pinconf.h. > > If add two parameter in .dt_node_to_map(), need change all functions, which > .dt_node_to_map = xxx_to_map(). and OF core part. Linus Walleij: Is my explain clear enough? I am preparing respin it? is okay use wrap function pinctrl_generic_pins_function_dt_node_to_map_ext()? Frank > > Frank > > > > > Yours, > > Linus Walleij
On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote: > On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote: > > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote: > > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote: > > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > > > > > > > > That said: in this case you're just adding a parameter, just add > > > > > the parameter and change all of the in-tree users to pass false > > > > > or whatever you need, these is just one (1) in-tree user anyway. > > > > > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to > > > > .dt_node_to_map() callback, add parameter will impact too much. > > > > > > Why do you say that. It already has many parameters, one more > > > or less doesn't matter. It's not like this call is performance-critical. > > > Just change the users. > > > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c, > > .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map; > > > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s > > declear. > > > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map() > > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in > > pinconf.h. > > > > If add two parameter in .dt_node_to_map(), need change all functions, which > > .dt_node_to_map = xxx_to_map(). and OF core part. > > Linus Walleij: > Is my explain clear enough? I am preparing respin it? > > is okay use wrap function > pinctrl_generic_pins_function_dt_node_to_map_ext()? I don't understand this patch. The function is called pinctrl_generic_pins_function_dt_node_to_map(). You have no pins. You're adding a parameter to make a function with *pins* in its name not use pins. The new function doesn't use pins but has pins in the name. At the very least function names should not be misleading. I was going to suggest pulling out the relevant portions and creating some helpers that could be used by multiple different-but-similar functions, but I don't actually even think that there's much in common. Most damningly I think, you don't actually read either the functions or pins properties at all and neither are permitted by your binding. So turns out you use neither pins or functions... You don't actually have any of these properties which runs counter to the goal of the function, which is parsing. With this in mind, it feels to me like you're trying way too hard to make use of a generic function when the right thing to do is probably just have an entirely custom function. Maybe that's a custom implementation in your driver, or a new function here, but I think writing that will highlight just how little of the code would be shared between the existing function and what your use-case needs: no pin configuration stuff, no reading of the devicetree other than the node names and no dealing with the label pointing to the "wrong" place. I recently bought a spacemit k1 board to go and write a sister function to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins and groups (because that's a pretty common pattern). I would be calling that pinctrl_generic_pinmux_dt_node_to_map(), because it that's the property it deals with. I have honestly got no idea what to call one for this situation since you don't have any of the properties in pinmux-node.yaml. Maybe that's a sign. Cheers, Conor.
On Wed, Mar 25, 2026 at 10:33:05AM +0000, Conor Dooley wrote: > On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote: > > On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote: > > > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote: > > > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote: > > > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote: > > > > > > > > > > That said: in this case you're just adding a parameter, just add > > > > > > the parameter and change all of the in-tree users to pass false > > > > > > or whatever you need, these is just one (1) in-tree user anyway. > > > > > > > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to > > > > > .dt_node_to_map() callback, add parameter will impact too much. > > > > > > > > Why do you say that. It already has many parameters, one more > > > > or less doesn't matter. It's not like this call is performance-critical. > > > > Just change the users. > > > > > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c, > > > .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map; > > > > > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s > > > declear. > > > > > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map() > > > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in > > > pinconf.h. > > > > > > If add two parameter in .dt_node_to_map(), need change all functions, which > > > .dt_node_to_map = xxx_to_map(). and OF core part. > > > > Linus Walleij: > > Is my explain clear enough? I am preparing respin it? > > > > is okay use wrap function > > pinctrl_generic_pins_function_dt_node_to_map_ext()? > > I don't understand this patch. The function is called > pinctrl_generic_pins_function_dt_node_to_map(). You have no pins. > You're adding a parameter to make a function with *pins* in its name not > use pins. The new function doesn't use pins but has pins in the name. > At the very least function names should not be misleading. > > I was going to suggest pulling out the relevant portions and creating > some helpers that could be used by multiple different-but-similar > functions, but I don't actually even think that there's much in common. > Most damningly I think, you don't actually read either the functions or > pins properties at all and neither are permitted by your binding. > So turns out you use neither pins or functions... > > You don't actually have any of these properties which runs counter to the > goal of the function, which is parsing. With this in mind, it feels to me > like you're trying way too hard to make use of a generic function when the > right thing to do is probably just have an entirely custom function. > Maybe that's a custom implementation in your driver, or a new function > here, but I think writing that will highlight just how little of the > code would be shared between the existing function and what your > use-case needs: no pin configuration stuff, no reading of the devicetree > other than the node names and no dealing with the label pointing to the > "wrong" place. > > I recently bought a spacemit k1 board to go and write a sister function > to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins > and groups (because that's a pretty common pattern). > I would be calling that pinctrl_generic_pinmux_dt_node_to_map(), > because it that's the property it deals with. I have honestly got no > idea what to call one for this situation since you don't have any of the > properties in pinmux-node.yaml. Maybe that's a sign. At v2, I implemented customize dt_node_to_map(), Linus Walleij think it is too similar with pinctrl_generic_pins_function_dt_node_to_map(), so ask me to enhanance and reuse pinctrl_generic_pins_function_dt_node_to_map(). Frank > > Cheers, > Conor.
On Wed, Mar 25, 2026 at 11:09:20AM -0400, Frank Li wrote:
> On Wed, Mar 25, 2026 at 10:33:05AM +0000, Conor Dooley wrote:
> > On Tue, Mar 24, 2026 at 04:16:10PM -0400, Frank Li wrote:
> > > On Fri, Mar 20, 2026 at 09:54:45AM -0400, Frank Li wrote:
> > > > On Fri, Mar 20, 2026 at 02:27:21PM +0100, Linus Walleij wrote:
> > > > > On Thu, Mar 19, 2026 at 12:04 AM Frank Li <Frank.li@nxp.com> wrote:
> > > > > > On Mon, Mar 16, 2026 at 10:37:28AM +0100, Linus Walleij wrote:
> > > > >
> > > > > > > That said: in this case you're just adding a parameter, just add
> > > > > > > the parameter and change all of the in-tree users to pass false
> > > > > > > or whatever you need, these is just one (1) in-tree user anyway.
> > > > > >
> > > > > > pinctrl_generic_pins_function_dt_node_to_map() directly feed to
> > > > > > .dt_node_to_map() callback, add parameter will impact too much.
> > > > >
> > > > > Why do you say that. It already has many parameters, one more
> > > > > or less doesn't matter. It's not like this call is performance-critical.
> > > > > Just change the users.
> > > >
> > > > In only user drivers/pinctrl/microchip/pinctrl-mpfs-mssio.c,
> > > > .dt_node_to_map = pinctrl_generic_pins_function_dt_node_to_map;
> > > >
> > > > pinctrl_generic_pins_function_dt_node_to_map() need match .dt_node_to_map()'s
> > > > declear.
> > > >
> > > > So it can't direct add two parameters in pinctrl_generic_pins_function_dt_node_to_map()
> > > > Need simple wrap function, which other in pinctrl-mpfs-mssio.c or in
> > > > pinconf.h.
> > > >
> > > > If add two parameter in .dt_node_to_map(), need change all functions, which
> > > > .dt_node_to_map = xxx_to_map(). and OF core part.
> > >
> > > Linus Walleij:
> > > Is my explain clear enough? I am preparing respin it?
> > >
> > > is okay use wrap function
> > > pinctrl_generic_pins_function_dt_node_to_map_ext()?
> >
> > I don't understand this patch. The function is called
> > pinctrl_generic_pins_function_dt_node_to_map(). You have no pins.
> > You're adding a parameter to make a function with *pins* in its name not
> > use pins. The new function doesn't use pins but has pins in the name.
> > At the very least function names should not be misleading.
> >
> > I was going to suggest pulling out the relevant portions and creating
> > some helpers that could be used by multiple different-but-similar
> > functions, but I don't actually even think that there's much in common.
> > Most damningly I think, you don't actually read either the functions or
> > pins properties at all and neither are permitted by your binding.
> > So turns out you use neither pins or functions...
> >
> > You don't actually have any of these properties which runs counter to the
> > goal of the function, which is parsing. With this in mind, it feels to me
> > like you're trying way too hard to make use of a generic function when the
> > right thing to do is probably just have an entirely custom function.
> > Maybe that's a custom implementation in your driver, or a new function
> > here, but I think writing that will highlight just how little of the
> > code would be shared between the existing function and what your
> > use-case needs: no pin configuration stuff, no reading of the devicetree
> > other than the node names and no dealing with the label pointing to the
> > "wrong" place.
> >
> > I recently bought a spacemit k1 board to go and write a sister function
> > to pinctrl_generic_pins_function_dt_node_to_map() that deals with pins
> > and groups (because that's a pretty common pattern).
> > I would be calling that pinctrl_generic_pinmux_dt_node_to_map(),
> > because it that's the property it deals with. I have honestly got no
> > idea what to call one for this situation since you don't have any of the
> > properties in pinmux-node.yaml. Maybe that's a sign.
>
> At v2, I implemented customize dt_node_to_map(), Linus Walleij think it is
> too similar with pinctrl_generic_pins_function_dt_node_to_map(), so ask me
> to enhanance and reuse pinctrl_generic_pins_function_dt_node_to_map().
Sure, and he's right that there's a lot similar. Everything you want to
do, other than looking at the mux state, is something that
pinctrl_generic_pins_function_dt_node_to_map() does. But bastardising
a function that's explicitly about reading pins and functions properties
to do things that have _neither_ is not a good implementation of that
review feedback.
If you're going to make something generic, the right level to hook in
IMO is at the pinctrl_generic_pins_function_dt_subnode_to_map() level,
I already know that 95% of the code in that function is identical to
what will be used for the spacemit k1 that uses the pinmux property
and changing its API is a lot easier than changing the API of something
that is written to match the dt_node_to_map callback.
You don't even benefit from the extra functionality that
pinctrl_generic_pins_function_dt_node_to_map() provides, because you
don't have ambiguity about where the phandle you're parsing from points.
In your case, it has to be the group node.
pinctrl_generic_pins_function_dt_subnode_to_map() could probably be
split into two parts, one that does the dt parsing portion and a second
portion that does the mapping to kernel data structures. The first
portion of that is the for loop. The second portion is everything after
the for loop and the bit that names the groups. IOW, do something like
what I am pasting here, and you create your own function that wraps
pinctrl_generic_to_map() in the way you want. I personally think this is
more tasteful than what you've done, and more importantly I am pretty
sure that this is what's needed to be able to maximise code reuse for
the devices that use pinmux. I didn't compile this or anything, it's
just speculative.
diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index efb39c6a67033..7f02af6d9f3e4 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -17,56 +17,30 @@
#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)
+static int pinctrl_generic_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,
+ const char **functions,
+ unsigned int *pins,
+ void *function_data)
{
struct device *dev = pctldev->dev;
- const char **functions;
const char *group_name;
unsigned long *configs;
- unsigned int num_configs, pin, *pins;
+ unsigned int num_configs;
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;
@@ -101,6 +75,52 @@ static int pinctrl_generic_pins_function_dt_subnode_to_map(struct pinctrl_dev *p
return ret;
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;
+ const char **functions;
+ unsigned int pin, *pins;
+ 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, NULL);
};
/*
© 2016 - 2026 Red Hat, Inc.