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

[Peter Rosin <peda@axentia.se>]

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");
> +
>