[U-Boot] [PATCH v2] ARM926ejs: Add routines to invalidate D-Cache

[Hong Xu <hong.xu@atmel.com>]

Hi Albert,

On 08/05/2011 02:13 PM, Albert ARIBAUD wrote:
> Hi Hong Xu,
>
> Le 05/08/2011 06:44, Hong Xu a ?crit :
>> After DMA operation, we need to maintain D-Cache coherency.
>> We need to clean cache (write back the dirty lines) and then
>> make the cache invalidate as well(hence CPU will fetch data
>> written by DMA controller from RAM).
>>
>> Tested on AT91SAM9261EK with Peripheral DMA controller.
>>
>> Signed-off-by: Hong Xu<hong.xu@atmel.com>
>> Tested-by: Elen Song<elen.song@atmel.com>
>> CC: Albert Aribaud<albert.aribaud@free.fr>
>> CC: Heiko Schocher<hs@denx.de>

[...]

>> +
>> +#ifdef CONFIG_SYS_CACHELINE_SIZE
>> + cache_line_len = CONFIG_SYS_CACHELINE_SIZE;
>> +#else
>> + cache_line_len = 32;
>
> Document magic number 32 -- for instance, indicate ARM architecture spec
> paragraph reference in a comment above it (and possibly emit a
> compile-time and/or run-time warning) -- or bail out with a compile
> error if CONFIG_SYS_CACHELINE_SIZE is not defined.

ARM926EJ-S Technical Reference Manual (r0p4/r0p5) only defines one cache 
line size, "32". From my understanding, the standard ARM926EJ-S 
implementation shall use 32 bytes cache line size. Of course this can be 
overwritten by CONFIG_SYS_CACHELINE_SIZE, but shall we force to define it?

Frankly I have not found a proper place to define this(aka. not using 32 
here), for example as a Macro.

>
>> +#endif
>> + /*
>> + * If start and stop are not aligned to cache-line,
>> + * the adjacent lines will be cleaned
>> + */
>> + if ((start& (cache_line_len - 1)) != 0)
>> + asm("mcr p15, 0, %0, c7, c10, 1" : : "r" (start));
>> + if ((stop& (cache_line_len - 1)) != 0)
>> + asm("mcr p15, 0, %0, c7, c10, 1" : : "r" (stop));
>> +
>> + mva = start& ~(cache_line_len - 1);
>> + while (mva< stop) {
>> + asm("mcr p15, 0, %0, c7, c6, 1" : : "r"(mva));
>> + mva += cache_line_len;
>> + }
>
> I, like Reinhard, prefer aligning start and stop and then looping
> through a single invalidate mcr.
>
> But I also want to make sure logs tell us anything weird with caches,
> and since unaligned start or stop invalidation could lead to trashing
> third party data, I would like the code to emit a run-time warning if
> that happens, like this:
>
> if ((start& (cache_line_len - 1)) != 0) {
> printf("WARNING: aligning start %x on start of cache line\n",
> start);
> start &= ~(cache_line_len - 1);
> }
> if ((stop& (cache_line_len - 1)) != (cache_line_len -1) ) {
> printf("WARNING: aligning stop %x on end of cache line\n",
> stop);
> start |= (cache_line_len - 1);
> }

I've tried to deal with the case that the (start, stop) is not aligned. 
If mis-align happens, the adjacent lines will be cleaned before 
invalidating. And from my view it's impossible for a driver to always 
pass aligned address to invalidate_dcache_range.

To answer your question in another email

 > How do you know the dirty data should be flushed rather than 
invalidated?

"Dirty" means this cache is changed by CPU but not has not been written 
into memory or WB. If we invalidate it, data will lost. In most cases, I 
do not see a situation why the dirty data shall not be written into memory.

BR,
Eric
>
[...]
> Amicalement,