Re: [PATCH v2] mux: gpio-mux: add support for 4:1 2-channels mux

[Srinivas Kandagatla]

"mux: gpio-mux: add support for multiple mux controls" would be more
reflective of what is done in  the patch.

On 5/6/26 1:33 PM, Andrea Tomassetti wrote:
> Some gpio multiplexers, like TMUX1209, offer differential 4:1
> or dual 4:1 single-ended channels. No binding changes are needed
> because the DT binding already supports #mux-control-cells
> with values 0 and 1. So, similarly to what was already done by the
> adg792a driver, the gpio-mux driver has to take into account
I guess adg792a does not have same issue(described below) as gpio mux

> the #mux-control-cells property and allocate as many controllers as
> advised by it.>

 drivers/mux/gpio.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index 4cc3202c58f3..ab3ed5247d2c 100644
> --- a/drivers/mux/gpio.c
> +++ b/drivers/mux/gpio.c
> @@ -52,12 +52,23 @@ static int mux_gpio_probe(struct platform_device *pdev)
>  	int pins;
>  	s32 idle_state;
>  	int ret;
> +	u32 cells;
> +	int i;
> 
>  	pins = gpiod_count(dev, "mux");
>  	if (pins < 0)
>  		return pins;
> 
> -	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio));
> +	ret = device_property_read_u32(dev, "#mux-control-cells", &cells);
> +	if (ret < 0)
> +		cells = 0;
> +
> +	if (cells >= 2) {

This code snippet is duplicated once again, may its time for this to go
to as helper function, either in core or somewhere common.

> +		dev_err(dev, "invalid control-cells %u, must be 0 or 1\n", cells);

 mux-control-cells would give useful message to user rather than just
control-cells.

> +		return -EINVAL;
> +	}
> +
> +	mux_chip = devm_mux_chip_alloc(dev, cells + 1, sizeof(*mux_gpio));

We are now allocating n mux_controls, however mux_gpio_set is not
protected against multiple controllers accessing the same gpio resource.


>  	if (IS_ERR(mux_chip))
>  		return PTR_ERR(mux_chip);
> 
> @@ -69,7 +80,9 @@ static int mux_gpio_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(mux_gpio->gpios),
>  				     "failed to get gpios\n");
>  	WARN_ON(pins != mux_gpio->gpios->ndescs);
> -	mux_chip->mux->states = BIT(pins);
> +
> +	for (i = 0; i < mux_chip->controllers; ++i)
> +		mux_chip->mux[i].states = BIT(pins);
> 
>  	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
>  	if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) {
> @@ -78,7 +91,8 @@ static int mux_gpio_probe(struct platform_device *pdev)
>  			return -EINVAL;
>  		}
> 
> -		mux_chip->mux->idle_state = idle_state;
> +		for (i = 0; i < mux_chip->controllers; ++i)
> +			mux_chip->mux[i].idle_state = idle_state;
>  	}
>.