[PATCH RFC 0/7] I2C Mux per channel bus speed

Marcus Folkesson posted 7 patches 1 week, 2 days ago
Documentation/i2c/i2c-topology.rst  | 176 ++++++++++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-davinci.c    |  37 ++++++--
drivers/i2c/i2c-mux.c               | 127 +++++++++++++++++++++++---
drivers/i2c/muxes/i2c-mux-ltc4306.c |   3 +
include/linux/i2c-mux.h             |  21 +++++
include/linux/i2c.h                 |  13 +++
6 files changed, 357 insertions(+), 20 deletions(-)
[PATCH RFC 0/7] I2C Mux per channel bus speed
Posted by Marcus Folkesson 1 week, 2 days ago
This is an RFC on how to implement a feature to have different bus
speeds on different channels with an I2C multiplexer/switch.

The benefit with this approach is that you may group devices after
the fastest bus speed they can handle.
A real-world example is that you could have e.g. a display running @400kHz
and a smart battery running @100kHz using the same I2C controller.

There are many corner cases where this may cause a problem for some
hardware topologies. I've tried to describe those I could think of
in the documentation, see Patch #7.

E.g. one risk is that if the mux driver does not disconnect channels
when Idle, this may cause a higher frequency to "leak" through to
devices that are supposed to run at lower bus speed.
This is not only a "problem" for changing bus speed but could also be
an issue for potential address conflicts.

The implementation is split up into several patches:

Patch #1 Introduce a callback for the i2c controller to set bus speed
Patch #2 Introduce idle state to the mux core.
Patch #3 Introduce functionality to adjust bus speed depending on mux
         channel.
Patch #4 Set idle state for an example mux driver
Patch #5 Cleanup i2c-davinci driver a bit to prepare it for set_clk_freq
Parch #6 Implement set_clk_freq for the i2c-davinci driver
Parch #7 Update documentation with this feature

[1] https://elixir.bootlin.com/linux/v6.16.7/source/Documentation/i2c/i2c-topology.rst#L240
[2] https://elixir.bootlin.com/linux/v6.16.7/source/Documentation/i2c/i2c-topology.rst#L298
[3] https://elixir.bootlin.com/linux/v6.16.7/source/Documentation/i2c/i2c-topology.rst#L322

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
Marcus Folkesson (7):
      i2c: core: add callback to change bus frequency
      i2c: mux: add idle_state property to i2c_mux_core
      i2c: mux: add support for per channel bus frequency
      i2c: mux: ltc4306: set correct idle_state in i2c_mux_core
      i2c: davinci: calculate bus freq from Hz instead of kHz
      i2c: davinci: add support for setting bus frequency
      docs: i2c: i2c-topology: add section about bus speed

 Documentation/i2c/i2c-topology.rst  | 176 ++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-davinci.c    |  37 ++++++--
 drivers/i2c/i2c-mux.c               | 127 +++++++++++++++++++++++---
 drivers/i2c/muxes/i2c-mux-ltc4306.c |   3 +
 include/linux/i2c-mux.h             |  21 +++++
 include/linux/i2c.h                 |  13 +++
 6 files changed, 357 insertions(+), 20 deletions(-)
---
base-commit: 22f20375f5b71f30c0d6896583b93b6e4bba7279
change-id: 20250913-i2c-mux-b0063de2ae4d

Best regards,
-- 
Marcus Folkesson <marcus.folkesson@gmail.com>
Re: [PATCH RFC 0/7] I2C Mux per channel bus speed
Posted by Peter Rosin 1 week, 1 day ago
Hi!

2025-09-22 at 08:20, Marcus Folkesson wrote:
> This is an RFC on how to implement a feature to have different bus
> speeds on different channels with an I2C multiplexer/switch.
> 
> The benefit with this approach is that you may group devices after
> the fastest bus speed they can handle.
> A real-world example is that you could have e.g. a display running @400kHz
> and a smart battery running @100kHz using the same I2C controller.
> 
> There are many corner cases where this may cause a problem for some
> hardware topologies. I've tried to describe those I could think of
> in the documentation, see Patch #7.
> 
> E.g. one risk is that if the mux driver does not disconnect channels
> when Idle, this may cause a higher frequency to "leak" through to
> devices that are supposed to run at lower bus speed.
> This is not only a "problem" for changing bus speed but could also be
> an issue for potential address conflicts.
> 
> The implementation is split up into several patches:
> 
> Patch #1 Introduce a callback for the i2c controller to set bus speed
> Patch #2 Introduce idle state to the mux core.
> Patch #3 Introduce functionality to adjust bus speed depending on mux
>          channel.
> Patch #4 Set idle state for an example mux driver
> Patch #5 Cleanup i2c-davinci driver a bit to prepare it for set_clk_freq
> Parch #6 Implement set_clk_freq for the i2c-davinci driver
> Parch #7 Update documentation with this feature
It seems excessive to add idle_state to struct i2c_mux_core for the sole
purpose of providing a warning in case the idle state runs on lower speed.
Especially so since the default idle behavior is so dependent on the mux.

