| Message ID | 1404775168-17884-3-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hello Simon, On 08-07-14 01:19, Simon Glass wrote: > At present arm defines CONFIG_SYS_GENERIC_GLOBAL_DATA, meaning that > the global_data pointer is set up in board_init_f(). However it is > actually set up before this, it just isn't zeroed. > > If we zero the global data before calling board_init_f() then we > don't need to define CONFIG_SYS_GENERIC_GLOBAL_DATA. > > Make this change to simplify the init process. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: None > > arch/arm/include/asm/config.h | 2 -- > arch/arm/lib/crt0.S | 7 +++++++ > 2 files changed, 7 insertions(+), 2 deletions(-) First of all, good idea.. I missed v1 apparently. > diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h > index 2a20a77..abf79e5 100644 > --- a/arch/arm/include/asm/config.h > +++ b/arch/arm/include/asm/config.h > @@ -7,8 +7,6 @@ > #ifndef _ASM_CONFIG_H_ > #define _ASM_CONFIG_H_ > > -#define CONFIG_SYS_GENERIC_GLOBAL_DATA > - This bricks aarch64 I guess. > #define CONFIG_LMB > #define CONFIG_SYS_BOOT_RAMDISK_HIGH > > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S > index dfc2de9..bbf3e41 100644 > --- a/arch/arm/lib/crt0.S > +++ b/arch/arm/lib/crt0.S > @@ -67,9 +67,16 @@ ENTRY(_main) > ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) > #endif > bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ > + mov r2, sp > sub sp, sp, #GD_SIZE /* allocate one GD above SP */ > bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ > mov r9, sp /* GD is above SP */ > + mov r1, r9 > + mov r0, #0 > +clr_gd: cmp r1, r2 /* while not at end of BSS */ > + strlo r0, [r1] /* clear 32-bit BSS word */ > + addlo r1, r1, #4 /* move to next */ The comment should be updated, it is not BSS which is cleared. > + blo clr_gd > mov r0, #0 > bl board_init_f > The mov r0, #0 for the argument could be removed, but at least deserves a comment if you do so. Regards, Jeroen
Hi Jeroen, On 8 July 2014 11:13, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: > Hello Simon, > > > On 08-07-14 01:19, Simon Glass wrote: >> >> At present arm defines CONFIG_SYS_GENERIC_GLOBAL_DATA, meaning that >> the global_data pointer is set up in board_init_f(). However it is >> actually set up before this, it just isn't zeroed. >> >> If we zero the global data before calling board_init_f() then we >> don't need to define CONFIG_SYS_GENERIC_GLOBAL_DATA. >> >> Make this change to simplify the init process. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v2: None >> >> arch/arm/include/asm/config.h | 2 -- >> arch/arm/lib/crt0.S | 7 +++++++ >> 2 files changed, 7 insertions(+), 2 deletions(-) > > > First of all, good idea.. I missed v1 apparently. > > >> diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h >> index 2a20a77..abf79e5 100644 >> --- a/arch/arm/include/asm/config.h >> +++ b/arch/arm/include/asm/config.h >> @@ -7,8 +7,6 @@ >> #ifndef _ASM_CONFIG_H_ >> #define _ASM_CONFIG_H_ >> -#define CONFIG_SYS_GENERIC_GLOBAL_DATA >> - > > > This bricks aarch64 I guess. I'll see if I can add it there also. > >> #define CONFIG_LMB >> #define CONFIG_SYS_BOOT_RAMDISK_HIGH >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S >> index dfc2de9..bbf3e41 100644 >> --- a/arch/arm/lib/crt0.S >> +++ b/arch/arm/lib/crt0.S >> @@ -67,9 +67,16 @@ ENTRY(_main) >> ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) >> #endif >> bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ >> + mov r2, sp >> sub sp, sp, #GD_SIZE /* allocate one GD above SP */ >> bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ >> mov r9, sp /* GD is above SP */ >> + mov r1, r9 >> + mov r0, #0 >> +clr_gd: cmp r1, r2 /* while not at end of BSS >> */ >> + strlo r0, [r1] /* clear 32-bit BSS word */ >> + addlo r1, r1, #4 /* move to next */ > > > The comment should be updated, it is not BSS which is cleared. Yes I have a new version which I'm testing and will send when ready. > >> + blo clr_gd >> mov r0, #0 >> bl board_init_f >> > > > The mov r0, #0 for the argument could be removed, but at > least deserves a comment if you do so. OK, I'll add that too. Regards, Simon
Hello Simon, On 08-07-14 20:41, Simon Glass wrote: > Hi Jeroen, > > On 8 July 2014 11:13, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: >>> diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h >>> index 2a20a77..abf79e5 100644 >>> --- a/arch/arm/include/asm/config.h >>> +++ b/arch/arm/include/asm/config.h >>> @@ -7,8 +7,6 @@ >>> #ifndef _ASM_CONFIG_H_ >>> #define _ASM_CONFIG_H_ >>> -#define CONFIG_SYS_GENERIC_GLOBAL_DATA >>> - >> >> This bricks aarch64 I guess. > I'll see if I can add it there also. If you manage actually boot an virtual aarch64 board (with console), could you report how you did it. I gave up at the fifth loader or something bl3.3? >>> #define CONFIG_LMB >>> #define CONFIG_SYS_BOOT_RAMDISK_HIGH >>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S >>> index dfc2de9..bbf3e41 100644 >>> --- a/arch/arm/lib/crt0.S >>> +++ b/arch/arm/lib/crt0.S >>> @@ -67,9 +67,16 @@ ENTRY(_main) >>> ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) >>> #endif >>> bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ >>> + mov r2, sp >>> sub sp, sp, #GD_SIZE /* allocate one GD above SP */ >>> bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ >>> mov r9, sp /* GD is above SP */ >>> + mov r1, r9 >>> + mov r0, #0 >>> +clr_gd: cmp r1, r2 /* while not at end of BSS nitpicking, personal taste I guess: could you hit enter after the clr_gd label? > >>> + blo clr_gd >>> mov r0, #0 >>> bl board_init_f >>> >> >> The mov r0, #0 for the argument could be removed, but at >> least deserves a comment if you do so. > OK, I'll add that too. I leave that up to you. Regards, Jeroen
Hi Jeroen, On 8 July 2014 14:00, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: > Hello Simon, > > > On 08-07-14 20:41, Simon Glass wrote: >> >> Hi Jeroen, >> >> On 8 July 2014 11:13, Jeroen Hofstee <jeroen@myspectrum.nl> wrote: >>>> >>>> diff --git a/arch/arm/include/asm/config.h >>>> b/arch/arm/include/asm/config.h >>>> index 2a20a77..abf79e5 100644 >>>> --- a/arch/arm/include/asm/config.h >>>> +++ b/arch/arm/include/asm/config.h >>>> @@ -7,8 +7,6 @@ >>>> #ifndef _ASM_CONFIG_H_ >>>> #define _ASM_CONFIG_H_ >>>> -#define CONFIG_SYS_GENERIC_GLOBAL_DATA >>>> - >>> >>> >>> This bricks aarch64 I guess. >> >> I'll see if I can add it there also. > > > If you manage actually boot an virtual aarch64 board (with console), > could you report how you did it. I gave up at the fifth loader or > something bl3.3? Have not managed, I might wait for real hardware... > > >>>> #define CONFIG_LMB >>>> #define CONFIG_SYS_BOOT_RAMDISK_HIGH >>>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S >>>> index dfc2de9..bbf3e41 100644 >>>> --- a/arch/arm/lib/crt0.S >>>> +++ b/arch/arm/lib/crt0.S >>>> @@ -67,9 +67,16 @@ ENTRY(_main) >>>> ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) >>>> #endif >>>> bic sp, sp, #7 /* 8-byte alignment for ABI compliance >>>> */ >>>> + mov r2, sp >>>> sub sp, sp, #GD_SIZE /* allocate one GD above SP */ >>>> bic sp, sp, #7 /* 8-byte alignment for ABI compliance >>>> */ >>>> mov r9, sp /* GD is above SP */ >>>> + mov r1, r9 >>>> + mov r0, #0 >>>> +clr_gd: cmp r1, r2 /* while not at end of >>>> BSS > > nitpicking, personal taste I guess: could you hit enter after the clr_gd > label? OK Regards, Simon
diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h index 2a20a77..abf79e5 100644 --- a/arch/arm/include/asm/config.h +++ b/arch/arm/include/asm/config.h @@ -7,8 +7,6 @@ #ifndef _ASM_CONFIG_H_ #define _ASM_CONFIG_H_ -#define CONFIG_SYS_GENERIC_GLOBAL_DATA - #define CONFIG_LMB #define CONFIG_SYS_BOOT_RAMDISK_HIGH diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index dfc2de9..bbf3e41 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -67,9 +67,16 @@ ENTRY(_main) ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) #endif bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ + mov r2, sp sub sp, sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r9, sp /* GD is above SP */ + mov r1, r9 + mov r0, #0 +clr_gd: cmp r1, r2 /* while not at end of BSS */ + strlo r0, [r1] /* clear 32-bit BSS word */ + addlo r1, r1, #4 /* move to next */ + blo clr_gd mov r0, #0 bl board_init_f
At present arm defines CONFIG_SYS_GENERIC_GLOBAL_DATA, meaning that the global_data pointer is set up in board_init_f(). However it is actually set up before this, it just isn't zeroed. If we zero the global data before calling board_init_f() then we don't need to define CONFIG_SYS_GENERIC_GLOBAL_DATA. Make this change to simplify the init process. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: None arch/arm/include/asm/config.h | 2 -- arch/arm/lib/crt0.S | 7 +++++++ 2 files changed, 7 insertions(+), 2 deletions(-)