linux-kernel - Re: [PATCH RFC 0/7] I2C Mux per channel bus speed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aNME9gWzazXTWtzw@gmail.com>
Date: Tue, 23 Sep 2025 22:37:10 +0200
From: Marcus Folkesson <marcus.folkesson@...il.com>
To: Peter Rosin <peda@...ntia.se>
Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Michael Hennerich <michael.hennerich@...log.com>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Andi Shyti <andi.shyti@...nel.org>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC 0/7] I2C Mux per channel bus speed


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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)