[U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Current code is supported only omap3 soc. this patch will support s5pc1xx(s5pc100 and s5pc110) soc also.
Signed-off-by: Minkyu Kang mk7.kang@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) { + /* 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
return 0;
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
?
Signed-off-by: Minkyu Kang mk7.kang@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" ?
/* 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?
Best regards
Dirk
Hi,
As he goes to home, I reply it instead.
On Fri, Sep 4, 2009 at 5:43 PM, Dirk Behmedirk.behme@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.
Signed-off-by: Minkyu Kang mk7.kang@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.
- /* 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.
Thank you, Kyungmin Park
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 Behmedirk.behme@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.kang@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
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com wrote:
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 Behmedirk.behme@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.
Yes, We really want it. At first development time, we don't know why the kernel is boot. almost same as s5pc100 but s5pc110 required cache flush.
Signed-off-by: Minkyu Kang mk7.kang@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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here.
Thank you, Kyungmin Park
Kyungmin Park wrote:
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com wrote:
Kyungmin Park wrote:
...
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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here.
What's about having this check in SoC specific code instead of Cortex A8 generic code, then?
E.g apply patch
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
and then create
cpu/arm_cortexa8/s5pcxxx/cache.S
with
invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ...
l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ...
etc.
That is, have the SoC dependent part in SoC specific directory/file.
Best regards
Dirk
Dear, Dirk
2009/9/4 Dirk Behme dirk.behme@googlemail.com
Kyungmin Park wrote:
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com
wrote:
Kyungmin Park wrote:
...
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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here.
What's about having this check in SoC specific code instead of Cortex A8 generic code, then?
E.g apply patch
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
and then create
cpu/arm_cortexa8/s5pcxxx/cache.S
with
invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ...
l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ...
etc.
That is, have the SoC dependent part in SoC specific directory/file.
Best regards
Dirk
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I know about the discussion of this issue between you and Jean-Christophe. but it is gone without resolving. So, I want to make issue again. anyway,,
Actually, we don't need the function of get_device_type() I think that function is omap specific function.. isn't it? but.. because of current code already use that function, I had to use that function If you have plan to move the cache routines into SoC, I think you can remove the argument for device_type. (check device type in omap3's cache routines)
And I want to remove CONFIG_L2_OFF also. We can know this through device type or soc type. How about make new function? e.g l2_off() or need_cache_flush() etc,
Please rework for removing dependency of omap3 soc first. And then, I'll make new patch for s5pc1xx soc.
Many thanks :) Minkyu Kang.
Minkyu Kang wrote:
Dear, Dirk
2009/9/4 Dirk Behme dirk.behme@googlemail.com
Kyungmin Park wrote:
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com
wrote:
Kyungmin Park wrote:
...
> + 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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we have to support both cpu simultaneously. with this reason. we check the device_type at here.
What's about having this check in SoC specific code instead of Cortex A8 generic code, then?
E.g apply patch
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
and then create
cpu/arm_cortexa8/s5pcxxx/cache.S
with
invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ...
l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ...
etc.
That is, have the SoC dependent part in SoC specific directory/file.
Best regards
Dirk
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I know about the discussion of this issue between you and Jean-Christophe. but it is gone without resolving. So, I want to make issue again. anyway,,
Actually, we don't need the function of get_device_type() I think that function is omap specific function.. isn't it? but.. because of current code already use that function, I had to use that function If you have plan to move the cache routines into SoC, I think you can remove the argument for device_type. (check device type in omap3's cache routines)
And I want to remove CONFIG_L2_OFF also. We can know this through device type or soc type. How about make new function? e.g l2_off() or need_cache_flush() etc,
Please rework for removing dependency of omap3 soc first.
Just to clarify: It's my understanding that this is already done by
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Do you agree? That is, when this patch is applied, then Samsung can go on. Correct?
If not correct, what is missing in above patch?
Best regards
Dirk
Dear Dirk,
2009/9/5 Dirk Behme dirk.behme@googlemail.com
Minkyu Kang wrote:
Dear, Dirk
2009/9/4 Dirk Behme dirk.behme@googlemail.com
Kyungmin Park wrote:
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com
wrote:
Kyungmin Park wrote:
...
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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we
have to support both cpu simultaneously. with this reason. we check the device_type at here.
What's about having this check in SoC specific code instead of Cortex A8 generic code, then?
E.g apply patch
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
and then create
cpu/arm_cortexa8/s5pcxxx/cache.S
with
invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ...
l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ...
etc.
That is, have the SoC dependent part in SoC specific directory/file.
Best regards
Dirk
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I know about the discussion of this issue between you and Jean-Christophe. but it is gone without resolving. So, I want to make issue again. anyway,,
Actually, we don't need the function of get_device_type() I think that function is omap specific function.. isn't it? but.. because of current code already use that function, I had to use that function If you have plan to move the cache routines into SoC, I think you can remove the argument for device_type. (check device type in omap3's cache routines)
And I want to remove CONFIG_L2_OFF also. We can know this through device type or soc type. How about make new function? e.g l2_off() or need_cache_flush() etc,
Please rework for removing dependency of omap3 soc first.
Just to clarify: It's my understanding that this is already done by
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Do you agree? That is, when this patch is applied, then Samsung can go on. Correct?
If not correct, what is missing in above patch?
Best regards
Dirk
I have two request.
1. I want to replace CONFIG_L2_OFF define to other function
for example..
if (need_cache_flush()) { /* or !l2_off() */ /* 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(); }
2. I don't want to use get_device_type() function (just call like this: v7_flush_decahe_all() )
How you think?
thanks Minkyu Kang
Dear Minkyu Kang,
Minkyu Kang wrote:
Dear Dirk,
2009/9/5 Dirk Behme dirk.behme@googlemail.com
Minkyu Kang wrote:
Dear, Dirk
2009/9/4 Dirk Behme dirk.behme@googlemail.com
Kyungmin Park wrote:
On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behmedirk.behme@googlemail.com
wrote:
Kyungmin Park wrote: ...
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" ;)
Agreed. DONT_NEED_CACHE_FLUSH?
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?
Basically agreed, of course we can think weak attribute but now we
have to support both cpu simultaneously. with this reason. we check the device_type at here.
What's about having this check in SoC specific code instead of Cortex A8 generic code, then?
E.g apply patch
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
and then create
cpu/arm_cortexa8/s5pcxxx/cache.S
with
invalidate_dcache() { if (get_device_type() == S5PC100_DEVICE) return(); ...
l2_cache_enable() { if (get_device_type() == S5PC100_DEVICE) return(); ...
etc.
That is, have the SoC dependent part in SoC specific directory/file.
Best regards
Dirk
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I know about the discussion of this issue between you and Jean-Christophe. but it is gone without resolving. So, I want to make issue again. anyway,,
Actually, we don't need the function of get_device_type() I think that function is omap specific function.. isn't it? but.. because of current code already use that function, I had to use that function If you have plan to move the cache routines into SoC, I think you can remove the argument for device_type. (check device type in omap3's cache routines)
And I want to remove CONFIG_L2_OFF also. We can know this through device type or soc type. How about make new function? e.g l2_off() or need_cache_flush() etc,
Please rework for removing dependency of omap3 soc first.
Just to clarify: It's my understanding that this is already done by
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Do you agree? That is, when this patch is applied, then Samsung can go on. Correct?
If not correct, what is missing in above patch?
Best regards
Dirk
I have two request.
- I want to replace CONFIG_L2_OFF define to other function
for example..
if (need_cache_flush()) { /* or !l2_off() */ /* 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(); }
- I don't want to use get_device_type() function
(just call like this: v7_flush_decahe_all() )
How you think?
I wonder if these two requests are
- nice to have topics? Or
- required changes?
And if these are required changes, if
- these can go on top of
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
so that we can apply above patch, you are able to go on to bring Samsung into mainline? And in parallel we discuss/change above topics? Or
- we have to rewrite
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
what will stall Samsungs mainline integration further?
Best regards
Dirk
Dear Dirk,
I have two request.
- I want to replace CONFIG_L2_OFF define to other function
for example..
if (need_cache_flush()) { /* or !l2_off() */ /* 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();}
- I don't want to use get_device_type() function
(just call like this: v7_flush_decahe_all() )
How you think?
I wonder if these two requests are
nice to have topics? Or
required changes?
request no.1 is partly "required" but not now, it is required for supporting s5pc110 soc. because of we want to use same binary at runtime, so have to avoid ifdef of cause, we can use other ways.. but i think it's good way that is removing ifdef
request no.2 is just "request" If you don't want to accept this, we can use the function. but I think get_device_type() does not fit with cpu common file
And if these are required changes, if
- these can go on top of
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
so that we can apply above patch, you are able to go on to bring Samsung into mainline? And in parallel we discuss/change above topics? Or
- we have to rewrite
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
what will stall Samsungs mainline integration further?
Best regards
Dirk
As a result, you can apply the patch. but I hope we will process my request soon.
Thanks :) Minkyu Kang
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread. When last I worked this, it seemed like the cache routines were going to be split.
1. generic cache routines 2. legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
Tom
Thanks :) Minkyu Kang
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
1) From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
3) In parallel, we should discuss how to further improve the OMAP3 cache stuff. What might be generic, what not, what isn't necessary etc.
4) Regarding (cache) code duplication, I vote for doing this duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3 cache
stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this duplication
first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
Tom wrote:
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3
cache stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this
duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
Although I did not understand all of the cache routine.. the s5pc100 soc doesn't need v7_flush_dcache_all function and other cache routines. But the s5pc110 soc needs this function, and it works fine without modification. (please see my patch)
I think.. v7_flush_decache_all function can share together.
If this function is not moved to soc layer, need to remove omap specific codes (checking device type)
Thanks. Minkyu Kang
Minkyu Kang wrote:
Tom wrote:
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3
cache stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this
duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
Although I did not understand all of the cache routine.. the s5pc100 soc doesn't need v7_flush_dcache_all function and other cache routines. But the s5pc110 soc needs this function, and it works fine without modification. (please see my patch)
Please send me a link to your patch.
Tom
I think.. v7_flush_decache_all function can share together.
If this function is not moved to soc layer, need to remove omap specific codes (checking device type)
Thanks. Minkyu Kang
Dear Tom,
2009/9/9 Tom Tom.Rix@windriver.com:
Minkyu Kang wrote:
Tom wrote:
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3
cache stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this
duplication first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
Although I did not understand all of the cache routine.. the s5pc100 soc doesn't need v7_flush_dcache_all function and other cache routines. But the s5pc110 soc needs this function, and it works fine without modification. (please see my patch)
Please send me a link to your patch.
Tom
I think.. v7_flush_decache_all function can share together.
If this function is not moved to soc layer, need to remove omap specific codes (checking device type)
Thanks. Minkyu Kang
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
http://lists.denx.de/pipermail/u-boot/2009-September/059889.html
top of this thread thanks.
Sandeep, Can you push this change to uboot-ti ? Tom
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Tom wrote:
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going to be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all. E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees the way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about removal of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3 cache
stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this duplication
first. That is, have working Samsung and OMAP3 code applied in parallel. While Jean-Christophe might cry "code duplication", I learned from OMAP3 boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more comfortable with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still discussing about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-arm Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
OK.
Will do
-----Original Message----- From: Tom [mailto:Tom.Rix@windriver.com] Sent: Thursday, September 10, 2009 3:02 PM To: Paulraj, Sandeep Cc: Dirk Behme; u-boot@lists.denx.de; Minkyu Kang Subject: Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
Sandeep, Can you push this change to uboot-ti ? Tom
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Tom wrote:
Dirk Behme wrote:
Tom wrote:
Minkyu Kang wrote:
Dear Dirk,
<snip>
I have lost track of this thread.
Yes, and I'm currently trying to get the track back ;)
When last I worked this, it seemed like the cache routines were going
to
be split.
- generic cache routines
- legacy soc cache routines.
Are the generic cache routines in place and can you use them? Else can you handle your soc specific cache routines as part of a legacy cache routine ?
The omap cache routines are dependent on omap rom code and fitting new routines in using the omap specifics may not be the best way to go.
I'm sure this is not the perfect way, but it seems to me that splitting all this stuff into several small steps would be the easier for all.
E.g.
- From the previous discussion I think we should apply
http://lists.denx.de/pipermail/u-boot/2009-August/058492.html
Independent of any discussion if this code is needed at all, if it is generic or not etc. the main advantage I understood is that it frees
the
way for Samsung.
2a) Then, what I understood from Minkyu, we need an additional patch (discussion?) how to switch CONFIG_L2_OFF from compile time to run time selection (for Samsung's multi board support)
2b) Then, what I understood from Minkyu, we should discuss about
removal
of get_device_type() function
- In parallel, we should discuss how to further improve the OMAP3
cache
stuff. What might be generic, what not, what isn't necessary etc.
- Regarding (cache) code duplication, I vote for doing this
duplication
first. That is, have working Samsung and OMAP3 code applied in
parallel.
While Jean-Christophe might cry "code duplication", I learned from
OMAP3
boards that is was easier to unify common code _after_ code duplication was done and therefore can be easily identified. Discussing about possible code duplication without being able to see (and test) the code is sometimes a little tricky ;)
As mentioned, doing this in several, small steps I feel more
comfortable
with. If we would have done step (1) already, it's my feeling that we would have reached step 2 or 3 already. But now, we are still
discussing
about the 'one big perfect patch'.
Best regards
Dirk
I am making this workflow up as I go.. but it seems like the way to resolve this is to work through the new sub-arm repo's
#1 should go to u-boot-ti first and then I will will merge it to u-boot-
arm
Then u-boot-samsung can sync to it and we will all be at a good starting point for #2. Can #1 go now to u-boot-ti ? If there is a merge problem, kick it back to the developer ;)
This patch moves the cache support out of A8 and into omap3 which is the correct place for it.
I assume the samsung changes are going to go first to u-boot-samsung Correct ?
For 2a) runtime cache enabling / disabling New feature I don't think omap needs so it should be at some board or soc level that does not impact omap.
For 2b) get_device_type. This is an omap-ism that goes away with #1
The means though that samsung needs its own cache routines. If they are going to do 2a) they will need them anyway.
For 3) I don't think omap needs improving at this point.
For 4) Lets see how much the samsung cache routines look like the omap once they are done. If it is a lot of cut-n-paste this is not worth arguing about a common routine will be easier to manage. If the cache code looks really different then it can live at the board or soc layer. As long as no one claims the cpu layer at the very least everyone's board will work.
Tom
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 12:45 Fri 04 Sep , Dirk Behme wrote:
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 Behmedirk.behme@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.
It's certainly not as the OMAP3 does not need to call the ROM code so we can share the same for for S3P and omap3
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.
no need for omap3
(*) Do you really need DCache flush? It always was Jean-Christophe's argument that U-Boot doesn't use any DCache at all.
It's true but U-Boot is not necessarely the first stage loader so yes we will clean all cache
"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.
multiple soc support will clearly simplify board support and improve code testing but it's need to have the possiblity to be disable as the compile to minimize the size impact
Best Regards, n J
Dear Kyungmin Park,
In message 9c9fda240909040234m4fdd7466ybb38d0d0618cdf32@mail.gmail.com you wrote:
...
#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.
Well, this definitely needs a comment, doesn;t it?
And I really dislike such board-specific parts in common code. If what you want to check is the CPU type, then isn;t there a more direct way?
And do we need a runtime check here? I think that decision can be made at compile time, right?
...
maybe not. now we only tested the smdkc100 but actual use is internal board for s5pc100 & s5pc110.
Do you use the very same U-Boot image on both SoCs?
Best regards,
Wolfgang Denk
On Fri, Sep 4, 2009 at 8:06 PM, Wolfgang Denkwd@denx.de wrote:
Dear Kyungmin Park,
In message 9c9fda240909040234m4fdd7466ybb38d0d0618cdf32@mail.gmail.com you wrote:
...
#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.
Well, this definitely needs a comment, doesn;t it?
And I really dislike such board-specific parts in common code. If what you want to check is the CPU type, then isn;t there a more direct way?
And do we need a runtime check here? I think that decision can be made at compile time, right?
To support two different CPU. If it determined at compile time. we can't support different CPUs
...
maybe not. now we only tested the smdkc100 but actual use is internal board for s5pc100 & s5pc110.
Do you use the very same U-Boot image on both SoCs?
Yes. One U-Boot image support 2 different CPU and 7 different boards. We don't want to make a u-boot for each board.
Thank you, Kyungmin Park
Dear Kyungmin Park,
In message 9c9fda240909040411w468baddv9cdd59fafeab2ffd@mail.gmail.com you wrote:
Do you use the very same U-Boot image on both SoCs?
Yes. One U-Boot image support 2 different CPU and 7 different boards. We don't want to make a u-boot for each board.
Ah, this explains a lot. Thanks for this explanation.
Um... do you have any plans how to handle this in a clean way that will scale for additional SoCs and boards as well, and that eventually can be used for other architectures, too?
I guess you will have to address the same issues in your Linux ports?
Do you consider using the device tree to implement the capability to adjust the software configuration to the specifics of the current hardware? I think this would be really beneficial both for U-Boot and Linux.
I have to admit that I'm pretty much stuck between a rock and a hard place. I highly appreciate the fact that Samsung starts contributing code to Linux and now also U-Boot mainline trees,. and as such I want to encourage you all as much as possible, and not put any road blocks in your way when it can be avoided.
I already hesitated when I rejected all these base address plus offset things and demanded these to be changed to C structs. I am well aware that is causes a lot of effort on your side without any immediately visible benefit (your code is already running, so why change it?).
On the other hand, I have to keep an eye at the overall code quality, and to make sure the coderemains maintenable.
From the patches I've seen so far I fear that we weill quickly see the
code copmplexity explode when it comes to handling different SoC and board combinations within a single U-Boot image. I would like to point out that we should not go the way that is currently being used in the ARM Linux kernel. We will not duplicate the MACH_ID concepts used there and the resulting code methods. Instead, when we need to add such flexible configuration, we will go the device tree way.
I hope such comment still comes in time to help you taking the right turns.
Best regards,
Wolfgang Denk
participants (8)
-
Dirk Behme -
Jean-Christophe PLAGNIOL-VILLARD -
Kyungmin Park -
Minkyu Kang -
Minkyu Kang -
Paulraj, Sandeep -
Tom -
Wolfgang Denk