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(-)
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>
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
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
© 2016 - 2025 Red Hat, Inc.