[i2c] mixed-speed I2C system

[Jean Delvare]

Hi Nishanth,

On Tue, 03 Apr 2007 20:44:33 -0500, Nishanth Menon wrote:
> Jean Delvare stated on 4/3/2007 3:27 AM:
> > Not really. For a given hardware setup, the max speed is a physical
> > attribute of the whole bus, basically defined as the minimum of the
> > maximum speed supported by each device. This holds whether or not a
>
> spec says:"13.5 Mixed speed modes on one serial bus system
> If a system has a combination of Hs-, Fast- and/or
> Standard-mode devices, it’s possible, by using an
> interconnection bridge, to have different bit rates between
> different devices (see Figs 24 and 25)."

Thanks for pointing this out, I didn't know such interconnection
bridges existed.

> Hmm... I don't agree in one specific scenario: HS and FS and STD speed
> devices divided by a bridge on the same I2C bus (ref: :i2cspec as given
> above). Defining it as a minimum of the max speed supported by all
> devices would not be optimal. In such cases, we'd need to swap speeds at
> xfer based, since we will have xfers for both devices which can arrive
> consecutively, we'd need to differentiate bus clock speeds at xfer
> level. if we all agree on this, it is just a matter of how..

Interconnection bridges, just as multiplexers, define bus segments, and
what matters as far as Standard mode vs. Fast mode I2C is concerned is
exactly that: bus segments. We don't yet fully support multiplexing,
but when we do, the idea will be to have one struct i2c_adapter for
every bus segment. This is what the motherboard-specific
i2c-amd756-s4882 driver does, for example.

> > device has been created in the device driver model. So attaching a
> > speed value to the devices will not cover all the possible cases. It
> > will fail as soon as there are additional devices connected on the bus
> > for which Linux doesn't have/need a driver, and in particular in the
> > multi-master case.
>
> Proposal I suggested is that we need information of speed capability in
> client and adaptor drivers. With this, i2c framework can intelligently
> manipulate speeds. the scenario mentioned of additional devices added on
> the bus once a configuration is compiled in is something i'd say invalid
> configuration-resolved atleast with a re-compile.

Right now, there are many cases were we do not declare all devices on
the I2C bus, in particular when we do not have (or do not need) a
driver for a given device. You can't suddenly state that this is
invalid.

Things might become a bit different now with the new model contributed
by David Brownell, which makes it easy to declare which I2C devices are
present. But it will take much time before all platforms and drivers
are converted to this new model. And anyway, there are a lot of systems
where we will _never_ declare all I2C devices. Think of PC
motherboards, for example.

So if we ever enable some form of automatic speed selection between
Standard mode and Fast mode, this will have to be enabled on a
voluntary basis, this can't be the default.

> > So I don't think that there is any way to enable fast-mode I2C
> > automatically. Instead I believe we need to default to standard I2C (as
> > we do now) and let the user explicitly switch to fast-mode I2C if
> > he/she knows that his/her bus setup will support it. I was thinking of
> > a sysfs attribute. I don't know if we also need an in-kernel interface.
>
> The main reason i am not infavour of this approach is due to
> co-existance of multispeed devices on the same bus. as i described
> previously.. i dont want to endup calling set_i2c_speed (xyz) before
> every call, and inviting all kind of race conditions where my speed gets
> re-set to an unsupported speed by other client drivers. by providing the
> speed as a flag (as in the patch i had posted), it can handle most of
> the scenarios.

I am totally fine with a flag to handle the HS mode, it should work
fine.

Calling set_i2c_speed() before every transaction is not going to
happen. HS mode is a per-device attribute, the master can switch to HS
mode automatically. Fast mode is a per-bus-segment attribute, you can't
change it.

> The trouble i have is with exteremely intelligent h/w folks who love to
> use the same controller on multiple SOCs (other than having multiple
> instances of the controller), with varied device speeds on each
> platform.. i have board specific files which can cater to this need..
> and calling speed a i2c resource I think is a matter of looking at it
> and saying it is a resource defining the bus characteristic.

No, speed isn't a resource, no matter how you look at it.

> scenarios:
> a) single speed memory mapped controllers
> something similar to this was done:
> http://linux.omap.com/pipermail/linux-omap-open-source/attachments/20070402/506956db/0001-Move-i2c-adaptor-speed-from-a-module-parameter-to-pl-0001.obj

You can't say "this was done", given that this is your own patch, and
it is only a proposal, your patch isn't upstream.

> b) range of devices, memory mapped controllers:
> start to end giving each speed.
> c) io mapped controllers
> use bus specific flags bits (0-7) to state that resource with IORESOURCE
> is speed..

This is totally ugly.

Just forget about it, please. Speed is an attribute, it's not a
resource. You can't abuse resources that way. The concept of a resource
is that it's something that cannot be used twice. You can't have two
devices requesting the same I/O region, for example. While you
obviously can have two I2C devices running at the same speed.

> I am not convinced that a two flag approach is good, esp if i would like
> to have a scenario like this:
> HS devices BRIDGE FS devices BRIDGE std devices
> each bridge filters out higher speed frequencies.. that should take care
> of a STD speed device not understanding FS clock rate..

But you cannot assume that all the hardware out there is connected that
way. In the general case we need to think in terms of segments.

> in this case, i think discrete speed levels should be good.

I think that we don't need these. The I2C specification says that
devices should stretch the clock if the master is faster than they
would like. So if, say, a slave can run at 300 kHz and the master runs
at 400 kHz, it should work just fine, the slave should stretch the
clock line to slow it down to 300 kHz. If it doesn't work, it means
that either the master device or the slave device does not fully
implement the I2C specification.

> > I'm curious. How does it work? Does this means that there is no need
> > for cooperation from i2c-core to implement I2C slave support in I2C bus
> > drivers? Or is i2c-pxa implementing things which should be moved to
> > i2c-core? This discussion probably belongs to a different thread
> > though, I doesn't seem to be related to bus speed issues at all.
> 
> with a google, i think i see
> http://marc.info/?l=linux-arm-kernel&m=116207345621115&w=2 as a patch to
> fix slave enable for pxa.
> basically should be here: drivers/i2c/busses/i2c-pxa.c

I know where the file is, thank you. My question was: how does it work?
If i2c-pxa already implements slave support without the help of
i2c-core, then what infrastructure are you asking for?

-- 
Jean Delvare