[U-Boot] [PATCH/RFC, 0/2] I2C rework -- what do you think?

[Heiko Schocher]

Hello ksi,

ksi at koi8.net wrote:
> Hi everyone,
>
> Here is the first draft of major I2C subsystem rework. This is not a patch
> that should be applied to the tree--it will break systems with I2C busses
> over multiplexers and probably something else--but rather a Request For
> Comments. I would like to here all the objections, suggestions etc. before I
> go on and undertake a big job to change all the boards and other stuff to
> the new code. It is a big job and I don't want to spend a lot of time doing
> something that nobody would accept.
>   

Yes, this would be a big job, and how do you want to test this for ~200
boards?

> My goal is to somehow untie I2C subsystem from platform-specific parts. This
>   

Isn t this so?

> is not an attempt for a "code cleanup" though some cleanup could be achieved
> in process. But this is not a goal, it is just a probable side effect.
>
> Primary goal is to allow multiple I2C busses on a board served by different
> I2C controllers (on-SoC, external, bit-banging.) As of now we have
> rudimentary support for multiple BUILT-IN adapters in some SoCs (e.g.
> fsl_i2c.c) but this support is platform specific and does NOT allow any
> additional (external etc.) adapters.
>   

Ah... okay, this lacks in the actual u-boot i2c layer. I see here to ways:

a) Rewriting the complete code. This is really a big change (maybe, I am not
    a I2C expert, the cleaner way, but this will need a long time and a
good test
    period ...)

b) It should be possible to extend the i2c hardware specific functions,
to allow
     more then one i2c platform adapter. Switching between those should
possible
     with the "i2c dev" command (Which I think is the reason for this
command)
     also "i2c bus" should be extend to choose, which "real" I2C Bus
     it uses. Something like this comes in my mind:
             i2c bus devid=[muxtype:muxaddr:muxchannel]
                 with  devid (an existing device id see "i2c dev" command)
                 if "devid=" is missing, old behaviour (backward compatible)


> This is not a problem for a vast majority of supported boards that usually
> have a single I2C bus but there are cases when boards have several busses
> served by different I2C adapters. I personally have the first prototype of
>   
[...]
> Here is a little manifest...
>
> First of all, there is a basic entity, I2C bus. It can be a simple I2C bus
> from the only I2C adapter on a SoC, several busses from several adapters,
> both on-SoC and external, and several busses growing off of several adapters
> via I2C multiplexers/switches (e.g. PCA9544 etc.) They are all the same for
> the rest of code accessed with single set of i2c_read()/i2c_write() and
> friends for all busses no matter what adapter they are on and if they are
> directly connected to an adapter or reached through a set of I2C mux chips.
>
> Any I2C operation is performed on the CURRENT bus. Current bus is set with
> i2c_set_bus_num(). In case of directly connected busses it merely changes a
> global variable that is used as an index into an array. If some busses are
> connected via I2C muxes, it disconnects the previously connected muxes if
> any and sets all chips along the path to the new bus so next access will go
> to that bus.
>
> All those paths are statically allocated at compile time according to
>   

Why must be this statically defined? Think of a Hardwaremanufacturer who
has several boardvariants. In your case, he must have for every board a
config file -> different binaries. In the case using "i2c bus" he has
one binary
which is able to work with all of his variants! And he can connect on a
running system on an actual not used I2C Bus, another mux for example,
and can configure this "new" bus per commandline without generating
a new binary, flashing, resetting the board!

Ah, now I understand why you have to change all config files for all
existing
boards which uses I2C. Think of my suggestion to integrate the "multi
real bus"
in the existing code, so you dont have to do this ... boards which have only
one Hardware I2C Bus automagically use this bus, because i2c_bus_cur = 0!

> board's config file. That means we can use I2C from flash before U-Boot is
> relocated to DRAM thus allowing us to use SPD for DRAM initialization. It is
>   

You can also use in actual code SPD EEprom for DRAM initialization. Guess
the following example:

SPD EEprom reached over 2 muxes:

define an Environment var like:
SPD_EEprom =pca9554a:70:5:pca9544a:71:4

now make bevor accessing the SPD EEprom:

i2c_mux_ident_muxstring_f (getenv ("SPD_EEprom");

and you can access the SPD EEProm! (And nice side effect, the way to your
SPD EEprom is not fix in the code, you can choose it easy per Env var!)

But, so my feeling, it would be a good idea, to connect a SPD EEprom not
over a mux, at least only on channel 0, so no mux commands are necessary
to read from the SPD EEprom after reset.

> also means we do NOT need that "i2c bus" or whatever command to setup a bus
> behind muxes for consecutive access--we just choose that bus number and
> that's it.
>   

That has nothing to do with the "i2c bus" command. This command
only adds new virtual buss to the "i2c dev" list (device number).
you can then select this bus with "i2c dev [device number]" or in
code with i2c_set_bus_num([device number]) . You see, this is also
just an integer, you have to choose!

> All busses are explicitely defined in boards' config files so we know
> exactly where all accesses are going.
>   

Actual Code too. Make a "i2c dev" and you get the actual bus number, if
it is a virtual
bus, you can use "i2c bus" to get a list of this virtual busses.
> I did test it on MPC8548CDS system and it works OK. There is a patch for
> MPC8548CDS.h in the patchset that changes it to the new I2C code and a
> speculative example of multiadapter multibus configuration with 3 adapters
> and 5 busses 3 of which are connected with I2C muxes, 2 from them are
> multihop.
>   

Nice, but why you ignored the existing interface for handling with muxes?
I think it is easier to extend the existing interface to support
multiple "real"
interfaces ...

> Please let me know guys what you think so I would be able to start with a
> big patch.


I think, the mux functionality is independent from the problem to have more
then one hardware I2C Bus, or?

So, maybe you could integrate your "multiple hardware I2C Bus"
concept (which looks good at a first look) in the existing code?
I vote for accesing the I2C Bus over a mux with the actual way:
- defining with the "i2c bus" command a new "virtual" i2c device
  and
- accesing this with the standard command "i2c dev"
because this is a more flexible way.

Wolfgang: Maybe we should have a "i2c" Tree for testing purposes,
if we step into this "multi hardware I2C busses" theme.

bye
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany