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

[Marcus Folkesson]

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>
> > ---
> >   drivers/i2c/i2c-mux.c   |  1 +
> >   include/linux/i2c-mux.h | 26 ++++++++++++++++++++++++++
> >   2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> > index 681a201c239b..edf16683dc83 100644
> > --- a/drivers/i2c/i2c-mux.c
> > +++ b/drivers/i2c/i2c-mux.c
> > @@ -247,6 +247,7 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
> >   	muxc->select = select;
> >   	muxc->deselect = deselect;
> >   	muxc->max_adapters = max_adapters;
> > +	muxc->idle_state = MUX_IDLE_AS_IS;
> 
> This is insufficient. AS_IS is simply not an adequate default.
> 
> For i2c-mux-gpmux, there is currently no way to dig out what
> the idle state is, as it is not exposed by the mux subsystem. For
> i2c-mux-gpio, the idle state depends on both the idle-state /and/
> the i2c-mux-idle-disconnect props. For i2c-mux-pca954x the idle
> state can be adjusted at runtime. Etc.
> 
> In short, idle state handling is a bit diverse, and I think this
> adds to that mess.
> 
> I think it will be a bit of work to come up with a scheme for the
> I2C mux core to accurately keep track of what the idle state is.
> One way to deal with that is to introduce a new "unknown" value
> that can be the default for all drivers that has not yet figured
> out how to feed the correct idle state to the core.
> 
> And that hints at why I think reusing the mux.h bindings #include
> is bad. The mux subsystem simply has no need for an unknown state,
> and adding that to mux.h is therefore out of place.

I see.

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)

[...]

struct i2c_mux_core {

    [...]

	/*
	 * The mux state to use when not active.
	 * 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;

    [...]
};


Would that be a better approach?

Thanks,
Marcus Folkesson