Re: [PATCH v11 2/7] i2c: mux: add idle_state property to i2c_mux_core

[Marcus Folkesson]

Hi Peter!

On Sun, Jul 19, 2026 at 02:39:34PM +0200, Peter Rosin wrote:
> On 2026-07-15 21:02, Marcus Folkesson wrote:
> > Hi Peter,
> > 
> > On Tue, Jul 14, 2026 at 01:29:01PM +0200, Peter Rosin wrote:
> > > Hi Marcus,
> > > 
> > > Sorry for the very late feedback.
> > > 
> > > On 2026-07-13 09:19, Marcus Folkesson wrote:
> > > > Muxes treat their channels differently when idle.
> > > > Let the mux core have this information to make it available for
> > > > internal use.
> > > > Reuse the same state values used by CONFIG_MULTIPLEXER.
> > > > 
> > > > Possible idle states are:
> > > > - MUX_IDLE_AS_IS: Leave channels as is when idle
> > > > - MUX_IDLE_DISCONNECT: Disconnect channel (set HiZ when idle)
> > > > - <n>: Enable channel n when idle
> > > > 
> > > > Default value is set to MUX_IDLE_AS_IS.
> > > > 
> > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx>


[...]

> > I think I will introduce a few defines in i2c-mux.c then;
> > 
> > 
> > #define I2C_MUX_IDLE_UNKNOWN	(-1)
> > #define I2C_MUX_IDLE_AS_IS      (-2)
> > #define I2C_MUX_IDLE_DISCONNECT (-3)
> 
> Hi!
> 
> Please keep AS_IS as -1 and DISCONNECT as -2. Using different
> values will make it difficult to get rid of the use of the
> defines belonging to the mux subsystem in case the actual
> value has crept into some .dtb or something like that.

Got it!

> 
> Also, I think the right thing to do is to put these defines
> in the i2c-mux.h header so that the drivers can find them.
> I assume .c was a typo?

Yep, it should be i2c-mux.h.

> 
> > [...]
> > 
> > struct i2c_mux_core {
> > 
> >      [...]
> > 
> > 	/*
> > 	 * The mux state to use when not active.
> 
> This is not 100% accurate. The value stored here is never
> actually used to set the idle state. The idle_state here is
> only what the driver has declared that the idle_state is.
> Perhaps word it like this instead?
> 
> 	* The mux state used by the driver when idle.
> 
> Agreed, subtle difference, but...

... even better. I will change to that.

> 
> In the future, drivers (most of them) could be changed to
> use this variable to store the actual idle state. But, as
> mentioned, that's not easy for i2c-mux-gpmux since the idle
> state is under the control of the the mux subsystem in that
> case.
> 
> Cheers,
> Peter
> 
> > 	 * Possible idle states are:
> > 	 *  - I2C_MUX_IDLE_UNKNOWN: Unknown idle state
> > 	 *  - I2C_MUX_IDLE_AS_IS: Leave channels as is when idle
> > 	 *  - I2C_MUX_IDLE_DISCONNECT: Disconnect channel (set HiZ when idle)
> > 	 *  - <n>: Enable channel n (starting from 0) when idle"
> > 	 *
> > 	 * Default value is set to I2C_MUX_IDLE_UNKNOWN.
> > 	 */
> > 	int idle_state;
> > 
> >      [...]
> > };

Thanks,
Marcus Folkesson