Hi Albert,

Thanks for the response!

On 11/10/2015 5:44 PM, Albert ARIBAUD wrote:

Hello Vignesh,

On Tue, 10 Nov 2015 14:29:54 +0530, Vignesh R vigneshr@ti.com wrote:

...

Hi,

With commit 7ae8350f67eea("ti: armv7: Move SPL SDRAM init to the right place, drop unused CONFIG_SPL_STACK") QSPI XIP boot appears to be broken on AM437x SK EVM.

Following UART initialization code (as indicated by TODO) causes the XIP boot failure.

In arch/arm/cpu/armv7/am33xx/board.c: @@ -275,9 +275,9 @@ void s_init(void) #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT) /* TODO: This does not work, gd is not available yet */ gd->baudrate = CONFIG_BAUDRATE; serial_init(); gd->have_console = 1; #endif

I was able to boot successfully from QSPI by commenting out the above code. But, could you suggest me what needs to be done as part of TODO in order to get QSPI XIP boot working?

Can't answer specifically on am437x, but basically, the problem you have here may be that the code is running in a C runtime environment in which only the global data is writable. This global data is a struct global_data (see include/asm-generic/global_data.h) which is supposed to be pointed to by the variable GD.

Can you detail the failure you are encountering?

Typically, GD is set up from within function board_init_f_mem(), before calling board_init_f(), or from arch/arm/lib/crt0.S.

So all depends on whether s_init() is executed before or after board_init_f_mem().

If s_init() runs before board_init_f_mem(), then you must move it to run after board_init_f_mem(). :)

If s_init() runs after board_init_f_mem() and you still have the issue, then your problem would be that gd is badly initialized. Is your board built for Thumb with a recent compiler, by any chance? I any case, can you test the value of gd when reaching the gd->baudrate line above?

Yes, s_init() is being called before call to _main(in arch/arm/lib/crt0.S that sets up GD) but all these calls are from arm generic files and nothing specific to am437x: reset (arch/arm/cpu/armv7/start.S) -> cpu_init_crit(arch/arm/cpu/armv7/start.S) -> lowlevel_init(arch/arm/cpu/armv7/lowlevel_init.S) -> s_init(arch/arm/cpu/armv7/board/am33xx.c)

The failure appears to be in serial_init(), it tries to access gd->flags which is not allocated yet and reads wrong value.

I was wondering whether entire UART initialization code in s_init() in arch/arm/cpu/armv7/board/am33xx.c can be moved to the end of board_init_f() where GD is accessible.

Regards Vignesh

Hello Albert,

On Wed, 11 Nov 2015 08:15:31 +0100, Albert ARIBAUD albert.u.boot@aribaud.net wrote:

Hello Vignesh,

On Wed, 11 Nov 2015 11:42:55 +0530, R, Vignesh vigneshr@ti.com wrote:

...

Hi Albert,

Thanks for the response!

On 11/10/2015 5:44 PM, Albert ARIBAUD wrote:

...

Hello Vignesh,

On Tue, 10 Nov 2015 14:29:54 +0530, Vignesh R vigneshr@ti.com wrote:

...

Hi,

With commit 7ae8350f67eea("ti: armv7: Move SPL SDRAM init to the right place, drop unused CONFIG_SPL_STACK") QSPI XIP boot appears to be broken on AM437x SK EVM.

Following UART initialization code (as indicated by TODO) causes the XIP boot failure.

In arch/arm/cpu/armv7/am33xx/board.c: @@ -275,9 +275,9 @@ void s_init(void) #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT) /* TODO: This does not work, gd is not available yet */ gd->baudrate = CONFIG_BAUDRATE; serial_init(); gd->have_console = 1; #endif

I was able to boot successfully from QSPI by commenting out the above code. But, could you suggest me what needs to be done as part of TODO in order to get QSPI XIP boot working?

Can't answer specifically on am437x, but basically, the problem you have here may be that the code is running in a C runtime environment in which only the global data is writable. This global data is a struct global_data (see include/asm-generic/global_data.h) which is supposed to be pointed to by the variable GD.

