Patchwork [U-Boot] ARM: MX5: Fix broken leftover TO-2 errata workaround

login
register
mail settings
Submitter David Jander
Date July 14, 2011, 11:56 a.m.
Message ID <1310644580-1005-1-git-send-email-david@protonic.nl>
Download mbox | patch
Permalink /patch/104668/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

David Jander - July 14, 2011, 11:56 a.m.
This check was broken. r3 does not contain the silicon revision anymore, so
we need to reload it.

Signed-off-by: David Jander <david@protonic.nl>
---
 arch/arm/cpu/armv7/mx5/lowlevel_init.S |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Stefano Babic - July 14, 2011, 12:14 p.m.
On 07/14/2011 01:56 PM, David Jander wrote:
> This check was broken. r3 does not contain the silicon revision anymore, so
> we need to reload it.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> ---

Hi David,

>  arch/arm/cpu/armv7/mx5/lowlevel_init.S |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> index ee4150d..6bb398f 100644
> --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> @@ -39,7 +39,9 @@
>  	orr r0, r0, #(1 << 23)		/* disable write allocate combine */
>  	orr r0, r0, #(1 << 22)		/* disable write allocate */
>  
> -	cmp r3, #0x10    /* r3 contains the silicon rev */
> +	ldr r1, =0x0
> +	ldr r3, [r1, #ROM_SI_REV]
> +	cmp r3, #0x10

You have to protect the code related to TO2 with CONFIG_MX51. As I can
see, the macro is called for MX.53, too, and the test produces wrong
results.

Best regards,
Stefano Babic
David Jander - July 14, 2011, 12:30 p.m.
On Thu, 14 Jul 2011 14:14:52 +0200
Stefano Babic <sbabic@denx.de> wrote:

> On 07/14/2011 01:56 PM, David Jander wrote:
> > This check was broken. r3 does not contain the silicon revision anymore, so
> > we need to reload it.
> > 
> > Signed-off-by: David Jander <david@protonic.nl>
> > ---
> 
> Hi David,
> 
> >  arch/arm/cpu/armv7/mx5/lowlevel_init.S |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..6bb398f 100644
> > --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
> > @@ -39,7 +39,9 @@
> >  	orr r0, r0, #(1 << 23)		/* disable write allocate
> > combine */ orr r0, r0, #(1 << 22)		/* disable write allocate
> > */ 
> > -	cmp r3, #0x10    /* r3 contains the silicon rev */
> > +	ldr r1, =0x0
> > +	ldr r3, [r1, #ROM_SI_REV]
> > +	cmp r3, #0x10
> 
> You have to protect the code related to TO2 with CONFIG_MX51. As I can
> see, the macro is called for MX.53, too, and the test produces wrong
> results.

Wow, you are right! So this code was actually broken in more ways than I
initially thought :-)
Ok, will fix it (again).
Btw, may I congratulate you for your good work in improving u-boot code
quality? It is starting to get to a level only known from LKML ;-)
Clearly the code was not reviewed as well when this was initially submitted.

Best regards,
Stefano Babic - July 14, 2011, 12:39 p.m.
On 07/14/2011 02:30 PM, David Jander wrote:
>> You have to protect the code related to TO2 with CONFIG_MX51. As I can
>> see, the macro is called for MX.53, too, and the test produces wrong
>> results.
> 
> Wow, you are right! So this code was actually broken in more ways than I
> initially thought :-)

Yes, with one shot you hit more bugs...

> Ok, will fix it (again).
> Btw, may I congratulate you for your good work in improving u-boot code
> quality? It is starting to get to a level only known from LKML ;-)

Congratulations go to all participants and reviewers of this ML, who are
helping to improve the quality of the code ;-)

> Clearly the code was not reviewed as well when this was initially submitted.

That is right - some issues was not seen and not fixed in time.

Best regards,
Stefano Babic

Patch

diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
index ee4150d..6bb398f 100644
--- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S
@@ -39,7 +39,9 @@ 
 	orr r0, r0, #(1 << 23)		/* disable write allocate combine */
 	orr r0, r0, #(1 << 22)		/* disable write allocate */
 
-	cmp r3, #0x10    /* r3 contains the silicon rev */
+	ldr r1, =0x0
+	ldr r3, [r1, #ROM_SI_REV]
+	cmp r3, #0x10
 
 	/* disable write combine for TO 2 and lower revs */
 	orrls r0, r0, #(1 << 25)