Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/2] mux: gpio: Add optional enable gpio
@ 2026-08-31 10:28 Tapio Reijonen
  2026-08-31 10:28 ` [PATCH v2 1/2] dt-bindings: mux: gpio-mux: Add enable-gpios Tapio Reijonen
  2026-08-31 10:28 ` [PATCH v2 2/2] mux: gpio: Add optional enable gpio Tapio Reijonen
  0 siblings, 2 replies; 4+ messages in thread
From: Tapio Reijonen @ 2026-08-31 10:28 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Greg Kroah-Hartman, devicetree, linux-kernel, Tapio Reijonen

Analog multiplexers have an enable input that puts all channels in the
high-impedance off state independent of the address inputs - the E input
of a 74HC4051, for example. gpio-mux has no way to describe it, so it can
neither reach the idle state of disconnect that the mux-controller
binding defines, nor keep the mux disabled while the address gpios are
updated one gpio controller at a time.

Add enable-gpios for that input, and use it in the driver.

Tested on an i.MX6SX board where two SFP cages share one SMBus behind
DG9411 analog switches: the address input selects the cage and the
enable input gates the bus, both driven by a PCA9501 I2C gpio expander.
With enable-gpios and an idle state of disconnect, the parent bus scans
empty between transfers while each cage stays reachable through its own
child bus. A logic analyser on the enable, address and bus lines over a
full boot shows 97 enable windows and 82 address changes, with no bus
activity outside a window and no address change inside one. Dropping
enable-gpios while keeping the disconnect idle state makes the driver
refuse to probe, as intended.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v2:
- Describe the enable input in terms of real multiplexers (the E input of
  a 74HC4051) instead of repeating the binding text, and say what needs
  it, as requested by Krzysztof.
- Add the missing gpio-mux: prefix to the binding patch subject.
- Limit enable-gpios to one entry, and require it for an idle state of
  disconnect in both the schema and the driver: without an enable gpio
  mux_gpio_set() drove the address gpios to the bit pattern of
  MUX_IDLE_DISCONNECT instead of disconnecting.
- Deassert the enable gpio while the mux gpios are updated, not only for
  the disconnect idle state, since they are not updated atomically.
- Show enable-gpios in the binding example.
- Link to v1: https://lore.kernel.org/r/20251105-add-external-mux-enable-gpio-v1-0-e59cba6f9e47@vaisala.com

---
Tapio Reijonen (2):
      dt-bindings: mux: gpio-mux: Add enable-gpios
      mux: gpio: Add optional enable gpio

 .../devicetree/bindings/mux/gpio-mux.yaml          | 22 ++++++++++++++
 drivers/mux/gpio.c                                 | 34 ++++++++++++++++++----
 2 files changed, 50 insertions(+), 6 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20250925-add-external-mux-enable-gpio-c9ba5beddd49

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] dt-bindings: mux: gpio-mux: Add enable-gpios
  2026-08-31 10:28 [PATCH v2 0/2] mux: gpio: Add optional enable gpio Tapio Reijonen
@ 2026-08-31 10:28 ` Tapio Reijonen
  2026-08-31 10:28 ` [PATCH v2 2/2] mux: gpio: Add optional enable gpio Tapio Reijonen
  1 sibling, 0 replies; 4+ messages in thread
From: Tapio Reijonen @ 2026-08-31 10:28 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Greg Kroah-Hartman, devicetree, linux-kernel, Tapio Reijonen

Analog multiplexers have an enable input besides the address inputs. On a
74HC4051 it is the E input: with E high, all eight switches are in the
high-impedance off state, independent of S0 to S2. A 74HC4067 works the
same way with four address inputs.

Two things need that input. The mux-controller binding describes an idle
state of disconnect, which a gpio-mux cannot reach today because the
address inputs always select a channel. And the address inputs are plain
gpios that may live on different gpio controllers, so they are not
updated in one go and the mux passes through the intermediate addresses
on every channel change unless it is disabled meanwhile.

