From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dirk Behme <dirk.behme@googlemail.com>
Date: Fri, 04 Sep 2009 13:43:08 +0200
Subject: [U-Boot] [PATCH] arm_cortexa8: support cache flush to other soc
In-Reply-To: <9c9fda240909040354h204ca677q946abc8c65d7aa0e@mail.gmail.com>
References: <4AA0CF18.9020807@samsung.com> <4AA0D335.505@googlemail.com>	
	<9c9fda240909040234m4fdd7466ybb38d0d0618cdf32@mail.gmail.com>	
	<4AA0EFB9.30801@googlemail.com>
	<9c9fda240909040354h204ca677q946abc8c65d7aa0e@mail.gmail.com>
Message-ID: <4AA0FD4C.3000706@googlemail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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