[PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework

Frank Li posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework
Posted by Frank Li 1 month, 1 week ago
Many boards use on-board mux chips (often controlled by GPIOs from an I2C
expander) to switch shared signals between peripherals.

Add a generic pinctrl driver built on top of the mux framework to
centralize mux handling and avoid probe ordering issues. Keep board-level
routing out of individual drivers and supports boot-time only mux
selection.

Ensure correct probe ordering, especially when the GPIO expander is probed
later.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pinctrl/Kconfig               |   9 ++
 drivers/pinctrl/Makefile              |   1 +
 drivers/pinctrl/pinctrl-generic-mux.c | 222 ++++++++++++++++++++++++++++++++++
 3 files changed, 232 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index afecd9407f5354f5b92223f8cd80d2f7a08f8e7d..0657eeeeb587fa5e68dc3c1e00be35608e243b80 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -274,6 +274,15 @@ config PINCTRL_GEMINI
 	select GENERIC_PINCONF
 	select MFD_SYSCON
 
+config PINCTRL_GENERIC_MUX
+	tristate "Generic Pinctrl driver by using multiplexer"
+	depends on MULTIPLEXER
+	select PINMUX
+	select GENERIC_PINCONF
+	help
+          Generic pinctrl driver by MULTIPLEXER framework to control on
+          board pin selection.
+
 config PINCTRL_INGENIC
 	bool "Pinctrl driver for the Ingenic JZ47xx SoCs"
 	default MACH_INGENIC
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f7d5d5f76d0c8becc0aa1d77c68b6ced924ea264..fcd1703440d24579636e8ddb6cbd83a0a982dfb7 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
 obj-$(CONFIG_PINCTRL_EP93XX)	+= pinctrl-ep93xx.o
 obj-$(CONFIG_PINCTRL_EYEQ5)	+= pinctrl-eyeq5.o
 obj-$(CONFIG_PINCTRL_GEMINI)	+= pinctrl-gemini.o
+obj-$(CONFIG_PINCTRL_GENERIC_MUX) += pinctrl-generic-mux.o
 obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
 obj-$(CONFIG_PINCTRL_K210)	+= pinctrl-k210.o
 obj-$(CONFIG_PINCTRL_K230)	+= pinctrl-k230.o
diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
new file mode 100644
index 0000000000000000000000000000000000000000..555dd2966e205e7f90a8bf8df3e46ed51d29d562
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-generic-mux.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic Pin Control Driver for Board-Level Mux Chips
+ * Copyright (C) 2026
+ */
+
+#include <linux/cleanup.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#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 "pinmux.h"
+
+struct mux_pin_function {
+	struct mux_state *mux_state;
+};
+
+struct mux_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+
+	/* mutex protecting the lists */
+	struct mutex lock;
+	int cur_select;
+};
+
+static int
+mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
+			  struct device_node *np_config,
+			  struct pinctrl_map **map, unsigned int *num_maps)
+{
+	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
+	struct mux_pin_function *function;
+	struct device *dev = mpctl->dev;
+	const char **pgnames;
+	int selector;
+	int group;
+	int ret;
+
+	*map = devm_kcalloc(dev, 1, sizeof(**map), GFP_KERNEL);
+	if (!map)
+		return -ENOMEM;
+
+	*num_maps = 0;
+
+	function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
+	if (!function) {
+		ret = -ENOMEM;
+		goto err_func;
+	}
+
+	pgnames = devm_kzalloc(dev, sizeof(*pgnames), GFP_KERNEL);
+	if (!pgnames) {
+		ret = -ENOMEM;
+		goto err_pgnames;
+	}
+
+	pgnames[0] = np_config->name;
+
+	guard(mutex)(&mpctl->lock);
+
+	selector = pinmux_generic_add_function(mpctl->pctl, np_config->name,
+					       pgnames, 1, function);
+	if (selector < 0) {
+		ret = selector;
+		goto err_add_func;
+	}
+
+	group = pinctrl_generic_add_group(mpctl->pctl, np_config->name, NULL, 0, mpctl);
+	if (group < 0) {
+		ret = group;
+		goto err_add_group;
+	}
+
+	function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
+	if (IS_ERR(function->mux_state)) {
+		ret = PTR_ERR(function->mux_state);
+		goto err_mux_state_get;
+	}
+
+	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
+	(*map)->data.mux.group = np_config->name;
+	(*map)->data.mux.function = np_config->name;
+
+	*num_maps = 1;
+
+	return 0;
+
+err_mux_state_get:
+	pinctrl_generic_remove_group(mpctl->pctl, group);
+err_add_group:
+	pinmux_generic_remove_function(mpctl->pctl, selector);
+err_add_func:
+	devm_kfree(dev, pgnames);
+err_pgnames:
+	devm_kfree(dev, function);
+err_func:
+	devm_kfree(dev, *map);
+
+	return ret;
+}
+
+static void
+mux_pinmux_dt_free_map(struct pinctrl_dev *pctldev, struct pinctrl_map *map,
+		       unsigned int num_maps)
+{
+	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
+
+	devm_kfree(mpctl->dev, map);
+}
+
+static const struct pinctrl_ops mux_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+	.dt_node_to_map = mux_pinmux_dt_node_to_map,
+	.dt_free_map = mux_pinmux_dt_free_map,
+};
+
+static int mux_pinmux_set_mux(struct pinctrl_dev *pctldev,
+			      unsigned int func_selector,
+			      unsigned int group_selector)
+{
+	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct function_desc *function;
+	struct mux_pin_function *func;
+	int ret;
+
+	guard(mutex)(&mpctl->lock);
+
+	function = pinmux_generic_get_function(pctldev, func_selector);
+	func = function->data;
+
+	if (mpctl->cur_select == func_selector)
+		return 0;
+
+	if (mpctl->cur_select >= 0 && mpctl->cur_select != func_selector)
+		return -EINVAL;
+
+	ret = mux_state_select(func->mux_state);
+	if (ret)
+		return ret;
+
+	mpctl->cur_select = func_selector;
+
+	return 0;
+}
+
+static const struct pinmux_ops mux_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = mux_pinmux_set_mux,
+};
+
+static int mux_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_pinctrl *mpctl;
+	struct pinctrl_desc *pctl_desc;
+	int ret;
+
+	mpctl = devm_kzalloc(dev, sizeof(*mpctl), GFP_KERNEL);
+	if (!mpctl)
+		return -ENOMEM;
+
+	mpctl->dev = dev;
+	mpctl->cur_select = -1;
+
+	platform_set_drvdata(pdev, mpctl);
+
+	pctl_desc = devm_kzalloc(dev, sizeof(*pctl_desc), GFP_KERNEL);
+	if (!pctl_desc)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(dev, &mpctl->lock);
+	if (ret)
+		return ret;
+
+	pctl_desc->name = dev_name(dev);
+	pctl_desc->owner = THIS_MODULE;
+	pctl_desc->pctlops = &mux_pinctrl_ops;
+	pctl_desc->pmxops = &mux_pinmux_ops;
+
+	ret = devm_pinctrl_register_and_init(dev, pctl_desc, mpctl,
+					     &mpctl->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
+
+	ret = pinctrl_enable(mpctl->pctl);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
+
+	return 0;
+}
+
+static const struct of_device_id mux_pinctrl_of_match[] = {
+	{ .compatible = "pinctrl-multiplexer" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mux_pinctrl_of_match);
+
+static struct platform_driver mux_pinctrl_driver = {
+	.driver = {
+		.name = "generic-pinctrl-mux",
+		.of_match_table = mux_pinctrl_of_match,
+	},
+	.probe = mux_pinctrl_probe,
+};
+module_platform_driver(mux_pinctrl_driver);
+
+MODULE_AUTHOR("Frank Li <Frank.Li@nxp.com>");
+MODULE_DESCRIPTION("Generic Pin Control Driver for Board-Level Mux Chips");
+MODULE_LICENSE("GPL");
+

-- 
2.43.0
Re: [PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework
Posted by Peter Rosin 1 month, 1 week ago
Hi!

2026-02-19 at 23:23, Frank Li wrote:
> Many boards use on-board mux chips (often controlled by GPIOs from an I2C
> expander) to switch shared signals between peripherals.
> 
> Add a generic pinctrl driver built on top of the mux framework to
> centralize mux handling and avoid probe ordering issues. Keep board-level
> routing out of individual drivers and supports boot-time only mux
> selection.
> 
> Ensure correct probe ordering, especially when the GPIO expander is probed
> later.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pinctrl/Kconfig               |   9 ++
>  drivers/pinctrl/Makefile              |   1 +
>  drivers/pinctrl/pinctrl-generic-mux.c | 222 ++++++++++++++++++++++++++++++++++
>  3 files changed, 232 insertions(+)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index afecd9407f5354f5b92223f8cd80d2f7a08f8e7d..0657eeeeb587fa5e68dc3c1e00be35608e243b80 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -274,6 +274,15 @@ config PINCTRL_GEMINI
>  	select GENERIC_PINCONF
>  	select MFD_SYSCON
>  
> +config PINCTRL_GENERIC_MUX
> +	tristate "Generic Pinctrl driver by using multiplexer"
> +	depends on MULTIPLEXER
> +	select PINMUX
> +	select GENERIC_PINCONF
> +	help
> +          Generic pinctrl driver by MULTIPLEXER framework to control on
> +          board pin selection.
> +
>  config PINCTRL_INGENIC
>  	bool "Pinctrl driver for the Ingenic JZ47xx SoCs"
>  	default MACH_INGENIC
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f7d5d5f76d0c8becc0aa1d77c68b6ced924ea264..fcd1703440d24579636e8ddb6cbd83a0a982dfb7 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
>  obj-$(CONFIG_PINCTRL_EP93XX)	+= pinctrl-ep93xx.o
>  obj-$(CONFIG_PINCTRL_EYEQ5)	+= pinctrl-eyeq5.o
>  obj-$(CONFIG_PINCTRL_GEMINI)	+= pinctrl-gemini.o
> +obj-$(CONFIG_PINCTRL_GENERIC_MUX) += pinctrl-generic-mux.o
>  obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
>  obj-$(CONFIG_PINCTRL_K210)	+= pinctrl-k210.o
>  obj-$(CONFIG_PINCTRL_K230)	+= pinctrl-k230.o
> diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..555dd2966e205e7f90a8bf8df3e46ed51d29d562
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic Pin Control Driver for Board-Level Mux Chips
> + * Copyright (C) 2026
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#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 "pinmux.h"
> +
> +struct mux_pin_function {
> +	struct mux_state *mux_state;
> +};
> +
> +struct mux_pinctrl {
> +	struct device *dev;
> +	struct pinctrl_dev *pctl;
> +
> +	/* mutex protecting the lists */
> +	struct mutex lock;
> +	int cur_select;
> +};
> +
> +static int
> +mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> +			  struct device_node *np_config,
> +			  struct pinctrl_map **map, unsigned int *num_maps)
> +{
> +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct mux_pin_function *function;
> +	struct device *dev = mpctl->dev;
> +	const char **pgnames;
> +	int selector;
> +	int group;
> +	int ret;
> +
> +	*map = devm_kcalloc(dev, 1, sizeof(**map), GFP_KERNEL);
> +	if (!map)
> +		return -ENOMEM;
> +
> +	*num_maps = 0;
> +
> +	function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> +	if (!function) {
> +		ret = -ENOMEM;
> +		goto err_func;
> +	}
> +
> +	pgnames = devm_kzalloc(dev, sizeof(*pgnames), GFP_KERNEL);
> +	if (!pgnames) {
> +		ret = -ENOMEM;
> +		goto err_pgnames;
> +	}
> +
> +	pgnames[0] = np_config->name;
> +
> +	guard(mutex)(&mpctl->lock);
> +
> +	selector = pinmux_generic_add_function(mpctl->pctl, np_config->name,
> +					       pgnames, 1, function);
> +	if (selector < 0) {
> +		ret = selector;
> +		goto err_add_func;
> +	}
> +
> +	group = pinctrl_generic_add_group(mpctl->pctl, np_config->name, NULL, 0, mpctl);
> +	if (group < 0) {
> +		ret = group;
> +		goto err_add_group;
> +	}
> +
> +	function->mux_state = devm_mux_state_get_from_np(pctldev->dev, NULL, np_config);
> +	if (IS_ERR(function->mux_state)) {
> +		ret = PTR_ERR(function->mux_state);
> +		goto err_mux_state_get;
> +	}
> +
> +	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
> +	(*map)->data.mux.group = np_config->name;
> +	(*map)->data.mux.function = np_config->name;
> +
> +	*num_maps = 1;
> +
> +	return 0;
> +
> +err_mux_state_get:
> +	pinctrl_generic_remove_group(mpctl->pctl, group);
> +err_add_group:
> +	pinmux_generic_remove_function(mpctl->pctl, selector);
> +err_add_func:
> +	devm_kfree(dev, pgnames);
> +err_pgnames:
> +	devm_kfree(dev, function);
> +err_func:
> +	devm_kfree(dev, *map);
> +
> +	return ret;
> +}
> +
> +static void
> +mux_pinmux_dt_free_map(struct pinctrl_dev *pctldev, struct pinctrl_map *map,
> +		       unsigned int num_maps)
> +{
> +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	devm_kfree(mpctl->dev, map);
> +}
> +
> +static const struct pinctrl_ops mux_pinctrl_ops = {
> +	.get_groups_count = pinctrl_generic_get_group_count,
> +	.get_group_name = pinctrl_generic_get_group_name,
> +	.get_group_pins = pinctrl_generic_get_group_pins,
> +	.dt_node_to_map = mux_pinmux_dt_node_to_map,
> +	.dt_free_map = mux_pinmux_dt_free_map,
> +};
> +
> +static int mux_pinmux_set_mux(struct pinctrl_dev *pctldev,
> +			      unsigned int func_selector,
> +			      unsigned int group_selector)
> +{
> +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct function_desc *function;
> +	struct mux_pin_function *func;
> +	int ret;
> +
> +	guard(mutex)(&mpctl->lock);
> +
> +	function = pinmux_generic_get_function(pctldev, func_selector);
> +	func = function->data;
> +
> +	if (mpctl->cur_select == func_selector)
> +		return 0;
> +
> +	if (mpctl->cur_select >= 0 && mpctl->cur_select != func_selector)
> +		return -EINVAL;
> +
> +	ret = mux_state_select(func->mux_state);

There is no matching call to mux_state_deselect(). Quoting the docs for
mux_state_select{,_delay}():

	On successfully selecting the mux-state, its mux-control will be
	locked until there is a call to mux_state_deselect(). If the
	mux-control is already selected when mux_state_select() is called,
	the caller will be blocked until mux_state_deselect() or
	mux_control_deselect() is called (by someone else).

	Therefore, make sure to call mux_state_deselect() when the
	operation is complete and the mux-control is free for others to
	use, but do not call mux_state_deselect() if mux_state_select()
	fails.

So, unless I'm missing something, you will get a deadlock if a pinctrl
have states backed by mux-states connected to the same mux-control (which
feels like the typical case) and you try to change between these pinctrl
states. Have you tried calling pinctrl_select_state() with this pinctrl
to mux things at runtime?

Cheers,
Peter

> +	if (ret)
> +		return ret;
> +
> +	mpctl->cur_select = func_selector;
> +
> +	return 0;
> +}
> +
> +static const struct pinmux_ops mux_pinmux_ops = {
> +	.get_functions_count = pinmux_generic_get_function_count,
> +	.get_function_name = pinmux_generic_get_function_name,
> +	.get_function_groups = pinmux_generic_get_function_groups,
> +	.set_mux = mux_pinmux_set_mux,
> +};
> +
> +static int mux_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mux_pinctrl *mpctl;
> +	struct pinctrl_desc *pctl_desc;
> +	int ret;
> +
> +	mpctl = devm_kzalloc(dev, sizeof(*mpctl), GFP_KERNEL);
> +	if (!mpctl)
> +		return -ENOMEM;
> +
> +	mpctl->dev = dev;
> +	mpctl->cur_select = -1;
> +
> +	platform_set_drvdata(pdev, mpctl);
> +
> +	pctl_desc = devm_kzalloc(dev, sizeof(*pctl_desc), GFP_KERNEL);
> +	if (!pctl_desc)
> +		return -ENOMEM;
> +
> +	ret = devm_mutex_init(dev, &mpctl->lock);
> +	if (ret)
> +		return ret;
> +
> +	pctl_desc->name = dev_name(dev);
> +	pctl_desc->owner = THIS_MODULE;
> +	pctl_desc->pctlops = &mux_pinctrl_ops;
> +	pctl_desc->pmxops = &mux_pinmux_ops;
> +
> +	ret = devm_pinctrl_register_and_init(dev, pctl_desc, mpctl,
> +					     &mpctl->pctl);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to register pinctrl.\n");
> +
> +	ret = pinctrl_enable(mpctl->pctl);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable pinctrl.\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mux_pinctrl_of_match[] = {
> +	{ .compatible = "pinctrl-multiplexer" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_pinctrl_of_match);
> +
> +static struct platform_driver mux_pinctrl_driver = {
> +	.driver = {
> +		.name = "generic-pinctrl-mux",
> +		.of_match_table = mux_pinctrl_of_match,
> +	},
> +	.probe = mux_pinctrl_probe,
> +};
> +module_platform_driver(mux_pinctrl_driver);
> +
> +MODULE_AUTHOR("Frank Li <Frank.Li@nxp.com>");
> +MODULE_DESCRIPTION("Generic Pin Control Driver for Board-Level Mux Chips");
> +MODULE_LICENSE("GPL");
> +
>
Re: [PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework
Posted by Daniel Baluta 1 month, 1 week ago
Hi Frank,

Few comments inline:


> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic Pin Control Driver for Board-Level Mux Chips
> + * Copyright (C) 2026
>
Copyright 2026 NXP

<snip>

> +struct mux_pin_function {
> +	struct mux_state *mux_state;
> +};
> +
> +struct mux_pinctrl {
> +	struct device *dev;
> +	struct pinctrl_dev *pctl;
> +
> +	/* mutex protecting the lists */

what lists? 


> +static int
> +mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> +			  struct device_node *np_config,
> +			  struct pinctrl_map **map, unsigned int *num_maps)
> +{
> +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct mux_pin_function *function;
> +	struct device *dev = mpctl->dev;
> +	const char **pgnames;
> +	int selector;
> +	int group;
> +	int ret;
> +
> +	*map = devm_kcalloc(dev, 1, sizeof(**map), GFP_KERNEL);
> +	if (!map)
> +		return -ENOMEM;
>
if (!*map) ?

> +
> +	*num_maps = 0;
> +
> +	function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> +	if (!function) {
> +		ret = -ENOMEM;
> +		goto err_func;
Do you really need this goto? Previously allocated memory is dev managed .
> +	}
> +
> +	pgnames = devm_kzalloc(dev, sizeof(*pgnames), GFP_KERNEL);
> +	if (!pgnames) {
> +		ret = -ENOMEM;
> +		goto err_pgnames;
>
Same here.

<snip>

> +err_mux_state_get:
> +	pinctrl_generic_remove_group(mpctl->pctl, group);
> +err_add_group:
> +	pinmux_generic_remove_function(mpctl->pctl, selector);
> +err_add_func:
> +	devm_kfree(dev, pgnames);
> +err_pgnames:
> +	devm_kfree(dev, function);
> +err_func:
> +	devm_kfree(dev, *map);
> +
> +	return ret;
> +}




Re: [PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework
Posted by Frank Li 1 month, 1 week ago
On Fri, Feb 20, 2026 at 10:50:41AM +0200, Daniel Baluta wrote:
> Hi Frank,
>
> Few comments inline:
>
>
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> > @@ -0,0 +1,222 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Generic Pin Control Driver for Board-Level Mux Chips
> > + * Copyright (C) 2026
> >
> Copyright 2026 NXP
>
> <snip>
>
> > +struct mux_pin_function {
> > +	struct mux_state *mux_state;
> > +};
> > +
> > +struct mux_pinctrl {
> > +	struct device *dev;
> > +	struct pinctrl_dev *pctl;
> > +
> > +	/* mutex protecting the lists */
>
> what lists? 
>
>
> > +static int
> > +mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> > +			  struct device_node *np_config,
> > +			  struct pinctrl_map **map, unsigned int *num_maps)
> > +{
> > +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> > +	struct mux_pin_function *function;
> > +	struct device *dev = mpctl->dev;
> > +	const char **pgnames;
> > +	int selector;
> > +	int group;
> > +	int ret;
> > +
> > +	*map = devm_kcalloc(dev, 1, sizeof(**map), GFP_KERNEL);
> > +	if (!map)
> > +		return -ENOMEM;
> >
> if (!*map) ?
>
> > +
> > +	*num_maps = 0;
> > +
> > +	function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> > +	if (!function) {
> > +		ret = -ENOMEM;
> > +		goto err_func;
> Do you really need this goto? Previously allocated memory is dev managed .

Yes, dt_node_to_map() may re-entry because some defer. we don't want to
defer free previous allocated memory to module remove and generally
pinctrl driver is never removed.

why use devm_ version is because pinctrl framework assume you use devm_ to
allocate map.

Frank

> > +	}
> > +
> > +	pgnames = devm_kzalloc(dev, sizeof(*pgnames), GFP_KERNEL);
> > +	if (!pgnames) {
> > +		ret = -ENOMEM;
> > +		goto err_pgnames;
> >
> Same here.
>
> <snip>
>
> > +err_mux_state_get:
> > +	pinctrl_generic_remove_group(mpctl->pctl, group);
> > +err_add_group:
> > +	pinmux_generic_remove_function(mpctl->pctl, selector);
> > +err_add_func:
> > +	devm_kfree(dev, pgnames);
> > +err_pgnames:
> > +	devm_kfree(dev, function);
> > +err_func:
> > +	devm_kfree(dev, *map);
> > +
> > +	return ret;
> > +}
>
>
>
>