* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot @ 2012-07-02 23:46 Zhong Hongbo 2012-07-05 11:53 ` Zhong Hongbo 0 siblings, 1 reply; 8+ messages in thread From: Zhong Hongbo @ 2012-07-02 23:46 UTC (permalink / raw) To: u-boot From: Zhong Hongbo <bocui107@gmail.com> In the spl booting step, When __bss_start is equal to __bss_end__, The loop will clear all the things in CPU space. If there are have the same address for this symbol, To skip the clear bss section. Signed-off-by: Hongbo Zhong <bocui107@gmail.com> --- arch/arm/cpu/armv7/start.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 76ccef1..c72f337 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -258,6 +258,8 @@ clear_bss: /* No relocation for SPL */ ldr r0, =__bss_start ldr r1, =__bss_end__ + cmp r0, r1 + beq skip_clbss #else ldr r0, _bss_start_ofs ldr r1, _bss_end_ofs @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* clear loop... */ add r0, r0, #4 cmp r0, r1 bne clbss_l +skip_clbss: /* * We are done. Do not return, instead branch to second part of board -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-02 23:46 [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot Zhong Hongbo @ 2012-07-05 11:53 ` Zhong Hongbo 2012-07-05 12:19 ` Albert ARIBAUD 2012-07-05 17:35 ` Albert ARIBAUD 0 siblings, 2 replies; 8+ messages in thread From: Zhong Hongbo @ 2012-07-05 11:53 UTC (permalink / raw) To: u-boot Hi Albert, Could you applied the patch to the arm tree? Thanks, hongbo On 07/03/2012 07:46 AM, Zhong Hongbo wrote: > From: Zhong Hongbo <bocui107@gmail.com> > > In the spl booting step, When __bss_start is equal to __bss_end__, > The loop will clear all the things in CPU space. If there are have > the same address for this symbol, To skip the clear bss section. > > Signed-off-by: Hongbo Zhong <bocui107@gmail.com> > --- > arch/arm/cpu/armv7/start.S | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > index 76ccef1..c72f337 100644 > --- a/arch/arm/cpu/armv7/start.S > +++ b/arch/arm/cpu/armv7/start.S > @@ -258,6 +258,8 @@ clear_bss: > /* No relocation for SPL */ > ldr r0, =__bss_start > ldr r1, =__bss_end__ > + cmp r0, r1 > + beq skip_clbss > #else > ldr r0, _bss_start_ofs > ldr r1, _bss_end_ofs > @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* clear loop... */ > add r0, r0, #4 > cmp r0, r1 > bne clbss_l > +skip_clbss: > > /* > * We are done. Do not return, instead branch to second part of board > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-05 11:53 ` Zhong Hongbo @ 2012-07-05 12:19 ` Albert ARIBAUD 2012-07-05 12:34 ` Zhong Hongbo 2012-07-05 17:35 ` Albert ARIBAUD 1 sibling, 1 reply; 8+ messages in thread From: Albert ARIBAUD @ 2012-07-05 12:19 UTC (permalink / raw) To: u-boot Hi Zhong Hongbo, On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> wrote: > Hi Albert, > > Could you applied the patch to the arm tree? > > Thanks, > hongbo > On 07/03/2012 07:46 AM, Zhong Hongbo wrote: > > From: Zhong Hongbo <bocui107@gmail.com> > > > > In the spl booting step, When __bss_start is equal to __bss_end__, > > The loop will clear all the things in CPU space. If there are have > > the same address for this symbol, To skip the clear bss section. > > > > Signed-off-by: Hongbo Zhong <bocui107@gmail.com> > > --- > > arch/arm/cpu/armv7/start.S | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > > index 76ccef1..c72f337 100644 > > --- a/arch/arm/cpu/armv7/start.S > > +++ b/arch/arm/cpu/armv7/start.S > > @@ -258,6 +258,8 @@ clear_bss: > > /* No relocation for SPL */ > > ldr r0, =__bss_start > > ldr r1, =__bss_end__ > > + cmp r0, r1 > > + beq skip_clbss > > #else > > ldr r0, _bss_start_ofs > > ldr r1, _bss_end_ofs > > @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* > > clear loop... */ add r0, r0, #4 > > cmp r0, r1 > > bne clbss_l > > +skip_clbss: Clearly the loop was wrong in that it should implement a "for (r0 = start; r0 < end; r0++)" but actually implements a "for (r0 = start; r0 != end; r0++)". I'd rather the loop be fixed to match the intended implementation rather than worked around. Please rewrite your patch to turn: > clbss_l:str r2, [r0] /* clear loop...*/ > add r0, r0, #4 > cmp r0, r1 > bne clbss_l Into something like > clbss_l:cmp r0, r1 > blo clbss_d > str r2, [r0] /* clear loop...*/ > add r0, r0, #4 > b clbss_l > clbss_d: Thanks in advance. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-05 12:19 ` Albert ARIBAUD @ 2012-07-05 12:34 ` Zhong Hongbo 0 siblings, 0 replies; 8+ messages in thread From: Zhong Hongbo @ 2012-07-05 12:34 UTC (permalink / raw) To: u-boot Hi Albert, On 07/05/2012 08:19 PM, Albert ARIBAUD wrote: > Hi Zhong Hongbo, > > On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> > wrote: >> Hi Albert, >> >> Could you applied the patch to the arm tree? >> >> Thanks, >> hongbo >> On 07/03/2012 07:46 AM, Zhong Hongbo wrote: >>> From: Zhong Hongbo <bocui107@gmail.com> >>> >>> In the spl booting step, When __bss_start is equal to __bss_end__, >>> The loop will clear all the things in CPU space. If there are have >>> the same address for this symbol, To skip the clear bss section. >>> >>> Signed-off-by: Hongbo Zhong <bocui107@gmail.com> >>> --- >>> arch/arm/cpu/armv7/start.S | 3 +++ >>> 1 files changed, 3 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S >>> index 76ccef1..c72f337 100644 >>> --- a/arch/arm/cpu/armv7/start.S >>> +++ b/arch/arm/cpu/armv7/start.S >>> @@ -258,6 +258,8 @@ clear_bss: >>> /* No relocation for SPL */ >>> ldr r0, =__bss_start >>> ldr r1, =__bss_end__ >>> + cmp r0, r1 >>> + beq skip_clbss >>> #else >>> ldr r0, _bss_start_ofs >>> ldr r1, _bss_end_ofs >>> @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* >>> clear loop... */ add r0, r0, #4 >>> cmp r0, r1 >>> bne clbss_l >>> +skip_clbss: > > Clearly the loop was wrong in that it should implement a "for (r0 = > start; r0 < end; r0++)" but actually implements a "for (r0 = > start; r0 != end; r0++)". Ok, Good to known, I will send V2 > > I'd rather the loop be fixed to match the intended implementation > rather than worked around. Please rewrite your patch to turn: Ok, Thanks, hongbo > >> clbss_l:str r2, [r0] /* clear loop...*/ >> add r0, r0, #4 >> cmp r0, r1 >> bne clbss_l > > Into something like > >> clbss_l:cmp r0, r1 >> blo clbss_d >> str r2, [r0] /* clear loop...*/ >> add r0, r0, #4 >> b clbss_l >> clbss_d: > > Thanks in advance. > > Amicalement, > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-05 11:53 ` Zhong Hongbo 2012-07-05 12:19 ` Albert ARIBAUD @ 2012-07-05 17:35 ` Albert ARIBAUD 2012-07-06 11:42 ` Zhong Hongbo 1 sibling, 1 reply; 8+ messages in thread From: Albert ARIBAUD @ 2012-07-05 17:35 UTC (permalink / raw) To: u-boot Hi Zhong Hongbo, On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> wrote: > Hi Albert, > > Could you applied the patch to the arm tree? > > Thanks, > hongbo > On 07/03/2012 07:46 AM, Zhong Hongbo wrote: > > From: Zhong Hongbo <bocui107@gmail.com> > > > > In the spl booting step, When __bss_start is equal to __bss_end__, > > The loop will clear all the things in CPU space. If there are have > > the same address for this symbol, To skip the clear bss section. > > > > Signed-off-by: Hongbo Zhong <bocui107@gmail.com> > > --- > > arch/arm/cpu/armv7/start.S | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > > index 76ccef1..c72f337 100644 > > --- a/arch/arm/cpu/armv7/start.S > > +++ b/arch/arm/cpu/armv7/start.S > > @@ -258,6 +258,8 @@ clear_bss: > > /* No relocation for SPL */ > > ldr r0, =__bss_start > > ldr r1, =__bss_end__ > > + cmp r0, r1 > > + beq skip_clbss > > #else > > ldr r0, _bss_start_ofs > > ldr r1, _bss_end_ofs > > @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* > > clear loop... */ add r0, r0, #4 > > cmp r0, r1 > > bne clbss_l > > +skip_clbss: Clearly the loop was wrong in that it should implement a "for (r0 = start; r0 < end; r0++)" but actually implements a "for (r0 = start; r0 != end; r0++)". I'd rather the loop be fixed to match the intended implementation rather than worked around. Please rewrite your patch to turn: > clbss_l:str r2, [r0] /* clear loop...*/ > add r0, r0, #4 > cmp r0, r1 > bne clbss_l Into something like > clbss_l:cmp r0, r1 > blo clbss_d > str r2, [r0] /* clear loop...*/ > add r0, r0, #4 > b clbss_l > clbss_d: Also, as Andreas points out, make sure the same fix is applied to all ARM start.S files which need it. Thanks in advance. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-05 17:35 ` Albert ARIBAUD @ 2012-07-06 11:42 ` Zhong Hongbo 2012-07-06 11:49 ` Andreas Bießmann 0 siblings, 1 reply; 8+ messages in thread From: Zhong Hongbo @ 2012-07-06 11:42 UTC (permalink / raw) To: u-boot On 07/06/2012 01:35 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> (by way of Albert ARIBAUD wrote: > Hi Zhong Hongbo, > > On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> > wrote: >> Hi Albert, >> >> Could you applied the patch to the arm tree? >> >> Thanks, >> hongbo >> On 07/03/2012 07:46 AM, Zhong Hongbo wrote: >>> From: Zhong Hongbo <bocui107@gmail.com> >>> >>> In the spl booting step, When __bss_start is equal to __bss_end__, >>> The loop will clear all the things in CPU space. If there are have >>> the same address for this symbol, To skip the clear bss section. >>> >>> Signed-off-by: Hongbo Zhong <bocui107@gmail.com> >>> --- >>> arch/arm/cpu/armv7/start.S | 3 +++ >>> 1 files changed, 3 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S >>> index 76ccef1..c72f337 100644 >>> --- a/arch/arm/cpu/armv7/start.S >>> +++ b/arch/arm/cpu/armv7/start.S >>> @@ -258,6 +258,8 @@ clear_bss: >>> /* No relocation for SPL */ >>> ldr r0, =__bss_start >>> ldr r1, =__bss_end__ >>> + cmp r0, r1 >>> + beq skip_clbss >>> #else >>> ldr r0, _bss_start_ofs >>> ldr r1, _bss_end_ofs >>> @@ -271,6 +273,7 @@ clbss_l:str r2, [r0] /* >>> clear loop... */ add r0, r0, #4 >>> cmp r0, r1 >>> bne clbss_l >>> +skip_clbss: > > Clearly the loop was wrong in that it should implement a "for (r0 = > start; r0 < end; r0++)" but actually implements a "for (r0 = > start; r0 != end; r0++)". > Yes? My new patch have do this. > I'd rather the loop be fixed to match the intended implementation > rather than worked around. Please rewrite your patch to turn: > Ok, I just found the issue have found in other arm platfor 2011 yeas, the detail information as following: commit 8f1da53508c78789ebeea98a92a3f55c3f84dc5d Author: Christian Riesch <christian.riesch@omicron.at> Date: Wed Nov 30 22:27:37 2011 +0000 arm, arm926ejs: Fix clear bss loop for zero length bss 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> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 339c5ed..bb4d00b 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -301,10 +301,12 @@ clear_bss: #endif mov r2, #0x00000000 /* clear */ -clbss_l:str r2, [r0] /* clear loop... */ +clbss_l:cmp r0, r1 /* clear loop... */ + bhs clbss_e /* if reached end of bss, exit */ + str r2, [r0] add r0, r0, #4 - cmp r0, r1 - bne clbss_l + b clbss_l +clbss_e: >> clbss_l:str r2, [r0] /* clear loop...*/ >> add r0, r0, #4 >> cmp r0, r1 >> bne clbss_l > > Into something like > >> clbss_l:cmp r0, r1 >> blo clbss_d >> str r2, [r0] /* clear loop...*/ >> add r0, r0, #4 >> b clbss_l >> clbss_d: > > Also, as Andreas points out, make sure the same fix is applied to all ARM start.S files which need it. Ok? Thanks, hongbo > > Thanks in advance. > > Amicalement, > ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-06 11:42 ` Zhong Hongbo @ 2012-07-06 11:49 ` Andreas Bießmann 2012-07-06 11:53 ` Zhong Hongbo 0 siblings, 1 reply; 8+ messages in thread From: Andreas Bießmann @ 2012-07-06 11:49 UTC (permalink / raw) To: u-boot Dear Zhong Hongbo, On 06.07.2012 13:42, Zhong Hongbo wrote: > On 07/06/2012 01:35 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> (by > way of Albert ARIBAUD wrote: >> Hi Zhong Hongbo, >> >> On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> >> wrote: <snip> > > Ok, I just found the issue have found in other arm platfor 2011 yeas, > the detail information as following: > > commit 8f1da53508c78789ebeea98a92a3f55c3f84dc5d > Author: Christian Riesch <christian.riesch@omicron.at> > Date: Wed Nov 30 22:27:37 2011 +0000 > > arm, arm926ejs: Fix clear bss loop for zero length bss > > 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> > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 339c5ed..bb4d00b 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -301,10 +301,12 @@ clear_bss: > #endif > mov r2, #0x00000000 /* clear > */ > > -clbss_l:str r2, [r0] /* clear loop... > */ > +clbss_l:cmp r0, r1 /* clear loop... */ > + bhs clbss_e /* if reached end of bss, exit */ > + str r2, [r0] > add r0, r0, #4 > - cmp r0, r1 > - bne clbss_l > + b clbss_l > +clbss_e: >>> clbss_l:str r2, [r0] /* clear loop...*/ >>> add r0, r0, #4 >>> cmp r0, r1 >>> bne clbss_l >> >> Into something like >> >>> clbss_l:cmp r0, r1 >>> blo clbss_d >>> str r2, [r0] /* clear loop...*/ >>> add r0, r0, #4 >>> b clbss_l >>> clbss_d: >> >> Also, as Andreas points out, make sure the same fix is applied to all ARM start.S files which need it. > > Ok? I think this is the thread we will proceed. So as I understand we had missed in 2011 to update the other start.S with a fix found by Christian Riesch for arm926ejs. So please do update _all_ start.S (of arm;) now. I think they do have all the same error cause we copied it over from one to the other when implementing the 'new relocation' stuff. Best regards Andreas Bie?mann ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot 2012-07-06 11:49 ` Andreas Bießmann @ 2012-07-06 11:53 ` Zhong Hongbo 0 siblings, 0 replies; 8+ messages in thread From: Zhong Hongbo @ 2012-07-06 11:53 UTC (permalink / raw) To: u-boot On 07/06/2012 07:49 PM, Andreas Bie?mann wrote: > Dear Zhong Hongbo, > > On 06.07.2012 13:42, Zhong Hongbo wrote: >> On 07/06/2012 01:35 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> (by >> way of Albert ARIBAUD wrote: >>> Hi Zhong Hongbo, >>> >>> On Thu, 05 Jul 2012 19:53:46 +0800, Zhong Hongbo <bocui107@gmail.com> >>> wrote: > > <snip> > >> >> Ok, I just found the issue have found in other arm platfor 2011 yeas, >> the detail information as following: >> >> commit 8f1da53508c78789ebeea98a92a3f55c3f84dc5d >> Author: Christian Riesch <christian.riesch@omicron.at> >> Date: Wed Nov 30 22:27:37 2011 +0000 >> >> arm, arm926ejs: Fix clear bss loop for zero length bss >> >> 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> >> >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 339c5ed..bb4d00b 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -301,10 +301,12 @@ clear_bss: >> #endif >> mov r2, #0x00000000 /* clear >> */ >> >> -clbss_l:str r2, [r0] /* clear loop... >> */ >> +clbss_l:cmp r0, r1 /* clear loop... */ >> + bhs clbss_e /* if reached end of bss, exit */ >> + str r2, [r0] >> add r0, r0, #4 >> - cmp r0, r1 >> - bne clbss_l >> + b clbss_l >> +clbss_e: >>>> clbss_l:str r2, [r0] /* clear loop...*/ >>>> add r0, r0, #4 >>>> cmp r0, r1 >>>> bne clbss_l >>> >>> Into something like >>> >>>> clbss_l:cmp r0, r1 >>>> blo clbss_d >>>> str r2, [r0] /* clear loop...*/ >>>> add r0, r0, #4 >>>> b clbss_l >>>> clbss_d: >>> >>> Also, as Andreas points out, make sure the same fix is applied to all ARM start.S files which need it. >> >> Ok? > > I think this is the thread we will proceed. > So as I understand we had missed in 2011 to update the other start.S > with a fix found by Christian Riesch for arm926ejs. So please do update > _all_ start.S (of arm;) now. I think they do have all the same error > cause we copied it over from one to the other when implementing the 'new > relocation' stuff. Agree, I will do it now. Thanks, hongbo > > Best regards > > Andreas Bie?mann > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-07-06 11:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-02 23:46 [U-Boot] [PATCH] armv7: Fix infinite loop for the spl boot Zhong Hongbo 2012-07-05 11:53 ` Zhong Hongbo 2012-07-05 12:19 ` Albert ARIBAUD 2012-07-05 12:34 ` Zhong Hongbo 2012-07-05 17:35 ` Albert ARIBAUD 2012-07-06 11:42 ` Zhong Hongbo 2012-07-06 11:49 ` Andreas Bießmann 2012-07-06 11:53 ` Zhong Hongbo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox