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

Marcus Folkesson <[email protected]>
Newsgroups org.kernel.vger.linux-i2c,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> > ---
> >   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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.