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

[Simon Glass]

Hi Stephen,

On Tue, Jul 30, 2013 at 2:00 PM, Stephen Warren <swarren at wwwdotorg.org>wrote:

> 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 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.
>
> 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 suppose you could mark i2c_controllers so that it is in the data section
with __attribute__((section(".data"))). That's what eynos does, for
example. It is valid since SPL or BCT has set up the SDRAM.


>
> 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.
>

True, although remember that very little init happens before relocation.
Here, I2C has moved to pre-reloc. But the vast majority of it happens after
reloc, so the actual impact of this problem is small, and there is a
workaround (above).

Regards,
Simon