[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[Vikram Narayanan]

bootstage_mark_name calls timer_get_boot_us which inturn calls
the arch timer. The arch timer uses the gd pointer to save the
lastinc count. A call to bootstage_mark_name here results in a
data abort as gd is uninitialized.

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
---
Though it is true that there are currently no users for this
feature, it isn't good to see an abort when someone tries to
use BOOTSTAGE. I was bugged by this error when trying to 
use this feature on i.Mx6Q based board.

 arch/arm/lib/board.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index e0cb635..2f09ab9 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -277,13 +277,13 @@ void board_init_f(ulong bootflag)
 	void *new_fdt = NULL;
 	size_t fdt_size = 0;
 
-	bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
-
 	/* Pointer is writable since we allocated a register for it */
 	gd = (gd_t *) ((CONFIG_SYS_INIT_SP_ADDR) & ~0x07);
 	/* compiler optimization barrier needed for GCC >= 3.4 */
 	__asm__ __volatile__("": : :"memory");
 
+	bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
+
 	memset((void *)gd, 0, sizeof(gd_t));
 
 	gd->mon_len = _bss_end_ofs;
-- 
1.7.4.1

[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[Simon Glass]

Hi Vikram,

On Sun, Dec 23, 2012 at 3:27 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> bootstage_mark_name calls timer_get_boot_us which inturn calls
> the arch timer. The arch timer uses the gd pointer to save the
> lastinc count. A call to bootstage_mark_name here results in a
> data abort as gd is uninitialized.
>
> Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>

Acked-by: Simon Glass <sjg@chromium.org>

This bug will affect boards which don't have their own
timer_get_boot_us() function.

Albert, I think we should try to get this in as a bug fix in the
upcoming release.

Thanks for the fix Vikram.

> ---
> Though it is true that there are currently no users for this
> feature, it isn't good to see an abort when someone tries to
> use BOOTSTAGE. I was bugged by this error when trying to
> use this feature on i.Mx6Q based board.
>
>  arch/arm/lib/board.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index e0cb635..2f09ab9 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -277,13 +277,13 @@ void board_init_f(ulong bootflag)
>         void *new_fdt = NULL;
>         size_t fdt_size = 0;
>
> -       bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
> -
>         /* Pointer is writable since we allocated a register for it */
>         gd = (gd_t *) ((CONFIG_SYS_INIT_SP_ADDR) & ~0x07);
>         /* compiler optimization barrier needed for GCC >= 3.4 */
>         __asm__ __volatile__("": : :"memory");
>
> +       bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
> +
>         memset((void *)gd, 0, sizeof(gd_t));
>
>         gd->mon_len = _bss_end_ofs;
> --
> 1.7.4.1
>

Regards,
Simon

[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[Albert ARIBAUD]

Hi Simon,

On Wed, 26 Dec 2012 12:39:32 -0800, Simon Glass <sjg@chromium.org>
wrote:
> Hi Vikram,
> 
> On Sun, Dec 23, 2012 at 3:27 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> > bootstage_mark_name calls timer_get_boot_us which inturn calls
> > the arch timer. The arch timer uses the gd pointer to save the
> > lastinc count. A call to bootstage_mark_name here results in a
> > data abort as gd is uninitialized.
> >
> > Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> 
> Acked-by: Simon Glass <sjg@chromium.org>
> 
> This bug will affect boards which don't have their own
> timer_get_boot_us() function.
> 
> Albert, I think we should try to get this in as a bug fix in the
> upcoming release.
> 
> Thanks for the fix Vikram.

Thanks Vikram and Simon; I'll pull this in today.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[Albert ARIBAUD]

On Thu, 27 Dec 2012 10:41:06 +0100, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
> 
> On Wed, 26 Dec 2012 12:39:32 -0800, Simon Glass <sjg@chromium.org>
> wrote:
> > Hi Vikram,
> > 
> > On Sun, Dec 23, 2012 at 3:27 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> > > bootstage_mark_name calls timer_get_boot_us which inturn calls
> > > the arch timer. The arch timer uses the gd pointer to save the
> > > lastinc count. A call to bootstage_mark_name here results in a
> > > data abort as gd is uninitialized.
> > >
> > > Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Wolfgang Denk <wd@denx.de>
> > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > 
> > Acked-by: Simon Glass <sjg@chromium.org>
> > 
> > This bug will affect boards which don't have their own
> > timer_get_boot_us() function.
> > 
> > Albert, I think we should try to get this in as a bug fix in the
> > upcoming release.
> > 
> > Thanks for the fix Vikram.
> 
> Thanks Vikram and Simon; I'll pull this in today.

... actually, this will depend on whether my crt0 change gets in for
2013.01, as with this change, gd is already initialized when
entering board_init_f().

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] arm/bootstage: Move the bootstage call after gd init

[Vikram Narayanan]

On 12/27/2012 3:52 PM, Albert ARIBAUD wrote:
> On Thu, 27 Dec 2012 10:41:06 +0100, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Hi Simon,
>>
>> On Wed, 26 Dec 2012 12:39:32 -0800, Simon Glass <sjg@chromium.org>
>> wrote:
>>> Hi Vikram,
>>>
>>> On Sun, Dec 23, 2012 at 3:27 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
>>>> bootstage_mark_name calls timer_get_boot_us which inturn calls
>>>> the arch timer. The arch timer uses the gd pointer to save the
>>>> lastinc count. A call to bootstage_mark_name here results in a
>>>> data abort as gd is uninitialized.
>>>>
>>>> Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> Cc: Wolfgang Denk <wd@denx.de>
>>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>>>
>>> Acked-by: Simon Glass <sjg@chromium.org>
>>>
>>> This bug will affect boards which don't have their own
>>> timer_get_boot_us() function.
>>>
>>> Albert, I think we should try to get this in as a bug fix in the
>>> upcoming release.
>>>
>>> Thanks for the fix Vikram.
>>
>> Thanks Vikram and Simon; I'll pull this in today.
>
> ... actually, this will depend on whether my crt0 change gets in for
> 2013.01, as with this change, gd is already initialized when
> entering board_init_f().

Yes. You are right. If your patch goes in, my patch need not be applied.

Regards,
Vikram