[U-Boot] [PATCH] overo: add SPL support

[Tom Rini]

On Mon, Dec 19, 2011 at 6:03 PM, Andreas Müller <schnitzeltony at gmx.de> wrote:
> On Thursday, December 15, 2011 10:12:59 PM Andreas Müller wrote:
>> On Thu, Dec 15, 2011 at 7:34 AM, Andreas Müller <schnitzeltony at gmx.de> wrote:
>> > I tried the following (as you can see I already commented out the
>> > i2c-read-
>>
>> write
>>
>> > for test):
>> >
>> > int get_board_revision(void)
>> > {
>> > #ifdef CONFIG_DRIVER_OMAP34XX_I2C
>> >
>> >        i2c_set_bus_num(TWL4030_I2C_BUS);
>> >        /*data = 0x01;
>> >        i2c_write(0x4B, 0x29, 1, &data, 1);
>> >        data = 0x0c;
>> >        i2c_write(0x4B, 0x2b, 1, &data, 1);
>> >        i2c_read(0x4B, 0x2a, 1, &data, 1);*/
>> >
>> > #endif
>> > ....
>> > }
>> >
>> > SPL Boot process hangs on i2c_set_bus_num ( tested by removing
>> >
>> > i2c_set_bus_num -> proper operation ) with console freeze:
>> > | U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
>> > | Texas Instruments Revision detection unimplemented
>> >
>> > The call stack for get_board_revision() is for SPL
>> >
>> > s_init()
>> > mem_init()
>> > do_sdrc_init(..)
>> > get_board_mem_timings(..)
>> > get_board_revision(..)
>> >
>> > It seems that the call to i2c_set_bus_num comes too early.
>>
>> Sorry for spamming but I face black magic:
>>
>> I added debug messages in omap24xx_i2c.c / i2c_set_bus_num().
>>
>> * Version 1:
>>
>> int i2c_set_bus_num(unsigned int bus)
>> {
>>       puts("i2c_set_bus_num called 1\n");
>>       if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
>>               printf("Bad bus: %d\n", bus);
>>               return -1;
>>       }
>>       printf("Bus: %d\n", bus);
>>
>> #if I2C_BUS_MAX == 3
>>       if (bus == 2) {
>>               puts("i2c_set_bus_num called 2\n");
>>               i2c_base = (struct i2c *)I2C_BASE3;
>>               puts("i2c_set_bus_num called 3\n");
>>       }
>>       else
>> #endif
>>       if (bus == 1) {
>>               puts("i2c_set_bus_num called 4\n");
>>               i2c_base = (struct i2c *)I2C_BASE2;
>>               puts("i2c_set_bus_num called 5\n");
>>       }
>>       else {
>>               puts("i2c_set_bus_num called 6\n");
>>               i2c_base = (struct i2c *)I2C_BASE1;
>>               puts("i2c_set_bus_num called 7\n");
>>       }
>>
>>       puts("i2c_set_bus_num called 8\n");
>> -->   current_bus = bus;
>>
>>       puts("i2c_set_bus_num called 9\n");
>>       if (!bus_initialized[current_bus])
>>               i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>>
>>       return 0;
>> }
>>
>> leads to
>>
>> i2c_set_bus_num called 1
>> Bus: 0
>> i2c_set_bus_num called 6
>> i2c_set_bus_num called 7
>> i2c_set_bus_num called 8
>>
>>
>> * Version 2:
>>
>> int i2c_set_bus_num(unsigned int bus)
>> {
>>       puts("i2c_set_bus_num called 1\n");
>>       if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
>>               printf("Bad bus: %d\n", bus);
>>               return -1;
>>       }
>>       printf("Bus: %d\n", bus);
>> -->   printf("CurrentBus: %d\n", current_bus);
>>       printf("AdrCurrentBus: %X\n", &current_bus);
>>
>> #if I2C_BUS_MAX == 3
>>       if (bus == 2) {
>>               puts("i2c_set_bus_num called 2\n");
>>               i2c_base = (struct i2c *)I2C_BASE3;
>>               puts("i2c_set_bus_num called 3\n");
>>       }
>>       else
>> #endif
>>       if (bus == 1) {
>>               puts("i2c_set_bus_num called 4\n");
>>               i2c_base = (struct i2c *)I2C_BASE2;
>>               puts("i2c_set_bus_num called 5\n");
>>       }
>>       else {
>>               puts("i2c_set_bus_num called 6\n");
>>               i2c_base = (struct i2c *)I2C_BASE1;
>>               puts("i2c_set_bus_num called 7\n");
>>       }
>>
>>       puts("i2c_set_bus_num called 8\n");
>>       current_bus = bus;
>>
>>       puts("i2c_set_bus_num called 9\n");
>>       if (!bus_initialized[current_bus])
>>               i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>>
>>       return 0;
>> }
>>
>> leads to
>>
>> i2c_set_bus_num called 1
>> Bus: 0
>>
>>
>> It seems that accessing 'current_bus' causes trouble. Has anybody an
>> explanation for that? I don't think this is related to i2c or overo. For a
>> better picture I attached memory mappings.
>>
> 'objdump -dSt' shows (the memory mappings I attached were not really helpful -
> sorry next time I know):
>
>    4020ae14 l     O .data      00000004 i2c_base
>    80000068 l     O .bss       00000004 current_bus
>    8000006c l     O .bss       0000000c bus_initialized
>
> 'i2c_base' is correctly located in SRAM but 'current_bus' and 'bus_initialized'
> are located in CS0 SDRAM which is at the time of call not yet initalized. This
> fits to the crash behaviour: Accessing 'i2c_base' does not cause trouble.
> How can I move 'current_bus' and 'bus_initialized' to SRAM?

Ah-ha!  Good work.  If you initialize them to a non-zero value,
statically (and make sure the code doesn't assume they're 0 by
default), this will change.

-- 
Tom