Patchwork [U-Boot,v2] cmd_boot: cleanup for 'go' command

login
register
mail settings
Submitter Kuo-Jung Su
Date April 29, 2013, 2:53 a.m.
Message ID <1367203988-5829-1-git-send-email-dantesu@gmail.com>
Download mbox | patch
Permalink /patch/240315/
State Changes Requested
Delegated to: Tom Rini
Headers show

Comments

Kuo-Jung Su - April 29, 2013, 2:53 a.m.
From: Kuo-Jung Su <dantesu@faraday-tech.com>

With MMU/D-Cache enabled, data might be retained at d-cache
rather than at DRAM when we execute 'go' command, and some
of the bare-metal softwares would always invalidate the entire
data cache at start-up, and then leads to a data lost issue.
Furthermore, the U-Boot not only relocates itself but also
updates symbol table at start-up, which means the i-cache
might also be dirty, so it would be better to invalidates the
i-cache alone with d-cache flush.

This patch is designed to fix the issues above.
It has been verified at ARM based systems, and should also work
at other platforms.

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
CC: Wolfgang Denk <wd@denx.de>
---
Changes for v2:
   - Update the commit log to state that i-cache is also invalidated.
   - DO NOT disable interrupts.

 common/cmd_boot.c |    7 +++++++
 1 file changed, 7 insertions(+)

--
1.7.9.5
Tom Rini - May 10, 2013, 12:06 p.m.
On Sun, Apr 28, 2013 at 04:53:08PM -0000, Kuo-Jung Su wrote:

> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> 
> With MMU/D-Cache enabled, data might be retained at d-cache
> rather than at DRAM when we execute 'go' command, and some
> of the bare-metal softwares would always invalidate the entire
> data cache at start-up, and then leads to a data lost issue.
> Furthermore, the U-Boot not only relocates itself but also
> updates symbol table at start-up, which means the i-cache
> might also be dirty, so it would be better to invalidates the
> i-cache alone with d-cache flush.
> 
> This patch is designed to fix the issues above.
> It has been verified at ARM based systems, and should also work
> at other platforms.

This patch has a few not trivially solvable problems.  First, the only
weak version of invalidate_icache_all is in common/cmd_cache.c.  The
only defined version of the function is for ARMv7 CPUs.  So, everyone
that is not ARMv7 and does not have CONFIG_CMD_CACHE will not build now.

And I'm not seeing a globally defined CONFIG to say "Yes, we are ARMv7"
or "Yes, we have icache to invalidate".

This actually seems related to the cache flush issue that Freescale
folks brought up a few weeks ago.  Or maybe that's my pre-coffee brain
talking.

Patch

diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index d3836fd..9e66364 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -50,6 +50,13 @@  static int do_go(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

 	printf ("## Starting application at 0x%08lX ...\n", addr);

+#ifndef CONFIG_SYS_DCACHE_OFF
+	flush_dcache_all();
+#endif
+#ifndef CONFIG_SYS_ICACHE_OFF
+	invalidate_icache_all();
+#endif
+
 	/*
 	 * pass address parameter as argv[0] (aka command name),
 	 * and all remaining args