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

Dirk Behme
2009-09-04T10:45:13Z

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