[U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework
Heiko Schocher
hs at denx.de
Wed Jul 31 07:01:22 CEST 2013
Hello Stephen,
Am 30.07.2013 21:22, schrieb 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:
Ok, great!
> ----------
> 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:
Hmm... do you have a debugger?
> ----------
> 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> I think this call comes from init_sequence_f[] -> init_func_i2c() ->
> i2c_init() -> i2c_init() == __i2c_init() -> i2c_init_bus() ->
No, we have now defined CONFIG_SYS_I2C and this calls
i2c_init_all() -> i2c_init_board()
and I think, on nvidia board i2c_init_board is defined. Yes, in the
i2c driver there it is ... calling process_nodes() ... so, the i2c
busses should be setup ...
> 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.
Feel free to post patches.
> 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()?
No, i2c_init_board gets called here very early:
init_func_i2c() -> i2c_init_all():
static int init_func_i2c(void)
{
puts("I2C: ");
#ifdef CONFIG_SYS_I2C
i2c_init_all();
#else
i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
#endif
puts("ready\n");
return (0);
}
and we have CONFIG_SYS_I2C defined (or, did you have it?)
and in drivers/i2c/i2c_core.c:
void i2c_init_all(void)
{
i2c_init_board();
i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM);
return;
}
Ah, I see, it is also called in board_init ...
> 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.
No, we need i2c before relocation for example to read SPD data
from eeprom, but this is on powerpc.
puts() is working, as your log shows.
I added in the comment from i2c_init_all:
/*
* i2c_init_all():
*
* not longer needed, will deleted. Actual init the SPD_BUS
* for compatibility.
* i2c_adap[] must be initialized beforehead with function pointers and
* data, including speed and slaveaddr.
*/
So the question raises, do we need this on arm?
bye,
Heiko
More information about the U-Boot
mailing list