[U-Boot] [PATCH 0/5] arm: Tidy up the cache aligning warning code

This series consolidates the code that warns about cache line alignment problems (with flush and invalidate). It adjusts it so that a warning is always displayed, except when building for SPL (since that bloats the code).
It is assumed that the cache is off in SPL.
Simon Glass (5): arm: Move check_cache_range() into a common place arm: Don't invalidate unaligned cache regions Add comments for debug() and pr_fmt Add warn_non_spl() to show a message in U-Boot proper arm: Show cache warnings in U-Boot proper only
arch/arm/cpu/arm11/cpu.c | 17 ----------------- arch/arm/cpu/arm926ejs/cache.c | 17 ----------------- arch/arm/cpu/armv7/cache_v7.c | 40 ++-------------------------------------- arch/arm/include/asm/cache.h | 2 ++ arch/arm/lib/cache.c | 22 ++++++++++++++++++++++ include/common.h | 12 ++++++++++++ 6 files changed, 38 insertions(+), 72 deletions(-)

This code is common, so move it into a common file.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/arm11/cpu.c | 17 ----------------- arch/arm/cpu/arm926ejs/cache.c | 17 ----------------- arch/arm/cpu/armv7/cache_v7.c | 17 ----------------- arch/arm/include/asm/cache.h | 2 ++ arch/arm/lib/cache.c | 22 ++++++++++++++++++++++ 5 files changed, 24 insertions(+), 51 deletions(-)
diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c index 1e4c214..7244c2e 100644 --- a/arch/arm/cpu/arm11/cpu.c +++ b/arch/arm/cpu/arm11/cpu.c @@ -69,23 +69,6 @@ void flush_dcache_all(void) asm volatile("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); }
-static int check_cache_range(unsigned long start, unsigned long stop) -{ - int ok = 1; - - if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (!ok) - debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", - start, stop); - - return ok; -} - void invalidate_dcache_range(unsigned long start, unsigned long stop) { if (!check_cache_range(start, stop)) diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c index 2839c86..2119382 100644 --- a/arch/arm/cpu/arm926ejs/cache.c +++ b/arch/arm/cpu/arm926ejs/cache.c @@ -29,23 +29,6 @@ void flush_dcache_all(void) ); }
-static int check_cache_range(unsigned long start, unsigned long stop) -{ - int ok = 1; - - if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (!ok) - debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", - start, stop); - - return ok; -} - void invalidate_dcache_range(unsigned long start, unsigned long stop) { if (!check_cache_range(start, stop)) diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c index dc309da..823a156 100644 --- a/arch/arm/cpu/armv7/cache_v7.c +++ b/arch/arm/cpu/armv7/cache_v7.c @@ -19,23 +19,6 @@ void v7_flush_dcache_all(void); void v7_invalidate_dcache_all(void);
-static int check_cache_range(unsigned long start, unsigned long stop) -{ - int ok = 1; - - if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) - ok = 0; - - if (!ok) - debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", - start, stop); - - return ok; -} - static u32 get_ccsidr(void) { u32 ccsidr; diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h index 1f63127..16e65c3 100644 --- a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@ -29,6 +29,8 @@ static inline void invalidate_l2_cache(void) } #endif
+int check_cache_range(unsigned long start, unsigned long stop); + void l2_cache_enable(void); void l2_cache_disable(void); void set_section_dcache(int section, enum dcache_option option); diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 3bd8710..642a952 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -10,6 +10,10 @@ #include <common.h> #include <malloc.h>
+#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif + /* * Flush range from all levels of d-cache/unified-cache. * Affects the range [start, start + size - 1]. @@ -46,6 +50,24 @@ __weak void flush_dcache_range(unsigned long start, unsigned long stop) /* An empty stub, real implementation should be in platform code */ }
+int check_cache_range(unsigned long start, unsigned long stop) +{ + int ok = 1; + + if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (!ok) { + debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + start, stop); + } + + return ok; +} + #ifdef CONFIG_SYS_NONCACHED_MEMORY /* * Reserve one MMU section worth of address space below the malloc() area that

On 06/20/2016 03:43 AM, Simon Glass wrote:
This code is common, so move it into a common file.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Marek Vasut marex@denx.de
arch/arm/cpu/arm11/cpu.c | 17 ----------------- arch/arm/cpu/arm926ejs/cache.c | 17 ----------------- arch/arm/cpu/armv7/cache_v7.c | 17 ----------------- arch/arm/include/asm/cache.h | 2 ++ arch/arm/lib/cache.c | 22 ++++++++++++++++++++++ 5 files changed, 24 insertions(+), 51 deletions(-)
diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c index 1e4c214..7244c2e 100644 --- a/arch/arm/cpu/arm11/cpu.c +++ b/arch/arm/cpu/arm11/cpu.c @@ -69,23 +69,6 @@ void flush_dcache_all(void) asm volatile("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); }
-static int check_cache_range(unsigned long start, unsigned long stop) -{
- int ok = 1;
- if (start & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (!ok)
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
- return ok;
-}
void invalidate_dcache_range(unsigned long start, unsigned long stop) { if (!check_cache_range(start, stop)) diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c index 2839c86..2119382 100644 --- a/arch/arm/cpu/arm926ejs/cache.c +++ b/arch/arm/cpu/arm926ejs/cache.c @@ -29,23 +29,6 @@ void flush_dcache_all(void) ); }
-static int check_cache_range(unsigned long start, unsigned long stop) -{
- int ok = 1;
- if (start & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (!ok)
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
- return ok;
-}
void invalidate_dcache_range(unsigned long start, unsigned long stop) { if (!check_cache_range(start, stop)) diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c index dc309da..823a156 100644 --- a/arch/arm/cpu/armv7/cache_v7.c +++ b/arch/arm/cpu/armv7/cache_v7.c @@ -19,23 +19,6 @@ void v7_flush_dcache_all(void); void v7_invalidate_dcache_all(void);
-static int check_cache_range(unsigned long start, unsigned long stop) -{
- int ok = 1;
- if (start & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (!ok)
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
- return ok;
-}
static u32 get_ccsidr(void) { u32 ccsidr; diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h index 1f63127..16e65c3 100644 --- a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@ -29,6 +29,8 @@ static inline void invalidate_l2_cache(void) } #endif
+int check_cache_range(unsigned long start, unsigned long stop);
void l2_cache_enable(void); void l2_cache_disable(void); void set_section_dcache(int section, enum dcache_option option); diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 3bd8710..642a952 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -10,6 +10,10 @@ #include <common.h> #include <malloc.h>
+#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif
/*
- Flush range from all levels of d-cache/unified-cache.
- Affects the range [start, start + size - 1].
@@ -46,6 +50,24 @@ __weak void flush_dcache_range(unsigned long start, unsigned long stop) /* An empty stub, real implementation should be in platform code */ }
+int check_cache_range(unsigned long start, unsigned long stop) +{
- int ok = 1;
- if (start & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;
- if (!ok) {
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
- }
- return ok;
+}
#ifdef CONFIG_SYS_NONCACHED_MEMORY /*
- Reserve one MMU section worth of address space below the malloc() area that

On Sun, Jun 19, 2016 at 07:43:01PM -0600, Simon Glass wrote:
This code is common, so move it into a common file.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Marek Vasut marex@denx.de
Applied to u-boot/master, thanks!

At present armv7 will unhappily invalidate a cache region and print an error message. Make it skip the operation instead, as it does with other cache operations.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/armv7/cache_v7.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c index 823a156..e59597e 100644 --- a/arch/arm/cpu/armv7/cache_v7.c +++ b/arch/arm/cpu/armv7/cache_v7.c @@ -44,27 +44,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) { u32 mva;
- /* - * If start address is not aligned to cache-line do not - * invalidate the first cache-line - */ - if (start & (line_len - 1)) { - 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 do not - * invalidate the last cache-line - */ - if (stop & (line_len - 1)) { - 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); - } + if (!check_cache_range(start, stop)) + return;
for (mva = start; mva < stop; mva = mva + line_len) { /* DCIMVAC - Invalidate data cache by MVA to PoC */

On 06/20/2016 03:43 AM, Simon Glass wrote:
At present armv7 will unhappily invalidate a cache region and print an error message. Make it skip the operation instead, as it does with other cache operations.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Marek Vasut marex@denx.de
arch/arm/cpu/armv7/cache_v7.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c index 823a156..e59597e 100644 --- a/arch/arm/cpu/armv7/cache_v7.c +++ b/arch/arm/cpu/armv7/cache_v7.c @@ -44,27 +44,8 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len) { u32 mva;
- /*
* If start address is not aligned to cache-line do not
* invalidate the first cache-line
*/
- if (start & (line_len - 1)) {
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 do not
* invalidate the last cache-line
*/
- if (stop & (line_len - 1)) {
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);
- }
if (!check_cache_range(start, stop))
return;
for (mva = start; mva < stop; mva = mva + line_len) { /* DCIMVAC - Invalidate data cache by MVA to PoC */

On Sun, Jun 19, 2016 at 07:43:02PM -0600, Simon Glass wrote:
At present armv7 will unhappily invalidate a cache region and print an error message. Make it skip the operation instead, as it does with other cache operations.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Marek Vasut marex@denx.de
Applied to u-boot/master, thanks!

Add a note to each of these so it is more obvious how they work.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/common.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/common.h b/include/common.h index f9f4605..a425dd9 100644 --- a/include/common.h +++ b/include/common.h @@ -100,6 +100,7 @@ typedef volatile unsigned char vu_char; #define _DEBUG 0 #endif
+/* Define this at the top of a file to add a prefix to debug messages */ #ifndef pr_fmt #define pr_fmt(fmt) fmt #endif @@ -115,6 +116,7 @@ typedef volatile unsigned char vu_char; printf(pr_fmt(fmt), ##args); \ } while (0)
+/* Show a message if DEBUG is defined in a file */ #define debug(fmt, args...) \ debug_cond(_DEBUG, fmt, ##args)

On Sun, Jun 19, 2016 at 07:43:03PM -0600, Simon Glass wrote:
Add a note to each of these so it is more obvious how they work.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

SPL tends to be more space-constrained that U-Boot proper. Some error messages are best suppressed in SPL. Add a macros to make this easy.
warn_non_spl() does nothing when built in SPL code.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/common.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/common.h b/include/common.h index a425dd9..b779f60 100644 --- a/include/common.h +++ b/include/common.h @@ -100,6 +100,12 @@ typedef volatile unsigned char vu_char; #define _DEBUG 0 #endif
+#ifdef CONFIG_SPL_BUILD +#define _SPL_BUILD 1 +#else +#define _SPL_BUILD 0 +#endif + /* Define this at the top of a file to add a prefix to debug messages */ #ifndef pr_fmt #define pr_fmt(fmt) fmt @@ -120,6 +126,10 @@ typedef volatile unsigned char vu_char; #define debug(fmt, args...) \ debug_cond(_DEBUG, fmt, ##args)
+/* Show a message if not in SPL */ +#define warn_non_spl(fmt, args...) \ + debug_cond(!_SPL_BUILD, fmt, ##args) + /* * An assertion is run-time check done in debug mode only. If DEBUG is not * defined then it is skipped. If DEBUG is defined and the assertion fails,

On Sun, Jun 19, 2016 at 07:43:04PM -0600, Simon Glass wrote:
SPL tends to be more space-constrained that U-Boot proper. Some error messages are best suppressed in SPL. Add a macros to make this easy.
warn_non_spl() does nothing when built in SPL code.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Avoid bloating the SPL image size.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/lib/cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 642a952..d330b09 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -61,8 +61,8 @@ int check_cache_range(unsigned long start, unsigned long stop) ok = 0;
if (!ok) { - debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", - start, stop); + warn_non_spl("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + start, stop); }
return ok;

On Sun, Jun 19, 2016 at 07:43:05PM -0600, Simon Glass wrote:
Avoid bloating the SPL image size.
Signed-off-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

On 20.06.2016 03:43, Simon Glass wrote:
Avoid bloating the SPL image size.
Signed-off-by: Simon Glass sjg@chromium.org
arch/arm/lib/cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 642a952..d330b09 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -61,8 +61,8 @@ int check_cache_range(unsigned long start, unsigned long stop) ok = 0;
if (!ok) {
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
warn_non_spl("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
}
return ok;
Since this i've, and probably many others, have a bunch of warnings on doing something in u-boot.
Just compiled and installed today new u-boot on my zynq-board.
" U-Boot 2016.09-rc1-00353-g2ac625b-dirty (Aug 02 2016 - 08:06:57 +0200)
I2C: ready DRAM: ECC disabled 256 MiB MMC: sdhci@e0100000: 0 SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16 MiB FPGA: id 0x2 (7z010) Net: ZYNQ GEM: e000b000, phyaddr 0, interface rgmii-id *CACHE: Misaligned operation at range [0fff03b0, 0fff03b4]*
Warning: ethernet@e000b000 (eth0) using random MAC address - aa:99:75:dd:1d:71 eth0: ethernet@e000b000 SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16 MiB device 0 offset 0xc0000, size 0x10000 SF: 65536 bytes @ 0xc0000 Read: OK ## Executing script at 000c0000 => dhcp *CACHE: Misaligned operation at range [0eb583b0, 0eb583b4]** **CACHE: Misaligned operation at range [0eb583b4, 0eb583b8]* BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 DHCP client bound to address 192.168.21.193 (1008 ms) => "
Do we have real trouble inside or can we quiet this warning?
cheers, Hannes

Hi,
On 2.8.2016 08:42, Hannes Schmelzer wrote:
On 20.06.2016 03:43, Simon Glass wrote:
Avoid bloating the SPL image size.
Signed-off-by: Simon Glass sjg@chromium.org
arch/arm/lib/cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 642a952..d330b09 100644 --- a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -61,8 +61,8 @@ int check_cache_range(unsigned long start, unsigned long stop) ok = 0; if (!ok) {
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
start, stop);
warn_non_spl("CACHE: Misaligned operation at range [%08lx,
%08lx]\n",
start, stop); } return ok;
Since this i've, and probably many others, have a bunch of warnings on doing something in u-boot.
Just compiled and installed today new u-boot on my zynq-board.
" U-Boot 2016.09-rc1-00353-g2ac625b-dirty (Aug 02 2016 - 08:06:57 +0200)
I2C: ready DRAM: ECC disabled 256 MiB MMC: sdhci@e0100000: 0 SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16 MiB FPGA: id 0x2 (7z010) Net: ZYNQ GEM: e000b000, phyaddr 0, interface rgmii-id *CACHE: Misaligned operation at range [0fff03b0, 0fff03b4]*
Warning: ethernet@e000b000 (eth0) using random MAC address - aa:99:75:dd:1d:71 eth0: ethernet@e000b000 SF: Detected S25FL128S_64K with page size 256 Bytes, erase size 64 KiB, total 16 MiB device 0 offset 0xc0000, size 0x10000 SF: 65536 bytes @ 0xc0000 Read: OK ## Executing script at 000c0000 => dhcp *CACHE: Misaligned operation at range [0eb583b0, 0eb583b4]** **CACHE: Misaligned operation at range [0eb583b4, 0eb583b8]* BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 DHCP client bound to address 192.168.21.193 (1008 ms) => "
Do we have real trouble inside or can we quiet this warning?
I have seen it too. Look at gem driver which is flushing cache which is not aligned. There must be some changes in the code to remove these warnings. Also worth to add this to test/py to capture it in future.
Thanks, Michal
participants (5)
-
Hannes Schmelzer
-
Marek Vasut
-
Michal Simek
-
Simon Glass
-
Tom Rini