Patchwork [U-Boot,1/2] command/cmd_cache.c: Add optional flush arguments

login
register
mail settings
Submitter Matthew McClintock
Date May 23, 2011, 6:38 p.m.
Message ID <1306175933-4950-1-git-send-email-msm@freescale.com>
Download mbox | patch
Permalink /patch/97025/
State Superseded
Headers show

Comments

Matthew McClintock - May 23, 2011, 6:38 p.m.
It might be desirable to have the ability to flush icache/dcache
within u-boot, this patch gives each arch the ability to provide
a flush_dcache/flush_icache function to let u-boot flush caches
from the prompt

Signed-off-by: Matthew McClintock <msm@freescale.com>
---
There are some checkpatch.pl warnings for formatting issues. In this case
I choose to keep the same formatting as was used in the file already:

#44: FILE: common/cmd_cache.c:48:
+               case 2: flush_icache();

ERROR: trailing statements should be on next line
#70: FILE: common/cmd_cache.c:78:
+               case 2: flush_dcache();

ERROR: return is not a function, parentheses are not required
#84: FILE: common/cmd_cache.c:96:
+               return (2);

total: 3 errors, 0 warnings, 83 lines checked

 common/cmd_cache.c |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)
Mike Frysinger - May 24, 2011, 12:25 a.m.
On Monday, May 23, 2011 14:38:52 Matthew McClintock wrote:
> There are some checkpatch.pl warnings for formatting issues. In this case
> I choose to keep the same formatting as was used in the file already:

this is fine i think.  otherwise, you'd send another patch to first fix the 
formatting and then base this on top of that.

> +void __flush_icache(void)
> +{
> +	/* please define arch specific flush_icache */
> +	printf("No arch specific flush_icache available!\n");
> +}
> +void flush_icache(void) __attribute__((weak, alias("__flush_icache")));

i dont see much point in having the indirection.  just do:

void __weak flush_icache(void)
{
	/* please define arch specific flush_icache */
	puts("No arch specific flush_icache available!\n");
}
-mike
McClintock Matthew-B29882 - May 24, 2011, 3:06 p.m.
On Mon, May 23, 2011 at 7:25 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> void __weak flush_icache(void)
> {
>        /* please define arch specific flush_icache */
>        puts("No arch specific flush_icache available!\n");
> }

This syntax does not quite work for me... how about something like:

void flush_icache(void) __attribute__((weak));
void flush_icache(void)
{
        /* please define arch specific flush_icache */
        puts("No arch specific flush_icache available!\n");
}

I seem to require separate declaration and definition for this to
work.... however the alias is not required.

-M
Mike Frysinger - May 24, 2011, 6:23 p.m.
On Tuesday, May 24, 2011 11:06:03 McClintock Matthew-B29882 wrote:
> On Mon, May 23, 2011 at 7:25 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > void __weak flush_icache(void)
> > {
> >        /* please define arch specific flush_icache */
> >        puts("No arch specific flush_icache available!\n");
> > }
> 
> This syntax does not quite work for me... how about something like:
> 
> void flush_icache(void) __attribute__((weak));
> void flush_icache(void)

now, that's exactly what i do not want.  i'm guessing you simply did not 
include linux/compiler.h and so was missing the __weak define.
-mike

Patch

diff --git a/common/cmd_cache.c b/common/cmd_cache.c
index 5cdd834..78d53df 100644
--- a/common/cmd_cache.c
+++ b/common/cmd_cache.c
@@ -27,17 +27,26 @@ 
 #include <common.h>
 #include <command.h>
 
-static int on_off (const char *);
+static int parse_argv(const char *);
+
+void __flush_icache(void)
+{
+	/* please define arch specific flush_icache */
+	printf("No arch specific flush_icache available!\n");
+}
+void flush_icache(void) __attribute__((weak, alias("__flush_icache")));
 
 int do_icache ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	switch (argc) {
 	case 2:			/* on / off	*/
-		switch (on_off(argv[1])) {
+		switch (parse_argv(argv[1])) {
 		case 0:	icache_disable();
 			break;
 		case 1:	icache_enable ();
 			break;
+		case 2: flush_icache();
+			break;
 		}
 		/* FALL TROUGH */
 	case 1:			/* get status */
@@ -50,15 +59,24 @@  int do_icache ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;
 }
 
+void __flush_dcache(void)
+{
+	printf("No arch specific flush_dcache available!\n");
+	/* please define arch specific flush_dcache */
+}
+void flush_dcache(void) __attribute__((weak, alias("__flush_dcache")));
+
 int do_dcache ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	switch (argc) {
 	case 2:			/* on / off	*/
-		switch (on_off(argv[1])) {
+		switch (parse_argv(argv[1])) {
 		case 0:	dcache_disable();
 			break;
 		case 1:	dcache_enable ();
 			break;
+		case 2: flush_dcache();
+			break;
 		}
 		/* FALL TROUGH */
 	case 1:			/* get status */
@@ -72,9 +90,11 @@  int do_dcache ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 }
 
-static int on_off (const char *s)
+static int parse_argv(const char *s)
 {
-	if (strcmp(s, "on") == 0) {
+	if (strcmp(s, "flush") == 0) {
+		return (2);
+	} else if (strcmp(s, "on") == 0) {
 		return (1);
 	} else if (strcmp(s, "off") == 0) {
 		return (0);
@@ -86,13 +106,13 @@  static int on_off (const char *s)
 U_BOOT_CMD(
 	icache,   2,   1,     do_icache,
 	"enable or disable instruction cache",
-	"[on, off]\n"
-	"    - enable or disable instruction cache"
+	"[on, off, flush]\n"
+	"    - enable, disable, or flush instruction cache"
 );
 
 U_BOOT_CMD(
 	dcache,   2,   1,     do_dcache,
 	"enable or disable data cache",
-	"[on, off]\n"
-	"    - enable or disable data (writethrough) cache"
+	"[on, off, flush]\n"
+	"    - enable, disable, or flush data (writethrough) cache"
 );