[U-Boot] QSPI XIP boot on am437x

[U-Boot] QSPI XIP boot on am437x

[Vignesh R]

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?


Thanks!

-- 
Regards
Vignesh

[U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

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?

> Thanks!

NP.

> Regards
> Vignesh

Amicalement,
-- 
Albert.

[U-Boot] QSPI XIP boot on am437x

[R, Vignesh]

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

[U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

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"

> Regards
> Vignesh



Amicalement,
-- 
Albert.

[U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

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

[U-Boot] QSPI XIP boot on am437x

[Vignesh R]

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

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?
-- 
Regards
Vignesh

[U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

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

[U-Boot] QSPI XIP boot on am437x

[Vignesh R]

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.

-- 
Regards
Vignesh

[U-Boot] QSPI XIP boot on am437x

[Albert ARIBAUD]

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

[U-Boot] QSPI XIP boot on am437x

[Vignesh R]

Hi Albert,

On 11/16/2015 05:16 PM, Albert ARIBAUD wrote:
> Hello Vignesh,
> 

[...]

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

Yeah, I will submit a patch shortly. Thanks!

-- 
Regards
Vignesh