[U-Boot] [PATCH] armv7:cache: D cache line size read method

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

(Added Hong Xu)

Hi Aneesh,

On 12/08/2011 11:16, Aneesh V wrote:
> Hi Albert, Lukasz,
>
> On Friday 12 August 2011 02:28 PM, Albert ARIBAUD wrote:
>> Hi Lukasz,
>>
>> On 12/08/2011 10:24, Lukasz Majewski wrote:
>>> This commit adds support for reading the D cache line size for armv7
>>> architecture.
>>>
>>> The get_dcache_line_size() function is supposed to work in conjunction
>>> with memalign call to provide D cache aligned DMA buffers.
>>>
>>> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>> CC: Aneesh V<aneesh@ti.com>
>>> CC: Albert ARIBAUD<albert.u.boot@aribaud.net>
>>> ---
>>> arch/arm/cpu/armv7/cache_v7.c | 25 +++++++++++++++++++++++++
>>> include/common.h | 1 +
>>> 2 files changed, 26 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/cache_v7.c
>>> b/arch/arm/cpu/armv7/cache_v7.c
>>> index 3e1e1bf..bd0b1a8 100644
>>> --- a/arch/arm/cpu/armv7/cache_v7.c
>>> +++ b/arch/arm/cpu/armv7/cache_v7.c
>>> @@ -246,6 +246,21 @@ static void v7_inval_tlb(void)
>>> CP15ISB;
>>> }
>>>
>>> +/* Read the cache line size (bytes) */
>>> +static int v7_dcache_line_size(void)
>>> +{
>>> + u32 ccsidr, log2_line_len;
>>> +
>>> + ccsidr = get_ccsidr();
>>> +
>>> + log2_line_len = ((ccsidr& CCSIDR_LINE_SIZE_MASK)>>
>>> + CCSIDR_LINE_SIZE_OFFSET) + 2;
>>> + /* Converting from words to bytes */
>>> + log2_line_len += 2;
>>> +
>>> + return 1<< log2_line_len;
>>> +}
>>> +
>>> void invalidate_dcache_all(void)
>>> {
>>> v7_maint_dcache_all(ARMV7_DCACHE_INVAL_ALL);
>>> @@ -303,6 +318,11 @@ void flush_cache(unsigned long start, unsigned
>>> long size)
>>> {
>>> flush_dcache_range(start, start + size);
>>> }
>>> +
>>> +int get_dcache_line_size(void)
>>> +{
>>> + return v7_dcache_line_size();
>>> +}
>>> #else /* #ifndef CONFIG_SYS_DCACHE_OFF */
>>> void invalidate_dcache_all(void)
>>> {
>>> @@ -327,6 +347,11 @@ void arm_init_before_mmu(void)
>>> void flush_cache(unsigned long start, unsigned long size)
>>> {
>>> }
>>> +
>>> +int get_dcache_line_size(void)
>>> +{
>>> + return 0;
>>> +}
>>> #endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
>>>
>>> #ifndef CONFIG_SYS_ICACHE_OFF
>>> diff --git a/include/common.h b/include/common.h
>>> index 12a1074..b535300 100644
>>> --- a/include/common.h
>>> +++ b/include/common.h
>>> @@ -622,6 +622,7 @@ void flush_dcache_range(unsigned long start,
>>> unsigned long stop);
>>> void invalidate_dcache_range(unsigned long start, unsigned long stop);
>>> void invalidate_dcache_all(void);
>>> void invalidate_icache_all(void);
>>> +int get_dcache_line_size(void);
>>>
>>> /* arch/$(ARCH)/lib/ticks.S */
>>> unsigned long long get_ticks(void);
>>
>> I like the idea of a global cache line accessor, but:
>>
>> 1) Does it have to be a function? I don't know cache implementations
>> where line size is not a constant, and a function might be overkill.
>>
>> 2) If it has to be a function, then I'd rather have the function
>> introduced in arch/arm/lib/cache.c as a weak function returning 32, with
>> armv7 providing a replacement.
>
> Since this is used from generic drivers I think this should be defined
> all ARCHs. How about something like this.
>
> int __weak get_dcache_line_size(void)
> {
> #if defined(CONFIG_SYS_CACHE_LINE_SIZE) && !defined(CONFIG_SYS_DCACHE_OFF)
> return CONFIG_SYS_CACHE_LINE_SIZE;
> #else
> return sizeof(int); /* or 0 - whatever is appropriate for memalign() */
> #endif
> }
>
> Then over-ride this from the respective ARCH implementation if there is
> a cache-type registers. I see that armv5/6/7 all can find this at
> run-time from CP15 registers.
>
>>
>> Aneesh, that patch could affect your patch series as the magic 32 could
>> then come from this weak function rather than be hard-coded.
>
> I think you are referring to Hong's series. In armv7 series we are
> getting the size by reading ccsidr register just like Lukasz is doing
> here - no magic values. I think in Hong's series also this can be found
> from the cache type CP15 register.

Correct, I misattributed the patch.

As for the cache line size, I see one reason for using a constant rather 
than dynamically finding it: aligning non-dynamic (either static local 
or global) buffers. If there are any in the code, they'll need an 
__attribute__((aligned())) directive, which can only take a constant.

OTOH, maybe they could be changed to dynamic buffers allocated at init 
time, which would make the handling of cache_aligned buffers homogeneous.

Amicalement,
-- 
Albert.