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

[Dirk Behme]

Dear Minkyu Kang,

Minkyu Kang wrote:
> Dear Dirk,
> 
> 2009/9/5 Dirk Behme <dirk.behme at googlemail.com>
> 
>> Minkyu Kang wrote:
>>
>>> Dear, Dirk
>>>
>>> 2009/9/4 Dirk Behme <dirk.behme at googlemail.com>
>>>
>>>  Kyungmin Park wrote:
>>>>> On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme<dirk.behme at 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?

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