[U-Boot] [PATCH] mpc83xx: remove some dead code, saving space

Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se --- arch/powerpc/cpu/mpc83xx/start.S | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index f7da14b..121c276 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -967,13 +967,6 @@ relocate_code: 30: li r3, 0 blr
-2: slwi r0,r0,2 /* re copy in reverse order ... y do we needed it? */ - add r8,r4,r0 - add r7,r3,r0 -3: lwzu r0,-4(r8) - stwu r0,-4(r7) - bdnz 3b - /* * Now flush the cache: note that we must start from a cache aligned * address. Otherwise we might miss one cache line.

On Tue, 23 Nov 2010 19:33:43 +0100 Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
applied.
Thanks,
Kim

Dear Joakim Tjernlund,
In message 1290537223-12160-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
arch/powerpc/cpu/mpc83xx/start.S | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index f7da14b..121c276 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -967,13 +967,6 @@ relocate_code: 30: li r3, 0 blr
-2: slwi r0,r0,2 /* re copy in reverse order ... y do we needed it? */
- add r8,r4,r0
- add r7,r3,r0
-3: lwzu r0,-4(r8)
- stwu r0,-4(r7)
- bdnz 3b
Why do you think this is dead code?
I see this about 30 lines above:
... 851 cmplw cr1,r3,r4 852 addi r0,r5,3 853 srwi. r0,r0,2 854 beq cr1,4f /* In place copy is not necessary */ 855 beq 7f /* Protect against 0 count */ 856 mtctr r0 857 bge cr1,2f ...
With your removal of the code, the "bge cr1,2f" will jump right into the relocation loop. I don't think this is intended?
Best regards,
Wolfgang Denk

Dear Joakim Tjernlund,
In message 1290537223-12160-1-git-send-email-Joakim.Tjernlund@transmode.se you wrote:
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se
arch/powerpc/cpu/mpc83xx/start.S | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index f7da14b..121c276 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -967,13 +967,6 @@ relocate_code: 30: li r3, 0 blr
-2: slwi r0,r0,2 /* re copy in reverse order ... y do we needed it? */
- add r8,r4,r0
- add r7,r3,r0
-3: lwzu r0,-4(r8)
- stwu r0,-4(r7)
- bdnz 3b
Why do you think this is dead code?
I see this about 30 lines above:
... 851 cmplw cr1,r3,r4 852 addi r0,r5,3 853 srwi. r0,r0,2 854 beq cr1,4f /* In place copy is not necessary */ 855 beq 7f /* Protect against 0 count */ 856 mtctr r0 857 bge cr1,2f ...
With your removal of the code, the "bge cr1,2f" will jump right into the relocation loop. I don't think this is intended?
Ah, sharp eyes! Didn't notice that branch. I do think that bge could be removed as well. It will only matter if the src and dst overlap and the distance between them is < 4 bytes. Anyhow, we don't need to remove this code now.
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
Jocke

On Tue, 30 Nov 2010 21:45:19 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
They are not.
-Scott

Scott Wood scottwood@freescale.com wrote on 2010/11/30 21:50:52:
On Tue, 30 Nov 2010 21:45:19 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
They are not.
Ah, I figured they would be these days. I doubt one needs to invalidate the icache though? Better safe than sorry I guess, one could probably optimize it a little bit though.
Jocke

On Tue, 30 Nov 2010 22:13:32 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/30 21:50:52:
On Tue, 30 Nov 2010 21:45:19 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
They are not.
Ah, I figured they would be these days.
Nope, I've seen weird stuff happen when I forget to sync the icache even on very recent chips (albeit 85xx and not 83xx).
I doubt one needs to invalidate the icache though? Better safe than sorry I guess, one could probably optimize it a little bit though.
You need to flush the dcache and then invalidate the icache.
-Scott

Scott Wood scottwood@freescale.com wrote on 2010/11/30 22:17:31:
On Tue, 30 Nov 2010 22:13:32 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/30 21:50:52:
On Tue, 30 Nov 2010 21:45:19 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
They are not.
Ah, I figured they would be these days.
Nope, I've seen weird stuff happen when I forget to sync the icache even on very recent chips (albeit 85xx and not 83xx).
I doubt one needs to invalidate the icache though? Better safe than sorry I guess, one could probably optimize it a little bit though.
You need to flush the dcache and then invalidate the icache.
but if one haven't executed any insn's at the new location yet there should not be any cache lines in the icache that match the new location.

On Tue, 30 Nov 2010 22:25:11 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/30 22:17:31:
On Tue, 30 Nov 2010 22:13:32 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
Scott Wood scottwood@freescale.com wrote on 2010/11/30 21:50:52:
On Tue, 30 Nov 2010 21:45:19 +0100 Joakim Tjernlund joakim.tjernlund@transmode.se wrote:
On a related note, I am not sure why the I and D cache needs to be flushed, aren't they coherent?
They are not.
Ah, I figured they would be these days.
Nope, I've seen weird stuff happen when I forget to sync the icache even on very recent chips (albeit 85xx and not 83xx).
I doubt one needs to invalidate the icache though? Better safe than sorry I guess, one could probably optimize it a little bit though.
You need to flush the dcache and then invalidate the icache.
but if one haven't executed any insn's at the new location yet there should not be any cache lines in the icache that match the new location.
Ah. That's a special case. :-)
Still, better to set a good example.
-Scott
participants (5)
-
Joakim Tjernlund
-
Joakim Tjernlund
-
Kim Phillips
-
Scott Wood
-
Wolfgang Denk