Can you detail the failure you are encountering?

Typically, GD is set up from within function board_init_f_mem(), before calling board_init_f(), or from arch/arm/lib/crt0.S.

So all depends on whether s_init() is executed before or after board_init_f_mem().

If s_init() runs before board_init_f_mem(), then you must move it to run after board_init_f_mem(). :)

If s_init() runs after board_init_f_mem() and you still have the issue, then your problem would be that gd is badly initialized. Is your board built for Thumb with a recent compiler, by any chance? I any case, can you test the value of gd when reaching the gd->baudrate line above?

Yes, s_init() is being called before call to _main(in arch/arm/lib/crt0.S that sets up GD) but all these calls are from arm generic files and nothing specific to am437x: reset (arch/arm/cpu/armv7/start.S) -> cpu_init_crit(arch/arm/cpu/armv7/start.S) -> lowlevel_init(arch/arm/cpu/armv7/lowlevel_init.S) -> s_init(arch/arm/cpu/armv7/board/am33xx.c)

The failure appears to be in serial_init(), it tries to access gd->flags which is not allocated yet and reads wrong value.

I feel a great disturbance in the Force...

...

I was wondering whether entire UART initialization code in s_init() in arch/arm/cpu/armv7/board/am33xx.c can be moved to the end of board_init_f() where GD is accessible.

I suggest you watch the list for other issues where access to gd->xxxx is broken -- it seems like several targets are experiencing this kind of problem. See for instance the thread with theis subject:

Revert "arm: Switch 32-bit ARM to using generic global_data setup"

Alternatively, you could test the patch at

http://patchwork.ozlabs.org/patch/542558/

Let us know if this solves your issue. If it does, then it will confirm the issue is with arch_setup_gd not being able to set up your GD for some reason.

IMPORTANT: patch 542558 is *not* a final patch (there will be at least a v3), and may even never land in u-boot/master; but some equivalent will eventually do. Right now, patch 542558 is only a way to test if your issue is a GD one.

...

Regards Vignesh

Amicalement,

Hello Vignesh,

On Fri, 13 Nov 2015 11:22:19 +0530, Vignesh R vigneshr@ti.com wrote:

Hi,

On 11/11/2015 02:33 PM, Albert ARIBAUD wrote:

[...]

...

Alternatively, you could test the patch at

http://patchwork.ozlabs.org/patch/542558/

Let us know if this solves your issue. If it does, then it will confirm the issue is with arch_setup_gd not being able to set up your GD for some reason.

IMPORTANT: patch 542558 is *not* a final patch (there will be at least a v3), and may even never land in u-boot/master; but some equivalent will eventually do. Right now, patch 542558 is only a way to test if your issue is a GD one.

The above patch affects the code that executes after s_init(), therefore I don't think above patch matters (_main is run after s_init()).

See (1) below.

As you said in your first reply "If s_init() runs before board_init_f_mem(), then you must move it to run after board_init_f_mem().", this is the problem with my case. s_init() is running *before* setting up of global_data in arch/arm/lib/crt0.S (in _main), hence IMO, calling serial_init() in s_init() is wrong. This has to be moved to board_init_f() (in arch/arm/cpu/armv7/am33xx/board.c). The comment in arch/arm/cpu/armv7/lowlevel_init.S that calls s_init() also says *not* to access global_data and not to try to start console. Therefore, I intend to do something like this:

diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c index bd14326cf479..351fc37b0483 100644 --- a/arch/arm/cpu/armv7/am33xx/board.c +++ b/arch/arm/cpu/armv7/am33xx/board.c @@ -256,6 +256,11 @@ void board_init_f(ulong dummy) { board_early_init_f(); sdram_init(); +#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)

•   gd->baudrate = CONFIG_BAUDRATE;
  

•   serial_init();
  

•   gd->have_console = 1;
  

+#endif } #endif /* end CONFIG_SPL_BUILD */

@@ -273,12 +278,6 @@ void s_init(void) set_uart_mux_conf(); setup_clocks_for_console(); uart_soft_reset(); -#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)

•   /* TODO: This does not work, gd is not available yet */
  

•   gd->baudrate = CONFIG_BAUDRATE;
  

•   serial_init();
  

•   gd->have_console = 1;
  

-#endif #if defined(CONFIG_SPL_AM33XX_ENABLE_RTC32K_OSC) /* Enable RTC32K clock */ rtc32k_enable();

Do you see any issues with above change?

(1) So your s_init runs even before board_init_f_mem(), right?

Your working fix seems to imply that as long as s_init() is run after board_init_f_mem (and any time before board_init_f) it will work. If so, then another, fix, preferable to the above, would be that the call to s_init be moved between those to board_init_f_mem and board_init_f. Can you test that?

Amicalement,

Hello Vignesh,

On Mon, 16 Nov 2015 16:00:58 +0530, Vignesh R vigneshr@ti.com wrote:

On 11/13/2015 11:58 AM, Albert ARIBAUD wrote:

...

Hello Vignesh,

On Fri, 13 Nov 2015 11:22:19 +0530, Vignesh R vigneshr@ti.com wrote:

...

Hi,

On 11/11/2015 02:33 PM, Albert ARIBAUD wrote:

[...]

...

Alternatively, you could test the patch at

http://patchwork.ozlabs.org/patch/542558/

Let us know if this solves your issue. If it does, then it will confirm the issue is with arch_setup_gd not being able to set up your GD for some reason.

IMPORTANT: patch 542558 is *not* a final patch (there will be at least a v3), and may even never land in u-boot/master; but some equivalent will eventually do. Right now, patch 542558 is only a way to test if your issue is a GD one.

The above patch affects the code that executes after s_init(), therefore I don't think above patch matters (_main is run after s_init()).

See (1) below.

...

As you said in your first reply "If s_init() runs before board_init_f_mem(), then you must move it to run after board_init_f_mem().", this is the problem with my case. s_init() is running *before* setting up of global_data in arch/arm/lib/crt0.S (in _main), hence IMO, calling serial_init() in s_init() is wrong. This has to be moved to board_init_f() (in arch/arm/cpu/armv7/am33xx/board.c). The comment in arch/arm/cpu/armv7/lowlevel_init.S that calls s_init() also says *not* to access global_data and not to try to start console. Therefore, I intend to do something like this:

diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c index bd14326cf479..351fc37b0483 100644 --- a/arch/arm/cpu/armv7/am33xx/board.c +++ b/arch/arm/cpu/armv7/am33xx/board.c @@ -256,6 +256,11 @@ void board_init_f(ulong dummy) { board_early_init_f(); sdram_init(); +#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)

•   gd->baudrate = CONFIG_BAUDRATE;
  

•   serial_init();
  

•   gd->have_console = 1;
  

+#endif } #endif /* end CONFIG_SPL_BUILD */

@@ -273,12 +278,6 @@ void s_init(void) set_uart_mux_conf(); setup_clocks_for_console(); uart_soft_reset(); -#if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)

•   /* TODO: This does not work, gd is not available yet */
  

•   gd->baudrate = CONFIG_BAUDRATE;
  

•   serial_init();
  

•   gd->have_console = 1;
  

-#endif #if defined(CONFIG_SPL_AM33XX_ENABLE_RTC32K_OSC) /* Enable RTC32K clock */ rtc32k_enable();

Do you see any issues with above change?

(1) So your s_init runs even before board_init_f_mem(), right?

Your working fix seems to imply that as long as s_init() is run after board_init_f_mem (and any time before board_init_f) it will work. If so, then another, fix, preferable to the above, would be that the call to s_init be moved between those to board_init_f_mem and board_init_f. Can you test that?

Yes, gd area gets initialized to 0 in board_init_f_mem(). Initializing the console thereafter fixes the issue. There is nothing between call to board_init_f_mem and board_init_f. board_init_f gets called right after board_init_f_mem (in arch/arm/lib/crt0.S), therefore I thought of moved in serial_init call to board_init_f as above.

Can you please submit a patch? Bugfixes are welcome even when the merge window is closed.

Amicalement,