E.g. the idle state is completely opaque to the driver of the pinctrl mux.
It simply has no way of knowing what the idle pinctrl state actually means,
and can therefore not report back a valid idle state to the i2c-mux core.

The general purpose mux is also problematic. There is currently no API
for the gpmux to dig out the idle state from the mux subsystem. That
can be fixed, of course, but the mux susbsystem might also grow a way
to change the idle state at runtime. Or some other consumer of the "mux
control" used by the I2C gpmux might set it to a new state without the
I2C gpmux having a chance to prevent it (or even know about it).

You can have a gpio mux that only muxes SDA while SCL is always forwarded
to all children. That might not be healthy for devices not expecting
overly high frequencies on the SCL pin. It's probably safe, but who knows?

The above are examples that make the warning inexact.

I'd prefer to just kill this idle state hand-holding from the code and
rely on documentation of the rules instead. Whoever sets this up must
understand I2C anyway; there are plenty of foot guns, so avoiding this
particular one (in a half-baked way) is no big help, methinks.

This has the added benefit of not muddying the waters for the idle state
defines owned by the mux subsystem.

Cheers,
Peter
Re: [PATCH RFC 0/7] I2C Mux per channel bus speed
Posted by Marcus Folkesson 1 week, 1 day ago
Hi Peter,

Thanks for your input!

On Tue, Sep 23, 2025 at 05:10:16PM +0200, Peter Rosin wrote:
> Hi!
> 
> 2025-09-22 at 08:20, Marcus Folkesson wrote:
> > This is an RFC on how to implement a feature to have different bus
> > speeds on different channels with an I2C multiplexer/switch.
> > 

[...]

> > Patch #1 Introduce a callback for the i2c controller to set bus speed
> > Patch #2 Introduce idle state to the mux core.
> > Patch #3 Introduce functionality to adjust bus speed depending on mux
> >          channel.
> > Patch #4 Set idle state for an example mux driver
> > Patch #5 Cleanup i2c-davinci driver a bit to prepare it for set_clk_freq
> > Parch #6 Implement set_clk_freq for the i2c-davinci driver
> > Parch #7 Update documentation with this feature
> It seems excessive to add idle_state to struct i2c_mux_core for the sole
> purpose of providing a warning in case the idle state runs on lower speed.
> Especially so since the default idle behavior is so dependent on the mux.
> 
> E.g. the idle state is completely opaque to the driver of the pinctrl mux.
> It simply has no way of knowing what the idle pinctrl state actually means,
> and can therefore not report back a valid idle state to the i2c-mux core.
> 
> The general purpose mux is also problematic. There is currently no API
> for the gpmux to dig out the idle state from the mux subsystem. That
> can be fixed, of course, but the mux susbsystem might also grow a way
> to change the idle state at runtime. Or some other consumer of the "mux
> control" used by the I2C gpmux might set it to a new state without the
> I2C gpmux having a chance to prevent it (or even know about it).
> 
> You can have a gpio mux that only muxes SDA while SCL is always forwarded
> to all children. That might not be healthy for devices not expecting
> overly high frequencies on the SCL pin. It's probably safe, but who knows?
> 
> The above are examples that make the warning inexact.
> 
> I'd prefer to just kill this idle state hand-holding from the code and
> rely on documentation of the rules instead. Whoever sets this up must
> understand I2C anyway; there are plenty of foot guns, so avoiding this
> particular one (in a half-baked way) is no big help, methinks.
> 
> This has the added benefit of not muddying the waters for the idle state
> defines owned by the mux subsystem.

I pretty much buy everything you say here. 

I later saw that, as you pointed out, e.g. pca954x let you set the idle
state at runtime which would have increased the complexity a bit.

So, I think it is better to do as you suggest; remove idle_state and
keep the rules in the documentation.

> 
> Cheers,
> Peter

Best regards,
Marcus Folkesson