Re: [PATCH v5 2/2] mux: gpio: Add optional enable gpio

["Alvin Šipraga" <alvin.sipraga@analog.com>]

On Thu, Nov 05, 2026 at 10:11:05AM +0000, Tapio Reijonen wrote:
> Some analog multiplexers have an enable input that disconnects all
> channels when deasserted, independent of the address inputs; on a
> 74HC4051 it is the E input. Add it as an optional gpio.
> 
> The mux gpios are not guaranteed to be updated atomically.
> gpiod_multi_set_value_cansleep() groups them per gpio controller, and
> only controllers implementing set_multiple() can possibly update theirs
> in a single write, so a mux with its address inputs spread over two
> controllers passes through the intermediate addresses on every change.
> Deassert the enable gpio for the duration of the update and assert it
> once the address is settled.
> 
> The enable gpio is also what makes an idle state of MUX_IDLE_DISCONNECT
> possible, which the mux core applies when the chip is registered and after
> every deselect: it leaves the enable gpio deasserted and the address
> inputs alone. Refuse that idle state without an enable gpio, because the
> address inputs cannot disconnect anything on their own and mux_gpio_set()
> would instead drive them to the bit pattern of MUX_IDLE_DISCONNECT.
> 
> Return the status of the gpio writes instead of zero. For a gpio on an
> i2c expander the write can fail, and mux_control_set() only invalidates
> the cached state when set() reports an error, so a failure reported as
> success would leave the mux on the previous channel with later selects
> of the same state skipping the write entirely.
> 
> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>

Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>

> ---
>  drivers/mux/gpio.c | 43 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index f9c7863e51b82fb76ed03cf17308fae74716cf6d..cb2f38c1a9e247478f97fffaf1d5ddae4315d1a7 100644
> --- a/drivers/mux/gpio.c
> +++ b/drivers/mux/gpio.c
> @@ -18,6 +18,7 @@
>  
>  struct mux_gpio {
>  	struct gpio_descs *gpios;
> +	struct gpio_desc *enable;
>  };
>  
>  static int mux_gpio_set(struct mux_control *mux, int state)
> @@ -25,12 +26,26 @@ static int mux_gpio_set(struct mux_control *mux, int state)
>  	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
>  	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
>  	u32 value = state;
> +	int ret;
> +
> +	/*
> +	 * The gpios might not be updated atomically, disable the mux
> +	 * meanwhile.
> +	 */
> +	ret = gpiod_set_value_cansleep(mux_gpio->enable, 0);
> +	if (ret)
> +		return ret;
> +
> +	if (state == MUX_IDLE_DISCONNECT)
> +		return 0;
>  
>  	bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
>  
> -	gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);
> +	ret = gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);
> +	if (ret)
> +		return ret;
>  
> -	return 0;
> +	return gpiod_set_value_cansleep(mux_gpio->enable, 1);
>  }
>  
>  static const struct mux_control_ops mux_gpio_ops = {
> @@ -63,6 +78,11 @@ static int mux_gpio_probe(struct platform_device *pdev)
>  	mux_gpio = mux_chip_priv(mux_chip);
>  	mux_chip->ops = &mux_gpio_ops;
>  
> +	mux_gpio->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(mux_gpio->enable))
> +		return dev_err_probe(dev, PTR_ERR(mux_gpio->enable),
> +				     "failed to get optional enable gpio\n");
> +
>  	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
>  	if (IS_ERR(mux_gpio->gpios))
>  		return dev_err_probe(dev, PTR_ERR(mux_gpio->gpios),
> @@ -71,15 +91,22 @@ static int mux_gpio_probe(struct platform_device *pdev)
>  	mux_chip->mux->states = BIT(pins);
>  
>  	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
> -	if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) {
> -		if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
> -			dev_err(dev, "invalid idle-state %u\n", idle_state);
> -			return -EINVAL;
> -		}
> +	if (ret < 0)
> +		idle_state = mux_chip->mux->idle_state;
>  
> -		mux_chip->mux->idle_state = idle_state;
> +	if (idle_state == MUX_IDLE_DISCONNECT && !mux_gpio->enable) {
> +		dev_err(dev, "idle-state disconnect requires enable-gpios\n");
> +		return -EINVAL;
>  	}
>  
> +	if (idle_state != MUX_IDLE_AS_IS && idle_state != MUX_IDLE_DISCONNECT &&
> +	    (idle_state < 0 || idle_state >= mux_chip->mux->states)) {
> +		dev_err(dev, "invalid idle-state %d\n", idle_state);
> +		return -EINVAL;
> +	}
> +
> +	mux_chip->mux->idle_state = idle_state;
> +
>  	ret = devm_regulator_get_enable_optional(dev, "mux");
>  	if (ret && ret != -ENODEV)
>  		return dev_err_probe(dev, ret, "failed to get/enable mux supply\n");
> 
> -- 
> 2.47.3
>