Hi Marek Vasut,

On Monday 08 August 2011 11:04 PM, Marek Vasut wrote:
> On Monday, August 08, 2011 10:01:19 AM Albert ARIBAUD wrote:
>> Hi Hong Xu,
>>
>> Le 08/08/2011 05:20, Hong Xu a écrit :
>>> After DMA operation, we need to maintain D-Cache coherency.
>>> So that the DCache must be invalidated (hence CPU will fetch
>>> data written by DMA controller from RAM).
>>>
>>> Tested on AT91SAM9261EK with Peripheral DMA controller.
>>>
>>> Signed-off-by: Hong Xu<hong...@atmel.com>
>>> Tested-by: Elen Song<elen.s...@atmel.com>
>>> CC: Albert Aribaud<albert.u.b...@aribaud.net>
>>> CC: Aneesh V<ane...@ti.com>
>>> CC: Reinhard Meyer<u-b...@emk-elektronik.de>
>>> CC: Heiko Schocher<h...@denx.de>
>>> ---
>>>
>>> V2:
>>>     Per Albert's suggestion, add invalidate_dcache_range
>>>
>>> V3:
>>>     invalidate_dcache_range emits warning when detecting unaligned buffer
>>>
>>>     invalidate_dcache_range won't clean any adjacent cache line when
>>>     detecting unaligned buffer and only round up/down the buffer address
>>>
>>> +   mva = start;
>>> +   if ((mva&   (cache_line_len - 1)) != 0) {
>>> +           printf("WARNING: %s - unaligned buffer detected, starting "
>>
>> I'd rather have a message about "cache", not "buffer", e.g.
>>
>>     printf("WARNING: %s - start address %x is not aligned\n"
>>       __FUNCTION__, start);
>
> __func__ is prefered in linux kernel :-)
>>
>>> +           mva&= ~(cache_line_len - 1);
>>> +   }
>>> +   if ((stop&   (cache_line_len - 1)) != 0) {
>>> +           printf("WARNING: %s - unaligned buffer detected, ending "
>>> +                   "address: 0x%08x\n", __FUNCTION__, stop);
>>
>> Ditto.
>
> Ditto.
>
>>
>>> +           stop = (stop | (cache_line_len - 1)) + 1;
>>> +   }
>>> +
>>> +   while (mva<   stop) {
>>> +           asm("mcr p15, 0, %0, c7, c6, 1" : : "r"(mva));
>>> +           mva += cache_line_len;
>>> +   }
>>
>> Thinking more about the degenerate case -- why not round *up* the start
>> address, and round *down* the stop address, that is, *reduce* the area
>> to the aligned portion rather than *expand* it into the unknown? That
>> would make data in "partially owned" cache lines safe from unwanted
>> invalidation. OTOH, it would not completely invalidate the caller's
>> data, but at least the malfunction would appear in the faulty calling
>> code, not elsewhere.
>
> That'd introduce even stranger behaviour and it'd be even more sickening to
> debug

I think the warning messages printed here will greatly help in
debugging such issues.

best regards,
Aneesh
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to