* [PATCH v5 1/5] i2c: core: add callback to change bus frequency
2026-02-13 11:06 [PATCH v5 0/5] I2C Mux per channel bus speed Marcus Folkesson
@ 2026-02-13 11:06 ` Marcus Folkesson
2026-02-13 11:14 ` Andy Shevchenko
2026-02-13 11:06 ` [PATCH v5 2/5] i2c: mux: add support for per channel " Marcus Folkesson
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 11:06 UTC (permalink / raw)
To: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel, Marcus Folkesson
All devices on the same I2C bus share the same clock line and the bus
frequency has therefor be chosen so that all attached devices are able
to tolarate that clock rate. IOW, the bus speed must be set for the
slowest attached device.
With I2C multiplexers/switches on the other hand, it would be possible
to have different "domains" that runs with different speeds.
Prepare for such a feature by provide an optional callback function to
change bus frequency.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
include/linux/i2c.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 20fd41b51d5c..d147e388dbab 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -742,6 +742,8 @@ struct i2c_adapter {
struct rt_mutex mux_lock;
int timeout; /* in jiffies */
+ int clock_hz;
+ int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_hz); /* Optional */
int retries;
struct device dev; /* the adapter device */
unsigned long locked_flags; /* owned by the I2C core */
@@ -835,6 +837,17 @@ i2c_unlock_bus(struct i2c_adapter *adapter, unsigned int flags)
adapter->lock_ops->unlock_bus(adapter, flags);
}
+static inline int
+i2c_adapter_set_clk_freq(struct i2c_adapter *adapter, u32 clock_hz)
+{
+ int ret = -EOPNOTSUPP;
+
+ if (adapter->set_clk_freq)
+ ret = adapter->set_clk_freq(adapter, clock_hz);
+
+ return ret;
+}
+
/**
* i2c_mark_adapter_suspended - Report suspended state of the adapter to the core
* @adap: Adapter to mark as suspended
--
2.52.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/5] i2c: core: add callback to change bus frequency
2026-02-13 11:06 ` [PATCH v5 1/5] i2c: core: add callback to change bus frequency Marcus Folkesson
@ 2026-02-13 11:14 ` Andy Shevchenko
2026-02-16 9:22 ` Marcus Folkesson
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2026-02-13 11:14 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
On Fri, Feb 13, 2026 at 12:06:50PM +0100, Marcus Folkesson wrote:
> All devices on the same I2C bus share the same clock line and the bus
> frequency has therefor be chosen so that all attached devices are able
> to tolarate that clock rate. IOW, the bus speed must be set for the
> slowest attached device.
>
> With I2C multiplexers/switches on the other hand, it would be possible
> to have different "domains" that runs with different speeds.
>
> Prepare for such a feature by provide an optional callback function to
> change bus frequency.
...
> struct i2c_adapter {
> + int clock_hz;
Why signed? Even inconsistent with the parameter of the below.
> + int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_hz); /* Optional */
It's already a huge struct, can we make this compile-time chosen
(when I²C muxes are not required, for example)?
...
> +static inline int
> +i2c_adapter_set_clk_freq(struct i2c_adapter *adapter, u32 clock_hz)
> +{
> + int ret = -EOPNOTSUPP;
> +
> + if (adapter->set_clk_freq)
> + ret = adapter->set_clk_freq(adapter, clock_hz);
> +
> + return ret;
The 'ret' is redundant and can be dropped completely. This makes the function
2LoCs shorter.
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/5] i2c: core: add callback to change bus frequency
2026-02-13 11:14 ` Andy Shevchenko
@ 2026-02-16 9:22 ` Marcus Folkesson
2026-02-16 9:31 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-16 9:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
Hi Andy,
On Fri, Feb 13, 2026 at 12:14:04PM +0100, Andy Shevchenko wrote:
> On Fri, Feb 13, 2026 at 12:06:50PM +0100, Marcus Folkesson wrote:
> > All devices on the same I2C bus share the same clock line and the bus
> > frequency has therefor be chosen so that all attached devices are able
> > to tolarate that clock rate. IOW, the bus speed must be set for the
> > slowest attached device.
> >
> > With I2C multiplexers/switches on the other hand, it would be possible
> > to have different "domains" that runs with different speeds.
> >
> > Prepare for such a feature by provide an optional callback function to
> > change bus frequency.
>
> ...
>
> > struct i2c_adapter {
>
> > + int clock_hz;
>
> Why signed? Even inconsistent with the parameter of the below.
>
> > + int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_hz); /* Optional */
>
> It's already a huge struct, can we make this compile-time chosen
> (when I²C muxes are not required, for example)?
Hrm, many bus drivers (k1, jz4780, stm32 to mention a few) already have
the clock value stored in their private data, so maybe it is better to
have this value in a uniform place in the i2c_adapter struct and make
those drivers use it instead?
Best regards,
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/5] i2c: core: add callback to change bus frequency
2026-02-16 9:22 ` Marcus Folkesson
@ 2026-02-16 9:31 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-02-16 9:31 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
On Mon, Feb 16, 2026 at 10:22:00AM +0100, Marcus Folkesson wrote:
> On Fri, Feb 13, 2026 at 12:14:04PM +0100, Andy Shevchenko wrote:
> > On Fri, Feb 13, 2026 at 12:06:50PM +0100, Marcus Folkesson wrote:
> > > All devices on the same I2C bus share the same clock line and the bus
> > > frequency has therefor be chosen so that all attached devices are able
> > > to tolarate that clock rate. IOW, the bus speed must be set for the
> > > slowest attached device.
> > >
> > > With I2C multiplexers/switches on the other hand, it would be possible
> > > to have different "domains" that runs with different speeds.
> > >
> > > Prepare for such a feature by provide an optional callback function to
> > > change bus frequency.
...
> > > struct i2c_adapter {
> >
> > > + int clock_hz;
> >
> > Why signed? Even inconsistent with the parameter of the below.
> >
> > > + int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_hz); /* Optional */
> >
> > It's already a huge struct, can we make this compile-time chosen
> > (when I²C muxes are not required, for example)?
>
> Hrm, many bus drivers (k1, jz4780, stm32 to mention a few) already have
> the clock value stored in their private data, so maybe it is better to
> have this value in a uniform place in the i2c_adapter struct and make
> those drivers use it instead?
Perhaps say it clearly in the commit message? It seems that commit message
currently focused on I²C mux.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 11:06 [PATCH v5 0/5] I2C Mux per channel bus speed Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 1/5] i2c: core: add callback to change bus frequency Marcus Folkesson
@ 2026-02-13 11:06 ` Marcus Folkesson
2026-02-13 11:21 ` Andy Shevchenko
2026-02-13 11:48 ` Peter Rosin
2026-02-13 11:06 ` [PATCH v5 3/5] i2c: davinci: calculate bus freq from Hz instead of kHz Marcus Folkesson
` (2 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 11:06 UTC (permalink / raw)
To: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel, Marcus Folkesson
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..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.
+ */
+ 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;
+ }
+ }
+
+ 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;
+
+ 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 adapter.
+ */
+ 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);
+}
+
static int __i2c_mux_master_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
{
@@ -46,11 +140,11 @@ static int __i2c_mux_master_xfer(struct i2c_adapter *adap,
/* 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);
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);
return ret;
}
@@ -65,11 +159,11 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
/* 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);
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);
return ret;
}
@@ -86,12 +180,12 @@ static int __i2c_mux_smbus_xfer(struct i2c_adapter *adap,
/* 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);
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);
return ret;
}
@@ -108,12 +202,12 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
/* 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);
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);
return ret;
}
@@ -223,6 +317,7 @@ struct i2c_adapter *i2c_root_adapter(struct device *dev)
}
EXPORT_SYMBOL_GPL(i2c_root_adapter);
+
struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
struct device *dev, int max_adapters,
int sizeof_priv, u32 flags,
@@ -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.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 11:06 ` [PATCH v5 2/5] i2c: mux: add support for per channel " Marcus Folkesson
@ 2026-02-13 11:21 ` Andy Shevchenko
2026-02-14 12:30 ` Marcus Folkesson
2026-02-13 11:48 ` Peter Rosin
1 sibling, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2026-02-13 11:21 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
On Fri, Feb 13, 2026 at 12:06:51PM +0100, 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.
...
> +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.
> + */
> + 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) {
Would it (ever) have any positive returned values?
Ditto for other similar cases.
> + 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;
> + }
> + }
> +
> + return muxc->select(muxc, priv->chan_id);
> +}
...
> @@ -223,6 +317,7 @@ struct i2c_adapter *i2c_root_adapter(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(i2c_root_adapter);
>
> +
> struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
Stray and unneeded change.
> struct device *dev, int max_adapters,
> int sizeof_priv, u32 flags,
...
> + of_property_read_u32(child, "clock-frequency", &priv->adap.clock_hz);
Why OF-centric APIs? Muxes may and do appear on other systems as well.
Okay, this function seems fully OF-centric :-(
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 11:21 ` Andy Shevchenko
@ 2026-02-14 12:30 ` Marcus Folkesson
2026-02-14 18:32 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-14 12:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 2429 bytes --]
Hi Andy!
Thank you for your comments, good and observant as usual :-)
On Fri, Feb 13, 2026 at 12:21:42PM +0100, Andy Shevchenko wrote:
[...]
> > +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.
> > + */
> > + 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) {
>
> Would it (ever) have any positive returned values?
> Ditto for other similar cases.
Nope, I will change to 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 ret;
> > + }
> > + }
> > +
> > + return muxc->select(muxc, priv->chan_id);
> > +}
>
> ...
>
> > @@ -223,6 +317,7 @@ struct i2c_adapter *i2c_root_adapter(struct device *dev)
> > }
> > EXPORT_SYMBOL_GPL(i2c_root_adapter);
> >
> > +
> > struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>
> Stray and unneeded change.
I will remove it.
>
> > struct device *dev, int max_adapters,
> > int sizeof_priv, u32 flags,
>
> ...
>
> > + of_property_read_u32(child, "clock-frequency", &priv->adap.clock_hz);
>
> Why OF-centric APIs? Muxes may and do appear on other systems as well.
> Okay, this function seems fully OF-centric :-(
:-/
>
> --
> With Best Regards,
> Andy Shevchenko
>
Best regards,
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-14 12:30 ` Marcus Folkesson
@ 2026-02-14 18:32 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-02-14 18:32 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski, linux-i2c, linux-kernel,
linux-arm-kernel
On Sat, Feb 14, 2026 at 01:30:29PM +0100, Marcus Folkesson wrote:
> On Fri, Feb 13, 2026 at 12:21:42PM +0100, Andy Shevchenko wrote:
[...]
> > > + of_property_read_u32(child, "clock-frequency", &priv->adap.clock_hz);
> >
> > Why OF-centric APIs? Muxes may and do appear on other systems as well.
> > Okay, this function seems fully OF-centric :-(
>
> :-/
Yeah, this is unfortunate, and probably in the future we can also have similar
stuff for all.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 11:06 ` [PATCH v5 2/5] i2c: mux: add support for per channel " Marcus Folkesson
2026-02-13 11:21 ` Andy Shevchenko
@ 2026-02-13 11:48 ` Peter Rosin
2026-02-13 16:05 ` Marcus Folkesson
1 sibling, 1 reply; 16+ messages in thread
From: Peter Rosin @ 2026-02-13 11:48 UTC (permalink / raw)
To: Marcus Folkesson, Wolfram Sang, Michael Hennerich,
Bartosz Golaszewski, Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel
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@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..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);
> +}
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 11:48 ` Peter Rosin
@ 2026-02-13 16:05 ` Marcus Folkesson
2026-02-13 16:35 ` Peter Rosin
0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 16:05 UTC (permalink / raw)
To: Peter Rosin
Cc: Wolfram Sang, Michael Hennerich, Bartosz Golaszewski, Andi Shyti,
Bartosz Golaszewski, linux-i2c, linux-kernel, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]
Hi Peter!
On Fri, Feb 13, 2026 at 12:48:13PM +0100, Peter Rosin wrote:
> Hi!
>
[...]
> > + 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.
Hrm, I think we are pointing at the same thing but naming it differently; M1 in this chain:
Root - P1 - M1 - M2 - P2 - D1
I'm thinking 'topmost' means the one closest to the root (ancestor most
far away), but I don't stick to that naming and I am happy to change it.
>
>
> > + 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...
Good point, I will change that.
>
> Cheers,
> Peter
Best regards,
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/5] i2c: mux: add support for per channel bus frequency
2026-02-13 16:05 ` Marcus Folkesson
@ 2026-02-13 16:35 ` Peter Rosin
0 siblings, 0 replies; 16+ messages in thread
From: Peter Rosin @ 2026-02-13 16:35 UTC (permalink / raw)
To: Marcus Folkesson
Cc: Wolfram Sang, Michael Hennerich, Bartosz Golaszewski, Andi Shyti,
Bartosz Golaszewski, linux-i2c, linux-kernel, linux-arm-kernel
Hi!
2026-02-13 at 17:05, Marcus Folkesson wrote:
> Hi Peter!
>
> On Fri, Feb 13, 2026 at 12:48:13PM +0100, Peter Rosin wrote:
>> Hi!
>>
>
> [...]
>
>>> + 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.
>
> Hrm, I think we are pointing at the same thing but naming it differently; M1 in this chain:
> Root - P1 - M1 - M2 - P2 - D1
>
> I'm thinking 'topmost' means the one closest to the root (ancestor most
> far away), but I don't stick to that naming and I am happy to change it.
That fact that we both point to M1 is incidental. If you instead have
Root - P1 - M0 - P0 - M1 - M2 - P2 - D1
you still want to "restart" locking from M1, not M0. I think.
Cheers,
Peter
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 3/5] i2c: davinci: calculate bus freq from Hz instead of kHz
2026-02-13 11:06 [PATCH v5 0/5] I2C Mux per channel bus speed Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 1/5] i2c: core: add callback to change bus frequency Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 2/5] i2c: mux: add support for per channel " Marcus Folkesson
@ 2026-02-13 11:06 ` Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 4/5] i2c: davinci: add support for setting bus frequency Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed Marcus Folkesson
4 siblings, 0 replies; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 11:06 UTC (permalink / raw)
To: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel, Marcus Folkesson,
Bartosz Golaszewski
The bus frequency is unnecessarily converted between Hz and kHz in
several places.
This is probably an old legacy from the old times (pre-devicetrees)
when the davinci_i2c_platform_data took the bus_freq in kHz.
Stick to Hz.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/i2c/busses/i2c-davinci.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index a773ba082321..761de5a814df 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -117,8 +117,6 @@
/* timeout for pm runtime autosuspend */
#define DAVINCI_I2C_PM_TIMEOUT 1000 /* ms */
-#define DAVINCI_I2C_DEFAULT_BUS_FREQ 100
-
struct davinci_i2c_dev {
struct device *dev;
void __iomem *base;
@@ -134,8 +132,8 @@ struct davinci_i2c_dev {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
#endif
- /* standard bus frequency (kHz) */
- unsigned int bus_freq;
+ /* standard bus frequency */
+ unsigned int bus_freq_hz;
/* Chip has a ICPFUNC register */
bool has_pfunc;
};
@@ -209,16 +207,16 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
if (device_is_compatible(dev->dev, "ti,keystone-i2c"))
d = 6;
- clk = ((input_clock / (psc + 1)) / (dev->bus_freq * 1000));
+ clk = ((input_clock / (psc + 1)) / (dev->bus_freq_hz));
/* Avoid driving the bus too fast because of rounding errors above */
- if (input_clock / (psc + 1) / clk > dev->bus_freq * 1000)
+ if (input_clock / (psc + 1) / clk > dev->bus_freq_hz)
clk++;
/*
* According to I2C-BUS Spec 2.1, in FAST-MODE LOW period should be at
* least 1.3uS, which is not the case with 50% duty cycle. Driving HIGH
* to LOW ratio as 1 to 2 is more safe.
*/
- if (dev->bus_freq > 100)
+ if (dev->bus_freq_hz > 100000)
clkl = (clk << 1) / 3;
else
clkl = (clk >> 1);
@@ -269,7 +267,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
dev_dbg(dev->dev, "CLKH = %d\n",
davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
- dev_dbg(dev->dev, "bus_freq = %dkHz\n", dev->bus_freq);
+ dev_dbg(dev->dev, "bus_freq_hz = %dHz\n", dev->bus_freq_hz);
/* Take the I2C module out of reset: */
@@ -760,9 +758,9 @@ static int davinci_i2c_probe(struct platform_device *pdev)
r = device_property_read_u32(&pdev->dev, "clock-frequency", &prop);
if (r)
- prop = DAVINCI_I2C_DEFAULT_BUS_FREQ;
+ prop = I2C_MAX_STANDARD_MODE_FREQ;
- dev->bus_freq = prop / 1000;
+ dev->bus_freq_hz = prop;
dev->has_pfunc = device_property_present(&pdev->dev, "ti,has-pfunc");
--
2.52.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 4/5] i2c: davinci: add support for setting bus frequency
2026-02-13 11:06 [PATCH v5 0/5] I2C Mux per channel bus speed Marcus Folkesson
` (2 preceding siblings ...)
2026-02-13 11:06 ` [PATCH v5 3/5] i2c: davinci: calculate bus freq from Hz instead of kHz Marcus Folkesson
@ 2026-02-13 11:06 ` Marcus Folkesson
2026-02-13 11:06 ` [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed Marcus Folkesson
4 siblings, 0 replies; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 11:06 UTC (permalink / raw)
To: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel, Marcus Folkesson,
Bartosz Golaszewski
Populate adapter with clock_hz and set_clk_freq to enable support for
dynamic bus frequency.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/i2c/busses/i2c-davinci.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 761de5a814df..3daa823dcb9e 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -279,6 +279,27 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
return 0;
}
+static int davinci_i2c_set_clk(struct i2c_adapter *adap, u32 clock_hz)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ if (dev->bus_freq_hz == clock_hz)
+ return 0;
+
+ dev->bus_freq_hz = clock_hz;
+
+ /* put I2C into reset */
+ davinci_i2c_reset_ctrl(dev, 0);
+
+ /* compute clock dividers */
+ i2c_davinci_calc_clk_dividers(dev);
+
+ /* Take the I2C module out of reset: */
+ davinci_i2c_reset_ctrl(dev, 1);
+
+ return 0;
+}
+
/*
* This routine does i2c bus recovery by using i2c_generic_scl_recovery
* which is provided by I2C Bus recovery infrastructure.
@@ -809,6 +830,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
adap->dev.parent = &pdev->dev;
adap->timeout = DAVINCI_I2C_TIMEOUT;
adap->dev.of_node = dev_of_node(&pdev->dev);
+ adap->clock_hz = dev->bus_freq_hz;
+ adap->set_clk_freq = davinci_i2c_set_clk;
if (dev->has_pfunc)
adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
--
2.52.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed
2026-02-13 11:06 [PATCH v5 0/5] I2C Mux per channel bus speed Marcus Folkesson
` (3 preceding siblings ...)
2026-02-13 11:06 ` [PATCH v5 4/5] i2c: davinci: add support for setting bus frequency Marcus Folkesson
@ 2026-02-13 11:06 ` Marcus Folkesson
2026-02-13 16:07 ` kernel test robot
4 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2026-02-13 11:06 UTC (permalink / raw)
To: Wolfram Sang, Peter Rosin, Michael Hennerich, Bartosz Golaszewski,
Andi Shyti, Bartosz Golaszewski
Cc: linux-i2c, linux-kernel, linux-arm-kernel, Marcus Folkesson
Describe what needs to be consideraed and taken into account
when using different bus speeds for different mux channels.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
Documentation/i2c/i2c-topology.rst | 176 +++++++++++++++++++++++++++++++++++++
1 file changed, 176 insertions(+)
diff --git a/Documentation/i2c/i2c-topology.rst b/Documentation/i2c/i2c-topology.rst
index 48fce0f7491b..2c4a1364ce82 100644
--- a/Documentation/i2c/i2c-topology.rst
+++ b/Documentation/i2c/i2c-topology.rst
@@ -367,6 +367,182 @@ When D1 or D2 are accessed, accesses to D3 and D4 are locked out while
accesses to D5 may interleave. When D3 or D4 are accessed, accesses to
all other devices are locked out.
+Bus Speed and I2C Multiplexers
+================================
+
+I2C bus multiplexers allows multiple downstream channels to be exposed
+as separate I2C adapters which also could set their own bus speed.
+
+The multiplexer itself cannot change the bus speed as it use the upstream
+clock and data lines to communicate with the downstream devices. The speed
+is therfor changed in the root adapter resulting in that the whole bus is
+affected.
+
+This increases the complexity of the topology and some considerations must
+be taken into.
+
+Bus speed
+----------
+
+Downstream channels of an I2C multiplexer can only operate at the same or
+lower bus speed as the upstream bus. This is because the upstream bus may
+have devices that cannot operate at higher speeds and those will be affected
+by the speed change.
+
+The example below illustrates the problem.
+The root adapter is operating at 100kHz. D2 can only operate with 100kHz,
+but D2 can operate at 400kHz. When D1 is selected, the bus speed of the
+root adapter would have to be is set to 400kHz, a speed that D2 may not support.
+
+This topology is therefor not allowed: ::
+
+ .----------. 400kHz .--------.
+ .--------. 100kHz | mux- |--------| dev D1 |
+ | root |--+-----| locked | '--------'
+ '--------' | | mux M1 |
+ | '----------'
+ | .--------.
+ '--| dev D2 |
+ '--------'
+
+
+This topology is allowed: ::
+
+ .----------. 100kHz .--------.
+ .--------. 400kHz | mux- |--------| dev D2 |
+ | root |--+-----| locked | '--------'
+ '--------' | mux M1 |--. 400kHz .--------.
+ '----------' '--------| dev D1 |
+ '--------'
+
+Preferred topology
+-------------------
+
+The preferred topology when using different bus speeds is to have the multiplexer
+connected directly to the root adapter without any devices as siblings.
+By this arrangement, the bus speed can be changed without affecting any other devices
+and many of the caveats are avoided.
+
+Other multiplexers in parallell is still okay as those are locked out during transfers.
+
+This is the preferred topology: ::
+
+ .----------. 100kHz .--------.
+ .--------. 400kHz | mux- |--------| dev D2 |
+ | root |--+-----| locked | '--------'
+ '--------' | mux M1 |--. 400kHz .--------.
+ '----------' '--------| dev D1 |
+ '--------'
+Locking
+--------
+
+If the multiplexer is mux-locked, transfers to D3 may interleave between the
+select-transfer-deselect to D1 or D2.
+This results in a situation where the bus speed to D3 may be lower than it
+is supposed to be. This is usually not a problem.
+
+This topology is allowed but some transfers to D3 may be at 100kHz: ::
+
+ .----------. 100kHz .--------.
+ .--------. 400kHz | mux- |--------| dev D1 |
+ | root |--+-----| locked | '--------'
+ '--------' | | mux M1 |--. 400kHz .--------.
+ | '----------' '--------| dev D2 |
+ | .--------. '--------'
+ '--| dev D3 |
+ '--------'
+
+Multiple muxes in series
+--------------------------
+
+When multiple muxes are used in series the same rules applies.
+
+Transfers to D3 may interleave between select-transfer-deselect to D1, which
+results that the bus speed to D2 or D3 will be at 100KHz.
+
+Transfers to D2 may interleave between select-transfer-deselect to D1, which
+results in that the bus speed to D1 may be at 400kHz as the transfer to D2
+will set the bus speed to before the transfer to D1 starts.
+
+This is probably a bad topology ::
+
+ .----------. 400kHz .----------. 100kHz .--------.
+ .--------.400kHz | mux- |--------| mux- |--------| dev D1 |
+ | root |--+----| locked | 400kHz | locked | '--------'
+ '--------' | | mux M1 |--. | mux M2 |
+ | '----------' | '----------'
+ | .--------. | .--------.
+ '--| dev D3 | '--| dev D2 |
+ '--------' '--------'
+
+Multiple muxes in parallell
+----------------------------
+
+When multiple muxes are used in parallell all access to other muxes are locked out
+so this is not a problem.
+
+If the muxes are mux-locked, access to D3 may still interleave though.
+
+In the example below, D3 may not interleave between select-transfer-deselect for D1
+or D2 as both muxes are parent-locked: ::
+
+
+ .----------. 100kHz .--------.
+ | parent- |----------| dev D1 |
+ .--| locked | '--------'
+ | | mux M1 |
+ | '----------'
+ | .----------. 400KHz .--------.
+ .--------. 400kHz | parent- |---------| dev D2 |
+ | root |--+------| locked | '--------'
+ '--------' | | mux M2 |
+ | '----------'
+ | .--------.
+ '--| dev D3 |
+ '--------'
+
+Idle state
+-----------
+
+Muxes have an idle state, which is the state the channels is put into when no channel
+is active. The state is typically one of the following:
+
+- All channels are disconnected
+- The last selected channel is left as-is
+- A predefined channel is selected
+
+Muxes that support an idle state where all channels are disconnected are preferred when using
+different bus speeds. Otherwise high bus speeds may "leak" through to devices that
+may not support that higher speed.
+
+Consider the following example: ::
+
+ .----------. 100kHz .--------.
+ .--------. 400kHz | mux- |--------| dev D1 |
+ | root |--+-----| locked | '--------'
+ '--------' | | mux M1 |--. 400kHz .--------.
+ | '----------' '--------| dev D2 |
+ | .--------. '--------'
+ '--| dev D3 |
+ '--------'
+
+If the idle state of M1 is:
+- All channels disconnected: No problem, D1 and D2 are not affected by communication
+ to D3.
+- Last selected channel: Problem if D1 was the last selected channel. High speed
+ communication to D3 will be "leaked" to D1.
+- Predefined channel: Problem, if the predefined channel D1. Set predefined channel
+ to D2 as D2 may handle 400kHz.
+
+Supported controllers
+-----------------------
+
+Not all I2C controllers support setting the bus speed dynamically.
+At the time of writint, the following controllers has support:
+
+============================ =============================================
+i2c-davinci Supports dynamic bus speed
+============================ =============================================
Mux type of existing device drivers
===================================
--
2.52.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed
2026-02-13 11:06 ` [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed Marcus Folkesson
@ 2026-02-13 16:07 ` kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2026-02-13 16:07 UTC (permalink / raw)
To: Marcus Folkesson, Wolfram Sang, Peter Rosin, Michael Hennerich,
Bartosz Golaszewski, Andi Shyti
Cc: oe-kbuild-all, linux-i2c, linux-kernel, linux-arm-kernel,
Marcus Folkesson
Hi Marcus,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 1f97d9dcf53649c41c33227b345a36902cbb08ad]
url: https://github.com/intel-lab-lkp/linux/commits/Marcus-Folkesson/i2c-core-add-callback-to-change-bus-frequency/20260213-191801
base: 1f97d9dcf53649c41c33227b345a36902cbb08ad
patch link: https://lore.kernel.org/r/20260213-i2c-mux-v5-5-fb2cbf9979b3%40gmail.com
patch subject: [PATCH v5 5/5] docs: i2c: i2c-topology: add section about bus speed
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
docutils: docutils (Docutils 0.21.2, Python 3.13.5, on linux)
reproduce: (https://download.01.org/0day-ci/archive/20260213/202602131725.COjgPVue-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602131725.COjgPVue-lkp@intel.com/
All warnings (new ones prefixed by >>):
The parent of level 2 sections cannot be reached. The parser is at section level 2 but the current node has only 0 parent section(s).
One reason may be a high level section used in a directive that parses its content into a base node not attached to the document
(up to Docutils 0.21, these sections were silently dropped). [docutils]
>> Documentation/i2c/i2c-topology.rst:436: WARNING: Literal block ends without a blank line; unexpected unindent. [docutils]
Documentation/i2c/i2c-topology.rst:531: ERROR: Unexpected indentation. [docutils]
>> Documentation/i2c/i2c-topology.rst:532: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
vim +436 Documentation/i2c/i2c-topology.rst
429
430 .----------. 100kHz .--------.
431 .--------. 400kHz | mux- |--------| dev D2 |
432 | root |--+-----| locked | '--------'
433 '--------' | mux M1 |--. 400kHz .--------.
434 '----------' '--------| dev D1 |
435 '--------'
> 436 Locking
437 --------
438
439 If the multiplexer is mux-locked, transfers to D3 may interleave between the
440 select-transfer-deselect to D1 or D2.
441 This results in a situation where the bus speed to D3 may be lower than it
442 is supposed to be. This is usually not a problem.
443
444 This topology is allowed but some transfers to D3 may be at 100kHz: ::
445
446 .----------. 100kHz .--------.
447 .--------. 400kHz | mux- |--------| dev D1 |
448 | root |--+-----| locked | '--------'
449 '--------' | | mux M1 |--. 400kHz .--------.
450 | '----------' '--------| dev D2 |
451 | .--------. '--------'
452 '--| dev D3 |
453 '--------'
454
455 Multiple muxes in series
456 --------------------------
457
458 When multiple muxes are used in series the same rules applies.
459
460 Transfers to D3 may interleave between select-transfer-deselect to D1, which
461 results that the bus speed to D2 or D3 will be at 100KHz.
462
463 Transfers to D2 may interleave between select-transfer-deselect to D1, which
464 results in that the bus speed to D1 may be at 400kHz as the transfer to D2
465 will set the bus speed to before the transfer to D1 starts.
466
467 This is probably a bad topology ::
468
469 .----------. 400kHz .----------. 100kHz .--------.
470 .--------.400kHz | mux- |--------| mux- |--------| dev D1 |
471 | root |--+----| locked | 400kHz | locked | '--------'
472 '--------' | | mux M1 |--. | mux M2 |
473 | '----------' | '----------'
474 | .--------. | .--------.
475 '--| dev D3 | '--| dev D2 |
476 '--------' '--------'
477
478 Multiple muxes in parallell
479 ----------------------------
480
481 When multiple muxes are used in parallell all access to other muxes are locked out
482 so this is not a problem.
483
484 If the muxes are mux-locked, access to D3 may still interleave though.
485
486 In the example below, D3 may not interleave between select-transfer-deselect for D1
487 or D2 as both muxes are parent-locked: ::
488
489
490 .----------. 100kHz .--------.
491 | parent- |----------| dev D1 |
492 .--| locked | '--------'
493 | | mux M1 |
494 | '----------'
495 | .----------. 400KHz .--------.
496 .--------. 400kHz | parent- |---------| dev D2 |
497 | root |--+------| locked | '--------'
498 '--------' | | mux M2 |
499 | '----------'
500 | .--------.
501 '--| dev D3 |
502 '--------'
503
504 Idle state
505 -----------
506
507 Muxes have an idle state, which is the state the channels is put into when no channel
508 is active. The state is typically one of the following:
509
510 - All channels are disconnected
511 - The last selected channel is left as-is
512 - A predefined channel is selected
513
514 Muxes that support an idle state where all channels are disconnected are preferred when using
515 different bus speeds. Otherwise high bus speeds may "leak" through to devices that
516 may not support that higher speed.
517
518 Consider the following example: ::
519
520 .----------. 100kHz .--------.
521 .--------. 400kHz | mux- |--------| dev D1 |
522 | root |--+-----| locked | '--------'
523 '--------' | | mux M1 |--. 400kHz .--------.
524 | '----------' '--------| dev D2 |
525 | .--------. '--------'
526 '--| dev D3 |
527 '--------'
528
529 If the idle state of M1 is:
530 - All channels disconnected: No problem, D1 and D2 are not affected by communication
531 to D3.
> 532 - Last selected channel: Problem if D1 was the last selected channel. High speed
533 communication to D3 will be "leaked" to D1.
534 - Predefined channel: Problem, if the predefined channel D1. Set predefined channel
535 to D2 as D2 may handle 400kHz.
536
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread