overo: add SPL support

PATCH New Andreas Müller 6 participants 17 messages Dec 15, 2011 – Dec 21, 2011

lore

Share & tools

lore

Andreas Müller

author

2011-12-15 14:34 UTC #0 lore raw

PATCH 1/1 overo: add SPL support

Quoted 16 lines · Steve Sakoman

I think you will also need to update the get_board_revision function to ensure that SPL works with very early Overo revisions.

Note this excerpt from the X-loader get_board_revision funtion:

	/* board revisions <= R2410 connect 4030 irq_1 to gpio112             */
	/* these boards should return a revision number of 0                  */
	/* the code below forces a 4030 RTC irq to ensure that gpio112 is low */
#ifdef CONFIG_DRIVER_OMAP34XX_I2C
	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
	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);

Quoted 4 lines

#endif
Yup, ugly, but this is the only way to detect revision properly on those boards.

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 -

Quoted 1 line

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. Before I dive into x- loader analysis: Has anybody an idea what goes wrong?

Andreas

Tom Rini

2011-12-15 18:32 UTC #1 lore raw

PATCH 1/1 overo: add SPL support

Quoted 45 lines · Andreas M?ller

Quoted 18 lines · Steve Sakoman
I think you will also need to update the get_board_revision function to ensure that SPL works with very early Overo revisions.
Note this excerpt from the X-loader get_board_revision funtion:
? ? ? /* board revisions <= R2410 connect 4030 irq_1 to gpio112 ? ? ? ? ? ? */ ? ? ? /* these boards should return a revision number of 0 ? ? ? ? ? ? ? ? ?*/ ? ? ? /* the code below forces a 4030 RTC irq to ensure that gpio112 is low */ #ifdef CONFIG_DRIVER_OMAP34XX_I2C
? ? ? i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
? ? ? 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
Quoted 2 lines
Yup, ugly, but this is the only way to detect revision properly on those boards.
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 -
Quoted 1 line
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

Quoted 8 lines

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. Before I dive into x- loader analysis: Has anybody an idea what goes wrong?

Without digging too hard into overo stuff, we've only called i2c_init() on the main bus and the x-load snippet is on the slave bus.

Signature · 1 line

Tom

Andreas Müller

author

2011-12-15 21:12 UTC #2 lore raw

PATCH 1/1 overo: add SPL support

Quoted 1 line · Andreas M?ller

