[U-Boot] [PATCH v 0/3] arm: changes in cache handling

[U-Boot] [PATCH v 0/3] arm: changes in cache handling

[Aneesh V]

- Avoid enabling caches for all ARM boards
- Enable caches for omap3/4
- Stronger barrier for armv7 cache-maintenance operations.

Aneesh V (3):
  arm: do not force d-cache enable on all boards
  omap: enable caches at system start-up
  armv7: stronger barrier for cache-maintenance operations

 arch/arm/cpu/armv7/cache_v7.c    |   12 +++++-----
 arch/arm/cpu/armv7/omap3/board.c |    8 +++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 +++++++
 arch/arm/lib/board.c             |    8 ++----
 arch/arm/lib/cache.c             |   12 +++++++++++
 doc/README.arm-caches            |   40 ++++++++++++++++++++++++++++++++++++++
 include/common.h                 |    1 +
 7 files changed, 78 insertions(+), 11 deletions(-)
 create mode 100644 doc/README.arm-caches

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Aneesh V]

c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
to board_init_r(). This enables d-cache for all ARM boards.
As a result some of the arm boards that are not cache-ready
are broken. Revert this change and allow platform code to
take the decision on d-cache enabling.

Also add some documentation for cache usage in ARM.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
MAKEALL pending. Will update the results tomorrow.
---
 arch/arm/lib/board.c  |    8 +++-----
 arch/arm/lib/cache.c  |   12 ++++++++++++
 doc/README.arm-caches |   40 ++++++++++++++++++++++++++++++++++++++++
 include/common.h      |    1 +
 4 files changed, 56 insertions(+), 5 deletions(-)
 create mode 100644 doc/README.arm-caches

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 90709d0..d093d5b 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -446,11 +446,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
 	monitor_flash_len = _end_ofs;
-	/*
-	 * Enable D$:
-	 * I$, if needed, must be already enabled in start.S
-	 */
-	dcache_enable();
+
+	/* Enable caches */
+	enable_caches();
 
 	debug ("monitor flash len: %08lX\n", monitor_flash_len);
 	board_init();	/* Setup chipselects */
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..b545fb7 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+
+/*
+ * Default implementation of enable_caches()
+ * Real implementation should be in platform code
+ */
+void __enable_caches(void)
+{
+	puts("WARNING: Caches not enabled\n");
+}
+void enable_caches(void)
+	__attribute__((weak, alias("__enable_caches")));
diff --git a/doc/README.arm-caches b/doc/README.arm-caches
new file mode 100644
index 0000000..9edc252
--- /dev/null
+++ b/doc/README.arm-caches
@@ -0,0 +1,40 @@
+Disabling I-cache:
+- Set CONFIG_SYS_ICACHE_OFF
+
+Disabling D-cache:
+- Set CONFIG_SYS_DCACHE_OFF
+
+Enabling I-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
+
+Enabling D-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
+
+Enabling Caches at System Startup:
+- Implement enable_caches() for your platform and enable the I-cache and
+  D-cache from this function. This function is called immediately
+  after relocation.
+
+Guidelines for Working with D-cache:
+
+Memory to Peripheral DMA:
+- Flush the buffer after the MPU writes the data and before the DMA is
+  initiated.
+
+Peripheral to Memory DMA:
+- Invalidate the buffer after the DMA is complete and before the MPU reads
+  it.
+
+Buffer Requirements:
+- Any buffer that is invalidated(that is, typically the peripheral to
+  memory DMA buffer) should be aligned to cache-line boundary both at
+  at the beginning and at the end of the buffer.
+
+Cleanup Before Linux:
+- cleanup_before_linux() should flush the D-cache, invalidate I-cache, and
+  disable MMU and caches.
+- The following sequence is advisable while disabling d-cache:
+  1. disable_dcache() - flushes and disables d-cache
+  2. invalidate_dcache_all() - invalid any entry that came to the cache
+	in the short period after the cache was flushed but before the
+	cache got disabled.
diff --git a/include/common.h b/include/common.h
index b994e70..c83b358 100644
--- a/include/common.h
+++ b/include/common.h
@@ -613,6 +613,7 @@ ulong	lcd_setmem (ulong);
 ulong	video_setmem (ulong);
 
 /* arch/$(ARCH)/lib/cache.c */
+void	enable_caches(void);
 void	flush_cache   (unsigned long, unsigned long);
 void	flush_dcache_all(void);
 void	flush_dcache_range(unsigned long start, unsigned long stop);
-- 
1.7.0.4

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Jason Liu]

Hi, Aneesh,

2011/8/1 Aneesh V <aneesh@ti.com>:
> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
> to board_init_r(). This enables d-cache for all ARM boards.
> As a result some of the arm boards that are not cache-ready
> are broken. Revert this change and allow platform code to
> take the decision on d-cache enabling.
>
> Also add some documentation for cache usage in ARM.
>
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
> MAKEALL pending. Will update the results tomorrow.
> ---
> ?arch/arm/lib/board.c ?| ? ?8 +++-----
> ?arch/arm/lib/cache.c ?| ? 12 ++++++++++++
> ?doc/README.arm-caches | ? 40 ++++++++++++++++++++++++++++++++++++++++
> ?include/common.h ? ? ?| ? ?1 +
> ?4 files changed, 56 insertions(+), 5 deletions(-)
> ?create mode 100644 doc/README.arm-caches
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 90709d0..d093d5b 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -446,11 +446,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
> ? ? ? ?gd->flags |= GD_FLG_RELOC; ? ? ?/* tell others: relocation done */
>
> ? ? ? ?monitor_flash_len = _end_ofs;
> - ? ? ? /*
> - ? ? ? ?* Enable D$:
> - ? ? ? ?* I$, if needed, must be already enabled in start.S
> - ? ? ? ?*/
> - ? ? ? dcache_enable();
> +
> + ? ? ? /* Enable caches */
> + ? ? ? enable_caches();
>
> ? ? ? ?debug ("monitor flash len: %08lX\n", monitor_flash_len);
> ? ? ? ?board_init(); ? /* Setup chipselects */
> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 92b61a2..b545fb7 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -53,3 +53,15 @@ void __flush_dcache_all(void)
> ?}
> ?void ? flush_dcache_all(void)
> ? ? ? ?__attribute__((weak, alias("__flush_dcache_all")));
> +
> +
> +/*
> + * Default implementation of enable_caches()
> + * Real implementation should be in platform code
> + */
> +void __enable_caches(void)
> +{
> + ? ? ? puts("WARNING: Caches not enabled\n");
> +}
> +void enable_caches(void)
> + ? ? ? __attribute__((weak, alias("__enable_caches")));


What about the following change?

#ifndef CONFIG_SYS_DCACHE_OFF
        dcache_enable();
#else
       puts("WARNING: Caches not enabled\n");
#endif

This can avoid adding the duplicated cache-enable code in every board later.
Just looked at your patches for omap3 and omap4:
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 98519a9..de0e90d 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -390,3 +390,11 @@ void v7_outer_cache_disable(void)
       omap3_update_aux_cr(0, 0x2);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+       /* Enable D-cache. I-cache is already enabled in start.S */
+       dcache_enable();
+}
+#endif
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index de4cc2a..6ea8a2e 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -139,3 +139,11 @@ void v7_outer_cache_disable(void)
       set_pl310_ctrl_reg(0);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+       /* Enable D-cache. I-cache is already enabled in start.S */
+       dcache_enable();
+}
+#endif

Maybe there will be many many duplicated code like this, do you wish that?

Jason

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Jason Liu]

2011/8/2 Jason Liu <liu.h.jason@gmail.com>:
> Hi, Aneesh,
>
> 2011/8/1 Aneesh V <aneesh@ti.com>:
>> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
>> to board_init_r(). This enables d-cache for all ARM boards.
>> As a result some of the arm boards that are not cache-ready
>> are broken. Revert this change and allow platform code to
>> take the decision on d-cache enabling.
>>
>> Also add some documentation for cache usage in ARM.
>>
>> Signed-off-by: Aneesh V <aneesh@ti.com>
>> ---
>> MAKEALL pending. Will update the results tomorrow.
>> ---
>> ?arch/arm/lib/board.c ?| ? ?8 +++-----
>> ?arch/arm/lib/cache.c ?| ? 12 ++++++++++++
>> ?doc/README.arm-caches | ? 40 ++++++++++++++++++++++++++++++++++++++++
>> ?include/common.h ? ? ?| ? ?1 +
>> ?4 files changed, 56 insertions(+), 5 deletions(-)
>> ?create mode 100644 doc/README.arm-caches
>>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index 90709d0..d093d5b 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -446,11 +446,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
>> ? ? ? ?gd->flags |= GD_FLG_RELOC; ? ? ?/* tell others: relocation done */
>>
>> ? ? ? ?monitor_flash_len = _end_ofs;
>> - ? ? ? /*
>> - ? ? ? ?* Enable D$:
>> - ? ? ? ?* I$, if needed, must be already enabled in start.S
>> - ? ? ? ?*/
>> - ? ? ? dcache_enable();
>> +
>> + ? ? ? /* Enable caches */
>> + ? ? ? enable_caches();
>>
>> ? ? ? ?debug ("monitor flash len: %08lX\n", monitor_flash_len);
>> ? ? ? ?board_init(); ? /* Setup chipselects */
>> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
>> index 92b61a2..b545fb7 100644
>> --- a/arch/arm/lib/cache.c
>> +++ b/arch/arm/lib/cache.c
>> @@ -53,3 +53,15 @@ void __flush_dcache_all(void)
>> ?}
>> ?void ? flush_dcache_all(void)
>> ? ? ? ?__attribute__((weak, alias("__flush_dcache_all")));
>> +
>> +
>> +/*
>> + * Default implementation of enable_caches()
>> + * Real implementation should be in platform code
>> + */
>> +void __enable_caches(void)
>> +{
>> + ? ? ? puts("WARNING: Caches not enabled\n");
>> +}
>> +void enable_caches(void)
>> + ? ? ? __attribute__((weak, alias("__enable_caches")));
>
>
> What about the following change?
>
> #ifndef CONFIG_SYS_DCACHE_OFF
> ? ? ? ?dcache_enable();
> #else
> ? ? ? puts("WARNING: Caches not enabled\n");
> #endif

Or better:

#ifdef CONFIG_SYS_DCACHE_ON
 ? ? ? ?dcache_enable();
 #else
 ? ? ? puts("WARNING: Caches not enabled\n");
 #endif

In fact, we can turn on I-cache safely by default, turn-off D-cache by default,
But give big warning to board-maintainer to fix it later if turn off D-cache.

This can keep the most of config file not change and keep using the
only one copy
of common d-cache enable function to avoid code duplication on every board.

Jason

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Wolfgang Denk]

Dear Jason Liu,

