| Message ID | 1310644580-1005-1-git-send-email-david@protonic.nl |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
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
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,
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
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)
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(-)