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

[Marcus Folkesson]

Hi Peter!

On Thu, Jul 23, 2026 at 01:04:11PM +0200, Peter Rosin wrote:
> Hi!
> 
> On 2026-07-19 16:59, Marcus Folkesson wrote:
> > Muxes treat their channels differently when idle.
> > Let the mux core have this information to make it available for
> > internal use.
> > 
> > Possible idle states are:
> >   - I2C_MUX_IDLE_AS_IS: Leave channels as is when idle
> >   - I2C_MUX_IDLE_DISCONNECT: Disconnect channel (set HiZ when idle)
> >   - I2C_MUX_IDLE_UNKNOWN: Unknown idle state
> >   - <n>: Enable channel n (starting from 0) when idle"
> > 
> > Default value is set to I2C_MUX_IDLE_UNKNOWN.
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx>


[...]

> > +/*
> > + * Mux drivers may only change idle_state, and may only do so
> > + * between allocation and registration of the mux controller.
> 
> This is simply not true. It is a limitation imposed by your series.
> The pca954x driver has a hook to change the idle state at runtime
> (the change takes effect the next time the mux is deselected).
> 
> Cheers,
> Peter

I see.
I will update pca954x driver to make use of
i2c_mux_set_idle_state()/i2c_mux_idle_state() instead.

As the state may be changed in runtime, I will also verify that the state is
allowed for the current clock configuration in i2c_mux_set_idle_state:


static inline int i2c_mux_set_idle_state(struct i2c_mux_core *muxc, int state)
{
	switch (state) {
	case I2C_MUX_IDLE_AS_IS:
	case I2C_MUX_IDLE_UNKNOWN:
		for (int i = 0; i < muxc->num_adapters; i++) {
			/*
			 * idle_state is incompatible with channels that have a
			 * different clock frequency than the parent adapter.
			 */
			if (muxc->adapter[i]->clock_Hz != muxc->parent->clock_Hz)
				return -EINVAL;
		}
		break;

	case I2C_MUX_IDLE_DISCONNECT:
		break;

	default:
		if (state < 0 || state >= muxc->num_adapters)
			return -EINVAL;

		/*
		 * idle_state cannot select a channel with a different clock
		 * frequency than the parent adapter.
		 */
		if (muxc->adapter[state]->clock_Hz != muxc->parent->clock_Hz)
			return -EINVAL;
		break;
	}

	muxc->idle_state = state;
	return 0;
}


Best regards,
Marcus Folkesson

Attachment: signature.asc
Description: PGP signature