Minkyu Kang wrote:
> Dear, Dirk
>
> 2009/9/4 Dirk Behme <dirk.be...@googlemail.com>
>
>> Kyungmin Park wrote:
>>> On Fri, Sep 4, 2009 at 7:45 PM, Dirk Behme<dirk.be...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
