Re: [PATCH v9 5/5] docs: i2c: i2c-topology: add section about bus speed

[Marcus Folkesson]

Hi all!

On Sat, May 30, 2026 at 11:17:05AM +0200, Wolfram Sang wrote:
> 
> > What makes the topology bad is D2. Without that device, it should
> > be OK. So, the question is which is more important?
> > 1. protect ignorant system designers from creating a topology
> >    that includes problematic devices like D2
> > 2. allow multi-level mux-locked muxes with variable bus-speed at all
> 
> My take is: as a generic operating system, we should try hard to
> maintain a reliable system. If a device gets stalled and blocks the bus
> because of a too high frequency, this is really bad.
> 
> Now, if this is a flaw of the HW design, we can only do so much but not
> more. If the HW design is OK, we should not introduce risks. This is why
> I want to disable this feature for MUX_IDLE_AS_IS. It cannot work in
> this setup, but the bus itself is likely designed OK and would work at
> the lowest speed.
> 
> > Checking the rules at run-time feels complicated to me, as devices
> > may come and go. Also, naming people "ignorant" over a mistake such
> > as the above in 1 is perhaps not all that fair. But, it feels sad
> > to disallow things that in fact do work.
> 
> Like I said, as a generic OS, we should play safe. If someone wants to
> implement a more risky version, it can be built on top of what we have.
> This is agreeing to "I want this and I know I am on my own now".
> Obviously, it reduces maintenance burden a lot.
> 
> > > ... unlike here. We have MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT defined
> > > already. And I'd think we should only allow bus speed switching for
> > > MUX_IDLE_DISCONNECT to avoid out-of-spec scenarios. Opinions?
> > 
> > It is probably not unwise to disallow MUX_IDLE_AS_IS in this context,
> > at least until someone actually needs it. Which does not seem all that
> > likely?
> 
> Exactly.

I agree as well.

> 
> > However, it seems like it should be fairly OK to allow a predefined
> > idle channel, as long as that channel is not lowering the bus speed
> > compared to the parent. But maybe supporting that can also wait for
> > an actual user?
> 
> I agree as well. I hope that Marcus' use-case is MUX_IDLE_DISCONNECT.

My use-case is MUX_IDLE_DISCONNECT :-)

> 
> > > > +Supported controllers
> > > > +-----------------------
> > > > +
> > > > +Not all I2C controllers support setting the bus speed dynamically.
> > > > +At the time of writing, the following controllers have support:
> > > > +
> > > > +============================   =============================================
> > > > +i2c-davinci                    Supports dynamic bus speed
> > > > +============================   =============================================
> > > 
> > > This paragaph is easy to get outdated. We can document that only
> > > controller drivers with the callback function implemented will work.
> > > People then can find out if that applies for their driver...
> > 
> > There are other such hard-to-maintain lists elsewhere in the text.
> > Not saying that this makes neither those nor this list a good idea,
> > but it is an explanation for adding one more.
> 
> You mean the list if a driver uses parent-locked or mux-locked. What
> about putting that information into the driver? The core could enforce
> that it is set to a meaningful value and not left uninitialized?

I would prefer to have this information in the driver.
But I must also say that I found the list in the documentation useful
when I started to work on this.

But it is hard to maintain and not optimal indeed.


> 
> Happy hacking everyone,
> 
>    Wolfram
> 

Best regards,
Marcus Folkesson