[U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
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.xu@atmel.com Tested-by: Elen Song elen.song@atmel.com CC: Albert Aribaud albert.u.boot@aribaud.net CC: Aneesh V aneesh@ti.com CC: Reinhard Meyer u-boot@emk-elektronik.de CC: Heiko Schocher hs@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
arch/arm/lib/cache.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 92b61a2..9e6aa47 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -53,3 +53,57 @@ void __flush_dcache_all(void) } void flush_dcache_all(void) __attribute__((weak, alias("__flush_dcache_all"))); + +/* + * The buffer range to be invalidated is [start, stop) + */ +void __invalidate_dcache_range(unsigned long start, unsigned long stop) +{ + int cache_line_len; + unsigned long mva; + +#ifdef CONFIG_ARM926EJS +#ifdef CONFIG_SYS_CACHELINE_SIZE + cache_line_len = CONFIG_SYS_CACHELINE_SIZE; +#else + /* + * ARM926EJ-S Technical Reference Manual, Chap 2.3.1 Table 2-9 + * only b'10, aka. 32 bytes cache line len is valid + */ + cache_line_len = 32; +#endif + mva = start; + if ((mva & (cache_line_len - 1)) != 0) { + printf("WARNING: %s - unaligned buffer detected, starting " + "address: 0x%08x\n", __FUNCTION__, start); + 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); + stop = (stop | (cache_line_len - 1)) + 1; + } + + while (mva < stop) { + asm("mcr p15, 0, %0, c7, c6, 1" : : "r"(mva)); + mva += cache_line_len; + } + + /* Drain the WB */ + asm("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +#endif + + return; +} +void invalidate_dcache_range(unsigned long start, unsigned long stop) + __attribute__((weak, alias("__invalidate_dcache_range"))); + +void __invalidate_dcache_all(void) +{ +#ifdef CONFIG_ARM926EJS + asm("mcr p15, 0, %0, c7, c6, 0" : : "r" (0)); +#endif + return; +} +void invalidate_dcache_all(void) + __attribute__((weak, alias("__invalidate_dcache_all")));
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 Xuhong.xu@atmel.com Tested-by: Elen Songelen.song@atmel.com CC: Albert Aribaudalbert.u.boot@aribaud.net CC: Aneesh Vaneesh@ti.com CC: Reinhard Meyeru-boot@emk-elektronik.de CC: Heiko Schocherhs@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);
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.
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.
Opinions?
Amicalement,
Hi Albert,
On 08/08/2011 04:01 PM, 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
[...]
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);
OK
- 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.
OK
- 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.
Opinions?
Agree :)
BR, Eric
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 Xuhong.xu@atmel.com Tested-by: Elen Songelen.song@atmel.com CC: Albert Aribaudalbert.u.boot@aribaud.net CC: Aneesh Vaneesh@ti.com CC: Reinhard Meyeru-boot@emk-elektronik.de CC: Heiko Schocherhs@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
Opinions?
Amicalement,
Le 08/08/2011 19:34, Marek Vasut a écrit :
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 tend to agree neither on the "stranger" part, nor on the "sickening". Can you develop your viewpoint? For your reference, here is mine:
As for "stranger": for me, an unexpected value (either a modification or an absence thereof) in a variable that no code is using right now, that is stranger than an unexpected value in a DMA buffer we just happen to have recently invalidated.
As for "sickening": for me, when a bug occurs, one of the requisites is to look up the console output, where a message about an unaligned cache invalidate address will stand out and quite quickly point at the root cause for that weird DMA buffer content problem -- OTOH, with "expanded" invalidates, the DMA buffer is OK so one might disregard the warning and not link it far later with that weird problem where an obscure global variable couldn't keep its value.
I do understand though that I dismissed flush-in-invalidate with an argument of "does what it says on the tin", and that one could say my proposal makes the function do less of what it says on the tin; my point was about the nature of the action, not its extent.
IOW, the sticker on the tin also says "use only with cacheline-aligned start and stop", so fair's fair: if you pass unaligned ranges, don't expect the invalidate to extend beyond the aligned part.
Amicalement,
Hi Marek Vasut,
On 08/09/2011 01:34 AM, 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 Xuhong.xu@atmel.com Tested-by: Elen Songelen.song@atmel.com CC: Albert Aribaudalbert.u.boot@aribaud.net CC: Aneesh Vaneesh@ti.com CC: Reinhard Meyeru-boot@emk-elektronik.de CC: Heiko Schocherhs@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 :-)
__func__ is C99 standard. __FUNCTION__ appears more in U-Boot. ;-) GCC manual says some older GCC only recognize __FUNCTION__ . If we rely on GCC, it looks __FUNCTION__ will reduce troubles.
BR, Eric
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.
[...]
On Tuesday, August 09, 2011 03:57:41 AM Hong Xu wrote:
Hi Marek Vasut,
On 08/09/2011 01:34 AM, 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 Xuhong.xu@atmel.com Tested-by: Elen Songelen.song@atmel.com CC: Albert Aribaudalbert.u.boot@aribaud.net CC: Aneesh Vaneesh@ti.com CC: Reinhard Meyeru-boot@emk-elektronik.de CC: Heiko Schocherhs@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 :-)
__func__ is C99 standard. __FUNCTION__ appears more in U-Boot. ;-)
This doesn't mean it's correct ;-) "majority proof" isn't a proof really.
GCC manual says some older GCC only recognize __FUNCTION__ . If we rely on GCC, it looks __FUNCTION__ will reduce troubles.
Do we support such ancient versions of GCC anyway ? Just to be clear, I'm fine with either way, just my 2.7183 cents ;-)
BR, Eric
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.
[...]
Hi Marek Vasut,
On 08/10/2011 03:55 AM, Marek Vasut wrote:
On Tuesday, August 09, 2011 03:57:41 AM Hong Xu wrote:
Hi Marek Vasut,
On 08/09/2011 01:34 AM, Marek Vasut wrote:
[...]
printf("WARNING: %s - start address %x is not aligned\n" __FUNCTION__, start);__func__ is prefered in linux kernel :-)
__func__ is C99 standard. __FUNCTION__ appears more in U-Boot. ;-)
This doesn't mean it's correct ;-) "majority proof" isn't a proof really.
GCC manual says some older GCC only recognize __FUNCTION__ . If we rely on GCC, it looks __FUNCTION__ will reduce troubles.
Do we support such ancient versions of GCC anyway ? Just to be clear, I'm fine with either way, just my 2.7183 cents ;-)
Agree. Just after last reply, I reconsidered the situation in the tearoom. __func__ looks better. ;-) I'll resend the patch soon, thanks.
BR, Eric
On Wednesday, August 10, 2011 03:45:11 AM Hong Xu wrote:
Hi Marek Vasut,
On 08/10/2011 03:55 AM, Marek Vasut wrote:
On Tuesday, August 09, 2011 03:57:41 AM Hong Xu wrote:
Hi Marek Vasut,
On 08/09/2011 01:34 AM, Marek Vasut wrote:
[...]
printf("WARNING: %s - start address %x is not aligned\n" __FUNCTION__, start);__func__ is prefered in linux kernel :-)
__func__ is C99 standard. __FUNCTION__ appears more in U-Boot. ;-)
This doesn't mean it's correct ;-) "majority proof" isn't a proof really.
GCC manual says some older GCC only recognize __FUNCTION__ . If we rely on GCC, it looks __FUNCTION__ will reduce troubles.
Do we support such ancient versions of GCC anyway ? Just to be clear, I'm fine with either way, just my 2.7183 cents ;-)
Agree. Just after last reply, I reconsidered the situation in the tearoom. __func__ looks better. ;-) I'll resend the patch soon, thanks.
Thanks, I'm very much looking forward to this patch in mainline.
BR, Eric
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 Xuhong.xu@atmel.com Tested-by: Elen Songelen.song@atmel.com CC: Albert Aribaudalbert.u.boot@aribaud.net CC: Aneesh Vaneesh@ti.com CC: Reinhard Meyeru-boot@emk-elektronik.de CC: Heiko Schocherhs@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
participants (4)
-
Albert ARIBAUD -
Aneesh V -
Hong Xu -
Marek Vasut