In message <CAB4PhKdj0prR3etkkz5PSqKPgS69CFZepY83Gw-aPQKcFkyKkQ@mail.gmail.com> you wrote:
>
> > What about the following change?
> >
> > #ifndef CONFIG_SYS_DCACHE_OFF
> >        dcache_enable();
> > #else
> >       puts("WARNING: Caches not enabled\n");
> > #endif
> 
> Or better:
> 
> #ifdef CONFIG_SYS_DCACHE_ON
>         dcache_enable();
>  #else
>        puts("WARNING: Caches not enabled\n");
>  #endif
>
> In fact, we can turn on I-cache safely by default, turn-off D-cache by default,
> But give big warning to board-maintainer to fix it later if turn off D-cache.

This is NOT better, it is worse.

The rule is that you don't need to #define a specific option if you
want default behaviour.  Default behaviour should be caches on, so
only "broken" boards where this does not work yet should be able to
opt out by defining some option.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are some things worth dying for.
	-- Kirk, "Errand of Mercy", stardate 3201.7

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Albert ARIBAUD]

Hi Jason,

Le 01/08/2011 18:33, Jason Liu a ?crit :

> Maybe there will be many many duplicated code like this, do you wish that?

I don't think this will or should be duplicated for each ARM board; more 
like suplicated by SoC, or more precisely, by ARM implementation (i.e., 
one cache handling for each of arch/arm/<architecture>/<implementation>) 
-- more or less.

> Jason

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Jason Liu]

Hi, Albert,

2011/8/2 Albert ARIBAUD <albert.u.boot@aribaud.net>:
> Hi Jason,
>
> Le 01/08/2011 18:33, Jason Liu a ?crit :
>
>> Maybe there will be many many duplicated code like this, do you wish that?
>
> I don't think this will or should be duplicated for each ARM board; more
> like suplicated by SoC, or more precisely, by ARM implementation (i.e., one
> cache handling for each of arch/arm/<architecture>/<implementation>) -- more
> or less.

Yes, not each ARM board, but should be a lot as the followings,

drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx31
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx35
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap24xx
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s3c64xx
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 tnetv107x
drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 15:21 lpc2292
drwxr-xr-x 2 r64343 r64343  4096 2011-04-13 13:00 s3c4510b
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 a320
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 at91rm9200
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 ep93xx
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 ks8695
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s3c24x0
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 armada100
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 davinci
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 kirkwood
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mb86r0x
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx25
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx27
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 nomadik
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 orion5x
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 pantheon
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 spear
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 versatile
drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 19:12 mx5
drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 10:20 omap3
drwxr-xr-x 2 r64343 r64343  4096 2011-07-28 17:23 omap4
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap-common
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 10:46 s5pc1xx
drwxr-xr-x 2 r64343 r64343  4096 2011-07-26 17:37 s5pc2xx
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s5p-common
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 tegra2
drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 u8500
drwxr-xr-x 3 r64343 r64343  4096 2011-07-29 15:21 npe

All these arm/<architecture>/<implementation>s will have the duplicated code.
can we consolidate it?

Jason

> Amicalement,
> --
> Albert.
>

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Albert ARIBAUD]

Le 02/08/2011 16:35, Jason Liu a ?crit :
> Hi, Albert,
>
> 2011/8/2 Albert ARIBAUD<albert.u.boot@aribaud.net>:
>> Hi Jason,
>>
>> Le 01/08/2011 18:33, Jason Liu a ?crit :
>>
>>> Maybe there will be many many duplicated code like this, do you wish that?
>>
>> I don't think this will or should be duplicated for each ARM board; more
>> like suplicated by SoC, or more precisely, by ARM implementation (i.e., one
>> cache handling for each of arch/arm/<architecture>/<implementation>) -- more
>> or less.
>
> Yes, not each ARM board, but should be a lot as the followings,
>
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx31
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx35
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap24xx
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s3c64xx
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 tnetv107x
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 15:21 lpc2292
> drwxr-xr-x 2 r64343 r64343  4096 2011-04-13 13:00 s3c4510b
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 a320
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 at91rm9200
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 ep93xx
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 ks8695
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s3c24x0
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 armada100
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 davinci
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 kirkwood
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mb86r0x
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx25
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 mx27
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 nomadik
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 orion5x
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 pantheon
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 spear
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 versatile
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 19:12 mx5
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-29 10:20 omap3
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-28 17:23 omap4
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 omap-common
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 10:46 s5pc1xx
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-26 17:37 s5pc2xx
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 s5p-common
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 tegra2
> drwxr-xr-x 2 r64343 r64343  4096 2011-07-27 11:04 u8500
> drwxr-xr-x 3 r64343 r64343  4096 2011-07-29 15:21 npe
>
> All these arm/<architecture>/<implementation>s will have the duplicated code.
> can we consolidate it?

It might be (partially) possible to factorize some of the code from 
implementations of the same <architecture> level (e.g., arm926ejs 
architecture for orion5x, kirkwood, etc).

This will be something that developers (and reviewers) will need to keep 
in mind when submitting patches that enable caches on ARM boards: such 
code should be split across ARM architecture and implementation, so that 
other implementations of the same arch will benefit from the common 
architecture part.

> Jason

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Aneesh V]

Hi Albert,

On Monday 01 August 2011 04:48 PM, Aneesh V wrote:
> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
> to board_init_r(). This enables d-cache for all ARM boards.
> As a result some of the arm boards that are not cache-ready
> are broken. Revert this change and allow platform code to
> take the decision on d-cache enabling.
>
> Also add some documentation for cache usage in ARM.
>
> Signed-off-by: Aneesh V<aneesh@ti.com>
> ---
> MAKEALL pending. Will update the results tomorrow.
> ---
>   arch/arm/lib/board.c  |    8 +++-----
>   arch/arm/lib/cache.c  |   12 ++++++++++++
>   doc/README.arm-caches |   40 ++++++++++++++++++++++++++++++++++++++++
>   include/common.h      |    1 +
>   4 files changed, 56 insertions(+), 5 deletions(-)
>   create mode 100644 doc/README.arm-caches
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 90709d0..d093d5b 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -446,11 +446,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
>   	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
>
>   	monitor_flash_len = _end_ofs;
> -	/*
> -	 * Enable D$:
> -	 * I$, if needed, must be already enabled in start.S
> -	 */
> -	dcache_enable();
> +
> +	/* Enable caches */
> +	enable_caches();
>
>   	debug ("monitor flash len: %08lX\n", monitor_flash_len);
>   	board_init();	/* Setup chipselects */
> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 92b61a2..b545fb7 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
>   }
>   void	flush_dcache_all(void)
>   	__attribute__((weak, alias("__flush_dcache_all")));
> +
> +
> +/*
> + * Default implementation of enable_caches()
> + * Real implementation should be in platform code
> + */
> +void __enable_caches(void)
> +{
> +	puts("WARNING: Caches not enabled\n");
> +}
> +void enable_caches(void)
> +	__attribute__((weak, alias("__enable_caches")));
> diff --git a/doc/README.arm-caches b/doc/README.arm-caches
> new file mode 100644
> index 0000000..9edc252
> --- /dev/null
> +++ b/doc/README.arm-caches
> @@ -0,0 +1,40 @@
> +Disabling I-cache:
> +- Set CONFIG_SYS_ICACHE_OFF
> +
> +Disabling D-cache:
> +- Set CONFIG_SYS_DCACHE_OFF
> +
> +Enabling I-cache:
> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
> +
> +Enabling D-cache:
> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
> +
> +Enabling Caches at System Startup:
> +- Implement enable_caches() for your platform and enable the I-cache and
> +  D-cache from this function. This function is called immediately
> +  after relocation.
> +
> +Guidelines for Working with D-cache:
> +
> +Memory to Peripheral DMA:
> +- Flush the buffer after the MPU writes the data and before the DMA is
> +  initiated.
> +
> +Peripheral to Memory DMA:
> +- Invalidate the buffer after the DMA is complete and before the MPU reads
> +  it.

The other discussion we are having about caches made me realize that I 
need to improve the above guideline like this:

Peripheral to Memory DMA:
- Invalidate the buffer before starting the DMA(This is for preventing
cache-line replacements from corrupting the DMA buffer during the
course of DMA).
- Invalidate the buffer after the DMA is complete and before the MPU
reads it.

I shall submit an updated patch for this.

best regards,
Aneesh

[U-Boot] [PATCH v 1/3] arm: do not force d-cache enable on all boards

[Albert ARIBAUD]

Hi Aneesh,

Le 05/08/2011 17:07, Aneesh V a ?crit :
> Hi Albert,
>
> On Monday 01 August 2011 04:48 PM, Aneesh V wrote:
>> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
>> to board_init_r(). This enables d-cache for all ARM boards.
>> As a result some of the arm boards that are not cache-ready
>> are broken. Revert this change and allow platform code to
>> take the decision on d-cache enabling.
>>
>> Also add some documentation for cache usage in ARM.
>>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>> MAKEALL pending. Will update the results tomorrow.
>> ---
>> arch/arm/lib/board.c | 8 +++-----
>> arch/arm/lib/cache.c | 12 ++++++++++++
>> doc/README.arm-caches | 40 ++++++++++++++++++++++++++++++++++++++++
>> include/common.h | 1 +
>> 4 files changed, 56 insertions(+), 5 deletions(-)
>> create mode 100644 doc/README.arm-caches
>>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index 90709d0..d093d5b 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -446,11 +446,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
>> gd->flags |= GD_FLG_RELOC; /* tell others: relocation done */
>>
>> monitor_flash_len = _end_ofs;
>> - /*
>> - * Enable D$:
>> - * I$, if needed, must be already enabled in start.S
>> - */
>> - dcache_enable();
>> +
>> + /* Enable caches */
>> + enable_caches();
>>
>> debug ("monitor flash len: %08lX\n", monitor_flash_len);
>> board_init(); /* Setup chipselects */
>> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
>> index 92b61a2..b545fb7 100644
>> --- a/arch/arm/lib/cache.c
>> +++ b/arch/arm/lib/cache.c
>> @@ -53,3 +53,15 @@ void __flush_dcache_all(void)
>> }
>> void flush_dcache_all(void)
>> __attribute__((weak, alias("__flush_dcache_all")));
>> +
>> +
>> +/*
>> + * Default implementation of enable_caches()
>> + * Real implementation should be in platform code
>> + */
>> +void __enable_caches(void)
>> +{
>> + puts("WARNING: Caches not enabled\n");
>> +}
>> +void enable_caches(void)
>> + __attribute__((weak, alias("__enable_caches")));
>> diff --git a/doc/README.arm-caches b/doc/README.arm-caches
>> new file mode 100644
>> index 0000000..9edc252
>> --- /dev/null
>> +++ b/doc/README.arm-caches
>> @@ -0,0 +1,40 @@
>> +Disabling I-cache:
>> +- Set CONFIG_SYS_ICACHE_OFF
>> +
>> +Disabling D-cache:
>> +- Set CONFIG_SYS_DCACHE_OFF
>> +
>> +Enabling I-cache:
>> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
>> +
>> +Enabling D-cache:
>> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
>> +
>> +Enabling Caches at System Startup:
>> +- Implement enable_caches() for your platform and enable the I-cache and
>> + D-cache from this function. This function is called immediately
>> + after relocation.
>> +
>> +Guidelines for Working with D-cache:
>> +
>> +Memory to Peripheral DMA:
>> +- Flush the buffer after the MPU writes the data and before the DMA is
>> + initiated.
>> +
>> +Peripheral to Memory DMA:
>> +- Invalidate the buffer after the DMA is complete and before the MPU
>> reads
>> + it.
>
> The other discussion we are having about caches made me realize that I
> need to improve the above guideline like this:
>
> Peripheral to Memory DMA:
> - Invalidate the buffer before starting the DMA(This is for preventing
> cache-line replacements from corrupting the DMA buffer during the
> course of DMA).