Add enable-gpios to describe the enable input, and require it for an idle
state of disconnect.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 .../devicetree/bindings/mux/gpio-mux.yaml          | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.yaml b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
index ef7e33ec85d479ce57c5fe4a780f81a08a6ab6ba..2dd94704fd7bde238af4e84af87c2ad8e955eb14 100644
--- a/Documentation/devicetree/bindings/mux/gpio-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
@@ -29,6 +29,14 @@ properties:
     description:
       Regulator to power on the multiplexer.
 
+  enable-gpios:
+    maxItems: 1
+    description:
+      Enable input of the multiplexer, which disconnects all channels when
+      deasserted, independent of mux-gpios. Deasserted while the mux-gpios
+      are updated and for an idle state of disconnect, which cannot be
+      reached with the mux-gpios alone and thus requires this gpio.
+
   '#mux-control-cells':
     enum: [ 0, 1 ]
 
@@ -47,11 +55,23 @@ anyOf:
   - required:
       - "#mux-state-cells"
 
+allOf:
+  - if:
+      properties:
+        idle-state:
+          const: -2
+      required:
+        - idle-state
+    then:
+      required:
+        - enable-gpios
+
 additionalProperties: false
 
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/mux/mux.h>
 
     mux: mux-controller {
         compatible = "gpio-mux";
@@ -59,6 +79,8 @@ examples:
 
         mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
               <&pioA 1 GPIO_ACTIVE_HIGH>;
+        enable-gpios = <&pioA 2 GPIO_ACTIVE_LOW>;
+        idle-state = <MUX_IDLE_DISCONNECT>;
     };
 
     adc-mux {

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] mux: gpio: Add optional enable gpio
  2026-08-31 10:28 [PATCH v2 0/2] mux: gpio: Add optional enable gpio Tapio Reijonen
  2026-08-31 10:28 ` [PATCH v2 1/2] dt-bindings: mux: gpio-mux: Add enable-gpios Tapio Reijonen
@ 2026-08-31 10:28 ` Tapio Reijonen
  2026-09-01  7:11   ` Peter Rosin
  1 sibling, 1 reply; 4+ messages in thread
From: Tapio Reijonen @ 2026-08-31 10:28 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Greg Kroah-Hartman, devicetree, linux-kernel, Tapio Reijonen

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 updated atomically. gpiod_multi_set_value_cansleep()
groups them per gpio controller, and only controllers implementing
set_multiple() 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.

Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
 drivers/mux/gpio.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index f9c7863e51b82fb76ed03cf17308fae74716cf6d..df5d24214bd5fa8c3c1d733b25d32ca88c06c8c0 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_gpio;
 };
 
 static int mux_gpio_set(struct mux_control *mux, int state)
@@ -26,10 +27,18 @@ static int mux_gpio_set(struct mux_control *mux, int state)
 	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
 	u32 value = state;
 
+	/* The gpios are not updated atomically, disable the mux meanwhile. */
+	gpiod_set_value_cansleep(mux_gpio->enable_gpio, 0);
+
+	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);
 
+	gpiod_set_value_cansleep(mux_gpio->enable_gpio, 1);
+
 	return 0;
 }
 
@@ -70,14 +79,27 @@ 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_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(mux_gpio->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(mux_gpio->enable_gpio),
+				     "failed to get optional 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) {
-			dev_err(dev, "invalid idle-state %u\n", idle_state);
-			return -EINVAL;
+	if (ret >= 0) {
+		if (idle_state == MUX_IDLE_DISCONNECT) {
+			if (!mux_gpio->enable_gpio)
+				return dev_err_probe(dev, -EINVAL,
+						     "idle-state disconnect requires enable-gpios\n");
+
+			mux_chip->mux->idle_state = MUX_IDLE_DISCONNECT;
+		} else if (idle_state != MUX_IDLE_AS_IS) {
+			if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
+				return dev_err_probe(dev, -EINVAL,
+						     "invalid idle-state %d\n",
+						     idle_state);
+			}
+			mux_chip->mux->idle_state = idle_state;
 		}
-
-		mux_chip->mux->idle_state = idle_state;
 	}
 
 	ret = devm_regulator_get_enable_optional(dev, "mux");

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] mux: gpio: Add optional enable gpio
  2026-08-31 10:28 ` [PATCH v2 2/2] mux: gpio: Add optional enable gpio Tapio Reijonen
