[U-Boot] [PATCH v5] arm: Fix clear bss loop for zero length bss

[U-Boot] [PATCH v5] arm: Fix clear bss loop for zero length bss

[Christian Riesch]

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

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

[U-Boot] [PATCH v5] arm: Fix clear bss loop for zero length bss

[Albert ARIBAUD]

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

[U-Boot] [PATCH v5] arm: Fix clear bss loop for zero length bss

[Christian Riesch]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH v5] arm: Fix clear bss loop for zero length bss

[Christian Riesch]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot