Hi Albert,
On Thursday, October 4, 2012 3:39:41 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Tue, 14 Aug 2012 15:17:09 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Remove a redundant '#ifndef CONFIG_SYS_DCACHE_OFF' nested in the same #ifndef.
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Albert Aribaud albert.u.boot@aribaud.net
.../arch/arm/cpu/arm1136/cpu.c | 2 -- 1 file changed, 2 deletions(-)
diff --git u-boot-4d3c95f.orig/arch/arm/cpu/arm1136/cpu.c u-boot-4d3c95f/arch/arm/cpu/arm1136/cpu.c index b98e3d9..1136c1d 100644 --- u-boot-4d3c95f.orig/arch/arm/cpu/arm1136/cpu.c +++ u-boot-4d3c95f/arch/arm/cpu/arm1136/cpu.c @@ -146,9 +146,7 @@ void enable_caches(void) #ifndef CONFIG_SYS_ICACHE_OFF icache_enable(); #endif -#ifndef CONFIG_SYS_DCACHE_OFF dcache_enable(); -#endif }
#else /* #ifndef CONFIG_SYS_DCACHE_OFF */
I'll NAK this one because:
- obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else /#endif is
there to provide either working D$ functions or empty ones;
- enable_caches() exists only in the "then" branch, not at all in
the "else" branch, which makes it a surprising exception;
- enable_caches() is the only function in the if/then/else acting on
I$ as well as D$;
... so I suspect it did not actually belong in the big if/then/else in the first place and should not be modified but moved after the #endif.
I agree, simply because with the current code, enable_caches() does not enable icache if CONFIG_SYS_ICACHE_OFF is not defined but CONFIG_SYS_DCACHE_OFF is.
But is it enough to move it? We could indeed move it after the #endif, but also change it to:
--- #if !defined(CONFIG_SYS_ICACHE_OFF) || !defined(CONFIG_SYS_DCACHE_OFF) void enable_caches(void) { #ifndef CONFIG_SYS_ICACHE_OFF icache_enable(); #endif #ifndef CONFIG_SYS_DCACHE_OFF dcache_enable(); #endif } #endif ---
In that way, the default __enable_caches() from cache.c (outputting "WARNING: Caches not enabled\n") would be linked if both CONFIG_SYS_ICACHE_OFF and CONFIG_SYS_DCACHE_OFF are defined.
Do you agree?
Please provide a V2 patch, I promise it'll get processed faster than this V1.
OK ;) , I'll do that after your answer to the question above.
Best regards, Benoît