[U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework

[Stephen Warren]

On 07/29/2013 10:28 PM, Heiko Schocher wrote:
> Hello Stephen,
> 
> Am 29.07.2013 18:12, schrieb Stephen Warren:
>> On 05/04/2013 06:01 AM, Heiko Schocher wrote:
>>> From: Simon Glass<sjg at chromium.org>
>>>
>>> This enables CONFIG_SYS_I2C on Tegra, updating existing boards and
>>> the Tegra
>>> i2c driver to support this.
>>
>> Heiko, the latest U-Boot tree hangs during boot on Tegra, and "git
>> bisect" points at this patch. Olof reported the issue to me.
>> Can you take a look at the code and see what might be wrong? Thanks.
>>
>> I suspect some kind of initialization ordering issue, since the boot
>> messages are:
>>
>> -----
>> U-Boot SPL 2013.07-rc3-00038-g880540d (Jul 29 2013 - 10:04:37)
>> U-Boot 2013.07-rc3-00038-g880540d (Jul 29 2013 - 10:04:37)
>>
>> TEGRA30
>> Board: NVIDIA Beaver
>> I2C:   Caller requested bad clock: periph=-49, parent=2
>> -----
>>
>> ... and that "bad clock" message implies to me that the I2C driver is
>> initializing before it has parsed the correct clock ID out of device
>> tree.
> 
> Hmm... looking in the patch ... I can see nothing which changes
> some initializing order ...

Yes, there's some initialization order issue; before this patch, I see
the I2C controller initialization, followed by some usage of it:

----------
U-Boot SPL 2013.07-rc3-00036-gd84eb85-dirty (Jul 30 2013 - 13:04:47)
U-Boot 2013.07-rc3-00036-gd84eb85-dirty (Jul 30 2013 - 13:04:47)

TEGRA30
Board: NVIDIA Beaver
DRAM:  2 GiB
i2c: controller bus 0 at 7000d000, periph_id 47, speed 100000: ok
i2c: controller bus 1 at 7000c000, periph_id 12, speed 100000: ok
i2c: controller bus 2 at 7000c400, periph_id 54, speed 100000: ok
i2c: controller bus 3 at 7000c500, periph_id 67, speed 100000: ok
i2c: controller bus 4 at 7000c700, periph_id 103, speed 100000: ok
MMC:   i2c_write: chip=0x2d, addr=0x32, len=0x1
----------

However with this patch applied, something starts using the controller
immediately, without it having been "probed" from device-tree:

----------
U-Boot SPL 2013.07-rc3-00037-g1f2ba72-dirty (Jul 30 2013 - 13:08:28)
U-Boot 2013.07-rc3-00037-g1f2ba72-dirty (Jul 30 2013 - 13:08:28)

TEGRA30
Board: NVIDIA Beaver
I2C:   i2c_init(speed=100000, slaveaddr=0xfe)
----------

i2c_init touches HW, but since process_nodes() hasn't run, none of the
parameters like register address or clock ID are yet known.

I think this call comes from init_sequence_f[] -> init_func_i2c() ->
i2c_init() -> i2c_init() == __i2c_init() -> i2c_init_bus() ->
I2C_ADAP->init(), although I didn't validate that in the running code,
just by code inspection.

The issue here is that the I2C core and/or Tegra driver seems to be
statically registering the I2C device objects, even though they should
be dynamically registered from device tree.

Should Tegra move its call of i2c_init_board() out of board_init() to
board_init_f(), and/or override __i2c_init() to call i2c_init_board()?

I think when init_sequence_f[] is running, there may be no serial
console to report errors. If so, moving the I2C initialization to that
early point sounds like a really bad idea.