Correct, but maybe you should indicate why this happens (not all people 
will realize that some *other* use of the cache might cause eviction of 
a cache line, thus a possible unexpected flush).

> - Invalidate the buffer after the DMA is complete and before the MPU
> reads it.
>
> I shall submit an updated patch for this.

Thanks!

> best regards,
> Aneesh

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 0/4] arm: changes in cache handling

[Aneesh V]

- Avoid enabling caches for all ARM boards
- Enable caches for omap3/4
- Stronger barrier for armv7 cache-maintenance operations.

V2:
* Rebased to latest HEAD of u-boot/master
* Improved the README
* Added a patch for removing the flush in invalidate
  and for printing a warning in such cases.

Aneesh V (4):
  arm: do not force d-cache enable on all boards
  omap: enable caches at system start-up
  armv7: stronger barrier for cache-maintenance operations
  armv7: cache: remove flush on un-aligned invalidate

 arch/arm/cpu/armv7/cache_v7.c    |   26 ++++++++++---------
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++
 arch/arm/lib/board.c             |    8 ++---
 arch/arm/lib/cache-pl310.c       |   15 ++++++----
 arch/arm/lib/cache.c             |   12 +++++++++
 doc/README.arm-caches            |   51 ++++++++++++++++++++++++++++++++++++++
 include/common.h                 |    1 +
 8 files changed, 106 insertions(+), 23 deletions(-)
 create mode 100644 doc/README.arm-caches

[U-Boot] [PATCH 0/4] arm: changes in cache handling

[Aneesh V]

Hi Wolfgang, Albert,

On Tuesday 09 August 2011 04:40 PM, Aneesh V wrote:
> - Avoid enabling caches for all ARM boards
> - Enable caches for omap3/4
> - Stronger barrier for armv7 cache-maintenance operations.
>
> V2:
> * Rebased to latest HEAD of u-boot/master
> * Improved the README
> * Added a patch for removing the flush in invalidate
>    and for printing a warning in such cases.

I forgot to add v2 tag in the subject line for this series. Shall I
reject and archive these patches in patchworks?

best regards,
Aneesh

[U-Boot] [PATCH 1/4] arm: do not force d-cache enable on all boards

[Aneesh V]

c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
to board_init_r(). This enables d-cache for all ARM boards.
As a result some of the arm boards that are not cache-ready
are broken. Revert this change and allow platform code to
take the decision on d-cache enabling.

Also add some documentation for cache usage in ARM.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
 Updated with additional guidelines in the README.
---
 arch/arm/lib/board.c  |    8 ++----
 arch/arm/lib/cache.c  |   12 +++++++++++
 doc/README.arm-caches |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h      |    1 +
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 doc/README.arm-caches

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 14a56f6..38a31d8 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
 	monitor_flash_len = _end_ofs;
-	/*
-	 * Enable D$:
-	 * I$, if needed, must be already enabled in start.S
-	 */
-	dcache_enable();
+
+	/* Enable caches */
+	enable_caches();
 
 	debug("monitor flash len: %08lX\n", monitor_flash_len);
 	board_init();	/* Setup chipselects */
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..b545fb7 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+
+/*
+ * Default implementation of enable_caches()
+ * Real implementation should be in platform code
+ */
+void __enable_caches(void)
+{
+	puts("WARNING: Caches not enabled\n");
+}
+void enable_caches(void)
+	__attribute__((weak, alias("__enable_caches")));
diff --git a/doc/README.arm-caches b/doc/README.arm-caches
new file mode 100644
index 0000000..a833eee
--- /dev/null
+++ b/doc/README.arm-caches
@@ -0,0 +1,51 @@
+Disabling I-cache:
+- Set CONFIG_SYS_ICACHE_OFF
+
+Disabling D-cache:
+- Set CONFIG_SYS_DCACHE_OFF
+
+Enabling I-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
+
+Enabling D-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
+
+Enabling Caches at System Startup:
+- Implement enable_caches() for your platform and enable the I-cache and
+  D-cache from this function. This function is called immediately
+  after relocation.
+
+Guidelines for Working with D-cache:
+
+Memory to Peripheral DMA:
+- Flush the buffer after the MPU writes the data and before the DMA is
+  initiated.
+
+Peripheral to Memory DMA:
+- Invalidate the buffer before starting the DMA. In case there are any dirty
+  lines from the DMA buffer in the cache, subsequent cache-line replacements
+  may corrupt the buffer in memory while the DMA is still going on. Cache-line
+  replacement can happen if the CPU tries to bring some other memory locations
+  into the cache while the DMA is going on.
+- Invalidate the buffer after the DMA is complete and before the MPU reads
+  it. This may be needed in addition to the invalidation before the DMA
+  mentioned above, because in some processors memory contents can spontaneously
+  come to the cache due to speculative memory access by the CPU. If this
+  happens with the DMA buffer while DMA is going on we have a coherency problem.
+
+Buffer Requirements:
+- Any buffer that is invalidated(that is, typically the peripheral to
+  memory DMA buffer) should be aligned to cache-line boundary both at
+  at the beginning and at the end of the buffer.
+- If the buffer is not cache-line aligned invalidation will be restricted
+  to the aligned part. That is, one cache-line at the respective boundary
+  may be left out while doing invalidation.
+
+Cleanup Before Linux:
+- cleanup_before_linux() should flush the D-cache, invalidate I-cache, and
+  disable MMU and caches.
+- The following sequence is advisable while disabling d-cache:
+  1. disable_dcache() - flushes and disables d-cache
+  2. invalidate_dcache_all() - invalid any entry that came to the cache
+	in the short period after the cache was flushed but before the
+	cache got disabled.
diff --git a/include/common.h b/include/common.h
index 12a1074..bd10f31 100644
--- a/include/common.h
+++ b/include/common.h
@@ -616,6 +616,7 @@ ulong	lcd_setmem (ulong);
 ulong	video_setmem (ulong);
 
 /* arch/$(ARCH)/lib/cache.c */
+void	enable_caches(void);
 void	flush_cache   (unsigned long, unsigned long);
 void	flush_dcache_all(void);
 void	flush_dcache_range(unsigned long start, unsigned long stop);
-- 
1.7.0.4

[U-Boot] [PATCH 2/4] omap: enable caches at system start-up

[Aneesh V]

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 4aaf97b..2c3d7f3 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -402,3 +402,11 @@ void v7_outer_cache_disable(void)
 	omap3_update_aux_cr(0, 0x2);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index 5943d61..8f7c375 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -299,3 +299,11 @@ void v7_outer_cache_disable(void)
 	set_pl310_ctrl_reg(0);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
-- 
1.7.0.4

[U-Boot] [PATCH 3/4] armv7: stronger barrier for cache-maintenance operations

[Aneesh V]

set-way operations need a DSB after them to ensure the
operation is complete. DMB may not be enough. Use DSB
after all operations instead of DMB.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/cache_v7.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 3e1e1bf..665f025 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -81,8 +81,8 @@ static void v7_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
@@ -108,8 +108,8 @@ static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_maint_dcache_level_setway(u32 level, u32 operation)
@@ -227,8 +227,8 @@ static void v7_dcache_maint_range(u32 start, u32 stop, u32 range_op)
 		break;
 	}
 
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 /* Invalidate TLB */
-- 
1.7.0.4

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Aneesh V]

Remove the flush of boundary cache-lines done as part
of invalidate on a non cache-line boundary aligned
buffer

Also, print a warning when this situation is recognized.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
 New in V2
---
 arch/arm/cpu/armv7/cache_v7.c |   14 ++++++++------
 arch/arm/lib/cache-pl310.c    |   15 +++++++++------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 665f025..3dda56b 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -181,21 +181,23 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
 	u32 mva;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_len - 1)) {
-		v7_dcache_clean_inval_range(start, start + 1, line_len);
+		printf("WARNING: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_len - 1) & ~(line_len - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_len - 1)) {
-		v7_dcache_clean_inval_range(stop, stop + 1, line_len);
+		printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_len - 1);
 	}
diff --git a/arch/arm/lib/cache-pl310.c b/arch/arm/lib/cache-pl310.c
index 36c629c..15fe304 100644
--- a/arch/arm/lib/cache-pl310.c
+++ b/arch/arm/lib/cache-pl310.c
@@ -26,6 +26,7 @@
 #include <asm/armv7.h>
 #include <asm/pl310.h>
 #include <config.h>
+#include <common.h>
 
 struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
 
@@ -89,21 +90,23 @@ void v7_outer_cache_inval_range(u32 start, u32 stop)
 	u32 pa, line_size = 32;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_size - 1)) {
-		v7_outer_cache_flush_range(start, start + 1);
+		printf("WARNING: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_size - 1) & ~(line_size - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_size - 1)) {
-		v7_outer_cache_flush_range(stop, stop + 1);
+		printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_size - 1);
 	}
-- 
1.7.0.4

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Anton Staaf]

I'm not sure what the larger context of this change is, but it seems
like a bad idea to me.  There are a lot of locations in U-Boot that
will end up causing an unaligned invalidate (ext2 and dos file system
code in particular).  And this change will cause those unaligned
invalidates to possibly throw away stores to adjacent variables.  If
you are going to make this change you should at least assert instead
of just printing a warning.  And there should be a concerted effort to
clean up the buffer management in U-Boot so that invalidates will
never be unaligned.  This is also a departure from the cache
management implementations in the Linux kernel, not that U-Boot has to
do exactly what they do, but I feel they have the correct
implementation, from the perspective of ensuring that all stores
actually make it to main memory.

Thanks,
    Anton

