[U-Boot] [PATCH v2] ARM Cortex8 Rename and move v7_flush_dcache_all to flush_dcache
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com ---
Changes in v2: Rebase against recent git head 3b9043a7c03290c9bdbef03848307263f5f3472c "omap3: bug fix for NOR boot support"
This patch updates
http://lists.denx.de/pipermail/u-boot/2009-July/055543.html
Second patch of this series
http://lists.denx.de/pipermail/u-boot/2009-July/055544.html
seems to be fixed already, so doesn't seem to be necessary any more.
Compile tested with ./MAKEALL ARM_CORTEX_A8, boot tested on BeagleBoard.
cpu/arm_cortexa8/cpu.c | 2 cpu/arm_cortexa8/omap3/Makefile | 1 cpu/arm_cortexa8/omap3/board.c | 2 cpu/arm_cortexa8/omap3/cache_flush.S | 118 +++++++++++++++++++++++++++++++++ cpu/arm_cortexa8/start.S | 86 ------------------------ include/asm-arm/arch-omap3/sys_proto.h | 2 6 files changed, 122 insertions(+), 89 deletions(-) create mode 100644 cpu/arm_cortexa8/omap3/cache_flush.S
Index: u-boot-main/cpu/arm_cortexa8/cpu.c =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/cpu.c +++ u-boot-main/cpu/arm_cortexa8/cpu.c @@ -64,7 +64,7 @@ int cleanup_before_linux(void) /* turn off L2 cache */ l2_cache_disable(); /* invalidate L2 cache also */ - v7_flush_dcache_all(get_device_type()); + flush_dcache(get_device_type()); #endif i = 0; /* mem barrier to sync up things */ Index: u-boot-main/cpu/arm_cortexa8/omap3/board.c =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/board.c +++ u-boot-main/cpu/arm_cortexa8/omap3/board.c @@ -201,7 +201,7 @@ void s_init(void) * Right now flushing at low MPU speed. * Need to move after clock init */ - v7_flush_dcache_all(get_device_type()); + flush_dcache(get_device_type()); #ifndef CONFIG_ICACHE_OFF icache_enable(); #endif Index: u-boot-main/cpu/arm_cortexa8/omap3/cache_flush.S =================================================================== --- /dev/null +++ u-boot-main/cpu/arm_cortexa8/omap3/cache_flush.S @@ -0,0 +1,118 @@ +/* + * armboot - Startup Code for OMAP3530/ARM Cortex CPU-core + * + * Copyright (c) 2004 Texas Instruments r-woodruff2@ti.com + * + * Copyright (c) 2001 Marius Gröger mag@sysgo.de + * Copyright (c) 2002 Alex Züpke azu@sysgo.de + * Copyright (c) 2002 Gary Jennejohn garyj@denx.de + * Copyright (c) 2003 Richard Woodruff r-woodruff2@ti.com + * Copyright (c) 2003 Kshitij kshitij@ti.com + * Copyright (c) 2006-2008 Syed Mohammed Khasim x0khasim@ti.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +/* + * flush_dcache() + * + * Flush the whole D-cache. + * + * Corrupted registers: r0-r5, r7, r9-r11 + * + * - mm - mm_struct describing address space + */ + .align 5 +.global flush_dcache +flush_dcache: + stmfd r13!, {r0 - r5, r7, r9 - r12, r14} + + mov r7, r0 @ take a backup of device type + cmp r0, #0x3 @ check if the device type is + @ GP + moveq r12, #0x1 @ set up to invalide L2 +smi: .word 0x01600070 @ Call SMI monitor (smieq) + cmp r7, #0x3 @ compare again in case its + @ lost + beq finished_inval @ if GP device, inval done + @ above + + mrc p15, 1, r0, c0, c0, 1 @ read clidr + ands r3, r0, #0x7000000 @ extract loc from clidr + mov r3, r3, lsr #23 @ left align loc bit field + beq finished_inval @ if loc is 0, then no need to + @ clean + mov r10, #0 @ start clean at cache level 0 +inval_loop1: + add r2, r10, r10, lsr #1 @ work out 3x current cache + @ level + mov r1, r0, lsr r2 @ extract cache type bits from + @ clidr + and r1, r1, #7 @ mask of the bits for current + @ cache only + cmp r1, #2 @ see what cache we have at + @ this level + blt skip_inval @ skip if no cache, or just + @ i-cache + mcr p15, 2, r10, c0, c0, 0 @ select current cache level + @ in cssr + mov r2, #0 @ operand for mcr SBZ + mcr p15, 0, r2, c7, c5, 4 @ flush prefetch buffer to + @ sych the new cssr&csidr, + @ with armv7 this is 'isb', + @ but we compile with armv5 + mrc p15, 1, r1, c0, c0, 0 @ read the new csidr + and r2, r1, #7 @ extract the length of the + @ cache lines + add r2, r2, #4 @ add 4 (line length offset) + ldr r4, =0x3ff + ands r4, r4, r1, lsr #3 @ find maximum number on the + @ way size + clz r5, r4 @ find bit position of way + @ size increment + ldr r7, =0x7fff + ands r7, r7, r1, lsr #13 @ extract max number of the + @ index size +inval_loop2: + mov r9, r4 @ create working copy of max + @ way size +inval_loop3: + orr r11, r10, r9, lsl r5 @ factor way and cache number + @ into r11 + orr r11, r11, r7, lsl r2 @ factor index number into r11 + mcr p15, 0, r11, c7, c6, 2 @ invalidate by set/way + subs r9, r9, #1 @ decrement the way + bge inval_loop3 + subs r7, r7, #1 @ decrement the index + bge inval_loop2 +skip_inval: + add r10, r10, #2 @ increment cache number + cmp r3, r10 + bgt inval_loop1 +finished_inval: + mov r10, #0 @ swith back to cache level 0 + mcr p15, 2, r10, c0, c0, 0 @ select current cache level + @ in cssr + mcr p15, 0, r10, c7, c5, 4 @ flush prefetch buffer, + @ with armv7 this is 'isb', + @ but we compile with armv5 + + ldmfd r13!, {r0 - r5, r7, r9 - r12, pc} + + Index: u-boot-main/cpu/arm_cortexa8/start.S =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/start.S +++ u-boot-main/cpu/arm_cortexa8/start.S @@ -414,89 +414,3 @@ fiq: bl do_fiq
#endif - -/* - * v7_flush_dcache_all() - * - * Flush the whole D-cache. - * - * Corrupted registers: r0-r5, r7, r9-r11 - * - * - mm - mm_struct describing address space - */ - .align 5 -.global v7_flush_dcache_all -v7_flush_dcache_all: - stmfd r13!, {r0 - r5, r7, r9 - r12, r14} - - mov r7, r0 @ take a backup of device type - cmp r0, #0x3 @ check if the device type is - @ GP - moveq r12, #0x1 @ set up to invalide L2 -smi: .word 0x01600070 @ Call SMI monitor (smieq) - cmp r7, #0x3 @ compare again in case its - @ lost - beq finished_inval @ if GP device, inval done - @ above - - mrc p15, 1, r0, c0, c0, 1 @ read clidr - ands r3, r0, #0x7000000 @ extract loc from clidr - mov r3, r3, lsr #23 @ left align loc bit field - beq finished_inval @ if loc is 0, then no need to - @ clean - mov r10, #0 @ start clean at cache level 0 -inval_loop1: - add r2, r10, r10, lsr #1 @ work out 3x current cache - @ level - mov r1, r0, lsr r2 @ extract cache type bits from - @ clidr - and r1, r1, #7 @ mask of the bits for current - @ cache only - cmp r1, #2 @ see what cache we have at - @ this level - blt skip_inval @ skip if no cache, or just - @ i-cache - mcr p15, 2, r10, c0, c0, 0 @ select current cache level - @ in cssr - mov r2, #0 @ operand for mcr SBZ - mcr p15, 0, r2, c7, c5, 4 @ flush prefetch buffer to - @ sych the new cssr&csidr, - @ with armv7 this is 'isb', - @ but we compile with armv5 - mrc p15, 1, r1, c0, c0, 0 @ read the new csidr - and r2, r1, #7 @ extract the length of the - @ cache lines - add r2, r2, #4 @ add 4 (line length offset) - ldr r4, =0x3ff - ands r4, r4, r1, lsr #3 @ find maximum number on the - @ way size - clz r5, r4 @ find bit position of way - @ size increment - ldr r7, =0x7fff - ands r7, r7, r1, lsr #13 @ extract max number of the - @ index size -inval_loop2: - mov r9, r4 @ create working copy of max - @ way size -inval_loop3: - orr r11, r10, r9, lsl r5 @ factor way and cache number - @ into r11 - orr r11, r11, r7, lsl r2 @ factor index number into r11 - mcr p15, 0, r11, c7, c6, 2 @ invalidate by set/way - subs r9, r9, #1 @ decrement the way - bge inval_loop3 - subs r7, r7, #1 @ decrement the index - bge inval_loop2 -skip_inval: - add r10, r10, #2 @ increment cache number - cmp r3, r10 - bgt inval_loop1 -finished_inval: - mov r10, #0 @ swith back to cache level 0 - mcr p15, 2, r10, c0, c0, 0 @ select current cache level - @ in cssr - mcr p15, 0, r10, c7, c5, 4 @ flush prefetch buffer, - @ with armv7 this is 'isb', - @ but we compile with armv5 - - ldmfd r13!, {r0 - r5, r7, r9 - r12, pc} Index: u-boot-main/cpu/arm_cortexa8/omap3/Makefile =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/Makefile +++ u-boot-main/cpu/arm_cortexa8/omap3/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB = $(obj)lib$(SOC).a
SOBJS := lowlevel_init.o +SOBJS += cache_flush.o SOBJS += reset.o
COBJS += board.o Index: u-boot-main/include/asm-arm/arch-omap3/sys_proto.h =================================================================== --- u-boot-main.orig/include/asm-arm/arch-omap3/sys_proto.h +++ u-boot-main/include/asm-arm/arch-omap3/sys_proto.h @@ -55,7 +55,7 @@ void secureworld_exit(void); void setup_auxcr(void); void try_unlock_memory(void); u32 get_boot_type(void); -void v7_flush_dcache_all(u32); +void flush_dcache(u32); void sr32(void *, u32, u32, u32); u32 wait_on_value(u32, u32, void *, u32); void sdelay(unsigned long);
Applying two indepenent OMAP3 patches resulted in missing GPMC_CONFIG_CS0_BASE. Patch "omap3: embedd gpmc_cs into gpmc config struct" removes GPMC_CONFIG_CS0_BASE, independent patch "omap3: bug fix for NOR boot support" introduces it's usage. Re-introduce GPMC_CONFIG_CS0_BASE.
Signed-off-by: Dirk Behme dirk.behme@googlemail.com
--- include/asm-arm/arch-omap3/cpu.h | 1 + 1 file changed, 1 insertion(+)
Index: u-boot-main/include/asm-arm/arch-omap3/cpu.h =================================================================== --- u-boot-main.orig/include/asm-arm/arch-omap3/cpu.h +++ u-boot-main/include/asm-arm/arch-omap3/cpu.h @@ -91,6 +91,7 @@ struct ctrl_id {
#define GPMC_BASE (OMAP34XX_GPMC_BASE) #define GPMC_CONFIG_CS0 0x60 +#define GPMC_CONFIG_CS0_BASE (GPMC_BASE + GPMC_CONFIG_CS0)
#ifndef __KERNEL_STRICT_NAMES #ifndef __ASSEMBLY__
Dear Dirk Behme,
In message 1249728369-2738-2-git-send-email-dirk.behme@googlemail.com you wrote:
Applying two indepenent OMAP3 patches resulted in missing GPMC_CONFIG_CS0_BASE. Patch "omap3: embedd gpmc_cs into gpmc config struct" removes GPMC_CONFIG_CS0_BASE, independent patch "omap3: bug fix for NOR boot support" introduces it's usage. Re-introduce GPMC_CONFIG_CS0_BASE.
Signed-off-by: Dirk Behme dirk.behme@googlemail.com
include/asm-arm/arch-omap3/cpu.h | 1 + 1 file changed, 1 insertion(+)
Applied, thanks.
Best regards,
Wolfgang Denk
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
Best regards
Dirk
On 15:47 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
so NACK
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:47 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
so NACK
NACK your NACK, see above.
Best regards
Dirk
On 16:35 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:47 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 16:35 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:47 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote:
Since there is only one version of flushing the dcache for arm_cortex8, rename v7_flush_dcache_all to the the generic name flush_dcache. Because the function is intended for only omap3 boards, move the function to the new file cache_flush.S.
Signed-off-by: Tom Rix Tom.Rix@windriver.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com
NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name - Then, you mention "this code is omap3 specific" - Then, you mention "this code is not needed at all"
Sorry, but this sounds somehow confusing, better let us stop here.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more. Then everybody (including you) can send additional patches for further improvements.
Best regards
Dirk
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
the current implementation is wrong
there is NO NEED to have a omap3 specific full cache code as the generic armv7 full cache work fine on omap3
that's why rename it or move it make no sense as we can use the same full cache code on cortex a8
the only part that could be soc specific will the l2 flush code
that's why I NACK this as it does not fix anything and just will end with duplicated code
and yes I do want to have armv7 name in the armv cache function
I known you will tell but the other does not I agree the other arm flush code need to be clean and it's plan anyway as I need it to add the mmu and d-cache support which will be done later
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more. Then everybody (including you) can send additional patches for further improvements.
It's plan as I'm working currentlty on the mmu and d-cache support for arm and the current arm implementation will be udpated really soon to allow this and after I'll take a look in the relocation scheme on arm
Best Regards, J.
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090808152633.GD17045@game.jcrosoft.org you wrote:
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
the current implementation is wrong
I try to stay out of this flame war, but Jean-Christophenm I cannot prevent myself from stating that indeed with each noew message from you in this thread you come up with new arguments. This is not exactly a convincing method, and it makes me (and probably others) wonder if you are just argumenting because you want to have some fight or if ther eis really some technical argument.
If there is one, it should remain contant, so we can try to understand what you mean.
and yes I do want to have armv7 name in the armv cache function
And this is something which does not make any sense to me. If we have a function flush_dcache() it should be indeed a genric one, so it can be used everywhere in the code where we need to flush the data cache. What would be the sense to stick the architecture name in it? If we - for example - decide we need to flush the data cache somewhere in the generic code, then I want to be able to write
flush_dcache();
Your proposal sounds as if we would end up with
#if defined(_ARMV7_) armv7_flush_dcache(); #elif defined(_OMAP3_) omap3__flush_dcache(); ... #elif defined(_MIPS32_) mips32_flush_dcache(); #elif defined(_MIPS64_) mips64_flush_dcache(); ...
This cannot be your intention?
A _generig_ name is a must, I think.
I known you will tell but the other does not I agree the other arm flush code need to be clean and it's plan anyway as I need it to add the mmu and d-cache support which will be done later
Again this argument of some clenup that might be performed some time in the future. If this should happen one day, then changing one funcktion name more or less probably makes only marginal difference.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more. Then everybody (including you) can send additional patches for further improvements.
It's plan as I'm working currentlty on the mmu and d-cache support for arm and the current arm implementation will be udpated really soon to allow this and after I'll take a look in the relocation scheme on arm
So this is all plans for some future release. Fine.
But what we're discussing here is a patch for the current release. And we've been told that other development depends on getting this into a working state, so I even tend to consider this a bug fix.
I think specific needs for the current release must have higher priority than some unspecific and unconfirmed plans for some later release.
Best regards,
Wolfgang Denk
On 20:03 Sat 08 Aug , Wolfgang Denk wrote:
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090808152633.GD17045@game.jcrosoft.org you wrote:
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
the current implementation is wrong
I try to stay out of this flame war, but Jean-Christophenm I cannot prevent myself from stating that indeed with each noew message from you in this thread you come up with new arguments. This is not exactly a convincing method, and it makes me (and probably others) wonder if you are just argumenting because you want to have some fight or if ther eis really some technical argument.
If there is one, it should remain contant, so we can try to understand what you mean.
and yes I do want to have armv7 name in the armv cache function
And this is something which does not make any sense to me. If we have a function flush_dcache() it should be indeed a genric one, so it can be used everywhere in the code where we need to flush the data cache. What would be the sense to stick the architecture name in it? If we - for example - decide we need to flush the data cache somewhere in the generic code, then I want to be able to write
flush_dcache();
Your proposal sounds as if we would end up with
#if defined(_ARMV7_) armv7_flush_dcache(); #elif defined(_OMAP3_) omap3__flush_dcache();
again no need of omap3 flush code so just armv7_flush_cache
... #elif defined(_MIPS32_) mips32_flush_dcache(); #elif defined(_MIPS64_) mips64_flush_dcache(); ...
This cannot be your intention?
no on more and more soc as cortex from armltd the difference between two soc for U-Boot point of view will be the cache version which we detect via the cp15 and have only one U-Boot for both
A _generig_ name is a must, I think.
I known you will tell but the other does not I agree the other arm flush code need to be clean and it's plan anyway as I need it to add the mmu and d-cache support which will be done later
Again this argument of some clenup that might be performed some time in the future. If this should happen one day, then changing one funcktion name more or less probably makes only marginal difference.
no the patch cleanup nothing it just move code in omap3 that is mostly armv7 specific and not omap3 specific. The only part that is omap3 specfic is non need anyway so move it make no sense
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more. Then everybody (including you) can send additional patches for further improvements.
It's plan as I'm working currentlty on the mmu and d-cache support for arm and the current arm implementation will be udpated really soon to allow this and after I'll take a look in the relocation scheme on arm
So this is all plans for some future release. Fine.
But what we're discussing here is a patch for the current release. And we've been told that other development depends on getting this into a working state, so I even tend to consider this a bug fix.
as the current code fix nothing and on contrary introduce a bug as the code in most armv7 specifc not omap3
the true fix will to remove the omap3 code as it's not needed
Best Regards, J.
Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 16:35 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:47 Sat 08 Aug , Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 12:46 Sat 08 Aug , Dirk Behme wrote: > Since there is only one version of flushing the dcache for > arm_cortex8, rename v7_flush_dcache_all to the the generic > name flush_dcache. Because the function is intended for > only omap3 boards, move the function to the new file > cache_flush.S. > > Signed-off-by: Tom Rix Tom.Rix@windriver.com > Signed-off-by: Dirk Behme dirk.behme@googlemail.com > --- NACK keep the arm arch version in the name
NACK the NACK, see below.
and as request to Tom remove the non need device_type and we can safely use the v7 cache flush for omap3 and other cortex a8
This can be done easily (later?) by an additional patch (when people have time for it?).
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
Sorry, but this sounds somehow confusing, better let us stop here.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more.
At the weekend, I missed the point why this code is OMAP3 specific and why it has to be moved to omap3 directory, so short update:
It calls OMAP3 ROM code. This doesn't work on other Cortex A8, e.g. Samsung. That is, it must be moved to not stall others any more.
(Thanks to Tom for remembering this!)
Best regards
Dirk
Applying the basic functionality (function move) now allows others to finally go on with their long waiting patches.
no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
Sorry, but this sounds somehow confusing, better let us stop here.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more.
At the weekend, I missed the point why this code is OMAP3 specific and why it has to be moved to omap3 directory, so short update:
It calls OMAP3 ROM code. This doesn't work on other Cortex A8, e.g. Samsung. That is, it must be moved to not stall others any more.
so I'll repeat this only once there no need to call the rom code as the generic armv7 cache full work fine
so no-need to use omap3 specific code
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
> Applying the basic functionality (function move) now allows others > to finally go on with their long waiting patches. > no this code is omap3 specific and there is no need ot this rename or move the function make no sense
Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
Sorry, but this sounds somehow confusing, better let us stop here.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more.
At the weekend, I missed the point why this code is OMAP3 specific and why it has to be moved to omap3 directory, so short update:
It calls OMAP3 ROM code. This doesn't work on other Cortex A8, e.g. Samsung. That is, it must be moved to not stall others any more.
so I'll repeat this only once there no need to call the rom code as the generic armv7 cache full work fine
so no-need to use omap3 specific code
I am not sure if the generic code can replace the omap3 code. I do not have access to the ROM code. From what I have seen on the l2_cache code, it seems that in some cases that the ROM code handles earlier revs of the hardware. I would be in favor of phasing in the new generic code so that if older hardware needed to use the ROM code it could. Not know knowing full effect of the change we should go slowly and provide options for legacy support up front. Tom
Best Regards, J.
Tom wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Applying the basic functionality (function move) now allows others >> to finally go on with their long waiting patches. >> > no this code is omap3 specific and there is no need ot this > rename or move the function make no sense > Yes, it is OMAP3 specific (as already mentioned in the patch description). So it's totally fine to move it to an OMAP3 specific file. So it's totally fine that others (!= OMAP3, e.g Samsung) can re-use arm_cortexa8 stuff without the burden of OMAP3 stuff.
the flush MUST NOT be soc specific as there is NO need to do this at all
so NACK
Let's summarize:
- First, you NACK because of device_type and function name
- Then, you mention "this code is omap3 specific"
- Then, you mention "this code is not needed at all"
Sorry, but this sounds somehow confusing, better let us stop here.
As already mentioned, let us apply the patch so that other Cortex A8 patches are not stalled any more.
At the weekend, I missed the point why this code is OMAP3 specific and why it has to be moved to omap3 directory, so short update:
It calls OMAP3 ROM code. This doesn't work on other Cortex A8, e.g. Samsung. That is, it must be moved to not stall others any more.
so I'll repeat this only once there no need to call the rom code as the generic armv7 cache full work fine
so no-need to use omap3 specific code
I am not sure if the generic code can replace the omap3 code. I do not have access to the ROM code. From what I have seen on the l2_cache code, it seems that in some cases that the ROM code handles earlier revs of the hardware. I would be in favor of phasing in the new generic code so that if older hardware needed to use the ROM code it could. Not know knowing full effect of the change we should go slowly and provide options for legacy support up front.
Yes. Agreed.
Best regards
Dirk
participants (4)
-
Dirk Behme -
Jean-Christophe PLAGNIOL-VILLARD -
Tom -
Wolfgang Denk