arm: changes in cache handling
cover arm: changes in cache handling
- 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
PATCH 1/3 arm: do not force d-cache enable on all boards
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.
---
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
arch/arm/lib/board.c +3 -5
@@ -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 */
arch/arm/lib/cache.c +12 -0
@@ -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" )));
doc/README.arm-caches new file +40 -0
@@ -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.
include/common.h +1 -0
@@ -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);
Signature · 1 line
1.7.0.4
PATCH 1/3 arm: do not force d-cache enable on all boards
Hi, Aneesh,
2011/8/1 Aneesh V <[email protected]>:
Quoted 48 lines
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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 <[email protected]>---
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
arch/arm/lib/board.c +0
-0@@ -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 instart.S
- ? ? ? ?*/
- ? ? ?dcache_enable();
+
+ ? ? ? /* Enable caches */
+ ? ? ?enable_caches();? ? ? ?debug ("monitor flash len: %08lX\n",
monitor_flash_len); ? ? ? ?board_init(); ? /* Setup chipselects */arch/arm/lib/cache.c +2
-0@@ -53,3 +53,15 @@ void __flush_dcache_all(void) ?} ?void ? flush_dcache_all(void) ? ? ? ?__attribute__((weak, alias("__flush_dcache_all"))); + +
arch/arm/lib/cache.c void __flush_dcache_all(void) · line 67
Quoted 10 lines
arch/arm/lib/cache.c +10
-0+/* +* 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(-)
arch/arm/cpu/armv7/omap3/board.c +8 -0
@@ -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
arch/arm/cpu/armv7/omap4/board.c +8 -0
@@ -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
PATCH 1/3 arm: do not force d-cache enable on all boards
2011/8/2 Jason Liu <[email protected]>:
Quoted 65 lines
Hi, Aneesh,
2011/8/1 Aneesh V <[email protected]>:
Quoted 48 lines
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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 <[email protected]>---
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
arch/arm/lib/board.c +0
-0@@ -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 instart.S
- ? ? ? ?*/
- ? ? ?dcache_enable();
+
+ ? ? ? /* Enable caches */
+ ? ? ?enable_caches();? ? ? ?debug ("monitor flash len: %08lX\n",
monitor_flash_len); ? ? ? ?board_init(); ? /* Setup chipselects */arch/arm/lib/cache.c +2
-0@@ -53,3 +53,15 @@ void __flush_dcache_all(void) ?} ?void ? flush_dcache_all(void) ? ? ? ?__attribute__((weak, alias("__flush_dcache_all"))); + +arch/arm/lib/cache.c void __flush_dcache_all(void) · line 67
Quoted 10 lines
arch/arm/lib/cache.c +10
-0+/* +* 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?
Quoted 5 lines
#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
PATCH 1/3 arm: do not force d-cache enable on all boards
Dear Jason Liu,
Quoted 11 lines · In message <CAB4PhKdj0prR3etkkz5PSqKPgS69CFZepY83Gw-aPQKcFkyKkQ@mail.gmail.com>…
Quoted 8 lines
What about the following change?
#ifndef CONFIG_SYS_DCACHE_OFF dcache_enable(); #else puts("WARNING: Caches not enabled\n");#endif
Or better:
Quoted 8 lines
#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
Signature · 5 lines
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
PATCH 1/3 arm: do not force d-cache enable on all boards
In reply to Jason LiuHi Jason,
Le 01/08/2011 18:33, Jason Liu a ?crit :
Quoted 1 line
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.
Quoted 1 line
Jason
Amicalement,
Signature · 1 line
Albert.
PATCH 1/3 arm: do not force d-cache enable on all boards
Hi, Albert,
2011/8/2 Albert ARIBAUD <[email protected]>:
Quoted 10 lines
Hi Jason,
Le 01/08/2011 18:33, Jason Liu a ?crit :
Quoted 1 line
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
Quoted 4 lines
Amicalement,
--
Albert.
PATCH 1/3 arm: do not force d-cache enable on all boards
Le 02/08/2011 16:35, Jason Liu a ?crit :
Quoted 51 lines
Hi, Albert,
2011/8/2 Albert ARIBAUD<[email protected]>:
Quoted 10 lines
Hi Jason,
Le 01/08/2011 18:33, Jason Liu a ?crit :
Quoted 1 line
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
Quoted 2 lines
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.
Quoted 1 line
Jason
Amicalement,
Signature · 1 line
Albert.
PATCH 1/3 arm: do not force d-cache enable on all boards
In reply to Aneesh VHi Albert,
Quoted 80 lines · Aneesh V
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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<[email protected]>---
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
arch/arm/lib/board.c +3
-5@@ -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 */ arch/arm/lib/cache.c +12
-0@@ -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" )));doc/README.arm-caches new file +16
-0@@ -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.
doc/README.arm-caches line 26
Quoted 10 lines
doc/README.arm-caches new file +10
-0+ +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
PATCH 1/3 arm: do not force d-cache enable on all boards
Hi Aneesh,
Le 05/08/2011 17:07, Aneesh V a ?crit :
Quoted 98 lines
Hi Albert,
Quoted 89 lines · Aneesh V
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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<[email protected]>---
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
arch/arm/lib/board.c +0
-0@@ -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 instart.S
- */
-dcache_enable();
+
+ /* Enable caches */
+enable_caches();debug ("monitor flash len: %08lX\n",
monitor_flash_len);board_init(); /* Setup chipselects */arch/arm/lib/cache.c +12
-0@@ -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" )));doc/README.arm-caches new file +25
-0@@ -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 MPUQuoted 2 lines
reads
+ it.The other discussion we are having about caches made me realize that I need to improve the above guideline like this:
Quoted 4 lines
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).
Quoted 4 lines
- Invalidate the buffer after the DMA is complete and before the MPU reads it.
I shall submit an updated patch for this.
Thanks!
Quoted 2 lines
best regards,
Aneesh
Amicalement,
Signature · 1 line
Albert.
cover arm: changes in cache handling
In reply to 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
cover arm: changes in cache handling
Hi Wolfgang, Albert,
Quoted 9 lines · 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.
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
PATCH 1/4 arm: do not force d-cache enable on all boards
In reply to Aneesh Vc2dd0d45540397704de9b13287417d21049d34c6 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.
---
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
arch/arm/lib/board.c +3 -5
@@ -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 */
arch/arm/lib/cache.c +12 -0
@@ -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" )));
doc/README.arm-caches new file +51 -0
51 lines not on this page · raw message
include/common.h +1 -0
@@ -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);
Signature · 1 line
1.7.0.4
PATCH 2/4 omap: enable caches at system start-up
In reply to Aneesh V--- arch/arm/cpu/armv7/omap3/board.c | 8 ++++++++ arch/arm/cpu/armv7/omap4/board.c | 8 ++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/omap3/board.c +8 -0
@@ -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
arch/arm/cpu/armv7/omap4/board.c +8 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH 3/4 armv7: stronger barrier for cache-maintenance operations
In reply to Aneesh Vset-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.
--- arch/arm/cpu/armv7/cache_v7.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
arch/arm/cpu/armv7/cache_v7.c +6 -6
@@ -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 */
Signature · 1 line
1.7.0.4
PATCH 4/4 armv7: cache: remove flush on un-aligned invalidate
In reply to Aneesh VRemove 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.
---
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(-)
arch/arm/cpu/armv7/cache_v7.c +8 -6
@@ -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 ); }
arch/arm/lib/cache-pl310.c +9 -6
@@ -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 ); }
Signature · 1 line
1.7.0.4
PATCH 4/4 armv7: cache: remove flush on un-aligned invalidate
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
Quoted 99 lines · 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 <[email protected]>---
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(-)arch/arm/cpu/armv7/cache_v7.c +0
-0@@ -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);
? ? ? ?}arch/arm/lib/cache-pl310.c +0
-0@@ -26,6 +26,7 @@?#include <
asm/armv7.h>
?#include <asm/pl310.h>
?#include <config.h>
+#include <common.h>?struct
pl310_regs*const pl310 = (structpl310_regs*)CONFIG_SYS_PL310_BASE;arch/arm/lib/cache-pl310.c +0
-0@@ -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
PATCH v3 4/4 armv7: cache: remove flush on un-aligned invalidate
Hi Anton,
Le 09/08/2011 18:39, Anton Staaf a ?crit :
Quoted 5 lines
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
Quoted 8 lines
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,
Signature · 1 line
Albert.
PATCH v2 4/4 armv7: cache: remove flush on un-aligned invalidate
In reply to Anton StaafHi Anton,
Quoted 2 lines · 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
Please see this thread for the context.
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135
Quoted 3 lines
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.
Quoted 8 lines
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
PATCH 4/4 armv7: cache: remove flush on un-aligned invalidate
Quoted 9 lines · Aneesh V
Hi Anton,
Quoted 3 lines · 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
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.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 9 lines
Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 3 lines
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
PATCH v2 4/4 armv7: cache: remove flush on un-aligned invalidate
Quoted 24 lines · Anton Staaf
Quoted 9 lines · Aneesh V
Hi Anton,
Quoted 3 lines · 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
Please see this thread for the context.
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135Ahh, I missed this thread (Its title became stale). :) But I completely agree with the outcome, we need to fix the unaligned buffer problem.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 10 lines
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
v2 cover arm: changes in cache handling
In reply to 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
PATCH v2 1/4 arm: do not force d-cache enable on all boards
In reply to Aneesh Vc2dd0d45540397704de9b13287417d21049d34c6 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.
---
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
arch/arm/lib/board.c +3 -5
@@ -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 */
arch/arm/lib/cache.c +12 -0
@@ -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" )));
doc/README.arm-caches new file +51 -0
51 lines not on this page · raw message
include/common.h +1 -0
@@ -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);
Signature · 1 line
1.7.0.4
[U-Boot] d-cache enable
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.
Signature · 6 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center Platform Group
[U-Boot] d-cache enable
Hi Lukasz,
Quoted 15 lines · 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:d26a82023af5771462f7223241ed18cfb7965f71After some research I can say that
flush_dcache_all()andinvalidate_dcache_all()are working(at least on my target).
Quoted 7 lines
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
Hi Aneesh,
Quoted 33 lines · On Fri, 12 Aug 2011 16:29:25 +0530 Aneesh V <[email protected]> wrote:
Hi Lukasz,
Quoted 15 lines · 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:d26a82023af5771462f7223241ed18cfb7965f71After some research I can say that
flush_dcache_all()andinvalidate_dcache_all()are working(at least on my target).Quoted 7 lines
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.
Quoted 2 lines
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.
Signature · 6 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center Platform Group
[U-Boot] d-cache enable
Hi Lukasz,
Quoted 38 lines · Lukasz Majewski
Hi Aneesh,
Quoted 33 lines · On Fri, 12 Aug 2011 16:29:25 +0530 Aneesh V <[email protected]> wrote:
Hi Lukasz,
Quoted 15 lines · 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:d26a82023af5771462f7223241ed18cfb7965f71After some research I can say that
flush_dcache_all()andinvalidate_dcache_all()are working(at least on my target).Quoted 7 lines
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.
Quoted 7 lines
Quoted 2 lines
best regards,
AneeshIt 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).
Quoted 1 line
Yes, they are working when I've realized how to use them :-).
Good to know that.
best regards,
Aneesh
[U-Boot] d-cache enable
In reply to Lukasz MajewskiHi Lukasz,
Quoted 3 lines · Lukasz Majewski
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,
Signature · 1 line
Albert.
[U-Boot] d-cache enable
Dear *ALL*,
Quoted 9 lines
Quoted 3 lines
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
Hi Reinhard,
Quoted 8 lines · Reinhard Meyer
Dear *ALL*,
Quoted 7 lines
Quoted 3 lines
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)?
Quoted 6 lines
Quoted 2 lines
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.
Quoted 1 line
Reinhard
Amicalement,
Signature · 1 line
Albert.
[U-Boot] d-cache enable
In reply to Albert ARIBAUDHi Albert,
Quoted 8 lines · On Fri, 12 Aug 2011 14:21:18 +0200 Albert ARIBAUD <[email protected]>…
Hi Lukasz,
Quoted 3 lines · Lukasz Majewski
It is embarrassing to admit, but I've
__wrongly__assumed that*_range()functions are accepting the start address and range for invalidation/flushing.
Quoted 7 lines
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.
Signature · 6 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center Platform Group
[U-Boot] d-cache enable
Quoted 16 lines · Lukasz Majewski
Hi Albert,
Quoted 11 lines · On Fri, 12 Aug 2011 14:21:18 +0200 Albert ARIBAUD<[email protected]>…
Hi Lukasz,
Quoted 3 lines · Lukasz Majewski
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)?
Quoted 7 lines
Quoted 4 lines
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,
Signature · 1 line
Albert.
PATCH v2 2/4 omap: enable caches at system start-up
In reply to Aneesh V--- arch/arm/cpu/armv7/omap3/board.c | 8 ++++++++ arch/arm/cpu/armv7/omap4/board.c | 8 ++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/omap3/board.c +8 -0
@@ -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
arch/arm/cpu/armv7/omap4/board.c +8 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH v2 3/4 armv7: stronger barrier for cache-maintenance operations
In reply to Aneesh Vset-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.
--- arch/arm/cpu/armv7/cache_v7.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
arch/arm/cpu/armv7/cache_v7.c +6 -6
@@ -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 */
Signature · 1 line
1.7.0.4
PATCH v2 4/4 armv7: cache: remove flush on un-aligned invalidate
In reply to Aneesh VRemove 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.
---
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(-)
arch/arm/cpu/armv7/cache_v7.c +8 -6
@@ -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 ); }
arch/arm/lib/cache-pl310.c +9 -6
@@ -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 ); }
Signature · 1 line
1.7.0.4
v3 cover arm: changes in cache handling
In reply to 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
v3 cover arm: changes in cache handling
Le 11/08/2011 16:35, Aneesh V a ?crit :
Quoted 28 lines
- 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 invalidateAneesh 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 invalidatearch/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(-)
Quoted 1 line
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,
Signature · 1 line
Albert.
V
v3 cover arm: changes in cache handling
Hi Albert,
Quoted 33 lines · Albert ARIBAUD
Le 11/08/2011 16:35, Aneesh V a ?crit :
Quoted 27 lines
- 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 invalidateAneesh 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 ? ? ? ? ? ?| ? 51Quoted 4 lines
++++++++++++++++++++++++++++++++++++++
?include/common.h ? ? ? ? ? ? ? ? | ? ?1 + ?8 files changed, 106 insertions(+), 23 deletions(-) ?create mode 100644 doc/README.arm-caches
Quoted 4 lines
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
PATCH v3 1/4 arm: do not force d-cache enable on all boards
In reply to Aneesh Vc2dd0d45540397704de9b13287417d21049d34c6 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.
---
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
arch/arm/lib/board.c +3 -5
@@ -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 */
arch/arm/lib/cache.c +12 -0
@@ -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" )));
doc/README.arm-caches new file +51 -0
51 lines not on this page · raw message
include/common.h +1 -0
@@ -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);
Signature · 1 line
1.7.0.4
PATCH v3 1/4 arm: do not force d-cache enable on all boards
Hi Aneesh,
Quoted 66 lines · Aneesh V
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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 <[email protected]>---
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-cachesarch/arm/lib/board.c +3
-5@@ -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 */ arch/arm/lib/cache.c +12
-0@@ -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" )));doc/README.arm-caches new file +1
-0@@ -0,0 +1,51 @@ +Disabling I-cache:
doc/README.arm-caches line 11
Quoted 10 lines
doc/README.arm-caches new file +10
-0+- 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
V
PATCH v3 1/4 arm: do not force d-cache enable on all boards
Hi Simon,
Quoted 73 lines · Simon Guinot
Hi Aneesh,
Quoted 70 lines · Aneesh V
c2dd0d45540397704de9b13287417d21049d34c6addeddcache_enable()toboard_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 <[email protected]>---
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
arch/arm/lib/board.c +0
-0@@ -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 instart.S
- ? ? ?*/
- ? ?dcache_enable();
+
+ ? ? /* Enable caches */
+ ? ?enable_caches();? ? ?
debug("monitor flash len: %08lX\n", monitor_flash_len); ? ? ?board_init(); ? /* Setup chipselects */arch/arm/lib/cache.c +12
-0@@ -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")));doc/README.arm-caches new file +5
-0@@ -0,0 +1,51 @@ +Disabling I-cache: +- Set CONFIG_SYS_ICACHE_OFF + +Disabling D-cache: +- Set CONFIG_SYS_DCACHE_OFF
Quoted 8 lines
doc/README.arm-caches line 11
Quoted 6 lines
doc/README.arm-caches new file +6
-0+ +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 forCONFIG_SYS_DCACHE_OFF?
Yes. That's a typo. Will fix it. Thank you for pointing it out.
best regards,
Aneesh
PATCH v4 1/4 arm: do not force d-cache enable on all boards
In reply to Aneesh Vc2dd0d45540397704de9b13287417d21049d34c6 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.
---
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
arch/arm/lib/board.c +3 -5
@@ -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 */
arch/arm/lib/cache.c +12 -0
@@ -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" )));
doc/README.arm-caches new file +51 -0
51 lines not on this page · raw message
include/common.h +1 -0
@@ -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);
Signature · 1 line
1.7.0.4
PATCH v3 2/4 omap: enable caches at system start-up
In reply to Aneesh V--- arch/arm/cpu/armv7/omap3/board.c | 8 ++++++++ arch/arm/cpu/armv7/omap4/board.c | 8 ++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/omap3/board.c +8 -0
@@ -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
arch/arm/cpu/armv7/omap4/board.c +8 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH v3 3/4 armv7: stronger barrier for cache-maintenance operations
In reply to Aneesh Vset-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.
--- arch/arm/cpu/armv7/cache_v7.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
arch/arm/cpu/armv7/cache_v7.c +6 -6
@@ -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 */
Signature · 1 line
1.7.0.4
PATCH v3 4/4 armv7: cache: remove flush on un-aligned invalidate
In reply to Aneesh VRemove 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.
---
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(-)
arch/arm/cpu/armv7/cache_v7.c +8 -6
@@ -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 ); }
arch/arm/lib/cache-pl310.c +9 -6
@@ -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 ); }
Signature · 1 line
1.7.0.4
PATCH 2/3 omap: enable caches at system start-up
In reply to Aneesh V--- arch/arm/cpu/armv7/omap3/board.c | 8 ++++++++ arch/arm/cpu/armv7/omap4/board.c | 8 ++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/omap3/board.c +8 -0
@@ -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
arch/arm/cpu/armv7/omap4/board.c +8 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH 3/3 armv7: stronger barrier for cache-maintenance operations
In reply to Aneesh Vset-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.
--- arch/arm/cpu/armv7/cache_v7.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
arch/arm/cpu/armv7/cache_v7.c +6 -6
@@ -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 */
Signature · 1 line
1.7.0.4
cover arm: changes in cache handling
In reply to Aneesh VHi Albert, Wolfgang,
Quoted 3 lines · Aneesh V
- 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