On Tue, Aug 9, 2011 at 4:10 AM, Aneesh V <aneesh@ti.com> wrote:
> Remove the flush of boundary cache-lines done as part
> of invalidate on a non cache-line boundary aligned
> buffer
>
> Also, print a warning when this situation is recognized.
>
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
> V2:
> ?New in V2
> ---
> ?arch/arm/cpu/armv7/cache_v7.c | ? 14 ++++++++------
> ?arch/arm/lib/cache-pl310.c ? ?| ? 15 +++++++++------
> ?2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
> index 665f025..3dda56b 100644
> --- a/arch/arm/cpu/armv7/cache_v7.c
> +++ b/arch/arm/cpu/armv7/cache_v7.c
> @@ -181,21 +181,23 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
> ? ? ? ?u32 mva;
>
> ? ? ? ?/*
> - ? ? ? ?* If start address is not aligned to cache-line flush the first
> - ? ? ? ?* line to prevent affecting somebody else's buffer
> + ? ? ? ?* If start address is not aligned to cache-line do not
> + ? ? ? ?* invalidate the first cache-line
> ? ? ? ? */
> ? ? ? ?if (start & (line_len - 1)) {
> - ? ? ? ? ? ? ? v7_dcache_clean_inval_range(start, start + 1, line_len);
> + ? ? ? ? ? ? ? printf("WARNING: %s - start address is not aligned - 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, start);
> ? ? ? ? ? ? ? ?/* move to next cache line */
> ? ? ? ? ? ? ? ?start = (start + line_len - 1) & ~(line_len - 1);
> ? ? ? ?}
>
> ? ? ? ?/*
> - ? ? ? ?* If stop address is not aligned to cache-line flush the last
> - ? ? ? ?* line to prevent affecting somebody else's buffer
> + ? ? ? ?* If stop address is not aligned to cache-line do not
> + ? ? ? ?* invalidate the last cache-line
> ? ? ? ? */
> ? ? ? ?if (stop & (line_len - 1)) {
> - ? ? ? ? ? ? ? v7_dcache_clean_inval_range(stop, stop + 1, line_len);
> + ? ? ? ? ? ? ? printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, stop);
> ? ? ? ? ? ? ? ?/* align to the beginning of this cache line */
> ? ? ? ? ? ? ? ?stop &= ~(line_len - 1);
> ? ? ? ?}
> diff --git a/arch/arm/lib/cache-pl310.c b/arch/arm/lib/cache-pl310.c
> index 36c629c..15fe304 100644
> --- a/arch/arm/lib/cache-pl310.c
> +++ b/arch/arm/lib/cache-pl310.c
> @@ -26,6 +26,7 @@
> ?#include <asm/armv7.h>
> ?#include <asm/pl310.h>
> ?#include <config.h>
> +#include <common.h>
>
> ?struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>
> @@ -89,21 +90,23 @@ void v7_outer_cache_inval_range(u32 start, u32 stop)
> ? ? ? ?u32 pa, line_size = 32;
>
> ? ? ? ?/*
> - ? ? ? ?* If start address is not aligned to cache-line flush the first
> - ? ? ? ?* line to prevent affecting somebody else's buffer
> + ? ? ? ?* If start address is not aligned to cache-line do not
> + ? ? ? ?* invalidate the first cache-line
> ? ? ? ? */
> ? ? ? ?if (start & (line_size - 1)) {
> - ? ? ? ? ? ? ? v7_outer_cache_flush_range(start, start + 1);
> + ? ? ? ? ? ? ? printf("WARNING: %s - start address is not aligned - 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, start);
> ? ? ? ? ? ? ? ?/* move to next cache line */
> ? ? ? ? ? ? ? ?start = (start + line_size - 1) & ~(line_size - 1);
> ? ? ? ?}
>
> ? ? ? ?/*
> - ? ? ? ?* If stop address is not aligned to cache-line flush the last
> - ? ? ? ?* line to prevent affecting somebody else's buffer
> + ? ? ? ?* If stop address is not aligned to cache-line do not
> + ? ? ? ?* invalidate the last cache-line
> ? ? ? ? */
> ? ? ? ?if (stop & (line_size - 1)) {
> - ? ? ? ? ? ? ? v7_outer_cache_flush_range(stop, stop + 1);
> + ? ? ? ? ? ? ? printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? __func__, stop);
> ? ? ? ? ? ? ? ?/* align to the beginning of this cache line */
> ? ? ? ? ? ? ? ?stop &= ~(line_size - 1);
> ? ? ? ?}
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Albert ARIBAUD]

Hi Anton,

Le 09/08/2011 18:39, Anton Staaf a ?crit :
> I'm not sure what the larger context of this change is, but it seems
> like a bad idea to me.  There are a lot of locations in U-Boot that
> will end up causing an unaligned invalidate (ext2 and dos file system
> code in particular).  And this change will cause those unaligned
> invalidates to possibly throw away stores to adjacent variables.  If
> you are going to make this change you should at least assert instead
> of just printing a warning.  And there should be a concerted effort to
> clean up the buffer management in U-Boot so that invalidates will
> never be unaligned.  This is also a departure from the cache
> management implementations in the Linux kernel, not that U-Boot has to
> do exactly what they do, but I feel they have the correct
> implementation, from the perspective of ensuring that all stores
> actually make it to main memory.

There's been a lot of discussion, both in July and August, about this 
and the the conclusion is that unaligned invalidates (and flushes) 
cannot reliably be done within the size and speed constraints of an 
embedded bootloader, and there are no 100% clean solutions without 
alignment, because with unaligned buffers, one might invalidate a cache 
line containing also deferred writes, or flush a line onto a buffer 
currently undergoing DMA. Linux can afford to spend more space and time 
than U-Boot to solving those issues. OTOH, aligning buffers is a 
relatively trivial change in the calling code that allows for much 
simpler and more efficient cache management.

The current cache patches on ARM are precisely the start of the effort 
to align buffers that you suggest, by warning the developer about 
unaligned invalidates/flushes through the console, and turning any side 
effect to an "inside-effect", so to speak: for instance the recent cache 
invalidate patch on arm926ejs prevents unaligned invalidates from 
affecting any other data than the (badly) invalidated buffer itself.

You have a point though that maybe a warning is not enough and that 
unalignments warrant an assert instead. OTOH, that might mean *a lot* of 
boards will completely cease working in this case, even for live 
debugging the issue with basic U-Boot commands (which many developers do 
I am sure).

Comments on this warning/assert point?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Aneesh V]

Hi Anton,

On Tuesday 09 August 2011 10:09 PM, Anton Staaf wrote:
> I'm not sure what the larger context of this change is, but it seems
> like a bad idea to me.  There are a lot of locations in U-Boot that

Please see this thread for the context.
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135


> will end up causing an unaligned invalidate (ext2 and dos file system
> code in particular).  And this change will cause those unaligned
> invalidates to possibly throw away stores to adjacent variables.  If

No. Those partial cache-lines on the boundary are left alone. They are
not invalidated. So, it still affects only the party calling the
invalidate.

> you are going to make this change you should at least assert instead
> of just printing a warning.  And there should be a concerted effort to
> clean up the buffer management in U-Boot so that invalidates will
> never be unaligned.  This is also a departure from the cache
> management implementations in the Linux kernel, not that U-Boot has to
> do exactly what they do, but I feel they have the correct
> implementation, from the perspective of ensuring that all stores
> actually make it to main memory.

Yes, I had implemented it in line with the kernel apporach. However,
with un-aligned buffers there is no perfect solution anyway. So, I
don't have a strong opinion on this. Leaving alone the boundary
cache-lines and printing a big warning seems reasonable enough.

best regards,
Aneesh

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Anton Staaf]

On Tue, Aug 9, 2011 at 11:48 PM, Aneesh V <aneesh@ti.com> wrote:
> Hi Anton,
>
> On Tuesday 09 August 2011 10:09 PM, Anton Staaf wrote:
>>
>> I'm not sure what the larger context of this change is, but it seems
>> like a bad idea to me. ?There are a lot of locations in U-Boot that
>
> Please see this thread for the context.
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135

Ahh, I missed this thread (Its title became stale).  :)  But I
completely agree with the outcome, we need to fix the unaligned buffer
problem.

>
>> will end up causing an unaligned invalidate (ext2 and dos file system
>> code in particular). ?And this change will cause those unaligned
>> invalidates to possibly throw away stores to adjacent variables. ?If
>
> No. Those partial cache-lines on the boundary are left alone. They are
> not invalidated. So, it still affects only the party calling the
> invalidate.

Ahh, you are correct.  I missed that the change would cause fewer
cache lines to be invalidated.  In this case I am much happier with
this change.  In light of this I still think the warning is a little
mild, since it means that the driver that called the invalidate is
certainly going to get the wrong values.  Perhaps changing it to an
error would be good (I realize that functionally it would be
identical, but it would be more potent psychologically).  I don't
think an assert is warranted in this case since as Albert points out
it would prevent "online" debugging of U-Boot which is a very useful
way of working.

>> you are going to make this change you should at least assert instead
>> of just printing a warning. ?And there should be a concerted effort to
>> clean up the buffer management in U-Boot so that invalidates will
>> never be unaligned. ?This is also a departure from the cache
>> management implementations in the Linux kernel, not that U-Boot has to
>> do exactly what they do, but I feel they have the correct
>> implementation, from the perspective of ensuring that all stores
>> actually make it to main memory.
>
> Yes, I had implemented it in line with the kernel apporach. However,
> with un-aligned buffers there is no perfect solution anyway. So, I
> don't have a strong opinion on this. Leaving alone the boundary
> cache-lines and printing a big warning seems reasonable enough.

Yup.

> best regards,
> Aneesh
>

I have a number of patches to fix unaligned buffer use in the
filesystem (ext2, dos, mmc and partition) code that I will start
sending upstream today.  Some of them will be more RFCs than patch
sets.  :)

Thanks,
    Anton

[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Aneesh V]

On Wednesday 10 August 2011 11:41 PM, Anton Staaf wrote:
> On Tue, Aug 9, 2011 at 11:48 PM, Aneesh V<aneesh@ti.com>  wrote:
>> Hi Anton,
>>
>> On Tuesday 09 August 2011 10:09 PM, Anton Staaf wrote:
>>>
>>> I'm not sure what the larger context of this change is, but it seems
>>> like a bad idea to me.  There are a lot of locations in U-Boot that
>>
>> Please see this thread for the context.
>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135
>
> Ahh, I missed this thread (Its title became stale).  :)  But I
> completely agree with the outcome, we need to fix the unaligned buffer
> problem.
>
>>
>>> will end up causing an unaligned invalidate (ext2 and dos file system
>>> code in particular).  And this change will cause those unaligned
>>> invalidates to possibly throw away stores to adjacent variables.  If
>>
>> No. Those partial cache-lines on the boundary are left alone. They are
>> not invalidated. So, it still affects only the party calling the
>> invalidate.
>
> Ahh, you are correct.  I missed that the change would cause fewer
> cache lines to be invalidated.  In this case I am much happier with
> this change.  In light of this I still think the warning is a little
> mild, since it means that the driver that called the invalidate is
> certainly going to get the wrong values.  Perhaps changing it to an
> error would be good (I realize that functionally it would be
> identical, but it would be more potent psychologically).  I don't
> think an assert is warranted in this case since as Albert points out
> it would prevent "online" debugging of U-Boot which is a very useful
> way of working.

Ok. I will change the warning to an error.

br,
Aneesh

[U-Boot] [PATCH v2 0/4] arm: changes in cache handling

[Aneesh V]

- Avoid enabling caches for all ARM boards
- Enable caches for omap3/4
- Stronger barrier for armv7 cache-maintenance operations.

V2:
* Rebased to latest HEAD of u-boot/master
* Improved the README
* Added a patch for removing the flush in invalidate
  and for printing a warning in such cases.

Aneesh V (4):
  arm: do not force d-cache enable on all boards
  omap: enable caches at system start-up
  armv7: stronger barrier for cache-maintenance operations
  armv7: cache: remove flush on un-aligned invalidate

 arch/arm/cpu/armv7/cache_v7.c    |   26 ++++++++++---------
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++
 arch/arm/lib/board.c             |    8 ++---
 arch/arm/lib/cache-pl310.c       |   15 ++++++----
 arch/arm/lib/cache.c             |   12 +++++++++
 doc/README.arm-caches            |   51 ++++++++++++++++++++++++++++++++++++++
 include/common.h                 |    1 +
 8 files changed, 106 insertions(+), 23 deletions(-)
 create mode 100644 doc/README.arm-caches

[U-Boot] [PATCH v2 1/4] arm: do not force d-cache enable on all boards

[Aneesh V]

c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
to board_init_r(). This enables d-cache for all ARM boards.
As a result some of the arm boards that are not cache-ready
are broken. Revert this change and allow platform code to
take the decision on d-cache enabling.

Also add some documentation for cache usage in ARM.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
 Updated with additional guidelines in the README.
---
 arch/arm/lib/board.c  |    8 ++----
 arch/arm/lib/cache.c  |   12 +++++++++++
 doc/README.arm-caches |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h      |    1 +
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 doc/README.arm-caches

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 14a56f6..38a31d8 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
 	monitor_flash_len = _end_ofs;
-	/*
-	 * Enable D$:
-	 * I$, if needed, must be already enabled in start.S
-	 */
-	dcache_enable();
+
+	/* Enable caches */
+	enable_caches();
 
 	debug("monitor flash len: %08lX\n", monitor_flash_len);
 	board_init();	/* Setup chipselects */
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..b545fb7 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+
+/*
+ * Default implementation of enable_caches()
+ * Real implementation should be in platform code
+ */
+void __enable_caches(void)
+{
+	puts("WARNING: Caches not enabled\n");
+}
+void enable_caches(void)
+	__attribute__((weak, alias("__enable_caches")));
diff --git a/doc/README.arm-caches b/doc/README.arm-caches
new file mode 100644
index 0000000..a833eee
--- /dev/null
+++ b/doc/README.arm-caches
@@ -0,0 +1,51 @@
+Disabling I-cache:
+- Set CONFIG_SYS_ICACHE_OFF
+
+Disabling D-cache:
+- Set CONFIG_SYS_DCACHE_OFF
+
+Enabling I-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
+
+Enabling D-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
+
+Enabling Caches at System Startup:
+- Implement enable_caches() for your platform and enable the I-cache and
+  D-cache from this function. This function is called immediately
+  after relocation.
+
+Guidelines for Working with D-cache:
+
+Memory to Peripheral DMA:
+- Flush the buffer after the MPU writes the data and before the DMA is
+  initiated.
+
+Peripheral to Memory DMA:
+- Invalidate the buffer before starting the DMA. In case there are any dirty
+  lines from the DMA buffer in the cache, subsequent cache-line replacements
+  may corrupt the buffer in memory while the DMA is still going on. Cache-line
+  replacement can happen if the CPU tries to bring some other memory locations
+  into the cache while the DMA is going on.
+- Invalidate the buffer after the DMA is complete and before the MPU reads
+  it. This may be needed in addition to the invalidation before the DMA
+  mentioned above, because in some processors memory contents can spontaneously
+  come to the cache due to speculative memory access by the CPU. If this
+  happens with the DMA buffer while DMA is going on we have a coherency problem.
+
+Buffer Requirements:
+- Any buffer that is invalidated(that is, typically the peripheral to
+  memory DMA buffer) should be aligned to cache-line boundary both at
+  at the beginning and at the end of the buffer.
+- If the buffer is not cache-line aligned invalidation will be restricted
+  to the aligned part. That is, one cache-line at the respective boundary
+  may be left out while doing invalidation.
+
+Cleanup Before Linux:
+- cleanup_before_linux() should flush the D-cache, invalidate I-cache, and
+  disable MMU and caches.
+- The following sequence is advisable while disabling d-cache:
+  1. disable_dcache() - flushes and disables d-cache
+  2. invalidate_dcache_all() - invalid any entry that came to the cache
+	in the short period after the cache was flushed but before the
+	cache got disabled.
diff --git a/include/common.h b/include/common.h
index 12a1074..bd10f31 100644
--- a/include/common.h
+++ b/include/common.h
@@ -616,6 +616,7 @@ ulong	lcd_setmem (ulong);
 ulong	video_setmem (ulong);
 
 /* arch/$(ARCH)/lib/cache.c */
+void	enable_caches(void);
 void	flush_cache   (unsigned long, unsigned long);
 void	flush_dcache_all(void);
 void	flush_dcache_range(unsigned long start, unsigned long stop);
-- 
1.7.0.4

[U-Boot] d-cache enable

[Lukasz Majewski]

Dear all,

As we know dcache is now enabled in u-boot.

I'm trying to make the S5P Goni target working with d-cache enabled.
There are some patches and ideas appearing on the list (e.g.
http://patchwork.ozlabs.org/patch/109199/ made by Aneesh V)


I'm currently using the u-boot/master branch,
SHA1: d26a82023af5771462f7223241ed18cfb7965f71 

After some research I can say that flush_dcache_all() and
invalidate_dcache_all() are working(at least on my target).

However I'm planning to use the "range" versions:
	flush_dcache_range((unsigned long) (buf), sizeof(buf));
	invalidate_dcache_range((unsigned long) (buf), sizeof(buf));

Those versions are not working on the Cortex-A8 (armv7) GONI target.
I'd like to ask if anybody was trying to use those functions
(defined at cache_v7.c) on other armv7 targets? 

I think that only flush or invalidation on the selective cache regions
are the way to go. The "*_dcache_all()" methods seems like an overkill.


-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group

[U-Boot] d-cache enable

[Aneesh V]

Hi Lukasz,

On Tuesday 09 August 2011 08:11 PM, Lukasz Majewski wrote:
> Dear all,
> 
> As we know dcache is now enabled in u-boot.
> 
> I'm trying to make the S5P Goni target working with d-cache enabled.
> There are some patches and ideas appearing on the list (e.g.
> http://patchwork.ozlabs.org/patch/109199/ made by Aneesh V)
> 
> 
> I'm currently using the u-boot/master branch,
> SHA1: d26a82023af5771462f7223241ed18cfb7965f71 
> 
> After some research I can say that flush_dcache_all() and
> invalidate_dcache_all() are working(at least on my target).
> 
> However I'm planning to use the "range" versions:
> 	flush_dcache_range((unsigned long) (buf), sizeof(buf));
> 	invalidate_dcache_range((unsigned long) (buf), sizeof(buf));
> 
> Those versions are not working on the Cortex-A8 (armv7) GONI target.
> I'd like to ask if anybody was trying to use those functions
> (defined at cache_v7.c) on other armv7 targets? 

I have tested cache on OMAP3(Cortex-A8) and OMAP4(Cortex-A9). Why do you
think it's not working for you. Did you run some tests? If so, what was
the result? Are you enabling only L1 or both L1 & L2? Can you try the
attached crude patch for testing caches. You might have to change the
addresses according to your platform. If you could run it, let me know
the results.

best regards,
Aneesh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-u-boot-cache-test.patch
Type: text/x-patch
Size: 0 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110812/7c7b6d81/attachment.bin 

[U-Boot] d-cache enable

[Lukasz Majewski]

Hi Aneesh,

On Fri, 12 Aug 2011 16:29:25 +0530
Aneesh V <aneesh@ti.com> wrote:

> Hi Lukasz,
> 
> On Tuesday 09 August 2011 08:11 PM, Lukasz Majewski wrote:
> > Dear all,
> > 
> > As we know dcache is now enabled in u-boot.
> > 
> > I'm trying to make the S5P Goni target working with d-cache enabled.
> > There are some patches and ideas appearing on the list (e.g.
> > http://patchwork.ozlabs.org/patch/109199/ made by Aneesh V)
> > 
> > 
> > I'm currently using the u-boot/master branch,
> > SHA1: d26a82023af5771462f7223241ed18cfb7965f71 
> > 
> > After some research I can say that flush_dcache_all() and
> > invalidate_dcache_all() are working(at least on my target).
> > 
> > However I'm planning to use the "range" versions:
> > 	flush_dcache_range((unsigned long) (buf), sizeof(buf));
> > 	invalidate_dcache_range((unsigned long) (buf), sizeof(buf));
> > 
> > Those versions are not working on the Cortex-A8 (armv7) GONI target.
> > I'd like to ask if anybody was trying to use those functions
> > (defined at cache_v7.c) on other armv7 targets? 
> 
> I have tested cache on OMAP3(Cortex-A8) and OMAP4(Cortex-A9). Why do
> you think it's not working for you. Did you run some tests? If so,
> what was the result? Are you enabling only L1 or both L1 & L2? Can
> you try the attached crude patch for testing caches. You might have
> to change the addresses according to your platform. If you could run
> it, let me know the results.
> 
> best regards,
> Aneesh

It is embarrassing to admit, but I've __wrongly__ assumed that *_range()
functions are accepting the start address and range for
invalidation/flushing. 

Yes, they are working when I've realized how to use them :-).

My tests are not so sophisticated as yours since I'm using TRACE32 to
observe if the cache has been invalidated/flushed. 

I will try to apply your test suite. Thanks for sharing them.

-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group

[U-Boot] d-cache enable

[Aneesh V]

Hi Lukasz,

On Friday 12 August 2011 05:11 PM, Lukasz Majewski wrote:
> Hi Aneesh,
> 
> On Fri, 12 Aug 2011 16:29:25 +0530
> Aneesh V <aneesh@ti.com> wrote:
> 
>> Hi Lukasz,
>>
>> On Tuesday 09 August 2011 08:11 PM, Lukasz Majewski wrote:
>>> Dear all,
>>>
>>> As we know dcache is now enabled in u-boot.
>>>
>>> I'm trying to make the S5P Goni target working with d-cache enabled.
>>> There are some patches and ideas appearing on the list (e.g.
>>> http://patchwork.ozlabs.org/patch/109199/ made by Aneesh V)
>>>
>>>
>>> I'm currently using the u-boot/master branch,
>>> SHA1: d26a82023af5771462f7223241ed18cfb7965f71 
>>>
>>> After some research I can say that flush_dcache_all() and
>>> invalidate_dcache_all() are working(at least on my target).
>>>
>>> However I'm planning to use the "range" versions:
>>> 	flush_dcache_range((unsigned long) (buf), sizeof(buf));
>>> 	invalidate_dcache_range((unsigned long) (buf), sizeof(buf));
>>>
>>> Those versions are not working on the Cortex-A8 (armv7) GONI target.
>>> I'd like to ask if anybody was trying to use those functions
>>> (defined at cache_v7.c) on other armv7 targets? 
>>
>> I have tested cache on OMAP3(Cortex-A8) and OMAP4(Cortex-A9). Why do
>> you think it's not working for you. Did you run some tests? If so,
>> what was the result? Are you enabling only L1 or both L1 & L2? Can
>> you try the attached crude patch for testing caches. You might have
>> to change the addresses according to your platform. If you could run
>> it, let me know the results.
>>
>> best regards,
>> Aneesh
> 
> It is embarrassing to admit, but I've __wrongly__ assumed that *_range()
> functions are accepting the start address and range for
> invalidation/flushing. 
> 

There is already some confusion around that. As pointed out by Reinhard
some MIPS implementations actually interpret the parameters that way.
But at least for arm, it's [strat, stop).

> Yes, they are working when I've realized how to use them :-).

Good to know that.

best regards,
Aneesh

[U-Boot] d-cache enable

[Albert ARIBAUD]

Hi Lukasz,

On 12/08/2011 13:41, Lukasz Majewski wrote:

> It is embarrassing to admit, but I've __wrongly__ assumed that *_range()
> functions are accepting the start address and range for
> invalidation/flushing.

Do you mean we're hitting again a confusion between *_range(start, stop) 
and *_range(start, length)?

If so, then the need to get rid of 'anonymous prototypes' becomes 
greater yet.

Amicalement,
-- 
Albert.

[U-Boot] d-cache enable

[Reinhard Meyer]

Dear *ALL*,
>> It is embarrassing to admit, but I've __wrongly__ assumed that *_range()
>> functions are accepting the start address and range for
>> invalidation/flushing.
> 
> Do you mean we're hitting again a confusion between *_range(start, stop) 
> and *_range(start, length)?
> 
> If so, then the need to get rid of 'anonymous prototypes' becomes 
> greater yet.

Maybe the parameters should even be of type (void *start, void *beyond_end) ?
We are talking of addresses here anyway. Most likely the calling place
has it as pointer, and inside the function it might have to be casted.

Reinhard

[U-Boot] d-cache enable

[Albert ARIBAUD]

Hi Reinhard,

On 12/08/2011 14:32, Reinhard Meyer wrote:
> Dear *ALL*,
>>> It is embarrassing to admit, but I've __wrongly__ assumed that *_range()
>>> functions are accepting the start address and range for
>>> invalidation/flushing.
>>
>> Do you mean we're hitting again a confusion between *_range(start, stop)
>> and *_range(start, length)?
>>
>> If so, then the need to get rid of 'anonymous prototypes' becomes
>> greater yet.
>
> Maybe the parameters should even be of type (void *start, void *beyond_end) ?
> We are talking of addresses here anyway. Most likely the calling place
> has it as pointer, and inside the function it might have to be casted.

Agreed, but that's an overall change to make across all architectures, 
not only ARM -- I don't want to add signature divergence to the current 
issue.

> Reinhard

Amicalement,
-- 
Albert.

[U-Boot] d-cache enable

[Lukasz Majewski]

Hi Albert,

On Fri, 12 Aug 2011 14:21:18 +0200
Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:

> Hi Lukasz,
> 
> On 12/08/2011 13:41, Lukasz Majewski wrote:
> 
> > It is embarrassing to admit, but I've __wrongly__ assumed that
> > *_range() functions are accepting the start address and range for
> > invalidation/flushing.
> 
> Do you mean we're hitting again a confusion between *_range(start,
> stop) and *_range(start, length)?
> 
> If so, then the need to get rid of 'anonymous prototypes' becomes 
> greater yet.
> 
> Amicalement,

Yes, it seems so. Maybe it is a matter of my programming habits. 
Anyway I should check semantics, before I started hacking.

-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group

[U-Boot] d-cache enable

[Albert ARIBAUD]

On 12/08/2011 15:08, Lukasz Majewski wrote:
> Hi Albert,
>
> On Fri, 12 Aug 2011 14:21:18 +0200
> Albert ARIBAUD<albert.u.boot@aribaud.net>  wrote:
>
>> Hi Lukasz,
>>
>> On 12/08/2011 13:41, Lukasz Majewski wrote:
>>
>>> It is embarrassing to admit, but I've __wrongly__ assumed that
>>> *_range() functions are accepting the start address and range for
>>> invalidation/flushing.
>>
>> Do you mean we're hitting again a confusion between *_range(start,
>> stop) and *_range(start, length)?
>>
>> If so, then the need to get rid of 'anonymous prototypes' becomes
>> greater yet.
>>
>> Amicalement,
>
> Yes, it seems so. Maybe it is a matter of my programming habits.
> Anyway I should check semantics, before I started hacking.

Your programming habits aside, I consider it bad that a language with 
positional paremeters allows them to have no names, and bad also that 
checking the intended semantics of a C function should require looking 
at its body, however informative that can be.

I know that C++ uses nameless function/method arguments for instance to 
limit "unused parameter" warnings in virtual method overrides, but that 
is barely a justification, and I see no other. Heck, why not go all K&R 
and remove arguments altogether then while we're at it?

Sorry, had to vent out my Friday rant.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 2/4] omap: enable caches at system start-up

[Aneesh V]

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 4aaf97b..2c3d7f3 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -402,3 +402,11 @@ void v7_outer_cache_disable(void)
 	omap3_update_aux_cr(0, 0x2);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index 5943d61..8f7c375 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -299,3 +299,11 @@ void v7_outer_cache_disable(void)
 	set_pl310_ctrl_reg(0);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
-- 
1.7.0.4

[U-Boot] [PATCH v2 3/4] armv7: stronger barrier for cache-maintenance operations

[Aneesh V]

set-way operations need a DSB after them to ensure the
operation is complete. DMB may not be enough. Use DSB
after all operations instead of DMB.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/cache_v7.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 3e1e1bf..665f025 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -81,8 +81,8 @@ static void v7_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
@@ -108,8 +108,8 @@ static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_maint_dcache_level_setway(u32 level, u32 operation)
@@ -227,8 +227,8 @@ static void v7_dcache_maint_range(u32 start, u32 stop, u32 range_op)
 		break;
 	}
 
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 /* Invalidate TLB */
-- 
1.7.0.4

