drivers/pinctrl/pinctrl-generic-mux.c | 76 ++++++++++++++++++++------- 1 file changed, 56 insertions(+), 20 deletions(-)
From: Chancel Liu <chancel.liu@nxp.com>
The pinctrl core re-parses a consumer's pinctrl on every probe attempt
(pinctrl_bind_pins() runs before the driver's probe(), and again on each
deferred-probe retry or re-bind), so .dt_node_to_map() must be side-effect
free and safe to call repeatedly.
mux_pinmux_dt_node_to_map() instead allocated provider-side resources on
every call - the mux_pin_function, the group name table and a mux_state
reference, plus a group name and a pinctrl_generic_add_group()
registration via pinctrl_generic_to_map(). All of these are allocated
with devm_*() against the provider device, so they live for the provider's
lifetime and are only released when the provider itself is unbound. The
.dt_free_map() path never touches them; it frees the per-consumer
pinctrl_map only. Every re-parse therefore adds another set of
provider-side allocations that are never reclaimed, so a consumer that
repeatedly defers probe or is re-bound leaks memory and mux_state
references on the provider without bound.
The groups and functions of a board-level mux are static and fully
described by the device tree, so build them once at probe time in the new
mux_pinctrl_probe_dt(). mux_pinmux_dt_node_to_map() then only looks up the
already registered group and builds the per-consumer mux map, which the
core frees via .dt_free_map. As the mux has no pin electrical
configuration, no config map is emitted.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260824022020.2812810-1-chancel.liu%40oss.nxp.com
Fixes: 34acc5a8adfb ("pinctrl: add generic board-level pinctrl driver using mux framework")
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
Assisted-by: VeroCoder:claude-opus-4-8
---
drivers/pinctrl/pinctrl-generic-mux.c | 76 ++++++++++++++++++++-------
1 file changed, 56 insertions(+), 20 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
index 202b72351efb..de85b4e75eb8 100644
--- a/drivers/pinctrl/pinctrl-generic-mux.c
+++ b/drivers/pinctrl/pinctrl-generic-mux.c
@@ -10,13 +10,11 @@
#include <linux/mutex.h>
#include <linux/mux/consumer.h>
#include <linux/platform_device.h>
-#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/slab.h>
#include "core.h"
-#include "pinconf.h"
#include "pinmux.h"
#include "pinctrl-utils.h"
@@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **maps, unsigned int *num_maps)
{
unsigned int num_reserved_maps = 0;
- struct mux_pin_function *function;
- const char **group_names;
int ret;
- function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
- if (!function)
- return -ENOMEM;
-
- group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
- if (!group_names)
- return -ENOMEM;
-
- function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
- if (IS_ERR(function->mux_state))
- return PTR_ERR(function->mux_state);
+ *maps = NULL;
+ *num_maps = 0;
- ret = pinctrl_generic_to_map(pctldev, np_config, np_config, maps,
- num_maps, &num_reserved_maps, group_names,
- 0, &np_config->name, NULL, 0);
+ if (pinctrl_get_group_selector(pctldev, np_config->name) < 0)
+ return -ENODEV;
+ ret = pinctrl_utils_reserve_map(pctldev, maps, &num_reserved_maps,
+ num_maps, 1);
if (ret)
return ret;
- ret = pinmux_generic_add_function(pctldev, np_config->name, group_names,
- 1, function);
+ ret = pinctrl_utils_add_map_mux(pctldev, maps, &num_reserved_maps,
+ num_maps, np_config->name,
+ np_config->name);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *maps, *num_maps);
+ *maps = NULL;
+ *num_maps = 0;
return ret;
}
return 0;
}
+static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
+ struct device_node *np)
+{
+ struct device *dev = pctldev->dev;
+
+ for_each_available_child_of_node_scoped(np, grp) {
+ struct mux_pin_function *function;
+ const char **group_names;
+ int ret;
+
+ function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
+ if (!function)
+ return -ENOMEM;
+
+ group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
+ if (!group_names)
+ return -ENOMEM;
+
+ group_names[0] = grp->name;
+
+ function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
+ if (IS_ERR(function->mux_state))
+ return dev_err_probe(dev, PTR_ERR(function->mux_state),
+ "failed to get mux-state for %pOFn\n",
+ grp);
+
+ ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "failed to add group %pOFn\n", grp);
+
+ ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
+ 1, function);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "failed to add function %pOFn\n", grp);
+ }
+
+ return 0;
+}
+
static const struct pinctrl_ops mux_pinctrl_ops = {
.get_groups_count = pinctrl_generic_get_group_count,
.get_group_name = pinctrl_generic_get_group_name,
@@ -157,6 +189,10 @@ static int mux_pinctrl_probe(struct platform_device *pdev)
if (ret)
return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
+ ret = mux_pinctrl_probe_dt(mpctl->pctl, dev->of_node);
+ if (ret)
+ return ret;
+
ret = pinctrl_enable(mpctl->pctl);
if (ret)
return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
--
2.50.1On Wed, Sep 16, 2026 at 12:04:35PM +0900, Chancel Liu wrote:
> From: Chancel Liu <chancel.liu@nxp.com>
>
...
>
> +static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
> + struct device_node *np)
> +{
> + struct device *dev = pctldev->dev;
> +
> + for_each_available_child_of_node_scoped(np, grp) {
> + struct mux_pin_function *function;
> + const char **group_names;
> + int ret;
> +
> + function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> + if (!function)
> + return -ENOMEM;
> +
> + group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
> + if (!group_names)
> + return -ENOMEM;
> +
> + group_names[0] = grp->name;
> +
> + function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
> + if (IS_ERR(function->mux_state))
> + return dev_err_probe(dev, PTR_ERR(function->mux_state),
> + "failed to get mux-state for %pOFn\n",
> + grp);
> +
> + ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to add group %pOFn\n", grp);
> +
> + ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
> + 1, function);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to add function %pOFn\n", grp);
> + }
> +
> + return 0;
> +}
This method also works, slice difference, if some pin have not used by
consumer, old method may save some memory.
leave to linus to decide which is better way, this one or add free() at
.dt_free_map()
Frank
> +
> static const struct pinctrl_ops mux_pinctrl_ops = {
> .get_groups_count = pinctrl_generic_get_group_count,
> .get_group_name = pinctrl_generic_get_group_name,
> @@ -157,6 +189,10 @@ static int mux_pinctrl_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
>
> + ret = mux_pinctrl_probe_dt(mpctl->pctl, dev->of_node);
> + if (ret)
> + return ret;
> +
> ret = pinctrl_enable(mpctl->pctl);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
> --
> 2.50.1
>
On 9/18/2026 5:02 AM, Frank Li wrote:
> On Wed, Sep 16, 2026 at 12:04:35PM +0900, Chancel Liu wrote:
>> From: Chancel Liu <chancel.liu@nxp.com>
>>
> ...
>>
>> +static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
>> + struct device_node *np)
>> +{
>> + struct device *dev = pctldev->dev;
>> +
>> + for_each_available_child_of_node_scoped(np, grp) {
>> + struct mux_pin_function *function;
>> + const char **group_names;
>> + int ret;
>> +
>> + function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
>> + if (!function)
>> + return -ENOMEM;
>> +
>> + group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
>> + if (!group_names)
>> + return -ENOMEM;
>> +
>> + group_names[0] = grp->name;
>> +
>> + function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
>> + if (IS_ERR(function->mux_state))
>> + return dev_err_probe(dev, PTR_ERR(function->mux_state),
>> + "failed to get mux-state for %pOFn\n",
>> + grp);
>> +
>> + ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret,
>> + "failed to add group %pOFn\n", grp);
>> +
>> + ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
>> + 1, function);
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret,
>> + "failed to add function %pOFn\n", grp);
>> + }
>> +
>> + return 0;
>> +}
>
> This method also works, slice difference, if some pin have not used by
> consumer, old method may save some memory.
>
> leave to linus to decide which is better way, this one or add free() at
> .dt_free_map()
>
> Frank
IMHO, the groups and functions of a board-level mux are static and fully
described by the devicetree, so probe time is the natural place to build
them, and dt_node_to_map() can then focus on mapping. Happy to go with
whichever Linus prefers.
Regards,
Chancel Liu
On Wed, Sep 16, 2026 at 12:04:35PM +0900, Chancel Liu wrote:
> From: Chancel Liu <chancel.liu@nxp.com>
>
> The pinctrl core re-parses a consumer's pinctrl on every probe attempt
> (pinctrl_bind_pins() runs before the driver's probe(), and again on each
> deferred-probe retry or re-bind), so .dt_node_to_map() must be side-effect
> free and safe to call repeatedly.
>
> mux_pinmux_dt_node_to_map() instead allocated provider-side resources on
> every call - the mux_pin_function, the group name table and a mux_state
> reference, plus a group name and a pinctrl_generic_add_group()
> registration via pinctrl_generic_to_map(). All of these are allocated
> with devm_*() against the provider device, so they live for the provider's
> lifetime and are only released when the provider itself is unbound. The
> .dt_free_map() path never touches them; it frees the per-consumer
> pinctrl_map only. Every re-parse therefore adds another set of
> provider-side allocations that are never reclaimed, so a consumer that
> repeatedly defers probe or is re-bound leaks memory and mux_state
> references on the provider without bound.
>
> The groups and functions of a board-level mux are static and fully
> described by the device tree, so build them once at probe time in the new
> mux_pinctrl_probe_dt(). mux_pinmux_dt_node_to_map() then only looks up the
> already registered group and builds the per-consumer mux map, which the
> core frees via .dt_free_map. As the mux has no pin electrical
> configuration, no config map is emitted.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260824022020.2812810-1-chancel.liu%40oss.nxp.com
> Fixes: 34acc5a8adfb ("pinctrl: add generic board-level pinctrl driver using mux framework")
> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> Assisted-by: VeroCoder:claude-opus-4-8
> ---
> drivers/pinctrl/pinctrl-generic-mux.c | 76 ++++++++++++++++++++-------
> 1 file changed, 56 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
> index 202b72351efb..de85b4e75eb8 100644
> --- a/drivers/pinctrl/pinctrl-generic-mux.c
> +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> @@ -10,13 +10,11 @@
> #include <linux/mutex.h>
> #include <linux/mux/consumer.h>
> #include <linux/platform_device.h>
> -#include <linux/pinctrl/pinconf-generic.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinmux.h>
> #include <linux/slab.h>
>
> #include "core.h"
> -#include "pinconf.h"
> #include "pinmux.h"
> #include "pinctrl-utils.h"
>
> @@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> struct pinctrl_map **maps, unsigned int *num_maps)
> {
> unsigned int num_reserved_maps = 0;
> - struct mux_pin_function *function;
> - const char **group_names;
> int ret;
>
> - function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
> - if (!function)
> - return -ENOMEM;
> -
> - group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
> - if (!group_names)
> - return -ENOMEM;
devm alloc is quite popular under drivers/pinctrl
airoha/pinctrl-airoha.c dt_node_to_map devm_kzalloc, devm_pinctrl_register_and_init
meson/pinctrl-amlogic-a4.c dt_node_to_map devm_kcalloc
pinctrl-at91.c at91_dt_node_to_map devm_kcalloc, devm_kfree
pinctrl-axp209.c dt_node_to_map devm_kcalloc
pinctrl-generic-mux.c mux_pinmux_dt_node_to_map devm_kcalloc, devm_kzalloc, devm_mux_state_get_from_np
pinctrl-generic.c pinctrl_generic_dt_node_to_map devm_kcalloc
pinctrl-single.c pcs_dt_node_to_map devm_kcalloc, devm_kfree, devm_kzalloc
pinctrl-st.c st_pctl_dt_node_to_map devm_kcalloc, devm_kfree
pinctrl-th1520.c th1520_pinctrl_dt_node_to_map devm_kasprintf, devm_kcalloc
pinctrl-tps6594.c dt_node_to_map devm_gpio_regmap_register, devm_kzalloc, devm_pinctrl_register
renesas/pinctrl-rza1.c rza1_dt_node_to_map devm_kcalloc, devm_kzalloc
renesas/pinctrl-rza2.c rza2_dt_node_to_map devm_kcalloc, devm_kzalloc
sophgo/pinctrl-sophgo-common.c sophgo_pctrl_dt_node_to_map devm_kasprintf, devm_kcalloc
starfive/pinctrl-starfive-jh7100.c starfive_dt_node_to_map devm_kasprintf, devm_kcalloc
starfive/pinctrl-starfive-jh7110.c jh7110_dt_node_to_map devm_kasprintf, devm_kcalloc
sunplus/sppctl.c dt_node_to_map devm_kcalloc
ti/pinctrl-ti-iodelay.c ti_iodelay_dt_node_to_map devm_kcalloc, devm_kfree, devm_kzalloc
does all have similar problem? I suspect this when do this patch, but I
have not tracked these life cycle.
Frank
> -
> - function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
> - if (IS_ERR(function->mux_state))
> - return PTR_ERR(function->mux_state);
> + *maps = NULL;
> + *num_maps = 0;
>
> - ret = pinctrl_generic_to_map(pctldev, np_config, np_config, maps,
> - num_maps, &num_reserved_maps, group_names,
> - 0, &np_config->name, NULL, 0);
> + if (pinctrl_get_group_selector(pctldev, np_config->name) < 0)
> + return -ENODEV;
>
> + ret = pinctrl_utils_reserve_map(pctldev, maps, &num_reserved_maps,
> + num_maps, 1);
> if (ret)
> return ret;
>
> - ret = pinmux_generic_add_function(pctldev, np_config->name, group_names,
> - 1, function);
> + ret = pinctrl_utils_add_map_mux(pctldev, maps, &num_reserved_maps,
> + num_maps, np_config->name,
> + np_config->name);
> if (ret < 0) {
> pinctrl_utils_free_map(pctldev, *maps, *num_maps);
> + *maps = NULL;
> + *num_maps = 0;
> return ret;
> }
>
> return 0;
> }
>
> +static int mux_pinctrl_probe_dt(struct pinctrl_dev *pctldev,
> + struct device_node *np)
> +{
> + struct device *dev = pctldev->dev;
> +
> + for_each_available_child_of_node_scoped(np, grp) {
> + struct mux_pin_function *function;
> + const char **group_names;
> + int ret;
> +
> + function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> + if (!function)
> + return -ENOMEM;
> +
> + group_names = devm_kcalloc(dev, 1, sizeof(*group_names), GFP_KERNEL);
> + if (!group_names)
> + return -ENOMEM;
> +
> + group_names[0] = grp->name;
> +
> + function->mux_state = devm_mux_state_get_from_np(dev, NULL, grp);
> + if (IS_ERR(function->mux_state))
> + return dev_err_probe(dev, PTR_ERR(function->mux_state),
> + "failed to get mux-state for %pOFn\n",
> + grp);
> +
> + ret = pinctrl_generic_add_group(pctldev, grp->name, NULL, 0, NULL);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to add group %pOFn\n", grp);
> +
> + ret = pinmux_generic_add_function(pctldev, grp->name, group_names,
> + 1, function);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to add function %pOFn\n", grp);
> + }
> +
> + return 0;
> +}
> +
> static const struct pinctrl_ops mux_pinctrl_ops = {
> .get_groups_count = pinctrl_generic_get_group_count,
> .get_group_name = pinctrl_generic_get_group_name,
> @@ -157,6 +189,10 @@ static int mux_pinctrl_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
>
> + ret = mux_pinctrl_probe_dt(mpctl->pctl, dev->of_node);
> + if (ret)
> + return ret;
> +
> ret = pinctrl_enable(mpctl->pctl);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
> --
> 2.50.1
>
>> @@ -38,39 +36,73 @@ mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
>> struct pinctrl_map **maps, unsigned int *num_maps)
>> {
>> unsigned int num_reserved_maps = 0;
>> - struct mux_pin_function *function;
>> - const char **group_names;
>> int ret;
>>
>> - function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
>> - if (!function)
>> - return -ENOMEM;
>> -
>> - group_names = devm_kcalloc(pctldev->dev, 1, sizeof(*group_names), GFP_KERNEL);
>> - if (!group_names)
>> - return -ENOMEM;
>
>
> devm alloc is quite popular under drivers/pinctrl
>
> airoha/pinctrl-airoha.c dt_node_to_map devm_kzalloc, devm_pinctrl_register_and_init
> meson/pinctrl-amlogic-a4.c dt_node_to_map devm_kcalloc
> pinctrl-at91.c at91_dt_node_to_map devm_kcalloc, devm_kfree
> pinctrl-axp209.c dt_node_to_map devm_kcalloc
> pinctrl-generic-mux.c mux_pinmux_dt_node_to_map devm_kcalloc, devm_kzalloc, devm_mux_state_get_from_np
> pinctrl-generic.c pinctrl_generic_dt_node_to_map devm_kcalloc
> pinctrl-single.c pcs_dt_node_to_map devm_kcalloc, devm_kfree, devm_kzalloc
> pinctrl-st.c st_pctl_dt_node_to_map devm_kcalloc, devm_kfree
> pinctrl-th1520.c th1520_pinctrl_dt_node_to_map devm_kasprintf, devm_kcalloc
> pinctrl-tps6594.c dt_node_to_map devm_gpio_regmap_register, devm_kzalloc, devm_pinctrl_register
> renesas/pinctrl-rza1.c rza1_dt_node_to_map devm_kcalloc, devm_kzalloc
> renesas/pinctrl-rza2.c rza2_dt_node_to_map devm_kcalloc, devm_kzalloc
> sophgo/pinctrl-sophgo-common.c sophgo_pctrl_dt_node_to_map devm_kasprintf, devm_kcalloc
> starfive/pinctrl-starfive-jh7100.c starfive_dt_node_to_map devm_kasprintf, devm_kcalloc
> starfive/pinctrl-starfive-jh7110.c jh7110_dt_node_to_map devm_kasprintf, devm_kcalloc
> sunplus/sppctl.c dt_node_to_map devm_kcalloc
> ti/pinctrl-ti-iodelay.c ti_iodelay_dt_node_to_map devm_kcalloc, devm_kfree, devm_kzalloc
>
> does all have similar problem? I suspect this when do this patch, but I
> have not tracked these life cycle.
>
> Frank
>
Not all, but many do. devm_* in .dt_node_to_map() only leaks when the
allocation is bound to the provider dev and isn't reclaimed by
.dt_free_map(). So it accumulates on every re-parse.
Here's the calling:
really_probe()
└─ pinctrl_bind_pins(dev)
└─ devm_pinctrl_get(dev)
└─ create_pinctrl()
└─ pinctrl_dt_to_map(p, pctldev)
└─ dt_to_map_one_config()
└─ ops->dt_node_to_map()
Every deferred-probe retry and every re-bind re-invokes .dt_node_to_map().
Regards,
Chancel Liu
© 2016 - 2026 Red Hat, Inc.