linux-kernel - Re: [PATCH v5 2/2] mux: gpio-mux: add support for enable GPIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMRc=McxxctNFRwKcHnZA2XjjDT3UKX_K67v6Z1nH9KpBimboA@mail.gmail.com>
Date: Tue, 27 Jan 2026 10:27:38 +0100
From: Bartosz Golaszewski <brgl@...nel.org>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>
Cc: Peter Rosin <peda@...ntia.se>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Srinivas Kandagatla <srini@...nel.org>, Johan Hovold <johan+linaro@...nel.org>, 
	Linus Walleij <linusw@...nel.org>, David Lechner <dlechner@...libre.com>, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/2] mux: gpio-mux: add support for enable GPIO

On Fri, Jan 23, 2026 at 3:58 PM Antoniu Miclaus
<antoniu.miclaus@...log.com> wrote:
>
> Add support for an optional enable GPIO to the gpio-mux driver. This
> allows the mux to be disabled before changing address lines and
> re-enabled after, preventing glitches that could briefly activate
> unintended channels during transitions.
>
> The enable GPIO is optional and the driver maintains backward
> compatibility with existing gpio-mux users.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> Changes in v5:
>  - Enhance code comment to explain high-impedance (high-Z) state when
>    mux is disabled
>  - Add context about downstream capacitance maintaining signal level
>    for analog multiplexers
> ---
>  drivers/mux/gpio.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index 4cc3202c58f3..b95d2c7a53ca 100644
> --- a/drivers/mux/gpio.c
> +++ b/drivers/mux/gpio.c
> @@ -19,6 +19,7 @@
>
>  struct mux_gpio {
>         struct gpio_descs *gpios;
> +       struct gpio_desc *enable;
>  };
>
>  static int mux_gpio_set(struct mux_control *mux, int state)
> @@ -27,10 +28,31 @@ static int mux_gpio_set(struct mux_control *mux, int state)
>         DECLARE_BITMAP(values, BITS_PER_TYPE(state));
>         u32 value = state;
>
> +       if (state == MUX_IDLE_DISCONNECT) {
> +               if (mux_gpio->enable)
> +                       gpiod_set_value_cansleep(mux_gpio->enable, 0);
> +               return 0;

This is optional so no need to check for NULL. Also: GPIO value
setters now return errors so it can be:

    return gpiod_set_value_cansleep();

> +       }
> +
> +       if (mux_gpio->enable) {
> +               /*
> +                * Disable the mux before changing address lines to prevent
> +                * glitches where an unintended channel could be briefly
> +                * activated during the transition. When disabled, all mux
> +                * outputs enter high-impedance (high-Z) state. For analog
> +                * signals, downstream capacitance typically maintains the
> +                * signal level during this brief disconnection.
> +                */
> +               gpiod_set_value_cansleep(mux_gpio->enable, 0);
> +       }

Same here.

> +
>         bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
>
>         gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);

This is not part of this change but would be useful to check this
return value as well.

>
> +       if (mux_gpio->enable)
> +               gpiod_set_value_cansleep(mux_gpio->enable, 1);
> +
>         return 0;
>  }
>
> @@ -71,9 +93,20 @@ static int mux_gpio_probe(struct platform_device *pdev)
>         WARN_ON(pins != mux_gpio->gpios->ndescs);
>         mux_chip->mux->states = BIT(pins);
>
> +       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 enable gpio\n");
> +
>         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) {
> +               if (idle_state == MUX_IDLE_DISCONNECT) {
> +                       if (!mux_gpio->enable) {
> +                               dev_err(dev,
> +                                       "invalid idle-state (MUX_IDLE_DISCONNECT requires enable-gpios)\n");
> +                               return -EINVAL;
> +                       }

Can we check it before and then decide whether we really need the GPIO or not?

> +               } else if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
>                         dev_err(dev, "invalid idle-state %u\n", idle_state);
>                         return -EINVAL;
>                 }
> --
> 2.43.0
>

Bart