[U-Boot] [PATCH v2 4/4] armv7: cache: remove flush on un-aligned invalidate

[Aneesh V]

Remove the flush of boundary cache-lines done as part
of invalidate on a non cache-line boundary aligned
buffer

Also, print a warning when this situation is recognized.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
 New in V2
---
 arch/arm/cpu/armv7/cache_v7.c |   14 ++++++++------
 arch/arm/lib/cache-pl310.c    |   15 +++++++++------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 665f025..3dda56b 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -181,21 +181,23 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
 	u32 mva;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_len - 1)) {
-		v7_dcache_clean_inval_range(start, start + 1, line_len);
+		printf("WARNING: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_len - 1) & ~(line_len - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_len - 1)) {
-		v7_dcache_clean_inval_range(stop, stop + 1, line_len);
+		printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_len - 1);
 	}
diff --git a/arch/arm/lib/cache-pl310.c b/arch/arm/lib/cache-pl310.c
index 36c629c..15fe304 100644
--- a/arch/arm/lib/cache-pl310.c
+++ b/arch/arm/lib/cache-pl310.c
@@ -26,6 +26,7 @@
 #include <asm/armv7.h>
 #include <asm/pl310.h>
 #include <config.h>
+#include <common.h>
 
 struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
 
@@ -89,21 +90,23 @@ void v7_outer_cache_inval_range(u32 start, u32 stop)
 	u32 pa, line_size = 32;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_size - 1)) {
-		v7_outer_cache_flush_range(start, start + 1);
+		printf("WARNING: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_size - 1) & ~(line_size - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_size - 1)) {
-		v7_outer_cache_flush_range(stop, stop + 1);
+		printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_size - 1);
 	}
-- 
1.7.0.4

[U-Boot] [PATCH v3 0/4] arm: changes in cache handling

[Aneesh V]

- Avoid enabling caches for all ARM boards
- Enable caches for omap3/4
- Stronger barrier for armv7 cache-maintenance operations.

V2:
* Rebased to latest HEAD of u-boot/master
* Improved the README
* Added a patch for removing the flush in invalidate
  and for printing a warning in such cases.

V3:
* Improve the error message on un-aligned invalidate

Aneesh V (4):
  arm: do not force d-cache enable on all boards
  omap: enable caches at system start-up
  armv7: stronger barrier for cache-maintenance operations
  armv7: cache: remove flush on un-aligned invalidate

 arch/arm/cpu/armv7/cache_v7.c    |   26 ++++++++++---------
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++
 arch/arm/lib/board.c             |    8 ++---
 arch/arm/lib/cache-pl310.c       |   15 ++++++----
 arch/arm/lib/cache.c             |   12 +++++++++
 doc/README.arm-caches            |   51 ++++++++++++++++++++++++++++++++++++++
 include/common.h                 |    1 +
 8 files changed, 106 insertions(+), 23 deletions(-)
 create mode 100644 doc/README.arm-caches

[U-Boot] [PATCH v3 0/4] arm: changes in cache handling

[Albert ARIBAUD]

Le 11/08/2011 16:35, Aneesh V a ?crit :
> - Avoid enabling caches for all ARM boards
> - Enable caches for omap3/4
> - Stronger barrier for armv7 cache-maintenance operations.
>
> V2:
> * Rebased to latest HEAD of u-boot/master
> * Improved the README
> * Added a patch for removing the flush in invalidate
>    and for printing a warning in such cases.
>
> V3:
> * Improve the error message on un-aligned invalidate
>
> Aneesh V (4):
>    arm: do not force d-cache enable on all boards
>    omap: enable caches at system start-up
>    armv7: stronger barrier for cache-maintenance operations
>    armv7: cache: remove flush on un-aligned invalidate
>
>   arch/arm/cpu/armv7/cache_v7.c    |   26 ++++++++++---------
>   arch/arm/cpu/armv7/omap3/board.c |    8 ++++++
>   arch/arm/cpu/armv7/omap4/board.c |    8 ++++++
>   arch/arm/lib/board.c             |    8 ++---
>   arch/arm/lib/cache-pl310.c       |   15 ++++++----
>   arch/arm/lib/cache.c             |   12 +++++++++
>   doc/README.arm-caches            |   51 ++++++++++++++++++++++++++++++++++++++
>   include/common.h                 |    1 +
>   8 files changed, 106 insertions(+), 23 deletions(-)
>   create mode 100644 doc/README.arm-caches

Please in the future leave history in individual patches (but you can 
keep int in cover letters as well). That is a burden, but it provides 
for better tracking on patchwork, where cover letters are not recorded, 
and individual patches are the only place where history can appear.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v3 0/4] arm: changes in cache handling

[V, Aneesh]

Hi Albert,

