public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
@ 2011-08-04  3:45 Hong Xu
  2011-08-04  7:11 ` Albert ARIBAUD
  0 siblings, 1 reply; 12+ messages in thread
From: Hong Xu @ 2011-08-04  3:45 UTC (permalink / raw)
  To: u-boot

After DMA transfer, 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: Heiko Schocher <hs@denx.de>
CC: Albert Aribaud <albert.aribaud@free.fr>
---
 arch/arm/lib/cache.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..216bde0 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,13 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+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")));
-- 
1.7.3.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-04  3:45 Hong Xu
@ 2011-08-04  7:11 ` Albert ARIBAUD
  0 siblings, 0 replies; 12+ messages in thread
From: Albert ARIBAUD @ 2011-08-04  7:11 UTC (permalink / raw)
  To: u-boot

Le 04/08/2011 05:45, Hong Xu a ?crit :
> After DMA transfer, 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: Heiko Schocher<hs@denx.de>
> CC: Albert Aribaud<albert.aribaud@free.fr>
> ---
>   arch/arm/lib/cache.c |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 92b61a2..216bde0 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -53,3 +53,13 @@ void	__flush_dcache_all(void)
>   }
>   void	flush_dcache_all(void)
>   	__attribute__((weak, alias("__flush_dcache_all")));
> +
> +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")));

NAK in this form.

I don't want invalide function introduced only in full invalidation 
form, because driver code will start using this, and it is the wrong 
thing to do as each caller can only tell what it wants invalidated, not 
what other code wants invalidated; a client doing a full invalidate 
might (and certainly will, to some extent) trash other data than its own.

So please provide also an invalidate_cache() function similar to the 
flush_cache() function, with start and size arguments, that callers 
should use.

full flush and invalidate should only be used by callers that *really* 
need it (bootm comes to mind).

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
@ 2011-08-08  3:20 Hong Xu
  2011-08-08  8:01 ` Albert ARIBAUD
  0 siblings, 1 reply; 12+ messages in thread
From: Hong Xu @ 2011-08-08  3:20 UTC (permalink / raw)
  To: u-boot

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")));
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08  3:20 [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache Hong Xu
@ 2011-08-08  8:01 ` Albert ARIBAUD
  2011-08-08  8:58   ` Hong Xu
  2011-08-08 17:34   ` Marek Vasut
  0 siblings, 2 replies; 12+ messages in thread
From: Albert ARIBAUD @ 2011-08-08  8:01 UTC (permalink / raw)
  To: u-boot

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.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

> +	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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08  8:01 ` Albert ARIBAUD
@ 2011-08-08  8:58   ` Hong Xu
  2011-08-08 17:34   ` Marek Vasut
  1 sibling, 0 replies; 12+ messages in thread
From: Hong Xu @ 2011-08-08  8:58 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08  8:01 ` Albert ARIBAUD
  2011-08-08  8:58   ` Hong Xu
@ 2011-08-08 17:34   ` Marek Vasut
  2011-08-08 17:56     ` Albert ARIBAUD
                       ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Marek Vasut @ 2011-08-08 17:34 UTC (permalink / raw)
  To: u-boot

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.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
> > 
> > +	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,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08 17:34   ` Marek Vasut
@ 2011-08-08 17:56     ` Albert ARIBAUD
  2011-08-09  1:57     ` Hong Xu
  2011-08-09 11:05     ` Aneesh V
  2 siblings, 0 replies; 12+ messages in thread
From: Albert ARIBAUD @ 2011-08-08 17:56 UTC (permalink / raw)
  To: u-boot

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,
-- 
Albert.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08 17:34   ` Marek Vasut
  2011-08-08 17:56     ` Albert ARIBAUD
@ 2011-08-09  1:57     ` Hong Xu
  2011-08-09 19:55       ` Marek Vasut
  2011-08-09 11:05     ` Aneesh V
  2 siblings, 1 reply; 12+ messages in thread
From: Hong Xu @ 2011-08-09  1:57 UTC (permalink / raw)
  To: u-boot

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 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
>>>
>>> +	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.
>

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-08 17:34   ` Marek Vasut
  2011-08-08 17:56     ` Albert ARIBAUD
  2011-08-09  1:57     ` Hong Xu
@ 2011-08-09 11:05     ` Aneesh V
  2 siblings, 0 replies; 12+ messages in thread
From: Aneesh V @ 2011-08-09 11:05 UTC (permalink / raw)
  To: u-boot

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.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
>>>
>>> +	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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-09  1:57     ` Hong Xu
@ 2011-08-09 19:55       ` Marek Vasut
  2011-08-10  1:45         ` Hong Xu
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2011-08-09 19:55 UTC (permalink / raw)
  To: u-boot

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 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
> >>> 
> >>> +	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.
> 
> [...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-09 19:55       ` Marek Vasut
@ 2011-08-10  1:45         ` Hong Xu
  2011-08-10  2:46           ` Marek Vasut
  0 siblings, 1 reply; 12+ messages in thread
From: Hong Xu @ 2011-08-10  1:45 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache
  2011-08-10  1:45         ` Hong Xu
@ 2011-08-10  2:46           ` Marek Vasut
  0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2011-08-10  2:46 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-08-10  2:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-08  3:20 [U-Boot] [PATCH] ARM926ejs: Add routines to invalidate D-Cache Hong Xu
2011-08-08  8:01 ` Albert ARIBAUD
2011-08-08  8:58   ` Hong Xu
2011-08-08 17:34   ` Marek Vasut
2011-08-08 17:56     ` Albert ARIBAUD
2011-08-09  1:57     ` Hong Xu
2011-08-09 19:55       ` Marek Vasut
2011-08-10  1:45         ` Hong Xu
2011-08-10  2:46           ` Marek Vasut
2011-08-09 11:05     ` Aneesh V
  -- strict thread matches above, loose matches on Subject: below --
2011-08-04  3:45 Hong Xu
2011-08-04  7:11 ` Albert ARIBAUD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox