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@0 {
clock-frequency = <100000>;
D1 {
...
};
};
i2c@1 {
D2 {
...
};
};
};
D3 {
...
}
};
Signed-off-by: Marcus Folkesson <marcus.folkesson@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..f125b4b8ae58 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -36,21 +36,113 @@ struct i2c_mux_priv {
u32 chan_id;
};
+static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
+{
+ struct i2c_adapter *parent;
+
+ while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
+ struct i2c_mux_priv *priv = adap->algo_data;
+
+ if (priv && priv->muxc && priv->muxc->mux_locked)
+ return priv->muxc;
+
+ adap = parent;
+ }
+
+ return NULL;
+}
+
+static int i2c_mux_select_chan(struct i2c_adapter *adap, u32 chan_id, u32 *oldclock)
+{
+ struct i2c_adapter *root = i2c_root_adapter(&adap->dev);
+ struct i2c_mux_priv *priv = adap->algo_data;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_mux_core *mux_locked_ancestor;
+ int ret;
+
+ if (priv->adap.clock_hz && priv->adap.clock_hz < root->clock_hz) {
+ mux_locked_ancestor = i2c_mux_first_mux_locked(adap);
+ *oldclock = root->clock_hz;
+
+ /*
+ * If there's a mux-locked mux in our ancestry, lock the parent
+ * of the first one. When locked with I2C_LOCK_ROOT_ADAPTER,
+ * this will recurse through all intermediate muxes (both mux-locked
+ * and parent-locked) up to the root adapter, ensuring the entire
+ * chain is locked.
+ */
+ 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)
+ dev_err(&adap->dev,
+ "Failed to set clock frequency %dHz on root adapter %s: %d\n",
+ priv->adap.clock_hz, root->name, ret);
+ }
+
+ return muxc->select(muxc, priv->chan_id);
+}
+
+static void i2c_mux_deselect_chan(struct i2c_adapter *adap, u32 chan_id, u32 oldclock)
+{
+ struct i2c_mux_priv *priv = adap->algo_data;
+ struct i2c_mux_core *mux_locked_ancestor;
+ struct i2c_mux_core *muxc = priv->muxc;
+ struct i2c_adapter *parent = muxc->parent;
+ struct i2c_adapter *root;
+ int ret;
+
+ if (muxc->deselect)
+ muxc->deselect(muxc, priv->chan_id);
+
+ if (oldclock && oldclock != priv->adap.clock_hz) {
+ mux_locked_ancestor = i2c_mux_first_mux_locked(adap);
+ root = i2c_root_adapter(&parent->dev);
+
+ /*
+ * If there's a mux-locked mux in our ancestry, lock the parent
+ * of the first one. When locked with I2C_LOCK_ROOT_ADAPTER,
+ * this will recurse through all intermediate muxes (both mux-locked
+ * and parent-locked) up to the root adapter, ensuring the entire
+ * chain is locked.
+ */
+ if (mux_locked_ancestor)
+ i2c_lock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
+
+ ret = i2c_adapter_set_clk_freq(root, oldclock);
+
+ if (mux_locked_ancestor)
+ i2c_unlock_bus(mux_locked_ancestor->parent, I2C_LOCK_ROOT_ADAPTER);
+
+ if (ret)
+ dev_err(&adap->dev,
+ "Failed to set clock frequency %dHz on root adapter %s: %d\n",
+ oldclock, root->name, ret);
+
+ }
+}
+
static int __i2c_mux_master_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
{
struct i2c_mux_priv *priv = adap->algo_data;
struct i2c_mux_core *muxc = priv->muxc;
struct i2c_adapter *parent = muxc->parent;
+ u32 oldclock = 0;
int ret;
/* Switch to the right mux port and perform the transfer. */
- ret = muxc->select(muxc, priv->chan_id);
+ ret = i2c_mux_select_chan(adap, priv->chan_id, &oldclock);
if (ret >= 0)
ret = __i2c_transfer(parent, msgs, num);
- if (muxc->deselect)
- muxc->deselect(muxc, priv->chan_id);
+
+ i2c_mux_deselect_chan(adap, priv->chan_id, oldclock);
return ret;
}
@@ -61,15 +153,16 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
struct i2c_mux_priv *priv = adap->algo_data;
struct i2c_mux_core *muxc = priv->muxc;
struct i2c_adapter *parent = muxc->parent;
+ u32 oldclock = 0;
int ret;
/* Switch to the right mux port and perform the transfer. */
- ret = muxc->select(muxc, priv->chan_id);
+ ret = i2c_mux_select_chan(adap, priv->chan_id, &oldclock);
if (ret >= 0)
ret = i2c_transfer(parent, msgs, num);
- if (muxc->deselect)
- muxc->deselect(muxc, priv->chan_id);
+
+ i2c_mux_deselect_chan(adap, priv->chan_id, oldclock);
return ret;
}
@@ -82,16 +175,17 @@ static int __i2c_mux_smbus_xfer(struct i2c_adapter *adap,
struct i2c_mux_priv *priv = adap->algo_data;
struct i2c_mux_core *muxc = priv->muxc;
struct i2c_adapter *parent = muxc->parent;
+ u32 oldclock = 0;
int ret;
/* Select the right mux port and perform the transfer. */
- ret = muxc->select(muxc, priv->chan_id);
+ ret = i2c_mux_select_chan(adap, priv->chan_id, &oldclock);
if (ret >= 0)
ret = __i2c_smbus_xfer(parent, addr, flags,
read_write, command, size, data);
- if (muxc->deselect)
- muxc->deselect(muxc, priv->chan_id);
+
+ i2c_mux_deselect_chan(adap, priv->chan_id, oldclock);
return ret;
}
@@ -104,16 +198,17 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
struct i2c_mux_priv *priv = adap->algo_data;
struct i2c_mux_core *muxc = priv->muxc;
struct i2c_adapter *parent = muxc->parent;
+ u32 oldclock = 0;
int ret;
/* Select the right mux port and perform the transfer. */
- ret = muxc->select(muxc, priv->chan_id);
+ ret = i2c_mux_select_chan(adap, priv->chan_id, &oldclock);
if (ret >= 0)
ret = i2c_smbus_xfer(parent, addr, flags,
read_write, command, size, data);
- if (muxc->deselect)
- muxc->deselect(muxc, priv->chan_id);
+
+ i2c_mux_deselect_chan(adap, priv->chan_id, oldclock);
return ret;
}
@@ -362,6 +457,32 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
}
}
+ of_property_read_u32(child, "clock-frequency", &priv->adap.clock_hz);
+
+ /* If the mux adapter has no clock-frequency property, inherit from parent */
+ if (!priv->adap.clock_hz)
+ priv->adap.clock_hz = parent->clock_hz;
+
+ /*
+ * Warn if the mux adapter is not parent-locked as
+ * this may cause issues for some hardware topologies.
+ */
+ if ((priv->adap.clock_hz < parent->clock_hz) && muxc->mux_locked)
+ dev_warn(muxc->dev,
+ "channel %u is slower than parent on a non parent-locked mux\n",
+ chan_id);
+
+ /* We don't support mux adapters faster than their parent */
+ if (priv->adap.clock_hz > parent->clock_hz) {
+ dev_err(muxc->dev,
+ "channel (%u) is faster (%u) than parent (%u)\n",
+ chan_id, priv->adap.clock_hz, parent->clock_hz);
+
+ of_node_put(mux_node);
+ ret = -EINVAL;
+ goto err_free_priv;
+ }
+
priv->adap.dev.of_node = child;
of_node_put(mux_node);
}
--
2.52.0Hi!
> +static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
> +{
> + struct i2c_adapter *parent;
> +
> + while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> + struct i2c_mux_priv *priv = adap->algo_data;
This assumption does not hold, making the cast pretty wild indeed. There
are other i2c_adapters with a parent besides muxes. See e.g. i2c_atr.c
Cheers,
Peter
> +
> + if (priv && priv->muxc && priv->muxc->mux_locked)
> + return priv->muxc;
> +
> + adap = parent;
> + }
> +
> + return NULL;
> +}
Hi Peter!
On Mon, Feb 16, 2026 at 05:40:37PM +0100, Peter Rosin wrote:
> Hi!
>
> > +static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
> > +{
> > + struct i2c_adapter *parent;
> > +
> > + while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> > + struct i2c_mux_priv *priv = adap->algo_data;
>
> This assumption does not hold, making the cast pretty wild indeed. There
> are other i2c_adapters with a parent besides muxes. See e.g. i2c_atr.c
I see. Hrm, not sure how to decide if it is a mux or not. The best I
could come up with is to look at the i2c_adapter.lock_ops. E.g.
while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
/*
* Check if this adapter is a mux channel by verifying its
* lock_ops. Only mux channels use these specific lock operations.
*/
if (adap->lock_ops == &i2c_mux_lock_ops ||
adap->lock_ops == &i2c_parent_lock_ops) {
struct i2c_mux_priv *priv = adap->algo_data;
if (priv->muxc->mux_locked)
return priv->muxc;
}
adap = parent;
}
Or do you have a better idea?
>
> Cheers,
> Peter
Thank you in advance,
Marcus Folkesson
Hi!
2026-02-16 at 19:50, Marcus Folkesson wrote:
> Hi Peter!
>
> On Mon, Feb 16, 2026 at 05:40:37PM +0100, Peter Rosin wrote:
>> Hi!
>>
>>> +static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
>>> +{
>>> + struct i2c_adapter *parent;
>>> +
>>> + while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
>>> + struct i2c_mux_priv *priv = adap->algo_data;
>>
>> This assumption does not hold, making the cast pretty wild indeed. There
>> are other i2c_adapters with a parent besides muxes. See e.g. i2c_atr.c
>
> I see. Hrm, not sure how to decide if it is a mux or not. The best I
> could come up with is to look at the i2c_adapter.lock_ops. E.g.
>
>
> while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> /*
> * Check if this adapter is a mux channel by verifying its
> * lock_ops. Only mux channels use these specific lock operations.
> */
> if (adap->lock_ops == &i2c_mux_lock_ops ||
> adap->lock_ops == &i2c_parent_lock_ops) {
> struct i2c_mux_priv *priv = adap->algo_data;
>
> if (priv->muxc->mux_locked)
> return priv->muxc;
> }
> adap = parent;
> }
>
> Or do you have a better idea?
That looks fragile. My recommendation would be to avoid trying to
guess how a potentially diverse adapter tree should be handled
locally in the mux code. To me, it would feel better to introduce
locking/recursion in i2c_adapter_set_clk_freq() for muxes (and
address translators), i.e. take inspiration from i2c_transfer()
and i2c_smbus_xfer().
I guess an unlocked __i2c_adapter_set_clk_freq() is needed.
Cheers,
Peter
Hi Peter!
On Tue, Feb 17, 2026 at 10:37:39AM +0100, Peter Rosin wrote:
> Hi!
>
> 2026-02-16 at 19:50, Marcus Folkesson wrote:
> > Hi Peter!
> >
> > On Mon, Feb 16, 2026 at 05:40:37PM +0100, Peter Rosin wrote:
> >> Hi!
> >>
> >>> +static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
> >>> +{
> >>> + struct i2c_adapter *parent;
> >>> +
> >>> + while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> >>> + struct i2c_mux_priv *priv = adap->algo_data;
> >>
> >> This assumption does not hold, making the cast pretty wild indeed. There
> >> are other i2c_adapters with a parent besides muxes. See e.g. i2c_atr.c
> >
> > I see. Hrm, not sure how to decide if it is a mux or not. The best I
> > could come up with is to look at the i2c_adapter.lock_ops. E.g.
> >
> >
> > while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> > /*
> > * Check if this adapter is a mux channel by verifying its
> > * lock_ops. Only mux channels use these specific lock operations.
> > */
> > if (adap->lock_ops == &i2c_mux_lock_ops ||
> > adap->lock_ops == &i2c_parent_lock_ops) {
> > struct i2c_mux_priv *priv = adap->algo_data;
> >
> > if (priv->muxc->mux_locked)
> > return priv->muxc;
> > }
> > adap = parent;
> > }
> >
> > Or do you have a better idea?
>
> That looks fragile. My recommendation would be to avoid trying to
> guess how a potentially diverse adapter tree should be handled
> locally in the mux code. To me, it would feel better to introduce
> locking/recursion in i2c_adapter_set_clk_freq() for muxes (and
> address translators), i.e. take inspiration from i2c_transfer()
> and i2c_smbus_xfer().
That would be a more robust solution indeed.
>
> I guess an unlocked __i2c_adapter_set_clk_freq() is needed.
>
Rethinking the whole locking approach;
If I follow the same locking logic as in i2c_transfer/__i2c_transfer, do I
really need do take any more locks than the root adapter with
I2C_LOCK_ROOT_ADAPTER?
As the frequency can only be lowered, an intermediate i2c message will
not mess anything up?
If so, do you think i2c_root_adapter() should be moved to i2c-core-base.c?
Then i2c_adapter_set(_root?)_clk_freq() could lookup the root adapter
and take the lock there.
> Cheers,
> Peter
Best regards,
Marcus Folkesson
Hi Peter,
On Wed, Feb 18, 2026 at 08:02:54AM +0100, Marcus Folkesson wrote:
> Hi Peter!
>
> On Tue, Feb 17, 2026 at 10:37:39AM +0100, Peter Rosin wrote:
> > Hi!
> >
> > 2026-02-16 at 19:50, Marcus Folkesson wrote:
> > > Hi Peter!
> > >
> > > On Mon, Feb 16, 2026 at 05:40:37PM +0100, Peter Rosin wrote:
> > >> Hi!
> > >>
> > >>> +static struct i2c_mux_core *i2c_mux_first_mux_locked(struct i2c_adapter *adap)
> > >>> +{
> > >>> + struct i2c_adapter *parent;
> > >>> +
> > >>> + while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> > >>> + struct i2c_mux_priv *priv = adap->algo_data;
> > >>
> > >> This assumption does not hold, making the cast pretty wild indeed. There
> > >> are other i2c_adapters with a parent besides muxes. See e.g. i2c_atr.c
> > >
> > > I see. Hrm, not sure how to decide if it is a mux or not. The best I
> > > could come up with is to look at the i2c_adapter.lock_ops. E.g.
> > >
> > >
> > > while ((parent = i2c_parent_is_i2c_adapter(adap)) != NULL) {
> > > /*
> > > * Check if this adapter is a mux channel by verifying its
> > > * lock_ops. Only mux channels use these specific lock operations.
> > > */
> > > if (adap->lock_ops == &i2c_mux_lock_ops ||
> > > adap->lock_ops == &i2c_parent_lock_ops) {
> > > struct i2c_mux_priv *priv = adap->algo_data;
> > >
> > > if (priv->muxc->mux_locked)
> > > return priv->muxc;
> > > }
> > > adap = parent;
> > > }
> > >
> > > Or do you have a better idea?
> >
> > That looks fragile. My recommendation would be to avoid trying to
> > guess how a potentially diverse adapter tree should be handled
> > locally in the mux code. To me, it would feel better to introduce
> > locking/recursion in i2c_adapter_set_clk_freq() for muxes (and
> > address translators), i.e. take inspiration from i2c_transfer()
> > and i2c_smbus_xfer().
>
> That would be a more robust solution indeed.
>
> >
> > I guess an unlocked __i2c_adapter_set_clk_freq() is needed.
> >
>
> Rethinking the whole locking approach;
> If I follow the same locking logic as in i2c_transfer/__i2c_transfer, do I
> really need do take any more locks than the root adapter with
> I2C_LOCK_ROOT_ADAPTER?
> As the frequency can only be lowered, an intermediate i2c message will
> not mess anything up?
>
> If so, do you think i2c_root_adapter() should be moved to i2c-core-base.c?
> Then i2c_adapter_set(_root?)_clk_freq() could lookup the root adapter
> and take the lock there.
I've reworked this during the weekend and think I've got some my
answers. I will send out a new version later today.
Thanks!
Best regards,
Marcus Folkesson
© 2016 - 2026 Red Hat, Inc.