On Sat, Aug 13, 2011 at 3:39 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 11/08/2011 16:35, Aneesh V a ?crit :
>>
>> - Avoid enabling caches for all ARM boards
>> - Enable caches for omap3/4
>> - Stronger barrier for armv7 cache-maintenance operations.
>>
>> V2:
>> * Rebased to latest HEAD of u-boot/master
>> * Improved the README
>> * Added a patch for removing the flush in invalidate
>> ? and for printing a warning in such cases.
>>
>> V3:
>> * Improve the error message on un-aligned invalidate
>>
>> Aneesh V (4):
>> ? arm: do not force d-cache enable on all boards
>> ? omap: enable caches at system start-up
>> ? armv7: stronger barrier for cache-maintenance operations
>> ? armv7: cache: remove flush on un-aligned invalidate
>>
>> ?arch/arm/cpu/armv7/cache_v7.c ? ?| ? 26 ++++++++++---------
>> ?arch/arm/cpu/armv7/omap3/board.c | ? ?8 ++++++
>> ?arch/arm/cpu/armv7/omap4/board.c | ? ?8 ++++++
>> ?arch/arm/lib/board.c ? ? ? ? ? ? | ? ?8 ++---
>> ?arch/arm/lib/cache-pl310.c ? ? ? | ? 15 ++++++----
>> ?arch/arm/lib/cache.c ? ? ? ? ? ? | ? 12 +++++++++
>> ?doc/README.arm-caches ? ? ? ? ? ?| ? 51
>> ++++++++++++++++++++++++++++++++++++++
>> ?include/common.h ? ? ? ? ? ? ? ? | ? ?1 +
>> ?8 files changed, 106 insertions(+), 23 deletions(-)
>> ?create mode 100644 doc/README.arm-caches
>
> Please in the future leave history in individual patches (but you can keep
> int in cover letters as well). That is a burden, but it provides for better
> tracking on patchwork, where cover letters are not recorded, and individual
> patches are the only place where history can appear.

I add history in both cover letter and in individual patches. However,
I change the
history of a patch only if there's some update in that revision. That
is, I do not
add something like:

Changes in V2:
None

Is that fine?

best regards,
Aneesh

[U-Boot] [PATCH v3 1/4] arm: do not force d-cache enable on all boards

[Aneesh V]

c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
to board_init_r(). This enables d-cache for all ARM boards.
As a result some of the arm boards that are not cache-ready
are broken. Revert this change and allow platform code to
take the decision on d-cache enabling.

Also add some documentation for cache usage in ARM.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
 Updated with additional guidelines in the README.
---
 arch/arm/lib/board.c  |    8 ++----
 arch/arm/lib/cache.c  |   12 +++++++++++
 doc/README.arm-caches |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h      |    1 +
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 doc/README.arm-caches

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 14a56f6..38a31d8 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
 	monitor_flash_len = _end_ofs;
-	/*
-	 * Enable D$:
-	 * I$, if needed, must be already enabled in start.S
-	 */
-	dcache_enable();
+
+	/* Enable caches */
+	enable_caches();
 
 	debug("monitor flash len: %08lX\n", monitor_flash_len);
 	board_init();	/* Setup chipselects */
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..b545fb7 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+
+/*
+ * Default implementation of enable_caches()
+ * Real implementation should be in platform code
+ */
+void __enable_caches(void)
+{
+	puts("WARNING: Caches not enabled\n");
+}
+void enable_caches(void)
+	__attribute__((weak, alias("__enable_caches")));
diff --git a/doc/README.arm-caches b/doc/README.arm-caches
new file mode 100644
index 0000000..a833eee
--- /dev/null
+++ b/doc/README.arm-caches
@@ -0,0 +1,51 @@
+Disabling I-cache:
+- Set CONFIG_SYS_ICACHE_OFF
+
+Disabling D-cache:
+- Set CONFIG_SYS_DCACHE_OFF
+
+Enabling I-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
+
+Enabling D-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
+
+Enabling Caches at System Startup:
+- Implement enable_caches() for your platform and enable the I-cache and
+  D-cache from this function. This function is called immediately
+  after relocation.
+
+Guidelines for Working with D-cache:
+
+Memory to Peripheral DMA:
+- Flush the buffer after the MPU writes the data and before the DMA is
+  initiated.
+
+Peripheral to Memory DMA:
+- Invalidate the buffer before starting the DMA. In case there are any dirty
+  lines from the DMA buffer in the cache, subsequent cache-line replacements
+  may corrupt the buffer in memory while the DMA is still going on. Cache-line
+  replacement can happen if the CPU tries to bring some other memory locations
+  into the cache while the DMA is going on.
+- Invalidate the buffer after the DMA is complete and before the MPU reads
+  it. This may be needed in addition to the invalidation before the DMA
+  mentioned above, because in some processors memory contents can spontaneously
+  come to the cache due to speculative memory access by the CPU. If this
+  happens with the DMA buffer while DMA is going on we have a coherency problem.
+
+Buffer Requirements:
+- Any buffer that is invalidated(that is, typically the peripheral to
+  memory DMA buffer) should be aligned to cache-line boundary both at
+  at the beginning and at the end of the buffer.
+- If the buffer is not cache-line aligned invalidation will be restricted
+  to the aligned part. That is, one cache-line at the respective boundary
+  may be left out while doing invalidation.
+
+Cleanup Before Linux:
+- cleanup_before_linux() should flush the D-cache, invalidate I-cache, and
+  disable MMU and caches.
+- The following sequence is advisable while disabling d-cache:
+  1. disable_dcache() - flushes and disables d-cache
+  2. invalidate_dcache_all() - invalid any entry that came to the cache
+	in the short period after the cache was flushed but before the
+	cache got disabled.
diff --git a/include/common.h b/include/common.h
index 12a1074..bd10f31 100644
--- a/include/common.h
+++ b/include/common.h
@@ -616,6 +616,7 @@ ulong	lcd_setmem (ulong);
 ulong	video_setmem (ulong);
 
 /* arch/$(ARCH)/lib/cache.c */
+void	enable_caches(void);
 void	flush_cache   (unsigned long, unsigned long);
 void	flush_dcache_all(void);
 void	flush_dcache_range(unsigned long start, unsigned long stop);
-- 
1.7.0.4

[U-Boot] [PATCH v3 1/4] arm: do not force d-cache enable on all boards

[Simon Guinot]

Hi Aneesh,

On Thu, Aug 11, 2011 at 08:05:42PM +0530, Aneesh V wrote:
> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
> to board_init_r(). This enables d-cache for all ARM boards.
> As a result some of the arm boards that are not cache-ready
> are broken. Revert this change and allow platform code to
> take the decision on d-cache enabling.
> 
> Also add some documentation for cache usage in ARM.
> 
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
> V2:
>  Updated with additional guidelines in the README.
> ---
>  arch/arm/lib/board.c  |    8 ++----
>  arch/arm/lib/cache.c  |   12 +++++++++++
>  doc/README.arm-caches |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/common.h      |    1 +
>  4 files changed, 67 insertions(+), 5 deletions(-)
>  create mode 100644 doc/README.arm-caches
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 14a56f6..38a31d8 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
>  
>  	monitor_flash_len = _end_ofs;
> -	/*
> -	 * Enable D$:
> -	 * I$, if needed, must be already enabled in start.S
> -	 */
> -	dcache_enable();
> +
> +	/* Enable caches */
> +	enable_caches();
>  
>  	debug("monitor flash len: %08lX\n", monitor_flash_len);
>  	board_init();	/* Setup chipselects */
> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 92b61a2..b545fb7 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
>  }
>  void	flush_dcache_all(void)
>  	__attribute__((weak, alias("__flush_dcache_all")));
> +
> +
> +/*
> + * Default implementation of enable_caches()
> + * Real implementation should be in platform code
> + */
> +void __enable_caches(void)
> +{
> +	puts("WARNING: Caches not enabled\n");
> +}
> +void enable_caches(void)
> +	__attribute__((weak, alias("__enable_caches")));
> diff --git a/doc/README.arm-caches b/doc/README.arm-caches
> new file mode 100644
> index 0000000..a833eee
> --- /dev/null
> +++ b/doc/README.arm-caches
> @@ -0,0 +1,51 @@
> +Disabling I-cache:
> +- Set CONFIG_SYS_ICACHE_OFF
> +
> +Disabling D-cache:
> +- Set CONFIG_SYS_DCACHE_OFF
> +
> +Enabling I-cache:
> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
> +
> +Enabling D-cache:
> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
               ^
Maybe you will prefer to check for CONFIG_SYS_DCACHE_OFF ?

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110814/108d6284/attachment.pgp 

[U-Boot] [PATCH v3 1/4] arm: do not force d-cache enable on all boards

[V, Aneesh]

Hi Simon,

On Sun, Aug 14, 2011 at 4:39 PM, Simon Guinot <simon@sequanux.org> wrote:
> Hi Aneesh,
>
> On Thu, Aug 11, 2011 at 08:05:42PM +0530, Aneesh V wrote:
>> c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
>> to board_init_r(). This enables d-cache for all ARM boards.
>> As a result some of the arm boards that are not cache-ready
>> are broken. Revert this change and allow platform code to
>> take the decision on d-cache enabling.
>>
>> Also add some documentation for cache usage in ARM.
>>
>> Signed-off-by: Aneesh V <aneesh@ti.com>
>> ---
>> V2:
>> ?Updated with additional guidelines in the README.
>> ---
>> ?arch/arm/lib/board.c ?| ? ?8 ++----
>> ?arch/arm/lib/cache.c ?| ? 12 +++++++++++
>> ?doc/README.arm-caches | ? 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>> ?include/common.h ? ? ?| ? ?1 +
>> ?4 files changed, 67 insertions(+), 5 deletions(-)
>> ?create mode 100644 doc/README.arm-caches
>>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index 14a56f6..38a31d8 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
>> ? ? ? gd->flags |= GD_FLG_RELOC; ? ? ?/* tell others: relocation done */
>>
>> ? ? ? monitor_flash_len = _end_ofs;
>> - ? ? /*
>> - ? ? ?* Enable D$:
>> - ? ? ?* I$, if needed, must be already enabled in start.S
>> - ? ? ?*/
>> - ? ? dcache_enable();
>> +
>> + ? ? /* Enable caches */
>> + ? ? enable_caches();
>>
>> ? ? ? debug("monitor flash len: %08lX\n", monitor_flash_len);
>> ? ? ? board_init(); ? /* Setup chipselects */
>> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
>> index 92b61a2..b545fb7 100644
>> --- a/arch/arm/lib/cache.c
>> +++ b/arch/arm/lib/cache.c
>> @@ -53,3 +53,15 @@ void ? ? ? __flush_dcache_all(void)
>> ?}
>> ?void flush_dcache_all(void)
>> ? ? ? __attribute__((weak, alias("__flush_dcache_all")));
>> +
>> +
>> +/*
>> + * Default implementation of enable_caches()
>> + * Real implementation should be in platform code
>> + */
>> +void __enable_caches(void)
>> +{
>> + ? ? puts("WARNING: Caches not enabled\n");
>> +}
>> +void enable_caches(void)
>> + ? ? __attribute__((weak, alias("__enable_caches")));
>> diff --git a/doc/README.arm-caches b/doc/README.arm-caches
>> new file mode 100644
>> index 0000000..a833eee
>> --- /dev/null
>> +++ b/doc/README.arm-caches
>> @@ -0,0 +1,51 @@
>> +Disabling I-cache:
>> +- Set CONFIG_SYS_ICACHE_OFF
>> +
>> +Disabling D-cache:
>> +- Set CONFIG_SYS_DCACHE_OFF
>> +
>> +Enabling I-cache:
>> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
>> +
>> +Enabling D-cache:
>> +- Make sure CONFIG_SYS_ICACHE_OFF is not set and call dcache_enable().
> ? ? ? ? ? ? ? ^
> Maybe you will prefer to check for CONFIG_SYS_DCACHE_OFF ?

