Hello Stephen,
Am 01.08.2013 22:32, schrieb Stephen Warren:
On 08/01/2013 12:02 AM, Heiko Schocher wrote:
Am 01.08.2013 07:39, schrieb Stephen Warren:
On 07/31/2013 10:32 PM, Heiko Schocher wrote:
Am 31.07.2013 21:31, schrieb Stephen Warren:
On 07/30/2013 11:46 PM, Heiko Schocher wrote:
...
- Actually initializing or using the I2C HW. That could certainly be
part of the per-I2C-controller init() function you mention.
For that purpose is the i2c_init function.
And why we could not do step 1 when we do step 2? Why doing step 1 for hw we later do not use?
I suppose you could. It seems conceptually /far/ simpler to just scan the DT once up-front rather than having to defer all this stuff until
on the other hand we ring for every ms boot time ... and here we want to scan a complete dt with maybe a lot of nodes, we do not want to use?
you actually use an I2C bus. If you do that, how can you know how many I2C buses exist without trying to use every possible one and seeing
Do I really need to know that?
Well, with the recent patches, U-Boot prints out a list of valid I2C
which patches? I am trying current U-Boot on some boards, I do not see such a list ... you mean on tegra based boards?
adapters at boot. So, I suppose someone must have thought it a good idea!
Ok, but then, printing this "list" should done after relocation. And the me known "I2C: ready" print on U-Boot boot, comes from init_func_i2c() ... which is ancient code from powerpc times ...
Perhaps the most relevant reason: why on earth wouldn't a user want to run e.g. "i2c list" or "i2c dev" and get back a list of valid I2C adapters, as opposed to poking around in the dark to see which exist?
Didn't you need also a list of devices on each i2c bus, if you use this argument?
But for this reason we have in the new framework the "i2c bus" command, which shows all busses. This could be adapted in the DT case, to look, which buses really exist and print them. No need to do this at boottime!
Also, in an old version of the multibus/multiadapter approach I made a option to add dynamically new i2c buses for example from the environment or from the "i2c bus" command ... maybe it is worth to think about such an option in the DT case?
see here: http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=commit;h=ccb680c8cf9002232bc45...
Patches welcome ;-)
which fail? Also, the mapping from I2C bus number to register address is
Hmm.. there are max TEGRA_I2C_NUM_CONTROLLERS. If one gets used, it scans the dt ... if only 1 adapter is used, only one is activated through i2c_init ...
only created by actually scanning the whole DT; there's no need to every I2C DT node to have a /aliases entry that dictates its U-Boot device ID, so you really do have to scan everything completely up-front before you can determine which registers to use.
But the i2c bus number is coded static all over the u-boot code (Should be changed) in the code. Saying a board has an i2c eeprom on bus "2", it calls i2c_set_bus_number(2) ... this "2" must be somewhere in the dt or?
Yes, the "2" must come from DT, or DT is pointless. We simply have to
Ok, puuh...
get rid of all the hard-coding. If we can't do that, then I'd strongly prefer to revert all the DT mess, and put the list of HW modules back into the source code. DT isn't adding anything at all unless we can actually use it exclusively.
Given how long this discussion is going on, can we please just revert the commit so that the code works for everyone who's trying to use it, then fix the problem later?
Yes, but not reverting the hole commit, please just remove the i2c_init_board() call in i2c_init_all() and test it, and send a new patch, thanks!
diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c index d1072e8..5f10eb3 100644 --- a/drivers/i2c/i2c_core.c +++ b/drivers/i2c/i2c_core.c @@ -246,7 +246,6 @@ void i2c_init_board(void) */ void i2c_init_all(void) { - i2c_init_board(); i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM); return; }
Or we make the "init_func_i2c()" function weak, and as the first step it is a dummy function, and boards which really need it, currently fail and must code it in board code? I would prefer this way, as it is anyway the goal to get rid of this function if all boards are using the new framework ...)
Can you or Simon try this?
@Simon: Hmm... just looking in an older version of the new framework... i2c_init_board() was from the beginning in i2c_init_all()... why didn't this poped up when you tested the tegra patches? Are there some changes in the DT/tegra code in the meantime?
bye, Heiko