Re: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc

7 Sep 2009

      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