Hello ksi,
ksi@koi8.net wrote:
On Wed, 28 Jan 2009, Heiko Schocher wrote:
Hello ksi, ksi@koi8.net schrieb:
On Tue, 27 Jan 2009, Heiko Schocher wrote:
ksi@koi8.net wrote:
[...]
OK,. But if I look in your patch you delete the "i2c bus" command, and this breaks at least 2 boards.
Yep. As I said it was not a real patch, just a request for comments. I will take care to make the real one work.
Ok.
As a matter of fact I can only see 2 boards that use that existing mux code, namely mgsuvd and mgcoge. That will be trivial to rework.
Yes, for those I made the "i2c bus" command. And it will follow soon 2 more boards ...
OK, I will look at those boards and make sure they work. Anyway it is just a work in progress; no actual patch submitted so far...
[...]
It is needed! If, for example an EEprom, is not always on the same virtual bus. Some boards have no mux, some 1, some 2 ... and if we have this virtual busses statically, we must have for all boards an extra u-boot binary. The only Hardware difference for this boards is, how things are connected to the I2C bus ... and it is not OK for this manufacturer, to have for every board a different u-boot binary!
So, why shouldnt it possible to add to your proposal a possibility to add virtual i2c busses dynamically? (using a list instead of a fix table for example)
It is definitely a possibility but it would break uniformity. One can not use lists while U-Boot is running out of ROM and DRAM is not active. That means we should use an additional mechanism for those busses added later on.
One solution is to make num_i2c_busses (or whatever) a variable initially set by config file define and then modified when new busses are added. It also means we should use pointers instead of just array of structures and bring in the whole bunch of list management functions like find_next, find_prev, find_first etc. It might make board developer's life slightly easier but would add complexity to U-Boot and increase its memory footprint.
OK, let us think a little about it.
I'm not sure yet that that little convenience is worth that.
OK, I will return to it after I have that template driven multibus soft I2C driver done.
Ok.
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 ...
It steers us into a bunch of board-specific hacks and quirks instead of making something uniform. We do not have a luxury of working DRAM when we starting up and that code is required to bring up DRAM. I'm trying to avoid separate code for startup and full blown operation.
Yes, I know, this is a problem.
I don't think it is needed at all. One has some number of busses on
the
board and all of them are active. Just switch to another bus and
you're
good to talk to that bus. If one needs something special, he can easily
achieve
it with writing to those muxes with existing read/write commands.
But it is in the code, because it is needed!
I will return to it when it comes to the real patch.
Ok, thanks.
[...]
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.
It is more flexible, I agree. But it multiplies entities. And doesn't add anything that couldn't be achieved with other existing commands.
Look, I2C bus behind muxes is just an already defined bus and a set of regular single-register I2C chips connected to that bus. In order to "add" that virtual bus one can simply switch to the appropriate existing bus and write a byte or two to those chips with existing i2c write commands.
That means that that "i2c bus" command in nothing more than a shortcut. It can be easily implemented even as U-Boot environmental variable that one can run...
Hmm.. so we need no i2c mux support?
CLI is easy, I'm now trying to make a template driven, self-generating
from
config file multi-bus software I2C driver...
or just try to make this
i2c_adap_t *i2c_adap[CONFIG_SYS_NUM_I2C_ADAPTERS] = CONFIG_SYS_I2C_ADAPTERS;
dynamically and extendable and I am happy ;-)
No, one can NOT use a dynamic set of adapters because this set determines which drivers got included into the final binary. This is COMPILE time define.
OK, let me make some real patch and then we'll return to our discussion :)
Ok.
Anyways thank you very much for a feedback, I really appreciate it. And I will try to make you happy :)
:-)
bye Heiko