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

Minkyu Kang
2009-09-07T01:31:49Z

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 Behme<dirk.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 at 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

--
from. prom.