[U-Boot] [PATCH] ARM: MX5: Remove broken leftover TO-2 errata workaround
This check is broken. r3 does not contain the silicon revision.
Signed-off-by: David Jander david@protonic.nl --- arch/arm/cpu/armv7/mx5/lowlevel_init.S | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..f17d200 100644 --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S @@ -39,11 +39,6 @@ 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 */ - - /* disable write combine for TO 2 and lower revs */ - orrls r0, r0, #(1 << 25) - mcr 15, 1, r0, c9, c0, 2 .endm /* init_l2cc */
On 07/14/2011 09:13 AM, David Jander wrote:
This check is broken. r3 does not contain the silicon revision.
Signed-off-by: David Jander david@protonic.nl
Hi David,
arch/arm/cpu/armv7/mx5/lowlevel_init.S | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..f17d200 100644 --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S @@ -39,11 +39,6 @@ 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 */
You are right. Nobody sets the r3 register, the test can be wrong.
- /* disable write combine for TO 2 and lower revs */
- orrls r0, r0, #(1 << 25)
However, you also remove the setup for TO2. To fix the TO2 issue, we should read correctly the revision number (from IIM or from a fixed address, I do not remember now), and then apply the compare to the read value.
Best regards, Stefano Babic
On Thu, 14 Jul 2011 10:16:51 +0200 Stefano Babic sbabic@denx.de wrote:
On 07/14/2011 09:13 AM, David Jander wrote:
This check is broken. r3 does not contain the silicon revision.
Signed-off-by: David Jander david@protonic.nl
Hi David,
arch/arm/cpu/armv7/mx5/lowlevel_init.S | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/mx5/lowlevel_init.S b/arch/arm/cpu/armv7/mx5/lowlevel_init.S index ee4150d..f17d200 100644 --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S @@ -39,11 +39,6 @@ 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 */
You are right. Nobody sets the r3 register, the test can be wrong.
- /* disable write combine for TO 2 and lower revs */
- orrls r0, r0, #(1 << 25)
However, you also remove the setup for TO2. To fix the TO2 issue, we should read correctly the revision number (from IIM or from a fixed address, I do not remember now), and then apply the compare to the read value.
Yes, you are right. But I don't know how to do it correctly. OTOH, it is broken now for all platforms. My patch fixes it for TO3 and newer. L2 write-combine has a significant performance impact, and I wonder how many boards there are still that use such an old (prototype silicon) processor. IMHO, the vast majority of MX51 users will benefit from this patch, and the rest shouldn't have any more problems than they have already, so can we just apply this, please?
Best regards,
On 07/14/2011 12:20 PM, David Jander wrote:
However, you also remove the setup for TO2. To fix the TO2 issue, we should read correctly the revision number (from IIM or from a fixed address, I do not remember now), and then apply the compare to the read value.
Yes, you are right. But I don't know how to do it correctly.
There is a similar code always in lowlevel_init.S
189 ldr r1, =0x0 190 ldr r3, [r1, #ROM_SI_REV] 191 cmp r3, #0x10
As we can suppose this is correct, the same code can be used in the macro.
OTOH, it is broken now for all platforms.
Agree we have to fix it. I only dislike to break some boards. As far as I know, there is many mx51evk boards sold by Freescale with the TO2 chip.
My patch fixes it for TO3 and newer. L2 write-combine has a significant performance impact, and I wonder how many boards there are still that use such an old (prototype silicon) processor.
I think only on the evaluation boards, but they were sold and delivered.
IMHO, the vast majority of MX51 users will benefit from this patch, and the rest shouldn't have any more problems than they have already, so can we just apply this, please?
Not as it is - I prefer we fix the test. Can you resubmit with the proposed changes ?
Best regards, Stefano Babic
On Thu, 14 Jul 2011 12:49:15 +0200 Stefano Babic sbabic@denx.de wrote:
On 07/14/2011 12:20 PM, David Jander wrote:
However, you also remove the setup for TO2. To fix the TO2 issue, we should read correctly the revision number (from IIM or from a fixed address, I do not remember now), and then apply the compare to the read value.
Yes, you are right. But I don't know how to do it correctly.
There is a similar code always in lowlevel_init.S
189 ldr r1, =0x0 190 ldr r3, [r1, #ROM_SI_REV] 191 cmp r3, #0x10
As we can suppose this is correct, the same code can be used in the macro.
Hmmm. Hadn't seen that part. Can we trust this?... because I have no means of testing for the TO2 case.
OTOH, it is broken now for all platforms.
Agree we have to fix it. I only dislike to break some boards. As far as I know, there is many mx51evk boards sold by Freescale with the TO2 chip.
Ah, ok. AFAICR, our EVK has a TO3, but I agree there might be low quantities of EVKs with TO2 still in use somewhere.
My patch fixes it for TO3 and newer. L2 write-combine has a significant performance impact, and I wonder how many boards there are still that use such an old (prototype silicon) processor.
I think only on the evaluation boards, but they were sold and delivered.
Ok.
IMHO, the vast majority of MX51 users will benefit from this patch, and the rest shouldn't have any more problems than they have already, so can we just apply this, please?
Not as it is - I prefer we fix the test. Can you resubmit with the proposed changes ?
Ok, thanks for pointing out the missing code. I will fix and re-submit.
Best regards,
participants (3)
-
David Jander -
David Jander -
Stefano Babic