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