* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF @ 2012-08-14 13:17 Benoît Thébaudeau 2012-10-04 13:39 ` Albert ARIBAUD 2012-10-04 20:04 ` [U-Boot] [PATCH v2] arm1136: Fix enable_caches() Benoît Thébaudeau 0 siblings, 2 replies; 10+ messages in thread From: Benoît Thébaudeau @ 2012-08-14 13:17 UTC (permalink / raw) To: u-boot 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 */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF 2012-08-14 13:17 [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF Benoît Thébaudeau @ 2012-10-04 13:39 ` Albert ARIBAUD 2012-10-04 16:57 ` Benoît Thébaudeau 2012-10-04 20:04 ` [U-Boot] [PATCH v2] arm1136: Fix enable_caches() Benoît Thébaudeau 1 sibling, 1 reply; 10+ messages in thread From: Albert ARIBAUD @ 2012-10-04 13:39 UTC (permalink / raw) To: u-boot 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: 1) obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else /#endif is there to provide either working D$ functions or empty ones; 2) enable_caches() exists only in the "then" branch, not at all in the "else" branch, which makes it a surprising exception; 3) 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. Please provide a V2 patch, I promise it'll get processed faster than this V1. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF 2012-10-04 13:39 ` Albert ARIBAUD @ 2012-10-04 16:57 ` Benoît Thébaudeau 2012-10-05 17:05 ` Albert ARIBAUD 0 siblings, 1 reply; 10+ messages in thread From: Benoît Thébaudeau @ 2012-10-04 16:57 UTC (permalink / raw) To: u-boot 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: > > 1) obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else /#endif is > there to provide either working D$ functions or empty ones; > > 2) enable_caches() exists only in the "then" branch, not at all in > the > "else" branch, which makes it a surprising exception; > > 3) 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF 2012-10-04 16:57 ` Benoît Thébaudeau @ 2012-10-05 17:05 ` Albert ARIBAUD 2012-10-05 17:36 ` Benoît Thébaudeau 0 siblings, 1 reply; 10+ messages in thread From: Albert ARIBAUD @ 2012-10-05 17:05 UTC (permalink / raw) To: u-boot Hi Beno?t, On Thu, 4 Oct 2012 18:57:19 +0200 (CEST), Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> wrote: > 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: > > > > 1) obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else /#endif is > > there to provide either working D$ functions or empty ones; > > > > 2) enable_caches() exists only in the "then" branch, not at all in > > the > > "else" branch, which makes it a surprising exception; > > > > 3) 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? When CONFIG_SYS_DCACHE_OFF, dcache functions are not compiled out, they are compiled in but empty. IOW, even with caches off, the API remains callable albeit useless. This is done so that client code does not have to test CONFIG_SYS_DCACHE_OFF before deciding to call the API or not. Therefore, for consistency, enable_caches() should be defined and empty even when both CONFIG_SYS_DCACHE_OFF and CONFIG_SYS_ICACHE_OFF are defined, which is not the case in the example above due to the enclosing #if/#endif. > > 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 Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF 2012-10-05 17:05 ` Albert ARIBAUD @ 2012-10-05 17:36 ` Benoît Thébaudeau 2012-10-05 18:15 ` Albert ARIBAUD 0 siblings, 1 reply; 10+ messages in thread From: Benoît Thébaudeau @ 2012-10-05 17:36 UTC (permalink / raw) To: u-boot Hi Albert, On Friday, October 5, 2012 7:05:19 PM, Albert ARIBAUD wrote: > Hi Beno?t, > > On Thu, 4 Oct 2012 18:57:19 +0200 (CEST), Beno?t Th?baudeau > <benoit.thebaudeau@advansee.com> wrote: > > > 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: > > > > > > 1) obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else > > > /#endif is > > > there to provide either working D$ functions or empty ones; > > > > > > 2) enable_caches() exists only in the "then" branch, not at all > > > in > > > the > > > "else" branch, which makes it a surprising exception; > > > > > > 3) 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? > > When CONFIG_SYS_DCACHE_OFF, dcache functions are not compiled out, > they > are compiled in but empty. IOW, even with caches off, the API remains > callable albeit useless. This is done so that client code does not > have to test CONFIG_SYS_DCACHE_OFF before deciding to call the API or > not. > > Therefore, for consistency, enable_caches() should be defined and > empty > even when both CONFIG_SYS_DCACHE_OFF and CONFIG_SYS_ICACHE_OFF are > defined, which is not the case in the example above due to the > enclosing #if/#endif. In the example, enable_caches() is still defined if both CONFIG_SYS_DCACHE_OFF and CONFIG_SYS_ICACHE_OFF are defined. See arch/arm/lib/cache.c: --- /* * Default implementation of enable_caches() * Real implementation should be in platform code */ void __enable_caches(void) { puts("WARNING: Caches not enabled\n"); } void enable_caches(void) __attribute__((weak, alias("__enable_caches"))); --- But you're right, it's not empty in this case. Is it that you want to remove this message in this case? This default implementation is used in the same way for several other ARM targets. Best regards, Beno?t ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF 2012-10-05 17:36 ` Benoît Thébaudeau @ 2012-10-05 18:15 ` Albert ARIBAUD 0 siblings, 0 replies; 10+ messages in thread From: Albert ARIBAUD @ 2012-10-05 18:15 UTC (permalink / raw) To: u-boot Hi Beno?t, On Fri, 5 Oct 2012 19:36:34 +0200 (CEST), Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> wrote: > Hi Albert, > > On Friday, October 5, 2012 7:05:19 PM, Albert ARIBAUD wrote: > > Hi Beno?t, > > > > On Thu, 4 Oct 2012 18:57:19 +0200 (CEST), Beno?t Th?baudeau > > <benoit.thebaudeau@advansee.com> wrote: > > > > > 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: > > > > > > > > 1) obviously the big #ifndef CONFIG_SYS_DCACHE_OFF / #else > > > > /#endif is > > > > there to provide either working D$ functions or empty ones; > > > > > > > > 2) enable_caches() exists only in the "then" branch, not at all > > > > in > > > > the > > > > "else" branch, which makes it a surprising exception; > > > > > > > > 3) 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? > > > > When CONFIG_SYS_DCACHE_OFF, dcache functions are not compiled out, > > they > > are compiled in but empty. IOW, even with caches off, the API remains > > callable albeit useless. This is done so that client code does not > > have to test CONFIG_SYS_DCACHE_OFF before deciding to call the API or > > not. > > > > Therefore, for consistency, enable_caches() should be defined and > > empty > > even when both CONFIG_SYS_DCACHE_OFF and CONFIG_SYS_ICACHE_OFF are > > defined, which is not the case in the example above due to the > > enclosing #if/#endif. > > In the example, enable_caches() is still defined if both CONFIG_SYS_DCACHE_OFF > and CONFIG_SYS_ICACHE_OFF are defined. See arch/arm/lib/cache.c: > --- > /* > * Default implementation of enable_caches() > * Real implementation should be in platform code > */ > void __enable_caches(void) > { > puts("WARNING: Caches not enabled\n"); > } > void enable_caches(void) > __attribute__((weak, alias("__enable_caches"))); > --- > > But you're right, it's not empty in this case. Is it that you want to remove > this message in this case? > > This default implementation is used in the same way for several other ARM > targets. OK, I see your point: enable_cache() needs not be defined here if caches are #define'd out, because there is a weak default definition in arch/arm/lib/cache.c that just prints a warning about caches not being enabled. But then, the same holds for flush_dcache_all and flush_cache, which are not compiled out from as enable_cache is -- and if they were, then the arch/arm/lib/cache.c versions would kick in, and those are not empty, especially __flush_cache which contains a conditional call to... arm1136_cache_flush() That is certainly not consistent... And on second thought, not defining a cache-related function at cpu level if caches are compiled out is more sensible than defining it empty. All this could undergo a greater change to move cpu-specifics out of the ARM library so that all lib functions are empty or generic, and define cpu-specific functions only if caches are compiled in... But I won't ask for something that's far beyond the goal of your patch. Conclusion: V2 is good as it is. I'll apply it on /next. > Best regards, > Beno?t Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2] arm1136: Fix enable_caches() 2012-08-14 13:17 [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF Benoît Thébaudeau 2012-10-04 13:39 ` Albert ARIBAUD @ 2012-10-04 20:04 ` Benoît Thébaudeau 2012-10-05 18:23 ` Albert ARIBAUD 1 sibling, 1 reply; 10+ messages in thread From: Benoît Thébaudeau @ 2012-10-04 20:04 UTC (permalink / raw) To: u-boot enable_caches() did not enable icache if CONFIG_SYS_ICACHE_OFF was not defined but CONFIG_SYS_DCACHE_OFF was. Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> Cc: Albert Aribaud <albert.u.boot@aribaud.net> --- This patch supersedes http://patchwork.ozlabs.org/patch/177263/ . Changes for v2: - Move enable_caches() outside of the main #if in order to fix this icache issue rather than doing only #if cosmetic cleanup. .../arch/arm/cpu/arm1136/cpu.c | 22 +++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c index b98e3d9..32a4c24 100644 --- u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c +++ u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c @@ -141,16 +141,6 @@ void flush_cache(unsigned long start, unsigned long size) flush_dcache_range(start, start + size); } -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 */ void invalidate_dcache_all(void) { @@ -172,3 +162,15 @@ void flush_cache(unsigned long start, unsigned long size) { } #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */ + +#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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2] arm1136: Fix enable_caches() 2012-10-04 20:04 ` [U-Boot] [PATCH v2] arm1136: Fix enable_caches() Benoît Thébaudeau @ 2012-10-05 18:23 ` Albert ARIBAUD 2012-10-31 14:41 ` Benoît Thébaudeau 0 siblings, 1 reply; 10+ messages in thread From: Albert ARIBAUD @ 2012-10-05 18:23 UTC (permalink / raw) To: u-boot Beno?t, On Thu, 4 Oct 2012 22:04:02 +0200 (CEST), Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> wrote: > enable_caches() did not enable icache if CONFIG_SYS_ICACHE_OFF was not defined > but CONFIG_SYS_DCACHE_OFF was. > > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > --- > This patch supersedes http://patchwork.ozlabs.org/patch/177263/ . > > Changes for v2: > - Move enable_caches() outside of the main #if in order to fix this icache > issue rather than doing only #if cosmetic cleanup. > > .../arch/arm/cpu/arm1136/cpu.c | 22 +++++++++++--------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > index b98e3d9..32a4c24 100644 > --- u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c > +++ u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > @@ -141,16 +141,6 @@ void flush_cache(unsigned long start, unsigned long size) > flush_dcache_range(start, start + size); > } > > -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 */ > void invalidate_dcache_all(void) > { > @@ -172,3 +162,15 @@ void flush_cache(unsigned long start, unsigned long size) > { > } > #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */ > + > +#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 > Applied to u-boot-arm/next, thanks! Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2] arm1136: Fix enable_caches() 2012-10-05 18:23 ` Albert ARIBAUD @ 2012-10-31 14:41 ` Benoît Thébaudeau 2012-11-10 11:29 ` Albert ARIBAUD 0 siblings, 1 reply; 10+ messages in thread From: Benoît Thébaudeau @ 2012-10-31 14:41 UTC (permalink / raw) To: u-boot Hi Albert, On Friday, October 5, 2012 8:23:14 PM, Albert ARIBAUD wrote; > On Thu, 4 Oct 2012 22:04:02 +0200 (CEST), Beno?t Th?baudeau > <benoit.thebaudeau@advansee.com> wrote: > > > enable_caches() did not enable icache if CONFIG_SYS_ICACHE_OFF was > > not defined > > but CONFIG_SYS_DCACHE_OFF was. > > > > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> > > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > > --- > > This patch supersedes http://patchwork.ozlabs.org/patch/177263/ . > > > > Changes for v2: > > - Move enable_caches() outside of the main #if in order to fix > > this icache > > issue rather than doing only #if cosmetic cleanup. > > > > .../arch/arm/cpu/arm1136/cpu.c | 22 > > +++++++++++--------- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c > > u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > > index b98e3d9..32a4c24 100644 > > --- u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c > > +++ u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > > @@ -141,16 +141,6 @@ void flush_cache(unsigned long start, unsigned > > long size) > > flush_dcache_range(start, start + size); > > } > > > > -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 */ > > void invalidate_dcache_all(void) > > { > > @@ -172,3 +162,15 @@ void flush_cache(unsigned long start, unsigned > > long size) > > { > > } > > #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */ > > + > > +#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 > > > > Applied to u-boot-arm/next, thanks! I know that's true (I had seen it applied), but it has vanished since then. Now, I can't find it in u-boot-arm/next nor in u-boot-arm/master. This is perhaps related to the rebase that you had to do at some point. Best regards, Beno?t ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2] arm1136: Fix enable_caches() 2012-10-31 14:41 ` Benoît Thébaudeau @ 2012-11-10 11:29 ` Albert ARIBAUD 0 siblings, 0 replies; 10+ messages in thread From: Albert ARIBAUD @ 2012-11-10 11:29 UTC (permalink / raw) To: u-boot Hi Beno?t, On Wed, 31 Oct 2012 15:41:02 +0100 (CET), Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> wrote: > Hi Albert, > > On Friday, October 5, 2012 8:23:14 PM, Albert ARIBAUD wrote; > > On Thu, 4 Oct 2012 22:04:02 +0200 (CEST), Beno?t Th?baudeau > > <benoit.thebaudeau@advansee.com> wrote: > > > > > enable_caches() did not enable icache if CONFIG_SYS_ICACHE_OFF was > > > not defined > > > but CONFIG_SYS_DCACHE_OFF was. > > > > > > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com> > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > > > --- > > > This patch supersedes http://patchwork.ozlabs.org/patch/177263/ . > > > > > > Changes for v2: > > > - Move enable_caches() outside of the main #if in order to fix > > > this icache > > > issue rather than doing only #if cosmetic cleanup. > > > > > > .../arch/arm/cpu/arm1136/cpu.c | 22 > > > +++++++++++--------- > > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > > > diff --git u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c > > > u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > > > index b98e3d9..32a4c24 100644 > > > --- u-boot-arm-1dfc916.orig/arch/arm/cpu/arm1136/cpu.c > > > +++ u-boot-arm-1dfc916/arch/arm/cpu/arm1136/cpu.c > > > @@ -141,16 +141,6 @@ void flush_cache(unsigned long start, unsigned > > > long size) > > > flush_dcache_range(start, start + size); > > > } > > > > > > -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 */ > > > void invalidate_dcache_all(void) > > > { > > > @@ -172,3 +162,15 @@ void flush_cache(unsigned long start, unsigned > > > long size) > > > { > > > } > > > #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */ > > > + > > > +#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 > > > > > > > Applied to u-boot-arm/next, thanks! > > I know that's true (I had seen it applied), but it has vanished since then. Now, > I can't find it in u-boot-arm/next nor in u-boot-arm/master. This is perhaps > related to the rebase that you had to do at some point. > > Best regards, > Beno?t RE-applied to u-boot-arm/master, thanks for pointing this out and apologies for the temporary disappearance. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-11-10 11:29 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-14 13:17 [U-Boot] [PATCH] arm1136: cosmetic: Remove double test on CONFIG_SYS_DCACHE_OFF Benoît Thébaudeau 2012-10-04 13:39 ` Albert ARIBAUD 2012-10-04 16:57 ` Benoît Thébaudeau 2012-10-05 17:05 ` Albert ARIBAUD 2012-10-05 17:36 ` Benoît Thébaudeau 2012-10-05 18:15 ` Albert ARIBAUD 2012-10-04 20:04 ` [U-Boot] [PATCH v2] arm1136: Fix enable_caches() Benoît Thébaudeau 2012-10-05 18:23 ` Albert ARIBAUD 2012-10-31 14:41 ` Benoît Thébaudeau 2012-11-10 11:29 ` Albert ARIBAUD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox