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

[Stephen Warren <swarren@wwwdotorg.org>]

On 07/30/2013 01:22 PM, Stephen Warren wrote:
> 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@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.

Oh, with the options Tegra has enabled, perhaps the call sequence is:

board_init_f() (which uses init_sequence_f[]) -> init_func_i2c() ->
i2c_init_all(), which then calls:

* i2c_init_board(), which is supposed to parse DT
* i2c_set_bus_num(), which will call I2C_ADAP->init

However, according to the comments near the top of arch/arm/lib/crt0.S,
board_init_f() is called in an environment where variable data (.data,
.bss) is not available, hence i2c_init_board() cannot possibly operate
correctly since its whole purpose is to fill in variable data structures
from DT.

I think the only way to solve this is not to use DT to instantiate
devices, or to move the I2C initialization after relocation etc.,
although the latter won't work on boards that need I2C up in order to
initialize DRAM.

It seems like much of U-Boot's initialization architecture simply wasn't
designed to accommodate dynamically initializing devices from DT.