@ 2026-09-01  7:11   ` Peter Rosin
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Rosin @ 2026-09-01  7:11 UTC (permalink / raw)
  To: Tapio Reijonen
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, devicetree, linux-kernel

Hi,

Thanks for the patches! This all looks nice, but I do have a
few cosmetic nits...

Den Mon, Aug 31, 2026 at 10:28:25AM +0000, skrev Tapio Reijonen:
> Analog multiplexers have an enable input that disconnects all channels

Some analog multiplexers ...
or perhaps just
Some multiplexers ...

> 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 updated atomically. gpiod_multi_set_value_cansleep()

The mux gpios are not guaranteed to be updated atomically.

> groups them per gpio controller, and only controllers implementing
> set_multiple() update theirs in a single write, so a mux with its address

... set_multiple() can possibly update theirs ...

> 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.
> 
> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
> ---
>  drivers/mux/gpio.c | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index f9c7863e51b82fb76ed03cf17308fae74716cf6d..df5d24214bd5fa8c3c1d733b25d32ca88c06c8c0 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_gpio;

Please drop the _gpio suffix from the name.

>  };
>  
>  static int mux_gpio_set(struct mux_control *mux, int state)
> @@ -26,10 +27,18 @@ static int mux_gpio_set(struct mux_control *mux, int state)
>  	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
>  	u32 value = state;
>  
> +	/* The gpios are not updated atomically, disable the mux meanwhile. */

	/*
	 * The gpios might not be updated atomically, disable the mux
	 * meanwhile.
	 */

> +	gpiod_set_value_cansleep(mux_gpio->enable_gpio, 0);
> +
> +	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);
>  
> +	gpiod_set_value_cansleep(mux_gpio->enable_gpio, 1);
> +
>  	return 0;
>  }
>  
> @@ -70,14 +79,27 @@ 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_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(mux_gpio->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(mux_gpio->enable_gpio),
> +				     "failed to get optional 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) {
> -			dev_err(dev, "invalid idle-state %u\n", idle_state);
> -			return -EINVAL;
> +	if (ret >= 0) {
> +		if (idle_state == MUX_IDLE_DISCONNECT) {
> +			if (!mux_gpio->enable_gpio)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "idle-state disconnect requires enable-gpios\n");
> +
> +			mux_chip->mux->idle_state = MUX_IDLE_DISCONNECT;
> +		} else if (idle_state != MUX_IDLE_AS_IS) {
> +			if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
> +				return dev_err_probe(dev, -EINVAL,
> +						     "invalid idle-state %d\n",
> +						     idle_state);
> +			}
> +			mux_chip->mux->idle_state = idle_state;

The new code is a bit convoluted and too deeply indented, methinks. I'd
prefer:

	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
	if (ret < 0)
		idle_state = mux_chip->mux->idle_state;

	if (idle_state == MUX_IDLE_AS_IS) {
	} else if (idle_state == MUX_IDLE_DISCONNECT && !mux_gpio->enable) {
		dev_err(dev, "idle-state disconnect requires enable-gpios\n");
		return -EINVAL;
	} else if (idle_state == MUX_IDLE_DISCONNECT) {
	} else if (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;

Using the "return dev_err_probe(dev, -EINVAL, ...)" pattern only buys
you an "error EINVAL" prefix but costs over-long lines. So, not worth
it to me.

Beware, the above snippet was written directly in the MUA and has never
seen a compiler. It probably has some silly bug, but something like
that...

Cheers,
Peter

>  		}
> -
> -		mux_chip->mux->idle_state = idle_state;
>  	}
>  
>  	ret = devm_regulator_get_enable_optional(dev, "mux");
> 
> -- 
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-01  7:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 10:28 [PATCH v2 0/2] mux: gpio: Add optional enable gpio Tapio Reijonen
2026-08-31 10:28 ` [PATCH v2 1/2] dt-bindings: mux: gpio-mux: Add enable-gpios Tapio Reijonen
2026-08-31 10:28 ` [PATCH v2 2/2] mux: gpio: Add optional enable gpio Tapio Reijonen
2026-09-01  7:11   ` Peter Rosin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox