Kyungmin Park wrote: > Hi, > > As he goes to home, I reply it instead.
Nice weekend then :) > On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behme<dirk.be...@googlemail.com> wrote: >> Dear Minkyu Kang, >> >> Minkyu Kang wrote: >>> Current code is supported only omap3 soc. >>> this patch will support s5pc1xx(s5pc100 and s5pc110) soc also. >> Thanks for this patch! >> >> How is this patch related to >> >> http://lists.denx.de/pipermail/u-boot/2009-August/058492.html >> > > It's not good idea to move invalidate_cache to omap directory. we need it. Well, yes and no ;) Most probably you (== Samsung) can't use the invalidate_dcache version we move in above patch to omap directory, because the version we move above is OMAP3 specific (it has calls to OMAP3 ROM code). So no, it's a good idea to move OMAP3 specific code to omap directory. But yes, you might need DCache flush (*). So the idea of above patch was to have your own (or generic) implementation, but let OMAP3 use the custom one where needed. (*) Do you really need DCache flush? It always was Jean-Christophe's argument that U-Boot doesn't use any DCache at all. >>> Signed-off-by: Minkyu Kang <mk7.k...@samsung.com> >>> --- >>> cpu/arm_cortexa8/cpu.c | 24 +++++++++++------------- >>> 1 files changed, 11 insertions(+), 13 deletions(-) >>> >>> diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c >>> index 5a5981e..3d430b1 100644 >>> --- a/cpu/arm_cortexa8/cpu.c >>> +++ b/cpu/arm_cortexa8/cpu.c >>> @@ -35,9 +35,6 @@ >>> #include <command.h> >>> #include <asm/system.h> >>> #include <asm/cache.h> >>> -#ifndef CONFIG_L2_OFF >>> -#include <asm/arch/sys_proto.h> >>> -#endif >>> >>> static void cache_flush(void); >>> >>> @@ -61,17 +58,18 @@ int cleanup_before_linux(void) >>> cache_flush(); >>> >>> #ifndef CONFIG_L2_OFF >>> - /* turn off L2 cache */ >>> - l2_cache_disable(); >>> - /* invalidate L2 cache also */ >>> - v7_flush_dcache_all(get_device_type()); >>> -#endif >>> - i = 0; >>> - /* mem barrier to sync up things */ >>> - asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >>> + if (get_device_type() != 0xC100) { >> Hmm, what is this "0xC100" ? > > Now we got two cpu, s5pc100 and s5pc110. In case of s5pc100 we don't > need to turn off l2 cache. but s5pc110 needs it. > So first check the device type, actually cpu type. then determine turn > off l2 cache or not. "0xC100" is the device type of s5pc100 then? So it should be if (get_device_type() != S5PC100_DEVICE) ? I hear some people crying "please use macro" ;) But I don't like this selection here. When we get additional similar SoCs, we will end with something like if (get_device_type() != 0xC100) || (get_device_type() != FOO) || (get_device_type() != BAR)) || ... { modifying each time cpu/arm_cortexa8/cpu.c. I would like more that we are able to compile the functionality based on the config file we use for compilation. E.g. provide emtpy l2_cache_disable(); function for SoCs that don't need it, but have functionality behind it where needed. With above patch, this would then become something like cpu/arm_cortexa8/s5pcxxx/dcache.S -> Implements invalidate_dcache() (or implement a Cortex A8 generic one in cpu/arm_cortexa8/cache.S) cpu/arm_cortexa8/s5pcxxx/cache_110.S -> Implements l2_cache_enable()/disable() cpu/arm_cortexa8/s5pcxxx/cache_100.S -> Implements *empty* l2_cache_enable()/disable() In cpu/arm_cortexa8/s5pcxxx/Makefile you then could have SOBJS-y += dcache.o SOBJS-$(CONFIG_S5PC100) += cache_100.o SOBJS-$(CONFIG_S5PC110) += cache_110.o What do you think about this? >>> + /* turn off L2 cache */ >>> + l2_cache_disable(); >>> + /* invalidate L2 cache also */ >>> + v7_flush_dcache_all(get_device_type()); >>> >>> -#ifndef CONFIG_L2_OFF >>> - l2_cache_enable(); >>> + i = 0; >>> + /* mem barrier to sync up things */ >>> + asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >>> + >>> + l2_cache_enable(); >>> + } >>> #endif >> What's about the order of #ifndef CONFIG_L2_OFF? >> >> While we had before >> >> >> #ifndef CONFIG_L2_OFF >> /* turn off L2 cache */ >> l2_cache_disable(); >> /* invalidate L2 cache also */ >> v7_flush_dcache_all(get_device_type()); >> #endif >> i = 0; >> /* mem barrier to sync up things */ >> asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >> #ifndef CONFIG_L2_OFF >> l2_cache_enable(); >> #endif >> >> >> after this patch we would have >> >> >> #ifndef CONFIG_L2_OFF >> if (get_device_type() != 0xC100) { >> /* turn off L2 cache */ >> l2_cache_disable(); >> /* invalidate L2 cache also */ >> v7_flush_dcache_all(get_device_type()); >> i = 0; >> /* mem barrier to sync up things */ >> asm("mcr p15, 0, %0, c7, c10, 4": :"r"(i)); >> >> l2_cache_enable(); >> } >> #endif >> >> Is this intended? > > maybe not. now we only tested the smdkc100 but actual use is internal > board for s5pc100 & s5pc110. > He will be modify it. Ok, thanks. Best regards Dirk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
