[U-Boot] [PATCH 2/8] armv7: cache maintenance operations for armv7

[Albert ARIBAUD]

Le 08/01/2011 14:17, Aneesh V a écrit :

>> Why use pointers here rather than weak functions?
>
> In fact, I hadn't thought about it. Maybe I was biased by the Linux
> implementation.The only reason I can think of is that pointer gives the
> flexibility of doing this assignment at run-time. Let's say we had a
> multi-platform u-boot that detects the SOC at run-time?

I know we consider multi-board u-boot binaries when boards are variant 
of a given SoC, that's one reason why we wanted relocation. I'm not sure 
about multi-SoC when SoC is a variant of a given cpu, though. Wolfgang, 
your opinion?

>>> +void flush_cache(unsigned long start, unsigned long size)
>>> +{
>>> + flush_dcache_range(start, start + size);
>>> +}
>>
>> This function is the only one which is defined to something non-empty
>> when CONFIG_SYS_NO_DCACHE is defined. Why is it not in the big #ifndef
>> for dcache above ?
>
> Although this function is non-empty, flush_dcache_range() is in turn
> empty. Effect will be the same, right?

Yes the effect is the same, but your patch results in a non-trivially 
empty function; I'd prefer it to be visibly empty when we compile 
without cache support.

>>> diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
>>> index 49ac9c7..7f9b171 100644
>>> --- a/arch/arm/cpu/armv7/config.mk
>>> +++ b/arch/arm/cpu/armv7/config.mk
>>> @@ -23,7 +23,7 @@
>>> PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
>>>
>>> # Make ARMv5 to allow more compilers to work, even though its v7a.
>>> -PLATFORM_CPPFLAGS += -march=armv5
>>> +PLATFORM_CPPFLAGS += -march=armv7-a
>>
>> Did you check that this does not break any board using armv7?
>
> I checked only Codesourcery tool chain.
> Linux kernel build for a v7 architecture processor uses armv7-a. Is it
> not fair to assume that the toolchain used for bootloader also supports
> it? Do we have to support those out-dated compilers?

Just because Linux uses armv7-a for a v7 arch does not mean we must have 
it for u-boot. For starters, U-boot does not always boot Linux. :)

As for out-dated compilers, that's the question I'm asking: do we 
consider e.g. ELDK 4.2 as outdated or not? It won't accept armv7-a.

>>> +/* some utility macros */
>>> +#define mask(start, end) \
>>> + (((1<< ((end) - (start) + 1)) - 1)<< (start))
>>> +
>>> +#define mask_n_get(reg, start, end) \
>>> + (((reg)& mask(start, end))>> (start))
>>
>> Seeing as these functions are only used in the ARMv7 cache C file, they
>> should be moved there.
>
> I plan to use a modified version of mask_n_get() and its set couterpart
> mask_n_set() in my subsequent works in more files.
>
> Can I keep it here itself or should I move it to an OMAP specific
> header file or can I move it to a more generic header file? Please
> suggest.

They're very generic actually. I think they should go to a gereric bit 
manipulation header, and be named a... bit... more explicitly. For 
instance, the name 'mask' does not show that the macro creates a range 
of 'one' bits from start to end.

> Best regards,
> Aneesh

Amicalement,
-- 
Albert.