Hello Simon,
Simon Glass wrote:
This series provides Heiko's upgraded I2C framework from a few years ago. I hope that we can bring this in and move boards over to it as time permits, rather than switching everything in one fell swoop which never happens.
Ok, lets try it!
To show it working I have enabled it for Tegra in a very rough way. It seems fine with my limited testing.
Great! Thanks! Patches for other i2c drivers can be found here:
http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus...
They just need a rebase and an update to your changes (and of course some tests)
In terms of changes, I have just fixed some checkpatch errors and fiddled with a couple of function signatures.
I will start a thread on the list with a few thoughts on this series at some point.
Ok, thanks. Here some thoughts comming in my mind:
- Why a "cur_adap" added in gd_t: - This points allways to the current used i2c adapter. - because gd_t is writeable when running in flash, complete multiadapter/multibus functionality is usable, when running in flash, which is needed for some SoCs. - using a pointer brings faster accesses to the i2c_adapter_t struct and saves some bytes here and there.
- init from a i2c controller: In current code all i2c controllers, as a precaution, getting initialized. In the new code, a i2c controller gets only initialized if it is used. This is done in i2c_set_bus_num().
Also, with this approach, we can easy add in a second step, a i2c_deinit() function, called from i2c_set_bus_num(), so we can easy deactivate a no longer used controller.
- added "hw_adapnr" in i2c_adapter_t struct: when for example a CPU has more then one i2c controller we can use this variable to differentiate which controller the actual i2c adapter uses.
- Maybe we should add a base_addr field in struct i2c_adapter? This would help for SoCs, who have more then one identical controller, just differ in their base_addr... (Currently I made a function, or an array which returns the base_addr, dependend on "hw_adapnr"). We should drop this, and introduce a "base_addr" field.
[...]
bye, Heiko