[PATCH v5 2/5] i2c: mux: add support for per channel bus frequency

[Peter Rosin]

Hi!

2026-02-13 at 12:06, Marcus Folkesson wrote:
> There may be several reasons why you may need to use a certain speed
> on an I2C bus. E.g.
> 
> - When several devices are attached to the bus, the speed must be
>   selected according to the slowest device.
> 
> - Electrical conditions may limit the usuable speed on the bus for
>   different reasons.
> 
> With an I2C multiplexer, it is possible to group the attached devices
> after their preferred speed by e.g. put all "slow" devices on a separate
> channel on the multiplexer.
> 
> Consider the following topology:
> 
>                       .----------. 100kHz .--------.
>     .--------. 400kHz |          |--------| dev D1 |
>     |  root  |--+-----| I2C MUX  |        '--------'
>     '--------'  |     |          |--. 400kHz .--------.
>                 |     '----------'  '-------| dev D2 |
>                 |  .--------.               '--------'
>                 '--| dev D3 |
>                    '--------'
> 
> One requirement with this design is that a multiplexer may only use the
> same or lower bus speed as its parent.
> Otherwise, if the multiplexer would have to increase the bus frequency,
> then all siblings (D3 in this case) would run into a clock speed it may
> not support.
> 
> The bus frequency for each channel is set in the devicetree. As the
> i2c-mux bindings import the i2c-controller schema, the clock-frequency
> property is already allowed.
> If no clock-frequency property is set, the channel inherit their parent
> bus speed.
> 
> The following example uses dt bindings to illustrate the topology above:
> 
>     i2c {
> 	clock-frequency = <400000>;
> 
>         i2c-mux {
>             i2c at 0 {
>                 clock-frequency = <100000>;
> 
>                 D1 {
>                     ...
>                 };
>             };
> 
>             i2c at 1 {
>                 D2 {
>                   ...
>                 };
>             };
>         };
> 
>         D3 {
>             ...
>         }
>     };
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson at gmail.com>
> ---
>  drivers/i2c/i2c-mux.c | 145 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 133 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index d59644e50f14..9116ee3fd979 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -36,6 +36,100 @@ struct i2c_mux_priv {
>  	u32 chan_id;
>  };
>  
> +static struct i2c_mux_core *i2c_mux_topmost_mux_locked(struct i2c_adapter *adap)
> +{
> +	struct i2c_adapter *curr = adap;
> +	struct i2c_adapter *parent;
> +	struct i2c_mux_core *result = NULL;
> +
> +	do {
> +		parent = i2c_parent_is_i2c_adapter(curr);
> +		if (parent) {
> +			struct i2c_mux_priv *priv = curr->algo_data;
> +
> +			if (priv && priv->muxc && priv->muxc->mux_locked)
> +				result = priv->muxc;
> +		}
> +		curr = parent;
> +	} while (parent);
> +
> +	return result;
> +}
> +
> +static int i2c_mux_select_chan(struct i2c_adapter *adap, u32 chan_id)
> +{
> +	struct i2c_mux_priv *priv = adap->algo_data;
> +	struct i2c_mux_core *muxc = priv->muxc;
> +	struct i2c_adapter *parent = muxc->parent;
> +	struct i2c_mux_core *mux_locked_ancestor = NULL;
> +	struct i2c_adapter *root;
> +	int ret;
> +
> +	if (priv->adap.clock_hz && priv->adap.clock_hz != parent->clock_hz) {
> +		mux_locked_ancestor = i2c_mux_topmost_mux_locked(adap);
> +		root = i2c_root_adapter(&adap->dev);
> +
> +		/*
> +		 * If there's a mux-locked mux in our ancestry, lock the parent
> +		 * of the topmost one. Mux-locked muxes don't propagate locking
> +		 * to their parents, so we must explicitly acquire the lock above
> +		 * the highest mux-locked ancestor to reach the root adapter.
> +		 */

As may be more apparent from my recent v4 response(?), this is not
what you want. You want mux_locked_ancestor to be the nearest
mux-locked ancestor.


> +		if (mux_locked_ancestor)
> +			i2c_lock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
> +
> +		ret = i2c_adapter_set_clk_freq(root, priv->adap.clock_hz);
> +
> +		if (mux_locked_ancestor)
> +			i2c_unlock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
> +
> +		if (ret < 0) {
> +			dev_err(&adap->dev,
> +				"Failed to set clock frequency %dHz on root adapter %s: %d\n",
> +				priv->adap.clock_hz, root->name, ret);
> +
> +			return ret;
> +		}
> +	}
> +

Here, you set the frequency /before/ ->select.

> +	return muxc->select(muxc, priv->chan_id);
> +}
> +
> +static void i2c_mux_deselect_chan(struct i2c_adapter *adap, u32 chan_id)
> +{
> +	struct i2c_mux_priv *priv = adap->algo_data;
> +	struct i2c_mux_core *muxc = priv->muxc;
> +	struct i2c_adapter *parent = muxc->parent;
> +	struct i2c_mux_core *mux_locked_ancestor = NULL;
> +	struct i2c_adapter *root;
> +	int ret;

And here, you restore the frequency, but /before/ ->deselect.

Thers's no symmetry in that...

Cheers,
Peter

> +	if (parent->clock_hz && parent->clock_hz != priv->adap.clock_hz) {
> +		mux_locked_ancestor = i2c_mux_topmost_mux_locked(adap);
> +		root = i2c_root_adapter(&parent->dev);
> +
> +		/*
> +		 * If there's a mux-locked mux in our ancestry, lock the parent
> +		 * of the topmost one. Mux-locked muxes don't propagate locking
> +		 * to their parents, so we must explicitly acquire the lock above
> +		 * the highest mux-locked ancestor to reach the root adapte> +		 */
> +		if (mux_locked_ancestor)
> +			i2c_lock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
> +
> +		ret = i2c_adapter_set_clk_freq(root, parent->clock_hz);
> +
> +		if (mux_locked_ancestor)
> +			i2c_unlock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
> +
> +		if (ret < 0)
> +			return;
> +	}
> +
> +	if (muxc->deselect)
> +		muxc->deselect(muxc, priv->chan_id);
> +}