| Message ID | 1322564285-6911-1-git-send-email-christian.riesch@omicron.at |
|---|---|
| State | Superseded |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Christian, Le 29/11/2011 11:58, Christian Riesch a écrit : > This patch fixes the clear bss loop for bss sections that have > zero length, i.e., where __bss_start == __bss_end__. Just out of curiosity, did you actually hit a scenario when you had a really empty BSS? Not that I intend to reject this patch because of this, mind. > Signed-off-by: Christian Riesch<christian.riesch@omicron.at> > Cc: Albert Aribaud<albert.u.boot@aribaud.net> > --- > Hi, > this is v5 of a patch out of my recent patchset > > [PATCH v3 00/15] Add an SPL to boot the da850evm from SPI > http://lists.denx.de/pipermail/u-boot/2011-November/111182.html > > Changes for v5: > - correct subject line > > Changes for v4: > - split the patchset since it is getting quite big > > Regards, Christian > > arch/arm/cpu/arm926ejs/start.S | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 8b5355b..772793c 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -299,10 +299,12 @@ clear_bss: > #endif > mov r2, #0x00000000 /* clear */ > > -clbss_l:str r2, [r0] /* clear loop... */ > +clbss_l:cmp r0, r1 /* clear loop... */ > + beq clbss_e For safety, you had better use a bge here, in case r1-r0 is not a multiple of 4. > + str r2, [r0] > add r0, r0, #4 > - cmp r0, r1 > - bne clbss_l > + b clbss_l > +clbss_e: > > #ifndef CONFIG_SPL_BUILD > bl coloured_LED_init Amicalement,
Hi Albert, On Wednesday, November 30, 2011, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Christian, > > Le 29/11/2011 11:58, Christian Riesch a écrit : >> >> This patch fixes the clear bss loop for bss sections that have >> zero length, i.e., where __bss_start == __bss_end__. > > Just out of curiosity, did you actually hit a scenario when you had a really empty BSS? Not that I intend to reject this patch because of this, mind. Yes, I did. The patchset in [1] (which is not a complete SPL but was for testing only, it hangs after initialization of the hardware) results in an empty bss. It took me quite some time to find out why my board didn't get out of the relocation code... ;-) [1] http://lists.denx.de/pipermail/u-boot/2011-November/109886.html > >> Signed-off-by: Christian Riesch<christian.riesch@omicron.at> >> Cc: Albert Aribaud<albert.u.boot@aribaud.net> >> --- >> Hi, >> this is v5 of a patch out of my recent patchset >> >> [PATCH v3 00/15] Add an SPL to boot the da850evm from SPI >> http://lists.denx.de/pipermail/u-boot/2011-November/111182.html >> >> Changes for v5: >> - correct subject line >> >> Changes for v4: >> - split the patchset since it is getting quite big >> >> Regards, Christian >> >> arch/arm/cpu/arm926ejs/start.S | 8 +++++--- >> 1 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 8b5355b..772793c 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -299,10 +299,12 @@ clear_bss: >> #endif >> mov r2, #0x00000000 /* clear */ >> >> -clbss_l:str r2, [r0] /* clear loop... */ >> +clbss_l:cmp r0, r1 /* clear loop... */ >> + beq clbss_e > > For safety, you had better use a bge here, in case r1-r0 is not a multiple of 4. Ok, I'll change that. > >> + str r2, [r0] >> add r0, r0, #4 >> - cmp r0, r1 >> - bne clbss_l >> + b clbss_l >> +clbss_e: >> >> #ifndef CONFIG_SPL_BUILD >> bl coloured_LED_init > Thanks for your comments! Regards, Christian > Amicalement, > -- > Albert. > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
Hi Albert, On Wed, Nov 30, 2011 at 9:13 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 29/11/2011 11:58, Christian Riesch a écrit : >> This patch fixes the clear bss loop for bss sections that have >> zero length, i.e., where __bss_start == __bss_end__. [...] >> diff --git a/arch/arm/cpu/arm926ejs/start.S >> b/arch/arm/cpu/arm926ejs/start.S >> index 8b5355b..772793c 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -299,10 +299,12 @@ clear_bss: >> #endif >> mov r2, #0x00000000 /* clear >> */ >> >> -clbss_l:str r2, [r0] /* clear loop... >> */ >> +clbss_l:cmp r0, r1 /* clear loop... >> */ >> + beq clbss_e > > > For safety, you had better use a bge here, in case r1-r0 is not a multiple > of 4. Shouldn't it be bhs instead because we are dealing with unsigned numbers here? bge is signed >=. Regards, Christian > > >> + str r2, [r0] >> add r0, r0, #4 >> - cmp r0, r1 >> - bne clbss_l >> + b clbss_l >> +clbss_e: >> >> #ifndef CONFIG_SPL_BUILD >> bl coloured_LED_init > > > Amicalement, > -- > Albert. > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 8b5355b..772793c 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -299,10 +299,12 @@ clear_bss: #endif mov r2, #0x00000000 /* clear */ -clbss_l:str r2, [r0] /* clear loop... */ +clbss_l:cmp r0, r1 /* clear loop... */ + beq clbss_e + str r2, [r0] add r0, r0, #4 - cmp r0, r1 - bne clbss_l + b clbss_l +clbss_e: #ifndef CONFIG_SPL_BUILD bl coloured_LED_init
This patch fixes the clear bss loop for bss sections that have zero length, i.e., where __bss_start == __bss_end__. Signed-off-by: Christian Riesch <christian.riesch@omicron.at> Cc: Albert Aribaud <albert.u.boot@aribaud.net> --- Hi, this is v5 of a patch out of my recent patchset [PATCH v3 00/15] Add an SPL to boot the da850evm from SPI http://lists.denx.de/pipermail/u-boot/2011-November/111182.html Changes for v5: - correct subject line Changes for v4: - split the patchset since it is getting quite big Regards, Christian arch/arm/cpu/arm926ejs/start.S | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)