Yes. That's a typo. Will fix it. Thank you for pointing it out.

best regards,
Aneesh

[U-Boot] [PATCH v4 1/4] arm: do not force d-cache enable on all boards

[Aneesh V]

c2dd0d45540397704de9b13287417d21049d34c6 added dcache_enable()
to board_init_r(). This enables d-cache for all ARM boards.
As a result some of the arm boards that are not cache-ready
are broken. Revert this change and allow platform code to
take the decision on d-cache enabling.

Also add some documentation for cache usage in ARM.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
* Updated with additional guidelines in the README.

V3:
* No change

V4:
* Fix typo in README
---
 arch/arm/lib/board.c  |    8 ++----
 arch/arm/lib/cache.c  |   12 +++++++++++
 doc/README.arm-caches |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h      |    1 +
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 doc/README.arm-caches

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 14a56f6..38a31d8 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -451,11 +451,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
 
 	monitor_flash_len = _end_ofs;
-	/*
-	 * Enable D$:
-	 * I$, if needed, must be already enabled in start.S
-	 */
-	dcache_enable();
+
+	/* Enable caches */
+	enable_caches();
 
 	debug("monitor flash len: %08lX\n", monitor_flash_len);
 	board_init();	/* Setup chipselects */
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 92b61a2..b545fb7 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -53,3 +53,15 @@ void	__flush_dcache_all(void)
 }
 void	flush_dcache_all(void)
 	__attribute__((weak, alias("__flush_dcache_all")));
+
+
+/*
+ * Default implementation of enable_caches()
+ * Real implementation should be in platform code
+ */
+void __enable_caches(void)
+{
+	puts("WARNING: Caches not enabled\n");
+}
+void enable_caches(void)
+	__attribute__((weak, alias("__enable_caches")));
diff --git a/doc/README.arm-caches b/doc/README.arm-caches
new file mode 100644
index 0000000..cd2b458
--- /dev/null
+++ b/doc/README.arm-caches
@@ -0,0 +1,51 @@
+Disabling I-cache:
+- Set CONFIG_SYS_ICACHE_OFF
+
+Disabling D-cache:
+- Set CONFIG_SYS_DCACHE_OFF
+
+Enabling I-cache:
+- Make sure CONFIG_SYS_ICACHE_OFF is not set and call icache_enable().
+
+Enabling D-cache:
+- Make sure CONFIG_SYS_DCACHE_OFF is not set and call dcache_enable().
+
+Enabling Caches at System Startup:
+- Implement enable_caches() for your platform and enable the I-cache and
+  D-cache from this function. This function is called immediately
+  after relocation.
+
+Guidelines for Working with D-cache:
+
+Memory to Peripheral DMA:
+- Flush the buffer after the MPU writes the data and before the DMA is
+  initiated.
+
+Peripheral to Memory DMA:
+- Invalidate the buffer before starting the DMA. In case there are any dirty
+  lines from the DMA buffer in the cache, subsequent cache-line replacements
+  may corrupt the buffer in memory while the DMA is still going on. Cache-line
+  replacement can happen if the CPU tries to bring some other memory locations
+  into the cache while the DMA is going on.
+- Invalidate the buffer after the DMA is complete and before the MPU reads
+  it. This may be needed in addition to the invalidation before the DMA
+  mentioned above, because in some processors memory contents can spontaneously
+  come to the cache due to speculative memory access by the CPU. If this
+  happens with the DMA buffer while DMA is going on we have a coherency problem.
+
+Buffer Requirements:
+- Any buffer that is invalidated(that is, typically the peripheral to
+  memory DMA buffer) should be aligned to cache-line boundary both at
+  at the beginning and at the end of the buffer.
+- If the buffer is not cache-line aligned invalidation will be restricted
+  to the aligned part. That is, one cache-line at the respective boundary
+  may be left out while doing invalidation.
+
+Cleanup Before Linux:
+- cleanup_before_linux() should flush the D-cache, invalidate I-cache, and
+  disable MMU and caches.
+- The following sequence is advisable while disabling d-cache:
+  1. disable_dcache() - flushes and disables d-cache
+  2. invalidate_dcache_all() - invalid any entry that came to the cache
+	in the short period after the cache was flushed but before the
+	cache got disabled.
diff --git a/include/common.h b/include/common.h
index 12a1074..bd10f31 100644
--- a/include/common.h
+++ b/include/common.h
@@ -616,6 +616,7 @@ ulong	lcd_setmem (ulong);
 ulong	video_setmem (ulong);
 
 /* arch/$(ARCH)/lib/cache.c */
+void	enable_caches(void);
 void	flush_cache   (unsigned long, unsigned long);
 void	flush_dcache_all(void);
 void	flush_dcache_range(unsigned long start, unsigned long stop);
-- 
1.7.0.4

[U-Boot] [PATCH v3 2/4] omap: enable caches at system start-up

[Aneesh V]

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 4aaf97b..2c3d7f3 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -402,3 +402,11 @@ void v7_outer_cache_disable(void)
 	omap3_update_aux_cr(0, 0x2);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index 5943d61..8f7c375 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -299,3 +299,11 @@ void v7_outer_cache_disable(void)
 	set_pl310_ctrl_reg(0);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
-- 
1.7.0.4

[U-Boot] [PATCH v3 3/4] armv7: stronger barrier for cache-maintenance operations

[Aneesh V]

set-way operations need a DSB after them to ensure the
operation is complete. DMB may not be enough. Use DSB
after all operations instead of DMB.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/cache_v7.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 3e1e1bf..665f025 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -81,8 +81,8 @@ static void v7_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
@@ -108,8 +108,8 @@ static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_maint_dcache_level_setway(u32 level, u32 operation)
@@ -227,8 +227,8 @@ static void v7_dcache_maint_range(u32 start, u32 stop, u32 range_op)
 		break;
 	}
 
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 /* Invalidate TLB */
-- 
1.7.0.4

[U-Boot] [PATCH v3 4/4] armv7: cache: remove flush on un-aligned invalidate

[Aneesh V]

Remove the flush of boundary cache-lines done as part
of invalidate on a non cache-line boundary aligned
buffer

Also, print a warning when this situation is recognized.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
V2:
* New in V2
V3:
* error notification instead of warning on un-aligned invalidate
---
 arch/arm/cpu/armv7/cache_v7.c |   14 ++++++++------
 arch/arm/lib/cache-pl310.c    |   15 +++++++++------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 665f025..1b4e808 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -181,21 +181,23 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
 	u32 mva;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_len - 1)) {
-		v7_dcache_clean_inval_range(start, start + 1, line_len);
+		printf("ERROR: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_len - 1) & ~(line_len - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_len - 1)) {
-		v7_dcache_clean_inval_range(stop, stop + 1, line_len);
+		printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_len - 1);
 	}
diff --git a/arch/arm/lib/cache-pl310.c b/arch/arm/lib/cache-pl310.c
index 36c629c..21d13f7 100644
--- a/arch/arm/lib/cache-pl310.c
+++ b/arch/arm/lib/cache-pl310.c
@@ -26,6 +26,7 @@
 #include <asm/armv7.h>
 #include <asm/pl310.h>
 #include <config.h>
+#include <common.h>
 
 struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
 
@@ -89,21 +90,23 @@ void v7_outer_cache_inval_range(u32 start, u32 stop)
 	u32 pa, line_size = 32;
 
 	/*
-	 * If start address is not aligned to cache-line flush the first
-	 * line to prevent affecting somebody else's buffer
+	 * If start address is not aligned to cache-line do not
+	 * invalidate the first cache-line
 	 */
 	if (start & (line_size - 1)) {
-		v7_outer_cache_flush_range(start, start + 1);
+		printf("ERROR: %s - start address is not aligned - 0x%08x\n",
+			__func__, start);
 		/* move to next cache line */
 		start = (start + line_size - 1) & ~(line_size - 1);
 	}
 
 	/*
-	 * If stop address is not aligned to cache-line flush the last
-	 * line to prevent affecting somebody else's buffer
+	 * If stop address is not aligned to cache-line do not
+	 * invalidate the last cache-line
 	 */
 	if (stop & (line_size - 1)) {
-		v7_outer_cache_flush_range(stop, stop + 1);
+		printf("ERROR: %s - stop address is not aligned - 0x%08x\n",
+			__func__, stop);
 		/* align to the beginning of this cache line */
 		stop &= ~(line_size - 1);
 	}
-- 
1.7.0.4

[U-Boot] [PATCH v 2/3] omap: enable caches at system start-up

[Aneesh V]

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/omap3/board.c |    8 ++++++++
 arch/arm/cpu/armv7/omap4/board.c |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 98519a9..de0e90d 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -390,3 +390,11 @@ void v7_outer_cache_disable(void)
 	omap3_update_aux_cr(0, 0x2);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index de4cc2a..6ea8a2e 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -139,3 +139,11 @@ void v7_outer_cache_disable(void)
 	set_pl310_ctrl_reg(0);
 }
 #endif
+
+#ifndef CONFIG_SYS_DCACHE_OFF
+void enable_caches(void)
+{
+	/* Enable D-cache. I-cache is already enabled in start.S */
+	dcache_enable();
+}
+#endif
-- 
1.7.0.4

[U-Boot] [PATCH v 3/3] armv7: stronger barrier for cache-maintenance operations

[Aneesh V]

set-way operations need a DSB after them to ensure the
operation is complete. DMB may not be enough. Use DSB
after all operations instead of DMB.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/cache_v7.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index 3e1e1bf..665f025 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -81,8 +81,8 @@ static void v7_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
@@ -108,8 +108,8 @@ static void v7_clean_inval_dcache_level_setway(u32 level, u32 num_sets,
 					: : "r" (setway));
 		}
 	}
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 static void v7_maint_dcache_level_setway(u32 level, u32 operation)
@@ -227,8 +227,8 @@ static void v7_dcache_maint_range(u32 start, u32 stop, u32 range_op)
 		break;
 	}
 
-	/* DMB to make sure the operation is complete */
-	CP15DMB;
+	/* DSB to make sure the operation is complete */
+	CP15DSB;
 }
 
 /* Invalidate TLB */
-- 
1.7.0.4

[U-Boot] [PATCH v 0/3] arm: changes in cache handling

[Aneesh V]

Hi Albert, Wolfgang,

On Monday 01 August 2011 04:48 PM, Aneesh V wrote:
> - Avoid enabling caches for all ARM boards
> - Enable caches for omap3/4
> - Stronger barrier for armv7 cache-maintenance operations.

Do you have any more comments on this series. I tend to agree with
Jason's views about code duplication. But looks like
CONFIG_SYS_DCACHE_ON is not acceptable.

If there is nothing more to be done, will you pull this series. It will
help many boards that are broken due to cache.

best regards,
Aneesh