I tried the following (as you can see I already commented out the i2c-read-

write

Quoted 23 lines

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

Quoted 7 lines

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.

Since I do not have JTAG for debugging, I think x-loader is also nice :(

Regards

Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot-spl.map.zip
Type: application/zip
Size: 10148 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111215/2581ad4e/attachment.zip>

Andreas Müller

author

2011-12-20 01:03 UTC #3 lore raw

PATCH 1/1 overo: add SPL support

Quoted 136 lines · Andreas M?ller

Quoted 2 lines · Andreas M?ller
I tried the following (as you can see I already commented out the i2c-read-
write
Quoted 25 lines
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
Quoted 7 lines
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

Quoted 8 lines

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?

Andreas

Tom Rini

2011-12-20 01:08 UTC #4 lore raw

PATCH 1/1 overo: add SPL support

Quoted 148 lines · Andreas M?ller

Quoted 136 lines · Andreas M?ller
Quoted 2 lines · Andreas M?ller
I tried the following (as you can see I already commented out the i2c-read-
write
Quoted 25 lines
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
Quoted 7 lines
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
Quoted 8 lines
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):

Quoted 8 lines

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

Signature · 1 line

Tom

Andreas Müller

author

2011-12-20 01:15 UTC #5 lore raw

PATCH 1/1 overo: add SPL support

Quoted 13 lines · Tom Rini

Quoted 7 lines
'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
Quoted 5 lines
'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?

Quoted 3 lines

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.

LOL: I tried already to set them to 0!

Wolfgang Denk

2011-12-20 05:43 UTC #6 lore raw

PATCH 1/1 overo: add SPL support

In reply to Tom Rini

Dear Tom Rini,

Quoted 5 lines · In message <CA+M6bX=bBFBaVm4aD2hVJfLXovtGjmKO9V40E+NyWtMtoaOV6A@mail.gmail.com>…

Quoted 5 lines
'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?

I guess you really, really must use i2c before relocation? If possible, this should be avoided in the first place.

Quoted 3 lines

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.

No, this is justy hiding the problem, and the next one who believes the value doesn't fit and changes it back to 0 runs into the issue again. Rather use an explicit

	int foo __attribute__ ((section(".data"))) = 0;

so everybody sees the real intention.

Best regards,

Wolfgang Denk

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ein weiser Herrscher kann in einem gro?en Land  mehr  Gutes  bewirken
als  in  einem kleinen - ein dummer Herrscher aber auch viel mehr Un-
fug. Da weise Herrscher seltener sind als dumme, war ich schon  immer
gegen gro?e Reiche skeptisch.                   - Herbert Rosendorfer

Tom Rini

2011-12-20 05:45 UTC #7 lore raw

PATCH 1/1 overo: add SPL support

Quoted 11 lines · Wolfgang Denk

Dear Tom Rini,
Quoted 5 lines · In message <CA+M6bX=bBFBaVm4aD2hVJfLXovtGjmKO9V40E+NyWtMtoaOV6A@mail.gmail.com>…
Quoted 5 lines
'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?
I guess you really, really must use i2c before relocation? ?If possible, this should be avoided in the first place.

Yes, board rev detection to know how to configure SDRAM.

Quoted 11 lines

Quoted 3 lines
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.
No, this is justy hiding the problem, and the next one who believes the value doesn't fit and changes it back to 0 runs into the issue again. ?Rather use an explicit
? ? ? ?int foo __attribute__ ((section(".data"))) = 0;
so everybody sees the real intention.

And I'll hit up <linux/compiler.h> and see if there's a shortcut and put it on my TODO list to clean up the existing areas (there's a few, but commented at least) that have the same need, but set to non-zero values. Thanks!

Signature · 1 line

Tom

Wolfgang Denk

2011-12-20 11:41 UTC #8 lore raw

PATCH 1/1 overo: add SPL support

Dear Tom Rini,

Quoted 5 lines · In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-w@mail.gmail.com>…

Quoted 3 lines
I guess you really, really must use i2c before relocation? =A0If possible, this should be avoided in the first place.
Yes, board rev detection to know how to configure SDRAM.

I don't consider this a valid reason (reading the SPD EEPROM would be such a reason). In almost all other cases it should be suffucient to configure the maximum number of memory banks and the maximum size of the memory banks and then use get_ram_size() to determine the actual amount of memory and to correctly initialize the memory controller.

Note that I don't insist on any changes to existing code here. This is just a recommendation which you may (or may not) consider for any future ports / implementations.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How much net work could a network work, if a network could net work?

Andreas Müller

author

2011-12-20 11:53 UTC #9 lore raw

PATCH 1/1 overo: add SPL support

Quoted 3 lines · you

Dear Tom Rini,
In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-

Quoted 11 lines · [email protected]> you wrote:

Quoted 4 lines
Quoted 2 lines
I guess you really, really must use i2c before relocation? =A0If possible, this should be avoided in the first place.
Yes, board rev detection to know how to configure SDRAM.
I don't consider this a valid reason (reading the SPD EEPROM would be such a reason). In almost all other cases it should be suffucient to configure the maximum number of memory banks and the maximum size of the memory banks and then use get_ram_size() to determine the actual amount of memory and to correctly initialize the memory controller.

Quoted 5 lines

Note that I don't insist on any changes to existing code here. This is just a recommendation which you may (or may not) consider for any future ports / implementations.

Dear Wolfgang Denk,

I agree to your concerns but - as I understood Steve Sakoman - here the situation is slightly different:
At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately this pin is also used for binary revision detection. Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must be done *before* SDRAM (timing) is set up, because the type of SDRAM is revision dependent.

Hope this helps

Andreas

Wolfgang Denk

2011-12-20 12:06 UTC #10 lore raw

PATCH 1/1 overo: add SPL support

Dear Andreas,

Quoted 3 lines · In message <[email protected]> you wrote:

I agree to your concerns but - as I understood Steve Sakoman - here the situation is slightly different:

I think you misunderstand.

Quoted 5 lines

At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately this pin is also used for binary revision detection. Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must be done *before* SDRAM (timing) is set up, because the type of SDRAM is revision dependent.

My suggestion was to check if memory initialization can not rather be done _without_ reading (and without otherwise knowing) the board type or revision. Usually this is possible, and I always prefer such auto-adjusting solutions over hard-wired approaches that break down when any of the expected inout data is not correct or not available.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
We are Microsoft. Unix is irrelevant. Openness is futile.  Prepare to
be assimilated.

Andreas Müller

author

2011-12-20 12:39 UTC #11 lore raw

PATCH 1/1 overo: add SPL support

Quoted 7 lines · you

Dear Andreas,
Quoted 3 lines · In message <[email protected]> you wrote:
I agree to your concerns but - as I understood Steve Sakoman - here the
situation is slightly different:
I think you misunderstand.

Yes I think too :-)

Quoted 7 lines

Quoted 6 lines
At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately this pin is also used for binary revision detection. Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must be done *before* SDRAM (timing) is set up, because the type of SDRAM is revision dependent.

Quoted 6 lines

My suggestion was to check if memory initialization can not rather be done _without_ reading (and without otherwise knowing) the board type or revision. Usually this is possible, and I always prefer such auto-adjusting solutions over hard-wired approaches that break down when any of the expected inout data is not correct or not available.

Dear Wolfgang,

I don't know if I want to jump also into these changes now - especially since I am quite new here..
But for my intererst - since it seems more error tolerant: How is SDRAM timing set up without exactly knowing what type is connected? Is there a good example implementation in u-boot(-spl)?

Best regards

Andreas

Aneesh V

2011-12-20 16:36 UTC #12 lore raw

PATCH 1/1 overo: add SPL support

Quoted 22 lines · Andreas M?ller

Quoted 7 lines · you
Dear Andreas,
Quoted 3 lines · In message<[email protected]> you wrote:
I agree to your concerns but - as I understood Steve Sakoman - here the
situation is slightly different:
I think you misunderstand.
Yes I think too :-)
Quoted 7 lines
Quoted 6 lines
At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately this pin is also used for binary revision detection. Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must be done *before* SDRAM (timing) is set up, because the type of SDRAM is revision dependent.
Quoted 6 lines
My suggestion was to check if memory initialization can not rather be done _without_ reading (and without otherwise knowing) the board type or revision. Usually this is possible, and I always prefer such auto-adjusting solutions over hard-wired approaches that break down when any of the expected inout data is not correct or not available.

Quoted 7 lines

Dear Wolfgang,
I don't know if I want to jump also into these changes now - especially since I am quite new here..
But for my intererst - since it seems more error tolerant: How is SDRAM timing set up without exactly knowing what type is connected? Is there a good example implementation in u-boot(-spl)?

Here is an example for automatic configuration with LPDDR2 memories in OMAP4.

http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/omap-common/emif-common.c;h=62678ff5d30739ce6dc0a4fdb5ea398acc3d31ea;hb=HEAD

Look at the function emif_get_device_details().

The automatic settings make use of timing values from JEDEC spec that is guaranteed to be safe for all devices, but not necessarily optimal(but for all LPDDR2 devices I have seen so far their timings are not different from the JEDEC defaults). In addition, we need to know density etc. which can be read from the LPDDR2 device itself.

Something similar may be possible for LPDDR (I presume you are using LPDDR), but will be somewhat complex, to say the least. Please note that we have compiled out the above code from OMAP4 SPL due to space constraints!

If something like 'int foo __attribute__ ((section(".data"))) = 0;' is solving your problem that may be the easier solution at the moment.

br,
Aneesh

Wolfgang Denk

2011-12-20 21:17 UTC #13 lore raw

PATCH 1/1 overo: add SPL support

In reply to Andreas Müller

Dear Andreas,

Quoted 6 lines · In message <[email protected]> you wrote:

I don't know if I want to jump also into these changes now - especially since I am quite new here..
But for my intererst - since it seems more error tolerant: How is SDRAM timing set up without exactly knowing what type is connected? Is there a good example implementation in u-boot(-spl)?

In most cases thare is not an arbitrary range of RAM types, but only a pretty small selection; even more often they differ only in size, not in timings. It depends on the set of possible configurations and capabilities of the meory controller how to proceed then. Typically you will start with very conservative settings, and from the sizes found you will know the correct configuration.

As for examples: probably not much in SPL, but a number of PPC board use this. See for example "board/tqc/tqm8xx/tqm8xx.c" which probes several banks of memory in 8, 9, and 10 column mode and then adjusts timing, even taking into account that the CPU and bus clock (and thus the refresh timings) are variable on some of these boards (you can set the CPU clock through the "cpuclk" environment variable).

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"There is no statute of limitations on stupidity."
- Randomly produced by a computer program called Markov3.

Igor Grinberg

2011-12-20 12:20 UTC #14 lore raw

PATCH 1/1 overo: add SPL support

In reply to Andreas Müller

Hi Andreas,

Quoted 23 lines · Andreas M?ller

Quoted 3 lines · you
Dear Tom Rini,
In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-
Quoted 11 lines · w at mail.gmail.com> you wrote:
Quoted 4 lines
Quoted 2 lines
I guess you really, really must use i2c before relocation? =A0If possible, this should be avoided in the first place.
Yes, board rev detection to know how to configure SDRAM.
I don't consider this a valid reason (reading the SPD EEPROM would be such a reason). In almost all other cases it should be suffucient to configure the maximum number of memory banks and the maximum size of the memory banks and then use get_ram_size() to determine the actual amount of memory and to correctly initialize the memory controller.
Quoted 5 lines
Note that I don't insist on any changes to existing code here. This is just a recommendation which you may (or may not) consider for any future ports / implementations.
Dear Wolfgang Denk,

Quoted 7 lines

I agree to your concerns but - as I understood Steve Sakoman - here the situation is slightly different:
At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately this pin is also used for binary revision detection. Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must be done *before* SDRAM (timing) is set up, because the type of SDRAM is revision dependent.

What about forging some very not optimized default DRAM settings, that suit any assembled DRAM and then when you have I2C access, reconfigure it - is it possible?

Signature · 2 lines

Regards,
Igor.

Steve Sakoman

2011-12-20 13:55 UTC #15 lore raw

PATCH 1/1 overo: add SPL support

Quoted 4 lines · Igor Grinberg

What about forging some very not optimized default DRAM settings, that suit any assembled DRAM and then when you have I2C access, reconfigure it - is it possible?

The board ID is used to determine some fairly fundamental things like how the address bits are multiplexed, bank size, number of banks, and timing.

Perhaps it might be possible to determine some non-optimal settings that can work with the current set of POP memories used, and also a scheme to modify the above on the fly while executing from said ram, but then one would have to revisit this every time a new vendor/type of POP was used.

That seems a lot more complex than the current method.

I suppose we could just drop support for the old boards in u-boot. Those folks could continue to use the current x-load solution. Or perhaps someone will come up with a more clever idea!

Steve

Andreas Müller

author

2011-12-21 01:00 UTC #16 lore raw

PATCH 1/1 overo: add SPL support

Quoted 13 lines · you

Quoted 3 lines · Igor Grinberg
What about forging some very not optimized default DRAM settings, that suit any assembled DRAM and then when you have I2C access, reconfigure it - is it possible?
The board ID is used to determine some fairly fundamental things like how the address bits are multiplexed, bank size, number of banks, and timing.
Perhaps it might be possible to determine some non-optimal settings that can work with the current set of POP memories used, and also a scheme to modify the above on the fly while executing from said ram, but then one would have to revisit this every time a new vendor/type of POP was used.

Quoted 4 lines

That seems a lot more complex than the current method.
I suppose we could just drop support for the old boards in u-boot. Those folks could continue to use the current x-load solution.

I am afraid you are right. It seems that the current U-boot/OMAP/SPL environment it is not designed to have i2c in early state when SDRAM is set up. My experience:
* After moving the i2c variables to SDRAM, i2c_set_bus_num (and thereby i2c_init) work but i2c_write hangs when calling udelay because timer is not yet running.
* This can be worked around by calling timer_init() within s_init(). With this setup booting is possible but I get MMC timeout messages from u-boot although kernel is loaded properly.
* I don't know if this is the reason for timeout but I think successful booting is just by accident because the register pointer to global data is not yet set (i2c and timer use global data). I tried to setup global data earlier but up to now without success. Even if successful I think the number of friends for such a deep change is limited...
Not to be misunderstood: This is no criticism to anybody. The experince that a design does not fit for a (corner case) request I have had in several projects :)

Quoted 2 lines

Or perhaps someone will come up with a more clever idea!

My suggestion:

* Soon: Clean up the patch already sent as reviewed and leave i2c TWL4030 shut- up out. As fallback for old boards leave x-load in oe meta-gumstix (maybe just create a different machine configuration).
* Later: Think about a 'more probing' approach as suggested in this thread (or a combination e.g if revision 1 detected, check size of SDRAM and with the result deciding if this is a revision 1/0).

Anyway: The informations regarding SDRAM I copied from x-load. Since this is a bit fishing in the dark: Are there documents from gumstix available, describing which memories were used (with which revision :) or are these secret IP?

Regards

Andreas