[U-Boot] [PATCH 0/21] Generic cross-architecture system restart support

[U-Boot] [PATCH 0/21] Generic cross-architecture system restart support

[Kyle Moffett]

Hello everyone,

This patch series creates a generic set of functions available to generic
code which can be easily hooked per-architecture or per-board.  I have
tried to be very careful that U-Boot builds and runs on all architectures
for the entire duration of this patchset.

This patchset is designed to produce zero change in behavior *except* for
one particular scenario:  Platforms which perform a "restart" with a simple
jump to the "_start" entrypoint (or similar) will no longer try to do that
on panic().

The new functions to be called from generic code are:

  int system_restart(void)
  void emergency_restart(void)

The specific platform hooks available are:

  int __arch_restart(void)
  int __board_restart(void)
  void __arch_emergency_restart(void)
  void __board_emergency_restart(void)

The first few patches set up the generic functions and hooks with a default
fallback to the existing "do_reset()" function.  Most of the rest are then
architecture-specific modifications necessary to convert away from the old
do_reset() prototype.  The last patch finally does away with that function.

In my previous discussions with Wolfgang Denk about the __arch_restart()
hook he requested that it should be mandatory for all architectures to
implement.  Unfortunately the MIPS and MicroBlaze architectures have no
architectural system-reset code at all, and others have only very limited
"soft-reboot" (IE: Jump-to-"_start").

Specifically, the MIPS do_reset() function used to look like this:
  _machine_restart();
  printf("*** restart failed ***\n");

When those platforms are fixed then it should be safe to remove the weak
fallback __arch_restart() function.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Kyle Moffett]

In preparation for making system restart use a generic set of hooks for
boards and architectures, we define some wrappers and weak stubs.

The new wrapper functions are:
  system_restart()     -  Normal system reboot (IE: user request)
  emergency_restart()  -  Critical error response (IE: panic(), etc)

During the process of converting all architectures to use the new hooks,
the system_restart() and emergency_restart() code will fall back to call
the existing do_reset() function.

The handler for the "reset" shell command is now do_generic_reset(),
which is a trivial wrapper around system_restart().

The new hooks (for architecture and board-support code to define) are:
  __arch_emergency_restart()
  __arch_restart()
  __board_emergency_restart()
  __board_restart()

By default the __(arch|board)_emergency_restart() functions just call
the corresponding __(arch|board)_restart() hook.  This works for all
hardware platforms which have a properly defined hardware reset
capability.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 api/api.c                 |    3 +-
 common/cmd_boot.c         |  128 ++++++++++++++++++++++++++++++++++++++++++++-
 common/cmd_bootm.c        |    8 +--
 common/hush.c             |    2 +-
 common/main.c             |    2 +-
 examples/api/libgenwrap.c |    5 +-
 include/_exports.h        |    2 +-
 include/command.h         |   13 +++++
 lib/vsprintf.c            |    2 +-
 9 files changed, 152 insertions(+), 13 deletions(-)

diff --git a/api/api.c b/api/api.c
index 853f010..b65fabd 100644
--- a/api/api.c
+++ b/api/api.c
@@ -131,7 +131,8 @@ static int API_puts(va_list ap)
  */
 static int API_reset(va_list ap)
 {
-	do_reset(NULL, 0, 0, NULL);
+	if (system_restart())
+		return API_ENODEV;
 
 	/* NOT REACHED */
 	return 0;
diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index 7b603d3..c0f26fc 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -58,6 +58,132 @@ int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return rcode;
 }
 
+/*
+ * emergency_restart()  -  Restart function to call from panic(), etc.
+ *
+ * This may be called from any context when something has gone terribly wrong
+ * and the system needs a best-effort reboot.
+ *
+ * This function will never return.
+ */
+__attribute__((__noreturn__))
+void emergency_restart(void)
+{
+	__board_emergency_restart();
+	__arch_emergency_restart();
+
+	/* Fallback to the old do_reset() until everything is converted. */
+	do_reset(NULL, 0, 0, NULL);
+
+	printf("EMERGENCY RESTART: All attempts to reboot failed!");
+	hang();
+}
+
+/*
+ * Architecture/board-specific emergency-restart hooks.
+ *
+ * These hooks default to the same as the normal restart hooks.
+ *
+ * WARNING: If your platform's "restart" hooks are not a "safe" then you
+ * must redefine these functions in your port as empty stubs.
+ *
+ * Specifically, the following assumptions must be true for these hooks:
+ *   (1) Safe to call from any context (interrupts, etc).
+ *   (2) Must use a hardware mechanism to reset *ALL* system state.
+ *   (3) They must not be interruptible (EG: with Ctrl-C).
+ */
+__attribute__((__weak__))
+void __board_emergency_restart(void)
+{
+	__board_restart();
+}
+
+__attribute__((__weak__))
+void __arch_emergency_restart(void)
+{
+	__arch_restart();
+}
+
+/*
+ * This MUST be called from normal interrupts-on context.  If it requires
+ * extended interaction with external hardware it SHOULD react to Ctrl-C.
+ *
+ * If this function fails to guarantee a clean reboot or receives a Ctrl-C
+ * keystroke it SHOULD return with an error (-1).
+ */
+int system_restart(void)
+{
+	int err;
+
+	/*
+	 * Print a nice message and wait a bit to make sure it goes out the
+	 * console properly.
+	 */
+	printf ("Restarting...\n");
+	udelay(50000);
+
+	/* First let the board code try to reboot */
+	err = __board_restart();
+	if (err)
+		goto failed;
+
+	/* Now call into the architecture-specific code */
+	err = __arch_restart();
+	if (err)
+		goto failed;
+
+	/* Fallback to the old do_reset() until everything is converted. */
+	err = do_reset(NULL, 0, 0, NULL);
+
+failed:
+	printf("*** SYSTEM RESTART FAILED ***\n");
+	return err;
+}
+
+/*
+ * Architecture/board-specific restart hooks.
+ *
+ * If your implementation of these functions does not meet the requirements
+ * for the emergency_restart() hooks described above then you MUST separately
+ * implement those hooks.
+ */
+__attribute__((__weak__))
+int __board_restart(void)
+{
+	/* Fallthrough to architecture-specific code */
+	return 0;
+}
+
+__attribute__((__weak__))
+int __arch_restart(void)
+{
+	/* Fallthrough to legacy do_reset() code */
+	return 0;
+}
+
+/*
+ * Generic reset command implementation
+ *
+ * This is what you get when you type "reset" at the command line.
+ */
+int do_generic_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return system_restart();
+}
+
+/*
+ * Empty legacy "do_reset" stub.
+ *
+ * This allows a platform using the new __board_restart() and
+ * __arch_restart() hooks to completely omit the old do_reset() function.
+ */
+int do_reset_stub(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return 0;
+}
+__attribute__((__weak__,__alias__("do_reset_stub")))
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+
 /* -------------------------------------------------------------------- */
 
 U_BOOT_CMD(
@@ -68,7 +194,7 @@ U_BOOT_CMD(
 );
 
 U_BOOT_CMD(
-	reset, 1, 0,	do_reset,
+	reset, 1, 0,	do_generic_reset,
 	"Perform RESET of the CPU",
 	""
 );
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 18019d6..60e4983 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -645,7 +645,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	if (ret < 0) {
 		if (ret == BOOTM_ERR_RESET)
-			do_reset (cmdtp, flag, argc, argv);
+			emergency_restart();
 		if (ret == BOOTM_ERR_OVERLAP) {
 			if (images.legacy_hdr_valid) {
 				if (image_get_type (&images.legacy_hdr_os_copy) == IH_TYPE_MULTI)
@@ -655,7 +655,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				puts ("ERROR: new format image overwritten - "
 					"must RESET the board to recover\n");
 				show_boot_progress (-113);
-				do_reset (cmdtp, flag, argc, argv);
+				emergency_restart();
 			}
 		}
 		if (ret == BOOTM_ERR_UNIMPLEMENTED) {
@@ -702,9 +702,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 #ifdef DEBUG
 	puts ("\n## Control returned to monitor - resetting...\n");
 #endif
-	do_reset (cmdtp, flag, argc, argv);
-
-	return 1;
+	emergency_restart();
 }
 
 /**
diff --git a/common/hush.c b/common/hush.c
index 8021a68..f9e57c9 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1036,7 +1036,7 @@ static void get_user_input(struct in_str *i)
 	if (n == -2) {
 	  puts("\nTimeout waiting for command\n");
 #  ifdef CONFIG_RESET_TO_RETRY
-	  do_reset(NULL, 0, 0, NULL);
+	  system_restart();
 #  else
 #	error "This currently only works with CONFIG_RESET_TO_RETRY enabled"
 #  endif
diff --git a/common/main.c b/common/main.c
index dcbacc9..abf77a5 100644
--- a/common/main.c
+++ b/common/main.c
@@ -450,7 +450,7 @@ void main_loop (void)
 			puts ("\nTimed out waiting for command\n");
 # ifdef CONFIG_RESET_TO_RETRY
 			/* Reinit board to run initialization code again */
-			do_reset (NULL, 0, 0, NULL);
+			system_restart();
 # else
 			return;		/* retry autoboot */
 # endif
diff --git a/examples/api/libgenwrap.c b/examples/api/libgenwrap.c
index 873cf34..31bfcf5 100644
--- a/examples/api/libgenwrap.c
+++ b/examples/api/libgenwrap.c
@@ -81,9 +81,10 @@ void __udelay(unsigned long usec)
 	ub_udelay(usec);
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int system_restart(void)
 {
-	ub_reset();
+	if (ub_reset())
+		return -1;
 	return 0;
 }
 
diff --git a/include/_exports.h b/include/_exports.h
index d89b65b..3d3e511 100644
--- a/include/_exports.h
+++ b/include/_exports.h
@@ -15,7 +15,7 @@ EXPORT_FUNC(free)
 EXPORT_FUNC(udelay)
 EXPORT_FUNC(get_timer)
 EXPORT_FUNC(vprintf)
-EXPORT_FUNC(do_reset)
+EXPORT_FUNC(system_restart)
 EXPORT_FUNC(getenv)
 EXPORT_FUNC(setenv)
 EXPORT_FUNC(simple_strtoul)
diff --git a/include/command.h b/include/command.h
index 8310fe5..ad8c915 100644
--- a/include/command.h
+++ b/include/command.h
@@ -101,6 +101,19 @@ extern int do_bootd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 extern int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 extern int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 
+/* Generic system restart functions */
+__attribute__((__noreturn__))
+void emergency_restart(void);
+int system_restart(void);
+
+/* Architecture/board-specific emergency-restart hooks. */
+void __board_emergency_restart(void);
+void __arch_emergency_restart(void);
+
+/* Architecture/board-specific restart hooks. */
+int __board_restart(void);
+int __arch_restart(void);
+
 #endif	/* __ASSEMBLY__ */
 
 /*
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 61e6f0d..295b60b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -674,6 +674,6 @@ void panic(const char *fmt, ...)
 	hang();
 #else
 	udelay (100000);	/* allow messages to go out */
-	do_reset (NULL, 0, 0, NULL);
+	emergency_restart();
 #endif
 }
-- 
1.7.2.3

[U-Boot] [PATCH 02/21] Replace do_reset() calls with {system, emergency}_restart()

[Kyle Moffett]

All explicit calls to do_reset() should now use either system_restart()
or emergency_restart() as appropriate.

This should not change any behavior because at present there are no
implementations of any arch-specific hooks, making those two functions
just minimal wrappers around the existing do_reset() functions.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 board/dave/PPChameleonEVB/PPChameleonEVB.c |    4 ++--
 board/esd/apc405/apc405.c                  |    4 ++--
 board/esd/ar405/ar405.c                    |    2 +-
 board/esd/ash405/ash405.c                  |    4 ++--
 board/esd/canbt/canbt.c                    |    2 +-
 board/esd/cpci405/cpci405.c                |    6 +++---
 board/esd/cpciiser4/cpciiser4.c            |    2 +-
 board/esd/du405/du405.c                    |    2 +-
 board/esd/hh405/hh405.c                    |    4 ++--
 board/esd/pci405/pci405.c                  |    4 ++--
 board/esd/plu405/plu405.c                  |    4 ++--
 board/esd/tasreg/tasreg.c                  |    4 ++--
 board/esd/voh405/voh405.c                  |    4 ++--
 board/esd/wuh405/wuh405.c                  |    4 ++--
 board/prodrive/pdnb3/pdnb3.c               |    2 +-
 board/sacsng/sacsng.c                      |    2 +-
 board/zeus/zeus.c                          |    4 ++--
 17 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/board/dave/PPChameleonEVB/PPChameleonEVB.c b/board/dave/PPChameleonEVB/PPChameleonEVB.c
index 8e26996..0c0c787 100644
--- a/board/dave/PPChameleonEVB/PPChameleonEVB.c
+++ b/board/dave/PPChameleonEVB/PPChameleonEVB.c
@@ -103,7 +103,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -136,7 +136,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/apc405/apc405.c b/board/esd/apc405/apc405.c
index def8a4f..31d8a4a 100644
--- a/board/esd/apc405/apc405.c
+++ b/board/esd/apc405/apc405.c
@@ -219,7 +219,7 @@ int misc_init_r(void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip(dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -255,7 +255,7 @@ int misc_init_r(void)
 				udelay(1000);
 		}
 		putc('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	/* restore gpio/cs settings */
diff --git a/board/esd/ar405/ar405.c b/board/esd/ar405/ar405.c
index 6ec507f..34936e6 100644
--- a/board/esd/ar405/ar405.c
+++ b/board/esd/ar405/ar405.c
@@ -112,7 +112,7 @@ int board_early_init_f (void)
 					udelay (1000);
 			}
 			putc ('\n');
-			do_reset (NULL, 0, 0, NULL);
+			emergency_restart();
 		}
 	}
 
diff --git a/board/esd/ash405/ash405.c b/board/esd/ash405/ash405.c
index 1b0365e..d45f70e 100644
--- a/board/esd/ash405/ash405.c
+++ b/board/esd/ash405/ash405.c
@@ -88,7 +88,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -121,7 +121,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/canbt/canbt.c b/board/esd/canbt/canbt.c
index cc537f2..1079a98 100644
--- a/board/esd/canbt/canbt.c
+++ b/board/esd/canbt/canbt.c
@@ -108,7 +108,7 @@ int board_early_init_f (void)
 				udelay (1000);
 		}
 		putc ('\n');
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	/*
diff --git a/board/esd/cpci405/cpci405.c b/board/esd/cpci405/cpci405.c
index 98a8584..df11d3e 100644
--- a/board/esd/cpci405/cpci405.c
+++ b/board/esd/cpci405/cpci405.c
@@ -160,7 +160,7 @@ int board_early_init_f(void)
 					udelay(1000);
 			}
 			putc('\n');
-			do_reset(NULL, 0, 0, NULL);
+			emergency_restart();
 		}
 	}
 #endif /* !CONFIG_CPCI405_VER2 */
@@ -288,7 +288,7 @@ int misc_init_r (void)
 		if (gunzip(dst, CONFIG_SYS_FPGA_MAX_SIZE,
 			   (uchar *)fpgadata, &len) != 0) {
 			printf("GUNZIP ERROR - must RESET board to recover\n");
-			do_reset(NULL, 0, 0, NULL);
+			emergency_restart();
 		}
 
 		status = fpga_boot(dst, len);
@@ -324,7 +324,7 @@ int misc_init_r (void)
 					udelay(1000);
 			}
 			putc('\n');
-			do_reset(NULL, 0, 0, NULL);
+			emergency_restart();
 		}
 
 		/* restore gpio/cs settings */
diff --git a/board/esd/cpciiser4/cpciiser4.c b/board/esd/cpciiser4/cpciiser4.c
index 8afc50d..74421fb 100644
--- a/board/esd/cpciiser4/cpciiser4.c
+++ b/board/esd/cpciiser4/cpciiser4.c
@@ -106,7 +106,7 @@ int board_early_init_f (void)
 				udelay (1000);
 		}
 		putc ('\n');
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	/*
diff --git a/board/esd/du405/du405.c b/board/esd/du405/du405.c
index c32d333..91646e2 100644
--- a/board/esd/du405/du405.c
+++ b/board/esd/du405/du405.c
@@ -106,7 +106,7 @@ int board_early_init_f (void)
 				udelay (1000);
 		}
 		putc ('\n');
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	/*
diff --git a/board/esd/hh405/hh405.c b/board/esd/hh405/hh405.c
index e9d2d36..5cb9db6 100644
--- a/board/esd/hh405/hh405.c
+++ b/board/esd/hh405/hh405.c
@@ -417,7 +417,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -450,7 +450,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/pci405/pci405.c b/board/esd/pci405/pci405.c
index c1bac6a..a2462d4 100644
--- a/board/esd/pci405/pci405.c
+++ b/board/esd/pci405/pci405.c
@@ -198,7 +198,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -231,7 +231,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/plu405/plu405.c b/board/esd/plu405/plu405.c
index 109d2dc..432d291 100644
--- a/board/esd/plu405/plu405.c
+++ b/board/esd/plu405/plu405.c
@@ -121,7 +121,7 @@ int misc_init_r(void)
 	if (gunzip(dst, CONFIG_SYS_FPGA_MAX_SIZE,
 		   (uchar *)fpgadata, &len) != 0) {
 		printf("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -157,7 +157,7 @@ int misc_init_r(void)
 				udelay(1000);
 		}
 		putc('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/tasreg/tasreg.c b/board/esd/tasreg/tasreg.c
index d2488b8..644cb0d 100644
--- a/board/esd/tasreg/tasreg.c
+++ b/board/esd/tasreg/tasreg.c
@@ -152,7 +152,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -185,7 +185,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/voh405/voh405.c b/board/esd/voh405/voh405.c
index 5f28a48..e03d6a1 100644
--- a/board/esd/voh405/voh405.c
+++ b/board/esd/voh405/voh405.c
@@ -117,7 +117,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -150,7 +150,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/esd/wuh405/wuh405.c b/board/esd/wuh405/wuh405.c
index d8d4bb5..d602261 100644
--- a/board/esd/wuh405/wuh405.c
+++ b/board/esd/wuh405/wuh405.c
@@ -85,7 +85,7 @@ int misc_init_r (void)
 	dst = malloc(CONFIG_SYS_FPGA_MAX_SIZE);
 	if (gunzip (dst, CONFIG_SYS_FPGA_MAX_SIZE, (uchar *)fpgadata, &len) != 0) {
 		printf ("GUNZIP ERROR - must RESET board to recover\n");
-		do_reset (NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	status = fpga_boot(dst, len);
@@ -118,7 +118,7 @@ int misc_init_r (void)
 				udelay(1000);
 		}
 		putc ('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/prodrive/pdnb3/pdnb3.c b/board/prodrive/pdnb3/pdnb3.c
index 928dd22..d41d659 100644
--- a/board/prodrive/pdnb3/pdnb3.c
+++ b/board/prodrive/pdnb3/pdnb3.c
@@ -181,7 +181,7 @@ int do_fpga_boot(unsigned char *fpgadata)
 				udelay(1000);
 		}
 		putc('\n');
-		do_reset(NULL, 0, 0, NULL);
+		emergency_restart();
 	}
 
 	puts("FPGA:  ");
diff --git a/board/sacsng/sacsng.c b/board/sacsng/sacsng.c
index 61cab87..3dc3f1b 100644
--- a/board/sacsng/sacsng.c
+++ b/board/sacsng/sacsng.c
@@ -826,7 +826,7 @@ void show_boot_progress (int status)
 	/*
 	 * Reset the board to retry initialization.
 	 */
-	do_reset (NULL, 0, 0, NULL);
+	emergency_restart();
     }
 }
 #endif /* CONFIG_SHOW_BOOT_PROGRESS */
diff --git a/board/zeus/zeus.c b/board/zeus/zeus.c
index 7e33d3f..b4119ec 100644
--- a/board/zeus/zeus.c
+++ b/board/zeus/zeus.c
@@ -372,7 +372,7 @@ int do_chkreset(cmd_tbl_t* cmdtp, int flag, int argc, char * const argv[])
 		/*
 		 * Reset the board for default to become valid
 		 */
-		do_reset(NULL, 0, 0, NULL);
+		system_restart();
 
 		return 0;
 	}
@@ -392,7 +392,7 @@ int do_chkreset(cmd_tbl_t* cmdtp, int flag, int argc, char * const argv[])
 		 */
 		logbuff_reset();
 
-		do_reset(NULL, 0, 0, NULL);
+		system_restart();
 
 		return 0;
 	}
-- 
1.7.2.3

[U-Boot] [PATCH 03/21] arm: Call "panic()" instead of "hang()" for div-by-zero

[Kyle Moffett]

Divide-by-zero errors should be controlled by the CONFIG_PANIC_HANG
configuration variable just like any other kind of trappable U-Boot bug.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Albert Aribaud <albert.aribaud@free.fr>
---
 arch/arm/lib/div0.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/lib/div0.c b/arch/arm/lib/div0.c
index 6267bf1..4ec25dc 100644
--- a/arch/arm/lib/div0.c
+++ b/arch/arm/lib/div0.c
@@ -21,10 +21,10 @@
  * MA 02111-1307 USA
  */
 
+#include <common.h>
+
 /* Replacement (=dummy) for GNU/Linux division-by zero handler */
 void __div0 (void)
 {
-	extern void hang (void);
-
-	hang();
+	panic("ERROR: divide by zero!");
 }
-- 
1.7.2.3

[U-Boot] [PATCH 04/21] arm: Replace unnecessary bad_mode() with panic()

[Kyle Moffett]

The bad_mode() function calls panic() and then would call the
reset_cpu() function, except that panic() never returns!

Replace all of the calls to bad_mode() with simple calls to panic().
This helps prepare for the followup patches to convert to generic
system-restart support.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Albert Aribaud <albert.aribaud@free.fr>
---
 arch/arm/lib/interrupts.c |   21 +++++++--------------
 1 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c
index 74ff5ce..156a23c 100644
--- a/arch/arm/lib/interrupts.c
+++ b/arch/arm/lib/interrupts.c
@@ -102,13 +102,6 @@ int disable_interrupts (void)
 }
 #endif
 
-
-void bad_mode (void)
-{
-	panic ("Resetting CPU ...\n");
-	reset_cpu (0);
-}
-
 void show_regs (struct pt_regs *regs)
 {
 	unsigned long flags;
@@ -150,42 +143,42 @@ void do_undefined_instruction (struct pt_regs *pt_regs)
 {
 	printf ("undefined instruction\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 void do_software_interrupt (struct pt_regs *pt_regs)
 {
 	printf ("software interrupt\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 void do_prefetch_abort (struct pt_regs *pt_regs)
 {
 	printf ("prefetch abort\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 void do_data_abort (struct pt_regs *pt_regs)
 {
 	printf ("data abort\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 void do_not_used (struct pt_regs *pt_regs)
 {
 	printf ("not used\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 void do_fiq (struct pt_regs *pt_regs)
 {
 	printf ("fast interrupt request\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 
 #ifndef CONFIG_USE_IRQ
@@ -193,6 +186,6 @@ void do_irq (struct pt_regs *pt_regs)
 {
 	printf ("interrupt request\n");
 	show_regs (pt_regs);
-	bad_mode ();
+	panic("Resetting CPU...\n");
 }
 #endif
-- 
1.7.2.3

[U-Boot] [PATCH 05/21] arm: cpux9k2: Remove unnecessary XF_do_reset assignment

[Kyle Moffett]

The exported function table already assigns XF_do_reset properly, so
there should be no need for this processor to manually reassign it.

This change is required before ARM is converted away from using the
legacy do_reset() handler.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Albert Aribaud <albert.aribaud@free.fr>
Cc: Reinhard Meyer <u-boot@emk-elektronik.de>
---
 board/BuS/eb_cpux9k2/cpux9k2.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/board/BuS/eb_cpux9k2/cpux9k2.c b/board/BuS/eb_cpux9k2/cpux9k2.c
index fe62a0f..473498c 100644
--- a/board/BuS/eb_cpux9k2/cpux9k2.c
+++ b/board/BuS/eb_cpux9k2/cpux9k2.c
@@ -111,7 +111,6 @@ int misc_init_r(void)
 		}
 	}
 #endif
-	gd->jt[XF_do_reset] = (void *) do_reset;
 
 #ifdef CONFIG_STATUS_LED
 	status_led_set(STATUS_LED_BOOT, STATUS_LED_BLINKING);
-- 
1.7.2.3

[U-Boot] [PATCH 06/21] arm: Rename nonstandard board_reset() as at91_board_reset()

[Kyle Moffett]

The ARM AT91 port calls a customized board_reset() function after
disabling interrupts and stopping the serial console.

Since this function is not architecture-generic and not compatible with
the new platform-independent meaning for __board_restart(), we rename it
here to be at91_board_reset().

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Albert Aribaud <albert.aribaud@free.fr>
Cc: Reinhard Meyer <u-boot@emk-elektronik.de>
---
 arch/arm/cpu/arm920t/at91/reset.c       |    4 ++--
 arch/arm/cpu/arm920t/at91rm9200/reset.c |    6 +++---
 board/atmel/at91rm9200dk/at91rm9200dk.c |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/reset.c b/arch/arm/cpu/arm920t/at91/reset.c
index 51043ec..c82a431 100644
--- a/arch/arm/cpu/arm920t/at91/reset.c
+++ b/arch/arm/cpu/arm920t/at91/reset.c
@@ -35,7 +35,7 @@
 #include <asm/arch/hardware.h>
 #include <asm/arch/at91_st.h>
 
-void  __attribute__((weak)) board_reset(void)
+void  __attribute__((weak)) at91_board_reset(void)
 {
 	/* true empty function for defining weak symbol */
 }
@@ -48,7 +48,7 @@ void reset_cpu(ulong ignored)
 	serial_exit();
 #endif
 
-	board_reset();
+	at91_board_reset();
 
 	/* Reset the cpu by setting up the watchdog timer */
 	writel(AT91_ST_WDMR_RSTEN | AT91_ST_WDMR_EXTEN | AT91_ST_WDMR_WDV(2),
diff --git a/arch/arm/cpu/arm920t/at91rm9200/reset.c b/arch/arm/cpu/arm920t/at91rm9200/reset.c
index 945ea2c..523f835 100644
--- a/arch/arm/cpu/arm920t/at91rm9200/reset.c
+++ b/arch/arm/cpu/arm920t/at91rm9200/reset.c
@@ -33,7 +33,7 @@
 #include <common.h>
 #include <asm/arch/hardware.h>
 
-void board_reset(void) __attribute__((__weak__));
+void at91_board_reset(void) __attribute__((__weak__));
 
 /*
  * Reset the cpu by setting up the watchdog timer and let him time out
@@ -47,8 +47,8 @@ void reset_cpu (ulong ignored)
 	serial_exit();
 #endif
 
-	if (board_reset)
-		board_reset();
+	if (at91_board_reset)
+		at91_board_reset();
 
 	/* this is the way Linux does it */
 
diff --git a/board/atmel/at91rm9200dk/at91rm9200dk.c b/board/atmel/at91rm9200dk/at91rm9200dk.c
index 49b5fe3..0fbf2a9 100644
--- a/board/atmel/at91rm9200dk/at91rm9200dk.c
+++ b/board/atmel/at91rm9200dk/at91rm9200dk.c
@@ -60,7 +60,7 @@ int board_init (void)
 	return 0;
 }
 
-void board_reset (void)
+void at91_board_reset (void)
 {
 	AT91PS_PIO pio = AT91C_BASE_PIOA;
 
-- 
1.7.2.3

[U-Boot] [PATCH 07/21] arm: Generic system restart support

[Kyle Moffett]

The ARM port has its own reset_cpu() dispatch for its various supported
CPU families, so the existing do_reset() function is simply altered to
use the new prototype for __arch_restart().

In addition, the debug message and delay are duplicated from the generic
code, so they are removed.

This reset code will probably work even when the CPU is in a bad state,
so no separate __arch_emergency_restart() function is required.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Albert Aribaud <albert.aribaud@free.fr>
---
 arch/arm/lib/reset.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
index 08e6acb..ec4861e 100644
--- a/arch/arm/lib/reset.c
+++ b/arch/arm/lib/reset.c
@@ -39,12 +39,8 @@
 
 #include <common.h>
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
-	puts ("resetting ...\n");
-
-	udelay (50000);				/* wait 50 ms */
-
 	disable_interrupts();
 	reset_cpu(0);
 
-- 
1.7.2.3

[U-Boot] [PATCH 08/21] avr32: Generic system restart support

[Kyle Moffett]

The AVR32 port uses the exact same restart code on all supported CPUs,
so that do_reset() function is simply altered to use the new prototype
for __arch_restart().

It will probably work even when the CPU is in a bad state, so no
separate __arch_emergency_restart() function is required.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Reinhard Meyer <u-boot@emk-elektronik.de>
---
 arch/avr32/cpu/cpu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/cpu/cpu.c b/arch/avr32/cpu/cpu.c
index e4489bb..ca2226b 100644
--- a/arch/avr32/cpu/cpu.c
+++ b/arch/avr32/cpu/cpu.c
@@ -76,7 +76,7 @@ void prepare_to_boot(void)
 		     "sync   0" : : "r"(0) : "memory");
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	/* This will reset the CPU core, caches, MMU and all internal busses */
 	__builtin_mtdr(8, 1 << 13);	/* set DC:DBE */
-- 
1.7.2.3

[U-Boot] [PATCH 09/21] blackfin: Replace "bfin_reset_or_hang()" with "panic()"

[Kyle Moffett]

The bfin_reset_or_hang function unnecessarily duplicates the panic()
logic based on CONFIG_PANIC_HANG.

This patch deletes 20 lines of code and just calls panic() instead.
This also makes the following generic-restart conversion patch simpler.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/cpu/cpu.h   |    1 -
 arch/blackfin/cpu/reset.c |   19 +------------------
 arch/blackfin/cpu/traps.c |    2 +-
 3 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/arch/blackfin/cpu/cpu.h b/arch/blackfin/cpu/cpu.h
index ba85e0b..e70560f 100644
--- a/arch/blackfin/cpu/cpu.h
+++ b/arch/blackfin/cpu/cpu.h
@@ -28,7 +28,6 @@
 #include <command.h>
 
 void board_reset(void) __attribute__((__weak__));
-void bfin_reset_or_hang(void) __attribute__((__noreturn__));
 void bfin_dump(struct pt_regs *reg);
 void bfin_panic(struct pt_regs *reg);
 void dump(struct pt_regs *regs);
diff --git a/arch/blackfin/cpu/reset.c b/arch/blackfin/cpu/reset.c
index 164afde..49d0cf8 100644
--- a/arch/blackfin/cpu/reset.c
+++ b/arch/blackfin/cpu/reset.c
@@ -80,27 +80,10 @@ static void bfin_reset(void)
  * PC relative call with a 25 bit immediate.  This is not enough
  * to get us from the top of SDRAM into L1.
  */
-__attribute__ ((__noreturn__))
-static inline void bfin_reset_trampoline(void)
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (board_reset)
 		board_reset();
 	while (1)
 		asm("jump (%0);" : : "a" (bfin_reset));
 }
-
-__attribute__ ((__noreturn__))
-void bfin_reset_or_hang(void)
-{
-#ifdef CONFIG_PANIC_HANG
-	hang();
-#else
-	bfin_reset_trampoline();
-#endif
-}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	bfin_reset_trampoline();
-	return 0;
-}
diff --git a/arch/blackfin/cpu/traps.c b/arch/blackfin/cpu/traps.c
index 09388aa..0cb833a 100644
--- a/arch/blackfin/cpu/traps.c
+++ b/arch/blackfin/cpu/traps.c
@@ -426,5 +426,5 @@ void bfin_panic(struct pt_regs *regs)
 	unsigned long tflags;
 	trace_buffer_save(tflags);
 	bfin_dump(regs);
-	bfin_reset_or_hang();
+	panic("PANIC: Blackfin internal error");
 }
-- 
1.7.2.3

[U-Boot] [PATCH 10/21] blackfin: Generic system restart support

[Kyle Moffett]

The BlackFin port uses the exact same restart code on all supported
CPUs, so that do_reset() function is simply altered to use the new
prototype for __arch_restart().

Reading the code and CPU documentation does not make it entirely clear
whether or not the implementation is safe to use when the CPU may be in
a bad or undefined state, but it *appears* to be OK.

As a result no separate __arch_emergency_restart() function should be
needed.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/cpu/cpu.h         |    1 -
 arch/blackfin/cpu/reset.c       |    4 +---
 board/bf537-stamp/bf537-stamp.c |    5 ++++-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/blackfin/cpu/cpu.h b/arch/blackfin/cpu/cpu.h
index e70560f..32f6414 100644
--- a/arch/blackfin/cpu/cpu.h
+++ b/arch/blackfin/cpu/cpu.h
@@ -27,7 +27,6 @@
 
 #include <command.h>
 
-void board_reset(void) __attribute__((__weak__));
 void bfin_dump(struct pt_regs *reg);
 void bfin_panic(struct pt_regs *reg);
 void dump(struct pt_regs *regs);
diff --git a/arch/blackfin/cpu/reset.c b/arch/blackfin/cpu/reset.c
index 49d0cf8..84be51a 100644
--- a/arch/blackfin/cpu/reset.c
+++ b/arch/blackfin/cpu/reset.c
@@ -80,10 +80,8 @@ static void bfin_reset(void)
  * PC relative call with a 25 bit immediate.  This is not enough
  * to get us from the top of SDRAM into L1.
  */
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
-	if (board_reset)
-		board_reset();
 	while (1)
 		asm("jump (%0);" : : "a" (bfin_reset));
 }
diff --git a/board/bf537-stamp/bf537-stamp.c b/board/bf537-stamp/bf537-stamp.c
index ec888d4..e1ae775 100644
--- a/board/bf537-stamp/bf537-stamp.c
+++ b/board/bf537-stamp/bf537-stamp.c
@@ -43,11 +43,14 @@ int checkboard(void)
 	return 0;
 }
 
-void board_reset(void)
+int __board_restart(void)
 {
 	/* workaround for weak pull ups on ssel */
 	if (CONFIG_BFIN_BOOT_MODE == BFIN_BOOT_SPI_MASTER)
 		bfin_reset_boot_spi_cs(GPIO_PF10);
+
+	/* Fall through to the architecture reset handler */
+	return 0;
 }
 
 #ifdef CONFIG_BFIN_MAC
-- 
1.7.2.3

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Kyle Moffett]

The i386 port has its own reset_cpu() dispatch for its various supported
CPU families, so the existing do_reset() function is simply altered to
use the new prototype for __arch_restart().

In addition, the debug message and delay are duplicated from the generic
code, so they are removed.

This reset code will probably work even when the CPU is in a bad state,
so no separate __arch_emergency_restart() function is required.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Graeme Russ <graeme.russ@gmail.com>
---
 arch/i386/cpu/cpu.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/i386/cpu/cpu.c b/arch/i386/cpu/cpu.c
index 2339cd4..b1454ba 100644
--- a/arch/i386/cpu/cpu.c
+++ b/arch/i386/cpu/cpu.c
@@ -122,10 +122,8 @@ int x86_cpu_init_r(void)
 }
 int cpu_init_r(void) __attribute__((weak, alias("x86_cpu_init_r")));
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
-	printf ("resetting ...\n");
-	udelay(50000);				/* wait 50 ms */
 	disable_interrupts();
 	reset_cpu(0);
 
-- 
1.7.2.3

[U-Boot] [PATCH 12/21] m68k: Generic system restart support

[Kyle Moffett]

The m68k port individually implements do_reset() for each of several
different CPU types.  As the functions are conditionally compiled based
on the board, this patch simply needs to alter each one to use the new
prototype for __arch_restart().

All of the do_reset() funtions which were converted appear to either use
a CPU hardware feature or an onboard watchdog unit to perform the system
reset.  As a hardware reset that will probably work even when the CPU is
in a bad state, so no separate __arch_emergency_restart() is required.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Jason Jin <jason.jin@freescale.com>
---
 arch/m68k/cpu/mcf5227x/cpu.c   |    2 +-
 arch/m68k/cpu/mcf523x/cpu.c    |    2 +-
 arch/m68k/cpu/mcf52x2/cpu.c    |   20 +++++++-------------
 arch/m68k/cpu/mcf52x2/cpu.h    |   33 ---------------------------------
 arch/m68k/cpu/mcf532x/cpu.c    |    2 +-
 arch/m68k/cpu/mcf5445x/cpu.c   |    2 +-
 arch/m68k/cpu/mcf547x_8x/cpu.c |    2 +-
 7 files changed, 12 insertions(+), 51 deletions(-)
 delete mode 100644 arch/m68k/cpu/mcf52x2/cpu.h

diff --git a/arch/m68k/cpu/mcf5227x/cpu.c b/arch/m68k/cpu/mcf5227x/cpu.c
index 09ef1d2..07b4595 100644
--- a/arch/m68k/cpu/mcf5227x/cpu.c
+++ b/arch/m68k/cpu/mcf5227x/cpu.c
@@ -33,7 +33,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile rcm_t *rcm = (rcm_t *) (MMAP_RCM);
 	udelay(1000);
diff --git a/arch/m68k/cpu/mcf523x/cpu.c b/arch/m68k/cpu/mcf523x/cpu.c
index 2376f97..62a8556 100644
--- a/arch/m68k/cpu/mcf523x/cpu.c
+++ b/arch/m68k/cpu/mcf523x/cpu.c
@@ -34,7 +34,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile ccm_t *ccm = (ccm_t *) MMAP_CCM;
 
diff --git a/arch/m68k/cpu/mcf52x2/cpu.c b/arch/m68k/cpu/mcf52x2/cpu.c
index 571d078..3427010 100644
--- a/arch/m68k/cpu/mcf52x2/cpu.c
+++ b/arch/m68k/cpu/mcf52x2/cpu.c
@@ -33,12 +33,11 @@
 #include <command.h>
 #include <asm/immap.h>
 #include <netdev.h>
-#include "cpu.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
 #ifdef	CONFIG_M5208
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile rcm_t *rcm = (rcm_t *)(MMAP_RCM);
 
@@ -141,13 +140,8 @@ int checkcpu(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
-	/* Call the board specific reset actions first. */
-	if(board_reset) {
-		board_reset();
-	}
-
 	mbar_writeByte(MCF_RCM_RCR,
 		       MCF_RCM_RCR_SOFTRST | MCF_RCM_RCR_FRCRSTOUT);
 	return 0;
@@ -176,7 +170,7 @@ int watchdog_init(void)
 #endif
 
 #ifdef	CONFIG_M5272
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile wdog_t *wdp = (wdog_t *) (MMAP_WDOG);
 
@@ -256,7 +250,7 @@ int watchdog_init(void)
 #endif				/* #ifdef CONFIG_M5272 */
 
 #ifdef	CONFIG_M5275
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile rcm_t *rcm = (rcm_t *)(MMAP_RCM);
 
@@ -336,7 +330,7 @@ int checkcpu(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	MCFRESET_RCR = MCFRESET_RCR_SOFTRST;
 	return 0;
@@ -353,7 +347,7 @@ int checkcpu(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	/* enable watchdog, set timeout to 0 and wait */
 	mbar_writeByte(MCFSIM_SYPCR, 0xc0);
@@ -383,7 +377,7 @@ int checkcpu(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	/* enable watchdog, set timeout to 0 and wait */
 	mbar_writeByte(SIM_SYPCR, 0xc0);
diff --git a/arch/m68k/cpu/mcf52x2/cpu.h b/arch/m68k/cpu/mcf52x2/cpu.h
deleted file mode 100644
index c1227eb..0000000
--- a/arch/m68k/cpu/mcf52x2/cpu.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- *  cpu.h
- *
- *  Copyright (c) 2009 Freescale Semiconductor, Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
- * MA 02110-1301 USA
- */
-
-#ifndef _CPU_H_
-#define _CPU_H_
-
-#include <command.h>
-
-/* Use this to create board specific reset functions */
-void board_reset(void) __attribute__((__weak__));
-
-#endif /* _CPU_H_ */
diff --git a/arch/m68k/cpu/mcf532x/cpu.c b/arch/m68k/cpu/mcf532x/cpu.c
index 3346784..8dba73e 100644
--- a/arch/m68k/cpu/mcf532x/cpu.c
+++ b/arch/m68k/cpu/mcf532x/cpu.c
@@ -34,7 +34,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile rcm_t *rcm = (rcm_t *) (MMAP_RCM);
 
diff --git a/arch/m68k/cpu/mcf5445x/cpu.c b/arch/m68k/cpu/mcf5445x/cpu.c
index 323a54e..7c81775 100644
--- a/arch/m68k/cpu/mcf5445x/cpu.c
+++ b/arch/m68k/cpu/mcf5445x/cpu.c
@@ -34,7 +34,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile rcm_t *rcm = (rcm_t *) (MMAP_RCM);
 	udelay(1000);
diff --git a/arch/m68k/cpu/mcf547x_8x/cpu.c b/arch/m68k/cpu/mcf547x_8x/cpu.c
index 7590f2c..3e94037 100644
--- a/arch/m68k/cpu/mcf547x_8x/cpu.c
+++ b/arch/m68k/cpu/mcf547x_8x/cpu.c
@@ -34,7 +34,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_reset(void)
 {
 	volatile gptmr_t *gptmr = (gptmr_t *) (MMAP_GPTMR);
 
-- 
1.7.2.3

[U-Boot] [PATCH 13/21] microblaze: Generic system restart support

[Kyle Moffett]

The MicroBlaze port only seems to have a single "microblaze-generic"
board right now with completely undocumented "restart" functionality.

Since the only do_reset() function in the port is in the board-specific
directory, it is renamed __board_restart().

It is unclear whether or not the code properly supports emergency
restart.  A new __board_emergency_restart() may need to be implemented.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Michal Simek <monstr@monstr.eu>
---
 .../xilinx/microblaze-generic/microblaze-generic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c b/board/xilinx/microblaze-generic/microblaze-generic.c
index 183e4dc..d7f0eaa 100644
--- a/board/xilinx/microblaze-generic/microblaze-generic.c
+++ b/board/xilinx/microblaze-generic/microblaze-generic.c
@@ -31,7 +31,7 @@
 #include <asm/microblaze_intc.h>
 #include <asm/asm.h>
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
 #ifdef CONFIG_SYS_GPIO_0
 	*((unsigned long *)(CONFIG_SYS_GPIO_0_ADDR)) =
-- 
1.7.2.3

[U-Boot] [PATCH 14/21] mips: Generic system restart support

[Kyle Moffett]

The MIPS port appears to use no generic hardware capability for
performing a CPU reset, therefore the do_reset() function can completely
go away.

Existing _machine_restart() functions are renamed __board_restart() to
allow the generic code to find them.

Two of the board ports just directly jump back to their FLASH reset
vector, therefore they have no-op __board_emergency_restart() functions.
(If the CPU is in an invalid state then that probably won't work).

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Shinya Kuribayashi <skuribay@pobox.com>
---
 arch/mips/cpu/cpu.c            |   13 -------------
 arch/mips/include/asm/reboot.h |   14 --------------
 board/incaip/incaip.c          |    4 ++--
 board/micronas/vct/vct.c       |    2 +-
 board/purple/purple.c          |   16 +++++++++++++---
 board/tb0229/tb0229.c          |   16 +++++++++++++---
 6 files changed, 29 insertions(+), 36 deletions(-)
 delete mode 100644 arch/mips/include/asm/reboot.h

diff --git a/arch/mips/cpu/cpu.c b/arch/mips/cpu/cpu.c
index 3ae397c..37ed1f1 100644
--- a/arch/mips/cpu/cpu.c
+++ b/arch/mips/cpu/cpu.c
@@ -26,7 +26,6 @@
 #include <netdev.h>
 #include <asm/mipsregs.h>
 #include <asm/cacheops.h>
-#include <asm/reboot.h>
 
 #define cache_op(op,addr)						\
 	__asm__ __volatile__(						\
@@ -38,18 +37,6 @@
 	:								\
 	: "i" (op), "R" (*(unsigned char *)(addr)))
 
-void __attribute__((weak)) _machine_restart(void)
-{
-}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	_machine_restart();
-
-	fprintf(stderr, "*** reset failed ***\n");
-	return 0;
-}
-
 void flush_cache(ulong start_addr, ulong size)
 {
 	unsigned long lsize = CONFIG_SYS_CACHELINE_SIZE;
diff --git a/arch/mips/include/asm/reboot.h b/arch/mips/include/asm/reboot.h
deleted file mode 100644
index 978d206..0000000
--- a/arch/mips/include/asm/reboot.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- *
- * Copyright (C) 1997, 1999, 2001, 06 by Ralf Baechle
- * Copyright (C) 2001 MIPS Technologies, Inc.
- */
-#ifndef _ASM_REBOOT_H
-#define _ASM_REBOOT_H
-
-extern void _machine_restart(void);
-
-#endif /* _ASM_REBOOT_H */
diff --git a/board/incaip/incaip.c b/board/incaip/incaip.c
index 3b30970..7eaf10e 100644
--- a/board/incaip/incaip.c
+++ b/board/incaip/incaip.c
@@ -27,13 +27,13 @@
 #include <asm/addrspace.h>
 #include <asm/inca-ip.h>
 #include <asm/io.h>
-#include <asm/reboot.h>
 
 extern uint incaip_get_cpuclk(void);
 
-void _machine_restart(void)
+int __board_restart(void)
 {
 	*INCA_IP_WDT_RST_REQ = 0x3f;
+	return -1;
 }
 
 static ulong max_sdram_size(void)
diff --git a/board/micronas/vct/vct.c b/board/micronas/vct/vct.c
index 7fc3507..06b7042 100644
--- a/board/micronas/vct/vct.c
+++ b/board/micronas/vct/vct.c
@@ -56,7 +56,7 @@ int board_early_init_f(void)
 	return 0;
 }
 
-void _machine_restart(void)
+void __board_restart(void)
 {
 	reg_write(DCGU_EN_WDT_RESET(DCGU_BASE), DCGU_MAGIC_WDT);
 	reg_write(WDT_TORR(WDT_BASE), 0x00);
diff --git a/board/purple/purple.c b/board/purple/purple.c
index 4e9e700..2ce9715 100644
--- a/board/purple/purple.c
+++ b/board/purple/purple.c
@@ -30,7 +30,6 @@
 #include <asm/io.h>
 #include <asm/addrspace.h>
 #include <asm/cacheops.h>
-#include <asm/reboot.h>
 
 #include "sconsole.h"
 
@@ -54,11 +53,22 @@ extern int	asc_serial_getc		(void);
 extern int	asc_serial_tstc		(void);
 extern void	asc_serial_setbrg	(void);
 
-void _machine_restart(void)
+int __board_restart(void)
 {
+	/* Jump to software-reset vector */
 	void (*f)(void) = (void *) 0xbfc00000;
-
 	f();
+	return 0;
+}
+
+/*
+ * The __board_restart() just jumps back to flash, which isn't safe to do in
+ * emergency conditions.  Since we don't have anything better to do, just
+ * fall through into the default hang().
+ */
+void __board_emergency_restart(void)
+{
+	return;
 }
 
 static void sdram_timing_init (ulong size)
diff --git a/board/tb0229/tb0229.c b/board/tb0229/tb0229.c
index d3f05b2..4684574 100644
--- a/board/tb0229/tb0229.c
+++ b/board/tb0229/tb0229.c
@@ -13,14 +13,24 @@
 #include <command.h>
 #include <asm/addrspace.h>
 #include <asm/io.h>
-#include <asm/reboot.h>
 #include <pci.h>
 
-void _machine_restart(void)
+int __board_restart(void)
 {
+	/* Jump to software-reset vector */
 	void (*f)(void) = (void *) 0xbfc00000;
-
 	f();
+	return 0;
+}
+
+/*
+ * The __board_restart() just jumps back to flash, which isn't safe to do in
+ * emergency conditions.  Since we don't have anything better to do, just
+ * fall through into the default hang().
+ */
+void __board_emergency_restart(void)
+{
+	return;
 }
 
 #if defined(CONFIG_PCI)
-- 
1.7.2.3

[U-Boot] [PATCH 15/21] nios2: Generic system restart support

[Kyle Moffett]

The Nios-II port appears to use no generic hardware capability for
performing a CPU reset.  Since all of the supported boards use the exact
same code to perform a jump-to-flash it goes into __arch_restart().

This means that Nios-II has a no-op __arch_emergency_restart() function.
If the CPU is in an invalid state then jump-to-FLASH probably won't
work.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Scott McNutt <smcnutt@psyent.com>
---
 arch/nios2/cpu/cpu.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c
index ef360ee..9f40188 100644
--- a/arch/nios2/cpu/cpu.c
+++ b/arch/nios2/cpu/cpu.c
@@ -40,10 +40,20 @@ int checkcpu (void)
 	return (0);
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	disable_interrupts();
 	/* indirect call to go beyond 256MB limitation of toolchain */
 	nios2_callr(CONFIG_SYS_RESET_ADDR);
 	return 0;
 }
+
+/*
+ * The __arch_restart() just jumps back to flash, which isn't safe to do in
+ * emergency conditions.  Since we don't have anything better to do, just
+ * fall through into the default hang().
+ */
+void __arch_emergency_restart(void)
+{
+	return;
+}
-- 
1.7.2.3

[U-Boot] [PATCH 16/21] powerpc: Generic system restart support

[Kyle Moffett]

The PowerPC processors supported by U-Boot have many different varieties
of hardware reset capabilites.

Some, such as the 7xx/74xx, require a board-specific hardware-reset
signal to be supplied to the CPU.

Others use CPU-specific restart registers or faulting instructions.

Several of the CPU-specific do_reset() functions called a custom helper
function "board_reset()" to allow board-specific override.

All of the do_reset() functions in arch/powerpc/ were changed to the new
prototype __arch_restart() and all PowerPC board/* changed to use the
function prototype __board_restart().

The "lwmon" board seems to keep its board-specific code littered in
ifdef blocks in the middle of common code, and its restart function was
no exception.  That do_reset() function was moved to board/lwmon/lwmon.c
where it belonged and changed to use the prototype __board_restart().

A few do_reset() functions in arch/powerpc/ used to be protected by
ifdefs due to conflicting board-specific definitions.  As they no longer
conflict, the excess #ifdefs are removed.

The ppmc7xx board does not appear to have any usable hardware reset
functionality, so it has a no-op __arch_emergency_restart() function.
If the CPU is in an invalid state then jump-to-RAM probably won't work.

All of the rest of the reset code will probably work even when the CPU
is in a bad state, so they should be fine with the defaults.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Stefan Roese <sr@denx.de>
Cc: Kim Phillips <kim.phillips@freescale.com>
Cc: Andy Fleming <afleming@gmail.com>
Cc: Kumar Gala <kumar.gala@freescale.com>
---
 arch/powerpc/cpu/74xx_7xx/cpu.c           |    7 +-----
 arch/powerpc/cpu/mpc512x/cpu.c            |    3 +-
 arch/powerpc/cpu/mpc5xx/cpu.c             |    2 +-
 arch/powerpc/cpu/mpc5xxx/cpu.c            |    3 +-
 arch/powerpc/cpu/mpc8220/cpu.c            |    2 +-
 arch/powerpc/cpu/mpc824x/cpu.c            |    2 +-
 arch/powerpc/cpu/mpc8260/cpu.c            |    6 +----
 arch/powerpc/cpu/mpc83xx/cpu.c            |    5 +---
 arch/powerpc/cpu/mpc85xx/cpu.c            |   17 +++++++++++----
 arch/powerpc/cpu/mpc86xx/cpu.c            |   16 +--------------
 arch/powerpc/cpu/mpc8xx/cpu.c             |   30 +----------------------------
 arch/powerpc/cpu/ppc4xx/cpu.c             |    8 +------
 board/amcc/yosemite/yosemite.c            |    3 +-
 board/eltec/bab7xx/bab7xx.c               |    4 +-
 board/eltec/elppc/elppc.c                 |    2 +-
 board/freescale/mpc8610hpcd/mpc8610hpcd.c |    3 +-
 board/freescale/mpc8641hpcn/mpc8641hpcn.c |    3 +-
 board/funkwerk/vovpn-gw/vovpn-gw.c        |    5 +---
 board/lwmon/lwmon.c                       |   22 +++++++++++++++++++++
 board/lwmon5/lwmon5.c                     |    3 +-
 board/pcippc2/pcippc2.c                   |    2 +-
 board/ppmc7xx/ppmc7xx.c                   |   18 ++++++++++++----
 board/sbc8641d/sbc8641d.c                 |    3 +-
 include/configs/VoVPN-GW.h                |    3 --
 include/configs/lwmon5.h                  |    1 -
 include/configs/yosemite.h                |    1 -
 26 files changed, 73 insertions(+), 101 deletions(-)

diff --git a/arch/powerpc/cpu/74xx_7xx/cpu.c b/arch/powerpc/cpu/74xx_7xx/cpu.c
index b6a31b4..24b531a 100644
--- a/arch/powerpc/cpu/74xx_7xx/cpu.c
+++ b/arch/powerpc/cpu/74xx_7xx/cpu.c
@@ -229,12 +229,8 @@ soft_restart(unsigned long addr)
 }
 
 
-#if !defined(CONFIG_PCIPPC2) && \
-    !defined(CONFIG_BAB7xx)  && \
-    !defined(CONFIG_ELPPC)   && \
-    !defined(CONFIG_PPMC7XX)
 /* no generic way to do board reset. simply call soft_reset. */
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong addr;
 	/* flush and disable I/D cache */
@@ -269,7 +265,6 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	return 1;
 }
-#endif
 
 /* ------------------------------------------------------------------------- */
 
diff --git a/arch/powerpc/cpu/mpc512x/cpu.c b/arch/powerpc/cpu/mpc512x/cpu.c
index a1a3bd4..c2dab2c 100644
--- a/arch/powerpc/cpu/mpc512x/cpu.c
+++ b/arch/powerpc/cpu/mpc512x/cpu.c
@@ -74,8 +74,7 @@ int checkcpu (void)
 }
 
 
-int
-do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr;
 	volatile immap_t *immap = (immap_t *) CONFIG_SYS_IMMR;
diff --git a/arch/powerpc/cpu/mpc5xx/cpu.c b/arch/powerpc/cpu/mpc5xx/cpu.c
index 5aa7f84..74193dc 100644
--- a/arch/powerpc/cpu/mpc5xx/cpu.c
+++ b/arch/powerpc/cpu/mpc5xx/cpu.c
@@ -138,7 +138,7 @@ int dcache_status (void)
 /*
  * Reset board
  */
-int do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 #if defined(CONFIG_PATI)
 	volatile ulong *addr = (ulong *) CONFIG_SYS_RESET_ADDRESS;
diff --git a/arch/powerpc/cpu/mpc5xxx/cpu.c b/arch/powerpc/cpu/mpc5xxx/cpu.c
index 0c1eebd..ed3b267 100644
--- a/arch/powerpc/cpu/mpc5xxx/cpu.c
+++ b/arch/powerpc/cpu/mpc5xxx/cpu.c
@@ -77,8 +77,7 @@ int checkcpu (void)
 
 /* ------------------------------------------------------------------------- */
 
-int
-do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr;
 	/* Interrupts and MMU off */
diff --git a/arch/powerpc/cpu/mpc8220/cpu.c b/arch/powerpc/cpu/mpc8220/cpu.c
index 64e0526..fa9d076 100644
--- a/arch/powerpc/cpu/mpc8220/cpu.c
+++ b/arch/powerpc/cpu/mpc8220/cpu.c
@@ -52,7 +52,7 @@ int checkcpu (void)
 
 /* ------------------------------------------------------------------------- */
 
-int do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	volatile gptmr8220_t *gptmr = (volatile gptmr8220_t *) MMAP_GPTMR;
 	ulong msr;
diff --git a/arch/powerpc/cpu/mpc824x/cpu.c b/arch/powerpc/cpu/mpc824x/cpu.c
index 44f91b2..c3188c9 100644
--- a/arch/powerpc/cpu/mpc824x/cpu.c
+++ b/arch/powerpc/cpu/mpc824x/cpu.c
@@ -92,7 +92,7 @@ int checkdcache (void)
 
 /*------------------------------------------------------------------- */
 
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr, addr;
 
diff --git a/arch/powerpc/cpu/mpc8260/cpu.c b/arch/powerpc/cpu/mpc8260/cpu.c
index 220c1e2..2c07147 100644
--- a/arch/powerpc/cpu/mpc8260/cpu.c
+++ b/arch/powerpc/cpu/mpc8260/cpu.c
@@ -236,9 +236,7 @@ void upmconfig (uint upm, uint * table, uint size)
 
 /* ------------------------------------------------------------------------- */
 
-#if !defined(CONFIG_HAVE_OWN_RESET)
-int
-do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr, addr;
 
@@ -268,9 +266,7 @@ do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 #endif
 	((void (*)(void)) addr) ();
 	return 1;
-
 }
-#endif	/* CONFIG_HAVE_OWN_RESET */
 
 /* ------------------------------------------------------------------------- */
 
diff --git a/arch/powerpc/cpu/mpc83xx/cpu.c b/arch/powerpc/cpu/mpc83xx/cpu.c
index 6635109..03f5154 100644
--- a/arch/powerpc/cpu/mpc83xx/cpu.c
+++ b/arch/powerpc/cpu/mpc83xx/cpu.c
@@ -126,8 +126,7 @@ int checkcpu(void)
 	return 0;
 }
 
-int
-do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr;
 #ifndef MPC83xx_RESET
@@ -136,8 +135,6 @@ do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 
 	volatile immap_t *immap = (immap_t *) CONFIG_SYS_IMMR;
 
-	puts("Resetting the board.\n");
-
 #ifdef MPC83xx_RESET
 
 	/* Interrupts and MMU off */
diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c
index 1aad2ba..c1690fa 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu.c
@@ -202,11 +202,11 @@ int checkcpu (void)
 
 /* ------------------------------------------------------------------------- */
 
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-/* Everything after the first generation of PQ3 parts has RSTCR */
 #if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
     defined(CONFIG_MPC8555) || defined(CONFIG_MPC8560)
+
+int __arch_restart(void)
+{
 	unsigned long val, msr;
 
 	/*
@@ -220,15 +220,22 @@ int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	val = mfspr(DBCR0);
 	val |= 0x70000000;
 	mtspr(DBCR0,val);
+	return 1;
+}
+
 #else
+
+/* Everything after the first generation of PQ3 parts has RSTCR */
+int __arch_restart(void)
+{
 	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
 	out_be32(&gur->rstcr, 0x2);	/* HRESET_REQ */
 	udelay(100);
-#endif
-
 	return 1;
 }
 
+#endif
+
 
 /*
  * Get timebase clock frequency
diff --git a/arch/powerpc/cpu/mpc86xx/cpu.c b/arch/powerpc/cpu/mpc86xx/cpu.c
index ffcc8e6..5633a3a 100644
--- a/arch/powerpc/cpu/mpc86xx/cpu.c
+++ b/arch/powerpc/cpu/mpc86xx/cpu.c
@@ -32,17 +32,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/*
- * Default board reset function
- */
-static void
-__board_reset(void)
-{
-	/* Do nothing */
-}
-void board_reset(void) __attribute__((weak, alias("__board_reset")));
-
-
 int
 checkcpu(void)
 {
@@ -123,14 +112,11 @@ checkcpu(void)
 }
 
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR;
 	volatile ccsr_gur_t *gur = &immap->im_gur;
 
-	/* Attempt board-specific reset */
-	board_reset();
-
 	/* Next try asserting HRESET_REQ */
 	out_be32(&gur->rstcr, MPC86xx_RSTCR_HRST_REQ);
 
diff --git a/arch/powerpc/cpu/mpc8xx/cpu.c b/arch/powerpc/cpu/mpc8xx/cpu.c
index 142cfa5..e73b33e 100644
--- a/arch/powerpc/cpu/mpc8xx/cpu.c
+++ b/arch/powerpc/cpu/mpc8xx/cpu.c
@@ -476,9 +476,7 @@ void upmconfig (uint upm, uint * table, uint size)
 
 /* ------------------------------------------------------------------------- */
 
-#ifndef CONFIG_LWMON
-
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	ulong msr, addr;
 
@@ -512,32 +510,6 @@ int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 1;
 }
 
-#else	/* CONFIG_LWMON */
-
-/*
- * On the LWMON board, the MCLR reset input of the PIC's on the board
- * uses a 47K/1n RC combination which has a 47us time  constant.  The
- * low  signal on the HRESET pin of the CPU is only 512 clocks = 8 us
- * and thus too short to reset the external hardware. So we  use  the
- * watchdog to reset the board.
- */
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	/* prevent triggering the watchdog */
-	disable_interrupts ();
-
-	/* make sure the watchdog is running */
-	reset_8xx_watchdog ((immap_t *) CONFIG_SYS_IMMR);
-
-	/* wait for watchdog reset */
-	while (1) {};
-
-	/* NOTREACHED */
-	return 1;
-}
-
-#endif	/* CONFIG_LWMON */
-
 /* ------------------------------------------------------------------------- */
 
 /*
diff --git a/arch/powerpc/cpu/ppc4xx/cpu.c b/arch/powerpc/cpu/ppc4xx/cpu.c
index 67f1fff..add4518 100644
--- a/arch/powerpc/cpu/ppc4xx/cpu.c
+++ b/arch/powerpc/cpu/ppc4xx/cpu.c
@@ -40,8 +40,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-void board_reset(void);
-
 /*
  * To provide an interface to detect CPU number for boards that support
  * more then one CPU, we implement the "weak" default functions here.
@@ -699,11 +697,8 @@ int ppc440spe_revB() {
 
 /* ------------------------------------------------------------------------- */
 
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
-#if defined(CONFIG_BOARD_RESET)
-	board_reset();
-#else
 #if defined(CONFIG_SYS_4xx_RESET_TYPE)
 	mtspr(SPRN_DBCR0, CONFIG_SYS_4xx_RESET_TYPE << 28);
 #else
@@ -712,7 +707,6 @@ int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 */
 	mtspr(SPRN_DBCR0, 0x30000000);
 #endif /* defined(CONFIG_SYS_4xx_RESET_TYPE) */
-#endif /* defined(CONFIG_BOARD_RESET) */
 
 	return 1;
 }
diff --git a/board/amcc/yosemite/yosemite.c b/board/amcc/yosemite/yosemite.c
index aaeab6f..74f675a 100644
--- a/board/amcc/yosemite/yosemite.c
+++ b/board/amcc/yosemite/yosemite.c
@@ -365,8 +365,9 @@ void hw_watchdog_reset(void)
 }
 #endif
 
-void board_reset(void)
+int __board_restart(void)
 {
 	/* give reset to BCSR */
 	*(unsigned char *)(CONFIG_SYS_BCSR_BASE | 0x06) = 0x09;
+	return -1;
 }
diff --git a/board/eltec/bab7xx/bab7xx.c b/board/eltec/bab7xx/bab7xx.c
index ea4897b..5cd1525 100644
--- a/board/eltec/bab7xx/bab7xx.c
+++ b/board/eltec/bab7xx/bab7xx.c
@@ -181,10 +181,10 @@ void after_reloc (ulong dest_addr)
 /* ------------------------------------------------------------------------- */
 
 /*
- * do_reset is done here because in this case it is board specific, since the
+ * reset is done here because in this case it is board specific, since the
  * 7xx CPUs can only be reset by external HW (the RTC in this case).
  */
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
 #if defined(CONFIG_RTC_MK48T59)
 	/* trigger watchdog immediately */
diff --git a/board/eltec/elppc/elppc.c b/board/eltec/elppc/elppc.c
index cb9ab86..e10a984 100644
--- a/board/eltec/elppc/elppc.c
+++ b/board/eltec/elppc/elppc.c
@@ -117,7 +117,7 @@ phys_size_t initdram (int board_type)
  * Register PI in the MPC 107 (at offset 0x41090 of the Embedded Utilities
  * Memory Block).
  */
-int do_reset (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
 	out8 (MPC107_EUMB_PI, 1);
 	return (0);
diff --git a/board/freescale/mpc8610hpcd/mpc8610hpcd.c b/board/freescale/mpc8610hpcd/mpc8610hpcd.c
index d7dd470..ee31588 100644
--- a/board/freescale/mpc8610hpcd/mpc8610hpcd.c
+++ b/board/freescale/mpc8610hpcd/mpc8610hpcd.c
@@ -314,7 +314,7 @@ int board_eth_init(bd_t *bis)
 	return pci_eth_init(bis);
 }
 
-void board_reset(void)
+int __board_restart(void)
 {
 	u8 *pixis_base = (u8 *)PIXIS_BASE;
 
@@ -322,4 +322,5 @@ void board_reset(void)
 
 	while (1)
 		;
+	return -1;
 }
diff --git a/board/freescale/mpc8641hpcn/mpc8641hpcn.c b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
index 166ff0c..60b52cf 100644
--- a/board/freescale/mpc8641hpcn/mpc8641hpcn.c
+++ b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
@@ -252,7 +252,7 @@ int board_eth_init(bd_t *bis)
 	return pci_eth_init(bis);
 }
 
-void board_reset(void)
+int __board_restart(void)
 {
 	u8 *pixis_base = (u8 *)PIXIS_BASE;
 
@@ -260,6 +260,7 @@ void board_reset(void)
 
 	while (1)
 		;
+	return -1;
 }
 
 #ifdef CONFIG_MP
diff --git a/board/funkwerk/vovpn-gw/vovpn-gw.c b/board/funkwerk/vovpn-gw/vovpn-gw.c
index a4bfbc9..14d495b 100644
--- a/board/funkwerk/vovpn-gw/vovpn-gw.c
+++ b/board/funkwerk/vovpn-gw/vovpn-gw.c
@@ -304,9 +304,7 @@ int misc_init_r (void)
 	return 0;
 }
 
-#if defined(CONFIG_HAVE_OWN_RESET)
-int
-do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
 	volatile ioport_t *iop;
 
@@ -314,7 +312,6 @@ do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	iop->pdat |= 0x00002000;	/* PC18 = HW_RESET */
 	return 1;
 }
-#endif	/* CONFIG_HAVE_OWN_RESET */
 
 #define ns2clk(ns) (ns / (1000000000 / CONFIG_8260_CLKIN) + 1)
 
diff --git a/board/lwmon/lwmon.c b/board/lwmon/lwmon.c
index 9d6c21f..4c99b68 100644
--- a/board/lwmon/lwmon.c
+++ b/board/lwmon/lwmon.c
@@ -1052,6 +1052,28 @@ void board_poweroff (void)
     while (1);
 }
 
+/*
+ * On the LWMON board, the MCLR reset input of the PIC's on the board
+ * uses a 47K/1n RC combination which has a 47us time  constant.  The
+ * low  signal on the HRESET pin of the CPU is only 512 clocks = 8 us
+ * and thus too short to reset the external hardware. So we  use  the
+ * watchdog to reset the board.
+ */
+int __board_restart(void)
+{
+	/* prevent triggering the watchdog */
+	disable_interrupts ();
+
+	/* make sure the watchdog is running */
+	reset_8xx_watchdog ((immap_t *) CONFIG_SYS_IMMR);
+
+	/* wait for watchdog reset */
+	while (1) {};
+
+	/* NOTREACHED */
+	return 1;
+}
+
 #ifdef CONFIG_MODEM_SUPPORT
 static int key_pressed(void)
 {
diff --git a/board/lwmon5/lwmon5.c b/board/lwmon5/lwmon5.c
index dd275bf..f659fd6 100644
--- a/board/lwmon5/lwmon5.c
+++ b/board/lwmon5/lwmon5.c
@@ -490,7 +490,8 @@ void video_get_info_str(int line_number, char *info)
 #endif /* CONFIG_CONSOLE_EXTRA_INFO */
 #endif /* CONFIG_VIDEO */
 
-void board_reset(void)
+int __board_restart(void)
 {
 	gpio_write_bit(CONFIG_SYS_GPIO_BOARD_RESET, 1);
+	return -1;
 }
diff --git a/board/pcippc2/pcippc2.c b/board/pcippc2/pcippc2.c
index 4a91458..568b6f1 100644
--- a/board/pcippc2/pcippc2.c
+++ b/board/pcippc2/pcippc2.c
@@ -69,7 +69,7 @@ phys_size_t initdram (int board_type)
 	return cpc710_ram_init ();
 }
 
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
 	out32 (REG (CPC0, SPOR), 0);
 	iobarrier_rw ();
diff --git a/board/ppmc7xx/ppmc7xx.c b/board/ppmc7xx/ppmc7xx.c
index 432d366..709fa33 100644
--- a/board/ppmc7xx/ppmc7xx.c
+++ b/board/ppmc7xx/ppmc7xx.c
@@ -84,14 +84,12 @@ int misc_init_r( void )
 
 
 /*
- * do_reset()
+ * __board_restart()
  *
- * Shell command to reset the board.
+ * Called when the board needs to be rebooted
  */
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __board_restart(void)
 {
-	printf( "Resetting...\n" );
-
 	/* Disabe and invalidate cache */
 	icache_disable();
 	dcache_disable();
@@ -106,6 +104,16 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 1;
 }
 
+/*
+ * The __board_restart() just jumps back to RAM, which isn't safe to do in
+ * emergency conditions.  Since we don't have anything better to do, just
+ * fall through into the default hang().
+ */
+void __board_emergency_restart(void)
+{
+	return;
+}
+
 int board_eth_init(bd_t *bis)
 {
 	return pci_eth_init(bis);
diff --git a/board/sbc8641d/sbc8641d.c b/board/sbc8641d/sbc8641d.c
index 5c30b26..4d8f17c 100644
--- a/board/sbc8641d/sbc8641d.c
+++ b/board/sbc8641d/sbc8641d.c
@@ -245,7 +245,7 @@ unsigned long get_board_sys_clk (ulong dummy)
 	return val;
 }
 
-void board_reset(void)
+int __board_restart(void)
 {
 #ifdef CONFIG_SYS_RESET_ADDRESS
 	ulong addr = CONFIG_SYS_RESET_ADDRESS;
@@ -272,6 +272,7 @@ void board_reset(void)
 	__asm__ __volatile__ ("mtspr	27, 4");
 	__asm__ __volatile__ ("rfi");
 #endif
+	return -1;
 }
 
 #ifdef CONFIG_MP
diff --git a/include/configs/VoVPN-GW.h b/include/configs/VoVPN-GW.h
index c06909f..796c9a0 100644
--- a/include/configs/VoVPN-GW.h
+++ b/include/configs/VoVPN-GW.h
@@ -47,9 +47,6 @@
 /* have reset_phy_r() function */
 #define CONFIG_RESET_PHY_R		1
 
-/* have special reset function */
-#define	CONFIG_HAVE_OWN_RESET		1
-
 /* allow serial and ethaddr to be overwritten */
 #define	CONFIG_ENV_OVERWRITE
 
diff --git a/include/configs/lwmon5.h b/include/configs/lwmon5.h
index a1ead70..a8c6348 100644
--- a/include/configs/lwmon5.h
+++ b/include/configs/lwmon5.h
@@ -49,7 +49,6 @@
 #define CONFIG_BOARD_EARLY_INIT_R	/* Call board_early_init_r	*/
 #define CONFIG_BOARD_POSTCLK_INIT	/* Call board_postclk_init	*/
 #define CONFIG_MISC_INIT_R		/* Call misc_init_r		*/
-#define CONFIG_BOARD_RESET		/* Call board_reset		*/
 
 /*
  * Base addresses -- Note these are effective addresses where the
diff --git a/include/configs/yosemite.h b/include/configs/yosemite.h
index 0cbef6f..1657d91 100644
--- a/include/configs/yosemite.h
+++ b/include/configs/yosemite.h
@@ -51,7 +51,6 @@
 
 #define CONFIG_BOARD_EARLY_INIT_F 1     /* Call board_early_init_f	*/
 #define CONFIG_MISC_INIT_R	1	/* call misc_init_r()		*/
-#define CONFIG_BOARD_RESET	1	/* call board_reset()		*/
 
 /*-----------------------------------------------------------------------
  * Base addresses -- Note these are effective addresses where the
-- 
1.7.2.3

[U-Boot] [PATCH 17/21] sh: Unify duplicate reset code

[Kyle Moffett]

All 3 SH processor variants have the same do_reset() and reset_cpu()
function implementations.  Furthermore, the only caller of the
reset_cpu() function on SH is do_reset().

To simplify and prepare for SH generic-restart support this patch
removes 38 lines of code by merging the functions together and moving
them into common code.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
---
 arch/sh/cpu/sh2/cpu.c      |    7 -------
 arch/sh/cpu/sh2/watchdog.c |    9 ---------
 arch/sh/cpu/sh3/cpu.c      |    7 -------
 arch/sh/cpu/sh3/watchdog.c |    9 ---------
 arch/sh/cpu/sh4/cpu.c      |    7 -------
 arch/sh/cpu/sh4/watchdog.c |    9 ---------
 arch/sh/lib/board.c        |   10 ++++++++++
 7 files changed, 10 insertions(+), 48 deletions(-)

diff --git a/arch/sh/cpu/sh2/cpu.c b/arch/sh/cpu/sh2/cpu.c
index 6bbedd9..a88fcad 100644
--- a/arch/sh/cpu/sh2/cpu.c
+++ b/arch/sh/cpu/sh2/cpu.c
@@ -59,13 +59,6 @@ int cleanup_before_linux(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	disable_interrupts();
-	reset_cpu(0);
-	return 0;
-}
-
 void flush_cache(unsigned long addr, unsigned long size)
 {
 
diff --git a/arch/sh/cpu/sh2/watchdog.c b/arch/sh/cpu/sh2/watchdog.c
index 0257d8d..df7f263 100644
--- a/arch/sh/cpu/sh2/watchdog.c
+++ b/arch/sh/cpu/sh2/watchdog.c
@@ -26,12 +26,3 @@ int watchdog_init(void)
 {
 	return 0;
 }
-
-void reset_cpu(unsigned long ignored)
-{
-	/* Address error with SR.BL=1 first. */
-	trigger_address_error();
-
-	while (1)
-		;
-}
diff --git a/arch/sh/cpu/sh3/cpu.c b/arch/sh/cpu/sh3/cpu.c
index 3e9caad..824daab 100644
--- a/arch/sh/cpu/sh3/cpu.c
+++ b/arch/sh/cpu/sh3/cpu.c
@@ -45,13 +45,6 @@ int cleanup_before_linux(void)
 	return 0;
 }
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	disable_interrupts();
-	reset_cpu(0);
-	return 0;
-}
-
 void flush_cache(unsigned long addr, unsigned long size)
 {
 
diff --git a/arch/sh/cpu/sh3/watchdog.c b/arch/sh/cpu/sh3/watchdog.c
index 90694f8..1dff27a 100644
--- a/arch/sh/cpu/sh3/watchdog.c
+++ b/arch/sh/cpu/sh3/watchdog.c
@@ -29,12 +29,3 @@ int watchdog_init(void)
 {
 	return 0;
 }
-
-void reset_cpu(unsigned long ignored)
-{
-	/* Address error with SR.BL=1 first. */
-	trigger_address_error();
-
-	while (1)
-		;
-}
diff --git a/arch/sh/cpu/sh4/cpu.c b/arch/sh/cpu/sh4/cpu.c
index f136758..264dafb 100644
--- a/arch/sh/cpu/sh4/cpu.c
+++ b/arch/sh/cpu/sh4/cpu.c
@@ -44,13 +44,6 @@ int cleanup_before_linux (void)
 	return 0;
 }
 
-int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	disable_interrupts();
-	reset_cpu (0);
-	return 0;
-}
-
 void flush_cache (unsigned long addr, unsigned long size)
 {
 	dcache_invalid_range( addr , addr + size );
diff --git a/arch/sh/cpu/sh4/watchdog.c b/arch/sh/cpu/sh4/watchdog.c
index d7e1703..cbc08f8 100644
--- a/arch/sh/cpu/sh4/watchdog.c
+++ b/arch/sh/cpu/sh4/watchdog.c
@@ -64,12 +64,3 @@ int watchdog_disable(void)
 	return 0;
 }
 #endif
-
-void reset_cpu(unsigned long ignored)
-{
-	/* Address error with SR.BL=1 first. */
-	trigger_address_error();
-
-	while (1)
-		;
-}
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c
index 968566c..07361b4 100644
--- a/arch/sh/lib/board.c
+++ b/arch/sh/lib/board.c
@@ -204,6 +204,16 @@ void sh_generic_init(void)
 
 /***********************************************************************/
 
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	disable_interrupts();
+
+	/* Address error with SR.BL=1 first. */
+	trigger_address_error();
+
+	while (1);
+}
+
 void hang(void)
 {
 	puts("Board ERROR\n");
-- 
1.7.2.3

[U-Boot] [PATCH 18/21] sh: Generic system restart support

[Kyle Moffett]

The SuperH port uses the exact same restart code on all supported CPUs,
so that do_reset() function is simply altered to use the new prototype
for __arch_restart().

It will probably work even when the CPU is in a bad state, so no
separate __arch_emergency_restart() function is required.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
---
 arch/sh/lib/board.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c
index 07361b4..9206311 100644
--- a/arch/sh/lib/board.c
+++ b/arch/sh/lib/board.c
@@ -204,7 +204,7 @@ void sh_generic_init(void)
 
 /***********************************************************************/
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	disable_interrupts();
 
-- 
1.7.2.3

[U-Boot] [PATCH 19/21] sparc: Unify duplicate reset code

[Kyle Moffett]

Both Sparc processor variants have the same do_reset() and reset_cpu()
function implementations.  Furthermore, the only caller of the
reset_cpu() function on Sparc is do_reset().

To simplify and prepare for Sparc generic-restart support this patch
removes 26 lines of code by merging the functions together and moving
them into common code.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Daniel Hellstrom <daniel@gaisler.com>
---
 arch/sparc/cpu/leon2/cpu.c |   18 ------------------
 arch/sparc/cpu/leon3/cpu.c |   19 -------------------
 arch/sparc/lib/board.c     |   11 +++++++++++
 3 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/arch/sparc/cpu/leon2/cpu.c b/arch/sparc/cpu/leon2/cpu.c
index 46512c7..7d9ae4a 100644
--- a/arch/sparc/cpu/leon2/cpu.c
+++ b/arch/sparc/cpu/leon2/cpu.c
@@ -40,24 +40,6 @@ int checkcpu(void)
 
 /* ------------------------------------------------------------------------- */
 
-void cpu_reset(void)
-{
-	/* Interrupts off */
-	disable_interrupts();
-
-	/* jump to restart in flash */
-	_reset_reloc();
-}
-
-int do_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
-{
-	cpu_reset();
-
-	return 1;
-}
-
-/* ------------------------------------------------------------------------- */
-
 #ifdef CONFIG_GRETH
 int cpu_eth_init(bd_t *bis)
 {
diff --git a/arch/sparc/cpu/leon3/cpu.c b/arch/sparc/cpu/leon3/cpu.c
index a1646e2..58c97a7 100644
--- a/arch/sparc/cpu/leon3/cpu.c
+++ b/arch/sparc/cpu/leon3/cpu.c
@@ -41,25 +41,6 @@ int checkcpu(void)
 	return 0;
 }
 
-/* ------------------------------------------------------------------------- */
-
-void cpu_reset(void)
-{
-	/* Interrupts off */
-	disable_interrupts();
-
-	/* jump to restart in flash */
-	_reset_reloc();
-}
-
-int do_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
-{
-	cpu_reset();
-
-	return 1;
-
-}
-
 u64 flash_read64(void *addr)
 {
 	return __raw_readq(addr);
diff --git a/arch/sparc/lib/board.c b/arch/sparc/lib/board.c
index 386cd04..128ece7 100644
--- a/arch/sparc/lib/board.c
+++ b/arch/sparc/lib/board.c
@@ -445,4 +445,15 @@ void hang(void)
 	for (;;) ;
 }
 
+int do_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+{
+	/* Interrupts off */
+	disable_interrupts();
+
+	/* jump to restart in flash */
+	_reset_reloc();
+
+	return 1;
+}
+
 /************************************************************************/
-- 
1.7.2.3

[U-Boot] [PATCH 20/21] sparc: Generic system restart support

[Kyle Moffett]

The Sparc port appears to use no generic hardware capability for
performing a CPU reset.  Since all of the supported boards use the exact
same code to perform a jump-to-flash it goes into __arch_restart().

This means that Sparc has a no-op __arch_emergency_restart() function.
If the CPU is in an invalid state then jump-to-FLASH probably won't
work.

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Daniel Hellstrom <daniel@gaisler.com>
---
 arch/sparc/lib/board.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/sparc/lib/board.c b/arch/sparc/lib/board.c
index 128ece7..e250274 100644
--- a/arch/sparc/lib/board.c
+++ b/arch/sparc/lib/board.c
@@ -445,7 +445,7 @@ void hang(void)
 	for (;;) ;
 }
 
-int do_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+int __arch_restart(void)
 {
 	/* Interrupts off */
 	disable_interrupts();
@@ -456,4 +456,14 @@ int do_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	return 1;
 }
 
+/*
+ * The __arch_restart() just jumps back to flash, which isn't safe to do in
+ * emergency conditions.  Since we don't have anything better to do, just
+ * fall through into the default hang().
+ */
+void __arch_emergency_restart(void)
+{
+	return;
+}
+
 /************************************************************************/
-- 
1.7.2.3

[U-Boot] [PATCH 21/21] Remove legacy do_reset() function

[Kyle Moffett]

All of the users of the legacy do_reset() function have been converted
to __arch_restart() or __board_restart() as appropriate, so the
compatibility calls to do_reset() may be removed.

In addition the do_generic_reset() function is renamed to the now-unused
name do_reset().

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
---
 common/cmd_boot.c |   27 +++------------------------
 include/command.h |    1 -
 2 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index c0f26fc..422d20c 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -72,9 +72,6 @@ void emergency_restart(void)
 	__board_emergency_restart();
 	__arch_emergency_restart();
 
-	/* Fallback to the old do_reset() until everything is converted. */
-	do_reset(NULL, 0, 0, NULL);
-
 	printf("EMERGENCY RESTART: All attempts to reboot failed!");
 	hang();
 }
@@ -129,11 +126,6 @@ int system_restart(void)
 
 	/* Now call into the architecture-specific code */
 	err = __arch_restart();
-	if (err)
-		goto failed;
-
-	/* Fallback to the old do_reset() until everything is converted. */
-	err = do_reset(NULL, 0, 0, NULL);
 
 failed:
 	printf("*** SYSTEM RESTART FAILED ***\n");
@@ -157,7 +149,7 @@ int __board_restart(void)
 __attribute__((__weak__))
 int __arch_restart(void)
 {
-	/* Fallthrough to legacy do_reset() code */
+	/* Some architectures have no generic reboot capability */
 	return 0;
 }
 
@@ -166,24 +158,11 @@ int __arch_restart(void)
  *
  * This is what you get when you type "reset" at the command line.
  */
-int do_generic_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	return system_restart();
 }
 
-/*
- * Empty legacy "do_reset" stub.
- *
- * This allows a platform using the new __board_restart() and
- * __arch_restart() hooks to completely omit the old do_reset() function.
- */
-int do_reset_stub(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-	return 0;
-}
-__attribute__((__weak__,__alias__("do_reset_stub")))
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
-
 /* -------------------------------------------------------------------- */
 
 U_BOOT_CMD(
@@ -194,7 +173,7 @@ U_BOOT_CMD(
 );
 
 U_BOOT_CMD(
-	reset, 1, 0,	do_generic_reset,
+	reset, 1, 0,	do_reset,
 	"Perform RESET of the CPU",
 	""
 );
diff --git a/include/command.h b/include/command.h
index ad8c915..d87f45d 100644
--- a/include/command.h
+++ b/include/command.h
@@ -99,7 +99,6 @@ extern int cmd_get_data_size(char* arg, int default_size);
 extern int do_bootd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 extern int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
-extern int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 
 /* Generic system restart functions */
 __attribute__((__noreturn__))
-- 
1.7.2.3

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Mike Frysinger]

On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
> +__attribute__((__noreturn__))
> +void emergency_restart(void)
> +{
> +	__board_emergency_restart();
> +	__arch_emergency_restart();
> +
> +	/* Fallback to the old do_reset() until everything is converted. */
> +	do_reset(NULL, 0, 0, NULL);
> +
> +	printf("EMERGENCY RESTART: All attempts to reboot failed!");

missing a trailing newline, and i'd just call puts() to bypass useless format 
string parsing

> +int system_restart(void)
> +{
> +	int err;
> +
> +	/*
> +	 * Print a nice message and wait a bit to make sure it goes out the
> +	 * console properly.
> +	 */
> +	printf ("Restarting...\n");

no space before the "(", and i'd use puts() here

> +	udelay(50000);

this doesnt sit well with me.  i dont see why this matters ... we dont have 
any delays today, and i dont think we should introduce any.

> +	printf("*** SYSTEM RESTART FAILED ***\n");

puts()

> +__attribute__((__weak__))
> +int __board_restart(void)
> +{
> +	/* Fallthrough to architecture-specific code */
> +	return 0;
> +}
> +
> +__attribute__((__weak__))
> +int __arch_restart(void)
> +{
> +	/* Fallthrough to legacy do_reset() code */
> +	return 0;
> +}

what use is there in having a return value ?  the do_reset() funcs today dont 
return, which means they have no meaningful reset value.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110307/c82df44f/attachment.pgp 

[U-Boot] [PATCH 0/21] Generic cross-architecture system restart support

[Graeme Russ]

Hi Kyle,

On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
> Hello everyone,
>
> This patch series creates a generic set of functions available to generic
> code which can be easily hooked per-architecture or per-board. ?I have
> tried to be very careful that U-Boot builds and runs on all architectures
> for the entire duration of this patchset.
>
> This patchset is designed to produce zero change in behavior *except* for
> one particular scenario: ?Platforms which perform a "restart" with a simple
> jump to the "_start" entrypoint (or similar) will no longer try to do that
> on panic().
>
> The new functions to be called from generic code are:
>
> ?int system_restart(void)
> ?void emergency_restart(void)
>
> The specific platform hooks available are:
>
> ?int __arch_restart(void)
> ?int __board_restart(void)
> ?void __arch_emergency_restart(void)
> ?void __board_emergency_restart(void)
>
> The first few patches set up the generic functions and hooks with a default
> fallback to the existing "do_reset()" function. ?Most of the rest are then
> architecture-specific modifications necessary to convert away from the old
> do_reset() prototype. ?The last patch finally does away with that function.
>
> In my previous discussions with Wolfgang Denk about the __arch_restart()
> hook he requested that it should be mandatory for all architectures to
> implement. ?Unfortunately the MIPS and MicroBlaze architectures have no
> architectural system-reset code at all, and others have only very limited
> "soft-reboot" (IE: Jump-to-"_start").
>
> Specifically, the MIPS do_reset() function used to look like this:
> ?_machine_restart();
> ?printf("*** restart failed ***\n");
>
> When those platforms are fixed then it should be safe to remove the weak
> fallback __arch_restart() function.
>

I recall seeing a patch series similar to this a little while ago. Is this
a Version 2 patch series? If so, what's different?

Regards,

Graeme

[U-Boot] [PATCH 09/21] blackfin: Replace "bfin_reset_or_hang()" with "panic()"

[Mike Frysinger]

On Monday, March 07, 2011 12:37:30 Kyle Moffett wrote:
> The bfin_reset_or_hang function unnecessarily duplicates the panic()
> logic based on CONFIG_PANIC_HANG.
> 
> This patch deletes 20 lines of code and just calls panic() instead.

hmm, didnt even realize this func existed in u-boot.  i'll merge this one 
patch through my tree in the next window since it doesnt rely on anything else 
in this series.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110307/74a00f77/attachment.pgp 

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Graeme Russ]

Hi Kyle

On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
> The i386 port has its own reset_cpu() dispatch for its various supported
> CPU families, so the existing do_reset() function is simply altered to
> use the new prototype for __arch_restart().
>
> In addition, the debug message and delay are duplicated from the generic
> code, so they are removed.
>
> This reset code will probably work even when the CPU is in a bad state,
> so no separate __arch_emergency_restart() function is required.

This part does not make much sense - If the CPU is in 'a bad state' then
it will probably be lights out anyway. As I understand it, an emergency
restart is a restart not initiated by the user (divide by zero, unhandled
exception etc), in which case i386 will make use of it

Regards,

Graeme

[U-Boot] [PATCH 21/21] Remove legacy do_reset() function

[Graeme Russ]

Hi Kyle

On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
> All of the users of the legacy do_reset() function have been converted
> to __arch_restart() or __board_restart() as appropriate, so the
> compatibility calls to do_reset() may be removed.
>
> In addition the do_generic_reset() function is renamed to the now-unused
> name do_reset().
>

If it is unused, why not remove it completely?

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Moffett, Kyle D]

On Mar 07, 2011, at 16:40, Mike Frysinger wrote:
> On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
>> +__attribute__((__noreturn__))
>> +void emergency_restart(void)
>> +{
>> +	__board_emergency_restart();
>> +	__arch_emergency_restart();
>> +
>> +	/* Fallback to the old do_reset() until everything is converted. */
>> +	do_reset(NULL, 0, 0, NULL);
>> +
>> +	printf("EMERGENCY RESTART: All attempts to reboot failed!");
> 
> missing a trailing newline, and i'd just call puts() to bypass useless format 
> string parsing

Ok, I'll go check for this in all the patches, thanks.


>> +	udelay(50000);
> 
> this doesnt sit well with me.  i dont see why this matters ... we dont have 
> any delays today, and i dont think we should introduce any.

Actually, several architectures had a printf()+udelay() already.  (i386 and ARM, IIRC).  Since I was moving the printf() to common code I figured I'd better move the udelay() as well.

I believe they had comments to the effect of "This udelay() prevents garbage on the serial console while rebooting", I would guess because it gives the FIFO time to finish flushing.

I can move it back to the individual architectures if you'd like.

>> +__attribute__((__weak__))
>> +int __board_restart(void)
>> +{
>> +	/* Fallthrough to architecture-specific code */
>> +	return 0;
>> +}
>> +
>> +__attribute__((__weak__))
>> +int __arch_restart(void)
>> +{
>> +	/* Fallthrough to legacy do_reset() code */
>> +	return 0;
>> +}
> 
> what use is there in having a return value ?  the do_reset() funcs today dont 
> return, which means they have no meaningful reset value.

Well, actually, a surprising number of them *do* return (at least on some board ports).

In particular I'm doing this for a custom board of ours (HWW-1U-1A) which *must* synchronize with Linux firmware running on another physical processor prior to asserting the reset line, otherwise it probably won't boot up correctly.  Since the synchronization may take an arbitrary amount of time (IE: Waiting for the other Linux to finish initializing), my board's __board_restart() function checks for Ctrl-C on the serial console.  If I actually *do* get a Ctrl-C I need to prevent the __arch_restart() hook from being run, by returning an error code.

The "emergency_restart()" best-effort reboot is obviously a special case, as the system is already wedged and there's nowhere useful to return *to*.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Moffett, Kyle D]

Hi!

On Mar 07, 2011, at 16:54, Graeme Russ wrote:
> On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
>> The i386 port has its own reset_cpu() dispatch for its various supported
>> CPU families, so the existing do_reset() function is simply altered to
>> use the new prototype for __arch_restart().
>> 
>> In addition, the debug message and delay are duplicated from the generic
>> code, so they are removed.
>> 
>> This reset code will probably work even when the CPU is in a bad state,
>> so no separate __arch_emergency_restart() function is required.
> 
> This part does not make much sense - If the CPU is in 'a bad state' then
> it will probably be lights out anyway. As I understand it, an emergency
> restart is a restart not initiated by the user (divide by zero, unhandled
> exception etc), in which case i386 will make use of it

I was considering unhandled exceptions, etc. to be "a bad state" :-D.

Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()".  Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default.

Hopefully I am making sense now?  Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand?

Cheers,
Kyle Moffett

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Mike Frysinger]

On Monday, March 07, 2011 16:56:31 Moffett, Kyle D wrote:
> On Mar 07, 2011, at 16:40, Mike Frysinger wrote:
> > On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
> >> +	udelay(50000);
> > 
> > this doesnt sit well with me.  i dont see why this matters ... we dont
> > have any delays today, and i dont think we should introduce any.
> 
> Actually, several architectures had a printf()+udelay() already.  (i386 and
> ARM, IIRC).  Since I was moving the printf() to common code I figured I'd
> better move the udelay() as well.
> 
> I believe they had comments to the effect of "This udelay() prevents
> garbage on the serial console while rebooting", I would guess because it
> gives the FIFO time to finish flushing.

Blackfin specifically does not do this.  personally, a half baked uart char is 
preferable to a (imo) useless hardcoded delay.  i wonder if they picked a 
number that seems to work, or actually calculated it based on the slowest baud 
times the deepest fifo times the extra bits (parity/stop/etc...).  i'd suspect 
the former which means this is even dumber.

> I can move it back to the individual architectures if you'd like.

if we cant agree to simply punt it, then yes, please do.  or make it a config 
option so board porters can opt out, and default it to on for only the 
existing arches.

> >> +__attribute__((__weak__))
> >> +int __board_restart(void)
> >> +{
> >> +	/* Fallthrough to architecture-specific code */
> >> +	return 0;
> >> +}
> >> +
> >> +__attribute__((__weak__))
> >> +int __arch_restart(void)
> >> +{
> >> +	/* Fallthrough to legacy do_reset() code */
> >> +	return 0;
> >> +}
> > 
> > what use is there in having a return value ?  the do_reset() funcs today
> > dont return, which means they have no meaningful reset value.
> 
> Well, actually, a surprising number of them *do* return (at least on some
> board ports).
> 
> In particular I'm doing this for a custom board of ours (HWW-1U-1A) which
> *must* synchronize with Linux firmware running on another physical
> processor prior to asserting the reset line, otherwise it probably won't
> boot up correctly.  Since the synchronization may take an arbitrary amount
> of time (IE: Waiting for the other Linux to finish initializing), my
> board's __board_restart() function checks for Ctrl-C on the serial
> console.  If I actually *do* get a Ctrl-C I need to prevent the
> __arch_restart() hook from being run, by returning an error code.

hrm, sounds dumb to me.  i guess we cant localize this though, and it isnt 
that much overhead, so i wont complain too much about the board part.

what about the arch hook ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110307/7b54667e/attachment.pgp 

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Graeme Russ]

Hi Kyle

On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D
<Kyle.D.Moffett@boeing.com> wrote:
> Hi!
>
> On Mar 07, 2011, at 16:54, Graeme Russ wrote:
>> On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
>>> The i386 port has its own reset_cpu() dispatch for its various supported
>>> CPU families, so the existing do_reset() function is simply altered to
>>> use the new prototype for __arch_restart().
>>>
>>> In addition, the debug message and delay are duplicated from the generic
>>> code, so they are removed.
>>>
>>> This reset code will probably work even when the CPU is in a bad state,
>>> so no separate __arch_emergency_restart() function is required.
>>
>> This part does not make much sense - If the CPU is in 'a bad state' then
>> it will probably be lights out anyway. As I understand it, an emergency
>> restart is a restart not initiated by the user (divide by zero, unhandled
>> exception etc), in which case i386 will make use of it
>
> I was considering unhandled exceptions, etc. to be "a bad state" :-D.
>
> Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". ?Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default.
>
> Hopefully I am making sense now? ?Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand?
>

I understand what you are doing now, thanks.

I think you can scrap this part of the description and I will have i386
start using __arch_emergency_restart() for 'Internal U-Boot Errors' such
as divide by zero, unhandled exception, general protection faults etc

I don't particularly like the 'emergency' naming - It's like if we don't
do it things will blow up :) I think 'automatic' might be a closer term

Is there anywhere yet where the code paths for the emergency and non
emergency variants differ in the way they end up resetting the board?

Regards,

Graeme

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Moffett, Kyle D]

On Mar 07, 2011, at 17:26, Graeme Russ wrote:
> On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote:
>> On Mar 07, 2011, at 16:54, Graeme Russ wrote:
>>> This part does not make much sense - If the CPU is in 'a bad state' then
>>> it will probably be lights out anyway. As I understand it, an emergency
>>> restart is a restart not initiated by the user (divide by zero, unhandled
>>> exception etc), in which case i386 will make use of it
>> 
>> I was considering unhandled exceptions, etc. to be "a bad state" :-D.
>> 
>> Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()".  Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default.
>> 
>> Hopefully I am making sense now?  Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand?
> 
> I understand what you are doing now, thanks.
> 
> I think you can scrap this part of the description and I will have i386
> start using __arch_emergency_restart() for 'Internal U-Boot Errors' such
> as divide by zero, unhandled exception, general protection faults etc
> 
> I don't particularly like the 'emergency' naming - It's like if we don't
> do it things will blow up :) I think 'automatic' might be a closer term

The name "emergency_restart()" was borrowed from the Linux kernel; the kernel shuts down disks and network interfaces prior to a regular restart but not during an emergency restart.  The best analogy is to the "EMERG" log level (KERN_EMERG inside the Linux kernel).

Furthermore, you should *not* directly call __arch_emergency_restart(), that is simply the architecture hook you provide to the generic code.  Your exception handlers should call "emergency_restart()" to allow board-specific code to override the reboot, for example by triggering an onboard watchdog to reset other hardware.

EG:

=> some_i386_trap_handler()
   => emergency_restart()
       => __board_emergency_restart()
       => __arch_emergency_restart()

=> do_reset()  [The new, generic version]
   => system_restart()
      => __board_restart()
      => __arch_restart()

The __{board,arch}_restart(), etc are just predefined weak functions that can be overridden by the implementation.

For example, my HWW-1U-1A board file effectively does this:

int __board_restart(void)
{
        while (poll_external_software()) {
                if (ctrlc())
                        return -1;
        }
        return 0;
}

void __board_emergency_restart(void)
{
        while (poll_external_software_uninterruptible())
                ;
}

During a normal restart, I allow the polling to be interrupted with Ctrl-C, but during an emergency restart I don't.  In both cases, my function just *returns* to allow the default MPC85xx code to actually perform the hardware-level restart (by poking at the "reset request" bit in a CPU register).


> Is there anywhere yet where the code paths for the emergency and non
> emergency variants differ in the way they end up resetting the board?

There are several U-Boot board ports whose do_reset() functions (now called "__board_restart()") just perform an unconditional jump to "_start" or a FLASH "soft-reset" address.  If the system has experienced an unexpected exception or other problem then that is not safe, and it would be better to just hang().

Cheers,
Kyle Moffett

[U-Boot] [PATCH 21/21] Remove legacy do_reset() function

[Moffett, Kyle D]

Hi!

On Mar 07, 2011, at 16:55, Graeme Russ wrote:
> On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
>> All of the users of the legacy do_reset() function have been converted
>> to __arch_restart() or __board_restart() as appropriate, so the
>> compatibility calls to do_reset() may be removed.
>> 
>> In addition the do_generic_reset() function is renamed to the now-unused
>> name do_reset().
> 
> If it is unused, why not remove it completely?

The "reset" command used to invoke "do_reset()".  Partway through this patch series I changed it to invoke a new "do_generic_reset()" function to preserve bisection.

The calltrace looked like this:

=> do_generic_reset()
   => system_restart()
      => __board_restart()
      => __arch_restart()
      => do_reset()

Now that nothing talks about "do_reset()" anymore, I delete that old function and rename "do_generic_reset()" over top of it:

=> do_reset()  [Same as old do_generic_reset()]
   => system_restart()
      => __board_restart()
      => __arch_restart()

Cheers,
Kyle Moffett

[U-Boot] [PATCH 21/21] Remove legacy do_reset() function

[Graeme Russ]

On Tue, Mar 8, 2011 at 10:00 AM, Moffett, Kyle D
<Kyle.D.Moffett@boeing.com> wrote:
> Hi!
>
> On Mar 07, 2011, at 16:55, Graeme Russ wrote:
>> On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote:
>>> All of the users of the legacy do_reset() function have been converted
>>> to __arch_restart() or __board_restart() as appropriate, so the
>>> compatibility calls to do_reset() may be removed.
>>>
>>> In addition the do_generic_reset() function is renamed to the now-unused
>>> name do_reset().
>>
>> If it is unused, why not remove it completely?
>
> The "reset" command used to invoke "do_reset()". ?Partway through this patch series I changed it to invoke a new "do_generic_reset()" function to preserve bisection.
>
> The calltrace looked like this:
>
> => do_generic_reset()
> ? => system_restart()
> ? ? ?=> __board_restart()
> ? ? ?=> __arch_restart()
> ? ? ?=> do_reset()
>
> Now that nothing talks about "do_reset()" anymore, I delete that old function and rename "do_generic_reset()" over top of it:
>
> => do_reset() ?[Same as old do_generic_reset()]
> ? => system_restart()
> ? ? ?=> __board_restart()
> ? ? ?=> __arch_restart()
>
> Cheers,
> Kyle Moffett

Go it - Thanks

Regards,

Graeme

[U-Boot] [PATCH 11/21] i386: Generic system restart support

[Graeme Russ]

On Tue, Mar 8, 2011 at 9:57 AM, Moffett, Kyle D
<Kyle.D.Moffett@boeing.com> wrote:
> On Mar 07, 2011, at 17:26, Graeme Russ wrote:
>> On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote:
>>> On Mar 07, 2011, at 16:54, Graeme Russ wrote:
>>>> This part does not make much sense - If the CPU is in 'a bad state' then
>>>> it will probably be lights out anyway. As I understand it, an emergency
>>>> restart is a restart not initiated by the user (divide by zero, unhandled
>>>> exception etc), in which case i386 will make use of it
>>>
>>> I was considering unhandled exceptions, etc. to be "a bad state" :-D.
>>>
>>> Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". ?Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default.
>>>
>>> Hopefully I am making sense now? ?Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand?
>>
>> I understand what you are doing now, thanks.
>>
>> I think you can scrap this part of the description and I will have i386
>> start using __arch_emergency_restart() for 'Internal U-Boot Errors' such
>> as divide by zero, unhandled exception, general protection faults etc
>>
>> I don't particularly like the 'emergency' naming - It's like if we don't
>> do it things will blow up :) I think 'automatic' might be a closer term
>
> The name "emergency_restart()" was borrowed from the Linux kernel; the kernel shuts down disks and network interfaces prior to a regular restart but not during an emergency restart. ?The best analogy is to the "EMERG" log level (KERN_EMERG inside the Linux kernel).

OK, makes sense to stick with emergency then

>
> Furthermore, you should *not* directly call __arch_emergency_restart(), that is simply the architecture hook you provide to the generic code. ?Your exception handlers should call "emergency_restart()" to allow board-specific code to override the reboot, for example by triggering an onboard watchdog to reset other hardware.
>
> EG:
>
> => some_i386_trap_handler()
> ? => emergency_restart()
> ? ? ? => __board_emergency_restart()
> ? ? ? => __arch_emergency_restart()
>
> => do_reset() ?[The new, generic version]
> ? => system_restart()
> ? ? ?=> __board_restart()
> ? ? ?=> __arch_restart()
>

OK

> The __{board,arch}_restart(), etc are just predefined weak functions that can be overridden by the implementation.
>
> For example, my HWW-1U-1A board file effectively does this:
>
> int __board_restart(void)
> {
> ? ? ? ?while (poll_external_software()) {
> ? ? ? ? ? ? ? ?if (ctrlc())
> ? ? ? ? ? ? ? ? ? ? ? ?return -1;
> ? ? ? ?}
> ? ? ? ?return 0;
> }
>
> void __board_emergency_restart(void)
> {
> ? ? ? ?while (poll_external_software_uninterruptible())
> ? ? ? ? ? ? ? ?;
> }
>
> During a normal restart, I allow the polling to be interrupted with Ctrl-C, but during an emergency restart I don't. ?In both cases, my function just *returns* to allow the default MPC85xx code to actually perform the hardware-level restart (by poking at the "reset request" bit in a CPU register).
>
>
>> Is there anywhere yet where the code paths for the emergency and non
>> emergency variants differ in the way they end up resetting the board?
>
> There are several U-Boot board ports whose do_reset() functions (now called "__board_restart()") just perform an unconditional jump to "_start" or a FLASH "soft-reset" address. ?If the system has experienced an unexpected exception or other problem then that is not safe, and it would be better to just hang().
>

OK - All sounds good to me. I'll try run it all up on my board 'soon'

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Graeme Russ]

On Tue, Mar 8, 2011 at 9:10 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday, March 07, 2011 16:56:31 Moffett, Kyle D wrote:
>> On Mar 07, 2011, at 16:40, Mike Frysinger wrote:
>> > On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
>> >> + ?udelay(50000);
>> >
>> > this doesnt sit well with me. ?i dont see why this matters ... we dont
>> > have any delays today, and i dont think we should introduce any.
>>
>> Actually, several architectures had a printf()+udelay() already. ?(i386 and
>> ARM, IIRC). ?Since I was moving the printf() to common code I figured I'd
>> better move the udelay() as well.
>>
>> I believe they had comments to the effect of "This udelay() prevents
>> garbage on the serial console while rebooting", I would guess because it
>> gives the FIFO time to finish flushing.
>
> Blackfin specifically does not do this. ?personally, a half baked uart char is
> preferable to a (imo) useless hardcoded delay. ?i wonder if they picked a
> number that seems to work, or actually calculated it based on the slowest baud
> times the deepest fifo times the extra bits (parity/stop/etc...). ?i'd suspect
> the former which means this is even dumber.
>
>> I can move it back to the individual architectures if you'd like.
>
> if we cant agree to simply punt it, then yes, please do. ?or make it a config
> option so board porters can opt out, and default it to on for only the
> existing arches.
>

Is there any way we could detect that the UART FIFO is empty and wait on
that condition (with a timeout in case the FIFO is never emptied)?

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Mike Frysinger]

On Monday, March 07, 2011 18:09:25 Graeme Russ wrote:
> On Tue, Mar 8, 2011 at 9:10 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Monday, March 07, 2011 16:56:31 Moffett, Kyle D wrote:
> >> On Mar 07, 2011, at 16:40, Mike Frysinger wrote:
> >> > On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
> >> >> +  udelay(50000);
> >> > 
> >> > this doesnt sit well with me.  i dont see why this matters ... we dont
> >> > have any delays today, and i dont think we should introduce any.
> >> 
> >> Actually, several architectures had a printf()+udelay() already.  (i386
> >> and ARM, IIRC).  Since I was moving the printf() to common code I
> >> figured I'd better move the udelay() as well.
> >> 
> >> I believe they had comments to the effect of "This udelay() prevents
> >> garbage on the serial console while rebooting", I would guess because it
> >> gives the FIFO time to finish flushing.
> > 
> > Blackfin specifically does not do this.  personally, a half baked uart
> > char is preferable to a (imo) useless hardcoded delay.  i wonder if they
> > picked a number that seems to work, or actually calculated it based on
> > the slowest baud times the deepest fifo times the extra bits
> > (parity/stop/etc...).  i'd suspect the former which means this is even
> > dumber.
> > 
> >> I can move it back to the individual architectures if you'd like.
> > 
> > if we cant agree to simply punt it, then yes, please do.  or make it a
> > config option so board porters can opt out, and default it to on for
> > only the existing arches.
> 
> Is there any way we could detect that the UART FIFO is empty and wait on
> that condition (with a timeout in case the FIFO is never emptied)?

that would require extending the serial API to add a new func, have each 
serial driver implement (i'm pretty sure most serial devices out there expose 
a bit to poll such status), and then having the code call it.  i'm not saying 
it cant be done, just outlining the requirements.

although i vaguely recall that Wolfgang is against this sort of thing since it 
kind of goes against the KISS principle ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110307/518432e3/attachment.pgp 

[U-Boot] [PATCH 15/21] nios2: Generic system restart support

[Scott McNutt]

Hi Kyle,

Kyle Moffett wrote:
> The Nios-II port appears to use no generic hardware capability for
> performing a CPU reset.  Since all of the supported boards use the exact
> same code to perform a jump-to-flash it goes into __arch_restart().
> 
> This means that Nios-II has a no-op __arch_emergency_restart() function.
> If the CPU is in an invalid state then jump-to-FLASH probably won't
> work.

If the CPU state is stable enough to call __arch_emergency_restart(),
a jump to the reset address should be fine. In many implementations
the reset code is in on-chip ROM. If things get screwed up enough
to affect the code in on-chip ROM, it probably won't matter what
gets called. ;-)

Thomas, any thoughts?

Regards,
--Scott

[U-Boot] [PATCH 15/21] nios2: Generic system restart support

[Moffett, Kyle D]

Hi!

On Mar 08, 2011, at 19:13, Scott McNutt wrote:
> Hi Kyle,
> 
> Kyle Moffett wrote:
>> The Nios-II port appears to use no generic hardware capability for
>> performing a CPU reset.  Since all of the supported boards use the exact
>> same code to perform a jump-to-flash it goes into __arch_restart().
>> 
>> This means that Nios-II has a no-op __arch_emergency_restart() function.
>> If the CPU is in an invalid state then jump-to-FLASH probably won't
>> work.
> 
> If the CPU state is stable enough to call __arch_emergency_restart(),
> a jump to the reset address should be fine. In many implementations
> the reset code is in on-chip ROM. If things get screwed up enough
> to affect the code in on-chip ROM, it probably won't matter what
> gets called. ;-)

I'm not at all familiar with the Nios-II hardware platform.  Is the on-chip ROM really safe to be called in arbitrary system states?

Using FLASH memory as an example, consider a FLASH driver in the middle of a programming cycle when an unexpected exception occurs:

*  FLASH programming in process
*  CPU takes an unexpected trap
*  CPU calls exception vector (possibly with interrupts disabled or enabled)
*  emergency_restart()
*  __arch_emergency_restart()
*  Boot ROM is called

Until the FLASH memory is reset and put back into a defined state it will be unusable, and if your boot process depends on the FLASH then serious problems will result.

In that case it would be better not to just hang than try to restart.

Basically the emergency_restart() code should handle being called even in the following scenarios:
  * Unexpected CPU exception or interrupt occurs
  * Interrupts on or off, or *from* an interrupt or trap handler.
  * FLASH or other peripherals in an undefined state

If that's the case for your onboard ROM then I will certainly remove the no-op emergency restart hook from this patch.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 15/21] nios2: Generic system restart support

[Scott McNutt]

Moffett, Kyle D wrote:
> Hi!
> 
> On Mar 08, 2011, at 19:13, Scott McNutt wrote:
>> Hi Kyle,
>>
>> Kyle Moffett wrote:
>>> The Nios-II port appears to use no generic hardware capability for
>>> performing a CPU reset.  Since all of the supported boards use the exact
>>> same code to perform a jump-to-flash it goes into __arch_restart().
>>>
>>> This means that Nios-II has a no-op __arch_emergency_restart() function.
>>> If the CPU is in an invalid state then jump-to-FLASH probably won't
>>> work.
>> If the CPU state is stable enough to call __arch_emergency_restart(),
>> a jump to the reset address should be fine. In many implementations
>> the reset code is in on-chip ROM. If things get screwed up enough
>> to affect the code in on-chip ROM, it probably won't matter what
>> gets called. ;-)
> 
  ... <snip> ...

> Basically the emergency_restart() code should handle being called even in the following scenarios:
>   * Unexpected CPU exception or interrupt occurs
>   * Interrupts on or off, or *from* an interrupt or trap handler.
>   * FLASH or other peripherals in an undefined state
> 
> If that's the case for your onboard ROM then I will certainly remove the no-op emergency restart hook from this patch.

 From a practical perspective, in the case of on-chip ROM, it would be
no different than a hard reset during any of the above activities.
But please don't remove it. Your position is sound and conservative.

I haven't had time to review the entire patch set yet. I'm assuming
the __board_emergency_restart() routine is where a board can override
the default arch-specific behavior? If so, you already provided the
hooks for more daring developers. ;-)

Regards,
--Scott

[U-Boot] [PATCH 0/21] Generic cross-architecture system restart support

[Wolfgang Denk]

Dear Kyle Moffett,

In message <1299519462-25320-1-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
> 
> This patchset is designed to produce zero change in behavior *except* for
> one particular scenario:  Platforms which perform a "restart" with a simple
> jump to the "_start" entrypoint (or similar) will no longer try to do that
> on panic().
> 
> The new functions to be called from generic code are:
> 
>   int system_restart(void)
>   void emergency_restart(void)

Could you please explain what these functions are supposed to do?

> The first few patches set up the generic functions and hooks with a default
> fallback to the existing "do_reset()" function.  Most of the rest are then
> architecture-specific modifications necessary to convert away from the old
> do_reset() prototype.  The last patch finally does away with that function.

Why do you replace a simple name like "reset" with a more complicated
name like "system_restart"?

What is emergency_restart() needed for?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I wish I had a bronze torc for every user who didn't read the manual.
                             - Terry Pratchett, _The Light Fantastic_

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear Kyle Moffett,

In message <1299519462-25320-2-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
> In preparation for making system restart use a generic set of hooks for
> boards and architectures, we define some wrappers and weak stubs.
> 
> The new wrapper functions are:
>   system_restart()     -  Normal system reboot (IE: user request)
>   emergency_restart()  -  Critical error response (IE: panic(), etc)

What is the difference between these two - and why do we need
different functions at all?

A reset is a reset is a reset, isn't it?

...
> +/*
> + * This MUST be called from normal interrupts-on context.  If it requires
> + * extended interaction with external hardware it SHOULD react to Ctrl-C.

??? Why such complicated restrictions for a simple command that is
just supposed to reset the board?

> + * If this function fails to guarantee a clean reboot or receives a Ctrl-C
> + * keystroke it SHOULD return with an error (-1).

A "reset" is supposed to take place immediately, and unconditionally.
If you need delays and ^C handling and other bells and whistles,
please add these to your own code, but not here.

> +int system_restart(void)
> +{
> +	int err;
> +
> +	/*
> +	 * Print a nice message and wait a bit to make sure it goes out the
> +	 * console properly.
> +	 */
> +	printf ("Restarting...\n");
> +	udelay(50000);
> +
> +	/* First let the board code try to reboot */
> +	err = __board_restart();
> +	if (err)
> +		goto failed;
> +
> +	/* Now call into the architecture-specific code */
> +	err = __arch_restart();
> +	if (err)
> +		goto failed;
> +
> +	/* Fallback to the old do_reset() until everything is converted. */
> +	err = do_reset(NULL, 0, 0, NULL);
> +
> +failed:
> +	printf("*** SYSTEM RESTART FAILED ***\n");
> +	return err;
> +}

You are making a simple thing pretty much complicated.  This adds lots
of code and I cannot see any benefits.

My initial feeling is a plain NAK, for this and the rest of the patch
series.  Why would we want all this?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
They're usually so busy thinking about what  happens  next  that  the
only  time they ever find out what is happening now is when they come
to look back on it.                 - Terry Pratchett, _Wyrd Sisters_

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Moffett, Kyle D]

Hi!

On Mar 13, 2011, at 15:24, Wolfgang Denk wrote:
> In message <1299519462-25320-2-git-send-email-Kyle.D.Moffett@boeing.com> you wrote:
>> In preparation for making system restart use a generic set of hooks for
>> boards and architectures, we define some wrappers and weak stubs.
>> 
>> The new wrapper functions are:
>>  system_restart()     -  Normal system reboot (IE: user request)
>>  emergency_restart()  -  Critical error response (IE: panic(), etc)
> 
> What is the difference between these two - and why do we need
> different functions at all?
> 
> A reset is a reset is a reset, isn't it?

That might be true *IF* all boards could actually perform a real hardware reset.

Some can't, and instead they just jump to their reset vector (Nios-II) or to flash (some ppc 74xx/7xx systems).

If the board just panic()ed or got an unhandled trap or exception, then you don't want to do a soft-reset that assumes everything is OK.  A startup in a bad environment like that could corrupt FLASH or worse.  Right now there is no way to tell the difference, but the lower-level arch-specific code really should care.

So system_restart() is what you use when the system is in a good normal operating condition.  The emergency_restart() is what gets called from panic() or in other places where a crash has happened.


> ...
>> +/*
>> + * This MUST be called from normal interrupts-on context.  If it requires
>> + * extended interaction with external hardware it SHOULD react to Ctrl-C.
> 
> ??? Why such complicated restrictions for a simple command that is
> just supposed to reset the board?

This is just documenting what the underlying hardware-specific code is guaranteed.  On some hardware a "system_restart()" may fail and return -1, on others the board needs to cleanly respond to interrupts while polling external hardware, etc.  This is analogous to the normal "sys_reboot()" code under Linux, which requires interrupts-on, etc.

This is to contrast with the emergency_restart() function which has no such API constraints.  It can be called from any context whatsoever and will do its best to either perform a full hardware reset or hang while trying.


>> + * If this function fails to guarantee a clean reboot or receives a Ctrl-C
>> + * keystroke it SHOULD return with an error (-1).
> 
> A "reset" is supposed to take place immediately, and unconditionally.
> If you need delays and ^C handling and other bells and whistles,
> please add these to your own code, but not here.

There's no Ctrl-C handling anywhere in this code, it will all be in my own __board_restart() hook.  As above, this documentation is just describing the guarantees provided to underlying __board_restart() and __arch_restart() hooks; if they check for Ctrl-C while polling external hardware and return an error then that's fine.

>> +int system_restart(void)
>> +{
>> +	int err;
>> +
>> +	/*
>> +	 * Print a nice message and wait a bit to make sure it goes out the
>> +	 * console properly.
>> +	 */
>> +	printf ("Restarting...\n");
>> +	udelay(50000);
>> +
>> +	/* First let the board code try to reboot */
>> +	err = __board_restart();
>> +	if (err)
>> +		goto failed;
>> +
>> +	/* Now call into the architecture-specific code */
>> +	err = __arch_restart();
>> +	if (err)
>> +		goto failed;
>> +
>> +	/* Fallback to the old do_reset() until everything is converted. */
>> +	err = do_reset(NULL, 0, 0, NULL);
>> +
>> +failed:
>> +	printf("*** SYSTEM RESTART FAILED ***\n");
>> +	return err;
>> +}
> 
> You are making a simple thing pretty much complicated.  This adds lots
> of code and I cannot see any benefits.

No, it's actually a net removal of code, the diffstat is:
  90 files changed, 341 insertions(+), 374 deletions(-)

The basic hook structure from system_restart() already existed individually in 6 different architectures (incl. PowerPC, Blackfin, ARM, MIPS...), each of which had its own "board_reset()" or "_machine_restart()" or similar.  This code makes it entirely generic, so as a new board implementor you can simply define a "__board_restart()" function to override (or enhance) the normal architecture-specific code.


> My initial feeling is a plain NAK, for this and the rest of the patch
> series.  Why would we want all this?

While I was going through the hooks I noticed that several of them were explicitly NOT safe if the board was in the middle of a panic() for whatever reason, so I split off the __*_emergency_restart() hooks separately to allow architectures to handle them cleanly.

My own board needs both processor modules to synchronize resets to allow them to come back up at all, which means that a "reset" may block for an arbitrary amount of time waiting for the other module to cleanly shut down and restart (or waiting for somebody to type "reset" on the other U-Boot).  If someone just types "reset" on the console, I want to allow them to hit Ctrl-C to interrupt the process.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear "Moffett, Kyle D",

In message <A60AEA13-1206-4699-9302-0DF9C0F9DE28@boeing.com> you wrote:
> 
> >> The new wrapper functions are:
> >>  system_restart()     -  Normal system reboot (IE: user request)
> >>  emergency_restart()  -  Critical error response (IE: panic(), etc)
> > 
> > What is the difference between these two - and why do we need
> > different functions at all?
> > 
> > A reset is a reset is a reset, isn't it?
>
> That might be true *IF* all boards could actually perform a real hardware reset.
>
> Some can't, and instead they just jump to their reset vector (Nios-II) or to flash (some ppc 74xx/7xx systems).

So this is the "reset" on these boards, then.

> If the board just panic()ed or got an unhandled trap or exception, then you
> don't want to do a soft-reset that assumes everything is OK.  A startup in
> a bad environment like that could corrupt FLASH or worse.  Right now there
> is no way to tell the difference, but the lower-level arch-specific code
> really should care.

I don't understand your chain of arguments.

If there really is no better way to implement the reset on such
boards, then what else can we do?

And if there are more things that could be done to provide a "better"
reset, then why should we not always do these?

> So system_restart() is what you use when the system is in a good normal
> operating condition.  The emergency_restart() is what gets called from panic()
> or in other places where a crash has happened.

Why?  What's the difference?

> >> + * If this function fails to guarantee a clean reboot or receives a Ctrl-C
> >> + * keystroke it SHOULD return with an error (-1).
> > 
> > A "reset" is supposed to take place immediately, and unconditionally.
> > If you need delays and ^C handling and other bells and whistles,
> > please add these to your own code, but not here.
>
> There's no Ctrl-C handling anywhere in this code, it will all be in my own
> __board_restart() hook.  As above, this documentation is just describing the

There is no ^C handling supposed to be in any reset hook.

You are changing user interfaces to very low-level and intentinally
simple commands in a complicated way, and I don;t see any advantage of
either this complexity nor your changes.

> guarantees provided to underlying __board_restart() and __arch_restart()
> hooks; if they check for Ctrl-C while polling external hardware and return
> an error then that's fine.

No, it is not, because it is not supposed to be done.

You could as well implement a "reset" cpmmand that actually turns on a
fan and the LCD backlight - that would be similarly useful.

> > My initial feeling is a plain NAK, for this and the rest of the patch
> > series.  Why would we want all this?
>
> While I was going through the hooks I noticed that several of them were
> explicitly NOT safe if the board was in the middle of a panic() for whatever

Can you please peovide some specific xamples?  I don't understand what
you are talking about.

> reason, so I split off the __*_emergency_restart() hooks separately to allow
> architectures to handle them cleanly.
>
> My own board needs both processor modules to synchronize resets to allow
> them to come back up at all, which means that a "reset" may block for an
> arbitrary amount of time waiting for the other module to cleanly shut down
> and restart (or waiting for somebody to type "reset" on the other U-Boot).
> If someone just types "reset" on the console, I want to allow them to hit
> Ctrl-C to interrupt the process.

This is not what the "reset" command is supposed to do.  The reset
command is supposed to be the software equivalent of someone pressing
the reset button on your board - to the extend possible to be
implemented in software.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If God had a beard, he'd be a UNIX programmer.

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Moffett, Kyle D]

On Mar 14, 2011, at 14:59, Wolfgang Denk wrote:
> In message <A60AEA13-1206-4699-9302-0DF9C0F9DE28@boeing.com> you wrote:
>> My own board needs both processor modules to synchronize resets to allow
>> them to come back up at all, which means that a "reset" may block for an
>> arbitrary amount of time waiting for the other module to cleanly shut down
>> and restart (or waiting for somebody to type "reset" on the other U-Boot).
>> If someone just types "reset" on the console, I want to allow them to hit
>> Ctrl-C to interrupt the process.
> 
> This is not what the "reset" command is supposed to do.  The reset
> command is supposed to be the software equivalent of someone pressing
> the reset button on your board - to the extend possible to be
> implemented in software.

On our boards, when the "reset" button is pressed in hardware, both processor modules on the board and all the attached hardware reset at the same time.

If just *one* of the 2 CPUs triggers the reset then only *some* of the attached hardware will be properly reset due to a hardware errata, and as a result the board will sometimes hang or corrupt DMA transfers to the SSDs shortly after reset.

The only way to reset either chip safely is by resetting both at the same time, which requires them to communicate before the reset and wait (possibly a long time) for the other board to agree to reset.  Yes, it's a royal pain, but we're stuck with this hardware for the time being, and if the board can't communicate then it might as well hang() anyways.

This same logic is also implemented in my Linux board-support code, so when one CPU requests a reset the other treats it as a Ctrl-Alt-Del.


>>> What is the difference between these two - and why do we need
>>> different functions at all?
>>> 
>>> A reset is a reset is a reset, isn't it?
>> 
>> That might be true *IF* all boards could actually perform a real hardware reset.
>> 
>> Some can't, and instead they just jump to their reset vector (Nios-II) or to flash (some ppc 74xx/7xx systems).
> 
> So this is the "reset" on these boards, then.
> 
>> If the board just panic()ed or got an unhandled trap or exception, then you
>> don't want to do a soft-reset that assumes everything is OK.  A startup in
>> a bad environment like that could corrupt FLASH or worse.  Right now there
>> is no way to tell the difference, but the lower-level arch-specific code
>> really should care.
> 
> I don't understand your chain of arguments.
> 
> If there really is no better way to implement the reset on such
> boards, then what else can we do?
> 
> And if there are more things that could be done to provide a "better"
> reset, then why should we not always do these?

If the board is in a panic() state it may well have still-running DMA transfers (such as USB URBs), or be in the middle of writing to FLASH.

Performing a jump to early-boot code which is only ever tested when everything is OK and devices are properly initialized is a great way to cause data corruption.

I know for a fact that our boards would rather hang forever than try to reset without cooperation from the other CPU.

>>> My initial feeling is a plain NAK, for this and the rest of the patch
>>> series.  Why would we want all this?
>> 
>> While I was going through the hooks I noticed that several of them were
>> explicitly NOT safe if the board was in the middle of a panic() for whatever
> 
> Can you please peovide some specific examples?  I don't understand what
> you are talking about.

Ok, using the ppmc7xx board as an example:

        /* Disable and invalidate cache */
        icache_disable();
        dcache_disable();

        /* Jump to cold reset point (in RAM) */
        _start();

        /* Should never get here */
        while(1)
                ;

This board uses the EEPRO100 driver, which appears to set up statically allocated TX and RX rings which the device performs DMA to/from.

If this board starts receiving packets and then panic()s, it will disable address translation and immediately re-relocate U-Boot into RAM, then zero the BSS.  If the network card tries to receive a packet after BSS is zeroed, it will read a packet buffer address of (probably) 0x0 from the RX ring and promptly overwrite part of U-Boot's memory at that address.

Depending on the initialization process and memory layout, other similar boards could start writing garbage values to FLASH control registers and potentially corrupt data.

Since the panic() path is so infrequently used and tested, it's better to be safe and hang() on the boards which do not have a reliable hardware-level reset than it is to cause undefined behavior or potentially corrupt data.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear "Moffett, Kyle D",

In message <613C8F89-3CE5-4C28-A48E-D5C3E8143A4C@boeing.com> you wrote:
>
> On our boards, when the "reset" button is pressed in hardware, both
> processor modules on the board and all the attached hardware reset at
> the same time.

OK.  So a sane design would provide a way for both of the processors
to do the same, for example by toggeling some GPIO or similar.

> If just *one* of the 2 CPUs triggers the reset then only *some* of
> the attached hardware will be properly reset due to a hardware
> errata, and as a result the board will sometimes hang or corrupt DMA
> transfers to the SSDs shortly after reset.
...
> Yes, it's a royal pain, but we're stuck with this hardware for the
> time being, and if the board can't communicate then it might as well
> hang() anyways.

Do you agree that this is a highly board-specific problem (I would
call it a hardware bug, but I don't insist you agree on that term),
and while there is a the need for you to work around such behaviour
there is little or no reason to do this, or anything like that, in
common code ?


> > And if there are more things that could be done to provide a "better"
> > reset, then why should we not always do these?
> 
> If the board is in a panic() state it may well have still-running DMA
> transfers (such as USB URBs), or be in the middle of writing to
> FLASH.

The same (at least having USB or other drivers still being enabled,
and USB writing it's SOF counters to RAM) can happen for any call to
the reset() function.  I see no reason for assuming there would be
better or worse conditions to perform a reset.

> Performing a jump to early-boot code which is only ever tested when
> everything is OK and devices are properly initialized is a great way
> to cause data corruption.

If there is a software way to prevent such issues, then these steps
should always be performed.

> I know for a fact that our boards would rather hang forever than try
> to reset without cooperation from the other CPU.

As mentioned above, this is a board specific issue that should not
influence common code design.

> >> While I was going through the hooks I noticed that several of them were
> >> explicitly NOT safe if the board was in the middle of a panic() for whatever
> > 
> > Can you please peovide some specific examples?  I don't understand what
> > you are talking about.
> 
> Ok, using the ppmc7xx board as an example:
> 
>         /* Disable and invalidate cache */
>         icache_disable();
>         dcache_disable();
> 
>         /* Jump to cold reset point (in RAM) */
>         _start();
> 
>         /* Should never get here */
>         while(1)
>                 ;
> 
> This board uses the EEPRO100 driver, which appears to set up
> statically allocated TX and RX rings which the device performs DMA
> to/from.
> 
> If this board starts receiving packets and then panic()s, it will
> disable address translation and immediately re-relocate U-Boot into
> RAM, then zero the BSS. If the network card tries to receive a packet
> after BSS is zeroed, it will read a packet buffer address of
> (probably) 0x0 from the RX ring and promptly overwrite part of
> U-Boot's memory at that address.

Agreed.  So this should be fixed.  One clean way to fix it would be to
help improving the driver model for U-Boot (read: create one) and
making sure drivers get deinitialized in such a case.

> Since the panic() path is so infrequently used and tested, it's
> better to be safe and hang() on the boards which do not have a
> reliable hardware-level reset than it is to cause undefined behavior
> or potentially corrupt data.

I disagree.  Instead of adding somewhat obscure alternate code paths
(which get tested even less frequently) we should focus oin fixing
such problems where we run into them.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Microsoft Multitasking:
                     several applications can crash at the same time.

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Moffett, Kyle D]

On Mar 14, 2011, at 16:38, Wolfgang Denk wrote:
> In message <613C8F89-3CE5-4C28-A48E-D5C3E8143A4C@boeing.com> you wrote:
>> 
>> If just *one* of the 2 CPUs triggers the reset then only *some* of
>> the attached hardware will be properly reset due to a hardware
>> errata, and as a result the board will sometimes hang or corrupt DMA
>> transfers to the SSDs shortly after reset.
> ...
>> Yes, it's a royal pain, but we're stuck with this hardware for the
>> time being, and if the board can't communicate then it might as well
>> hang() anyways.
> 
> Do you agree that this is a highly board-specific problem (I would
> call it a hardware bug, but I don't insist you agree on that term),
> and while there is a the need for you to work around such behaviour
> there is little or no reason to do this, or anything like that, in
> common code ?

Oh, absolutely.  I do think there still needs to be a separation between a "normal user-initiated restart" and an "panic-time emergency restart" though, see further on in this email.

The comment about Ctrl-C was simply because our board restart can be aborted by someone at the console because it may take a while for the other CPU to cleanly shut down and acknowledge.  Our __board_restart() watches for Ctrl-C to handle this nicely, and then returns an error, which makes do_reset() return to the user or script with an error.  Obviously in the middle of a panic() there is nothing useful to return to, so the code keeps polling regardless.

Such decisions on what is and is not "acceptable" to run on a panic() are better left to the individual boards and architectures.  Specifically, the separate board and arch hooks for regular and "emergency" restarts that I included in the patch:

  __arch_restart()
  __board_restart()
  __arch_emergency_restart()
  __board_emergency_restart()

>>> And if there are more things that could be done to provide a "better"
>>> reset, then why should we not always do these?
>> 
>> If the board is in a panic() state it may well have still-running DMA
>> transfers (such as USB URBs), or be in the middle of writing to
>> FLASH.
> 
> The same (at least having USB or other drivers still being enabled,
> and USB writing it's SOF counters to RAM) can happen for any call to
> the reset() function.  I see no reason for assuming there would be
> better or worse conditions to perform a reset.

I would argue that is a bug to be fixed.  Regardless of how various boards and architectures implement "reset", U-Boot should provide generic functionality to drivers and library code to allow them to indicate what they want:

  (1) A safe normal operational restart, with all hardware shut down (as much as is considered necessary on the platform).  Depending on the platform this may fail or take some time.

  (2) A critical error restart, where system state may be undefined and the calling code does not expect the function to ever return.

Linux has *both* of those cases in the kernel:  sys_reboot() and emergency_restart().

>> If this board starts receiving packets and then panic()s, it will
>> disable address translation and immediately re-relocate U-Boot into
>> RAM, then zero the BSS. If the network card tries to receive a packet
>> after BSS is zeroed, it will read a packet buffer address of
>> (probably) 0x0 from the RX ring and promptly overwrite part of
>> U-Boot's memory at that address.
> 
> Agreed.  So this should be fixed.  One clean way to fix it would be to
> help improving the driver model for U-Boot (read: create one) and
> making sure drivers get deinitialized in such a case.

This another excellent reason to have separate system_restart() and emergency_restart().  The "system_restart()" function would hook into the driver model and perform device shutdown (just like Linux), while the emergency_restart() function (and therefore panic()) would not.

The "jump to _start" case is very similar to Linux kexec().  There are two specific use-cases:
  (1) Safe reliable run-time handoff from one kernel to another
  (2) Emergency panic() call into another kernel to record the error and reboot safely

In the former case, the kernel runs all of its normal shutdown handlers and quiesces all DMA before passing control to the new kernel.  Under U-Boot this is analogous to the "system_restart()" function I added.  The "system_restart()" function would be the ideal central place to hook driver-model device shut-down.

In the latter case (similar to "emergency_restart()"), the new kernel runs in a sub-section of memory *completely-preallocated* at boot time, and explicitly ignores all other memory because there might be ongoing DMA transactions.  The "emergency kexec" case intentionally avoids running device shutdown handlers because the system state is not well defined.  Under U-Boot there is no way to run completely from a preallocated chunk of memory, which means that a panic()-time jump to "_start" is not safe.

Cheers,
Kyle Moffett

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear "Moffett, Kyle D",

In message <44A75130-ED4F-46D6-B0E4-12433CC15142@boeing.com> you wrote:
> 
> Oh, absolutely. I do think there still needs to be a separation
> between a "normal user-initiated restart" and an "panic-time
> emergency restart" though, see further on in this email.

These terms refer to another software level, though.

do_reset() is supposed to provide a hard reset function, nothing else
- as mentioned, the software way of pressing the reset button (or
pulling the plug).

A "normal user-initiated restart" would be the equivalent of a
"shutdown -r" in Linux context. We don't offer such a service in
U-Boot, as it has never been needed yet. If implemented, it would
probably call do_reset() as last of it's actions.

A "panic-time emergency restart" can be anything, too - again, it
would probably call do_reset() at the end.

> Such decisions on what is and is not "acceptable" to run on a panic()
> are better left to the individual boards and architectures.

This is a completely new topic now.   We have not been discussing the
implementation or function of panic() before.  This has nothing to do
with what do_reset() is supposed to do - do_reset() may (or may not)
be one action called by panic() [and if so, it should be the last
thing that panic() has been doing.]

> Specifically, the separate board and arch hooks for regular and
> "emergency" restarts that I included in the patch:
> 
>   __arch_restart()
>   __board_restart()
>   __arch_emergency_restart()
>   __board_emergency_restart()

Yes, and I object against such unneeded (for everybody else)
complexity.


> I would argue that is a bug to be fixed. Regardless of how various
> boards and architectures implement "reset", U-Boot should provide
> generic functionality to drivers and library code to allow them to
> indicate what they want:
> 
>   (1) A safe normal operational restart, with all hardware shut down
> (as much as is considered necessary on the platform). Depending on
> the platform this may fail or take some time.
> 
>   (2) A critical error restart, where system state may be undefined
> and the calling code does not expect the function to ever return.

This is overkill for a boot loader.  We assume that when anything goes
wrong we do the best we can to perform a hard reset.  Any halfway sane
hardware design will allow you do do that.  There is a zillion of ways
to do that - from causing a machine check, using a hardware watchdog,
messing limits of system monitor chips, etc. etc.  Or there is a
simple GPIO pin that triggers an external hard reset.

If some hardware provides no such option I will not hesitate to call
this a hardare bug and blame the hardware designer.

Workarounds for such bugs should be dealt with in board specific code.

> Linux has *both* of those cases in the kernel: sys_reboot() and
> emergency_restart().

Linux is an OS.  U-Boot is a boot loader.

Linux offers many things and services that are not available in
U-Boot.

I am even tempted to recommend you to boot Linux as part of your reset
sequence ;-)

> The "jump to _start" case is very similar to Linux kexec().  There are two specific use-cases:

The "jump to _start" case is something I consider a bug that should be
fixed.  I will not accept the existence of such code as reason to
build arbitrarily complex layers on top of it.  It may be the best
possible solution in the given case (which I actually doubt), but even
then it's just that: the best possible approximation to what
actuallyis needed.

>   (1) Safe reliable run-time handoff from one kernel to another
>   (2) Emergency panic() call into another kernel to record the error and reboot safely

U-Boot just provides "reset".


I think I understand what you have in mind, but I'm not going to
accept that, especially not in common code.  Sorry.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Remember that Beethoven wrote his first symphony in C ...

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Graeme Russ]

On 15/03/11 09:01, Wolfgang Denk wrote:
> Dear "Moffett, Kyle D",
> 
> In message <44A75130-ED4F-46D6-B0E4-12433CC15142@boeing.com> you wrote:
>>

[Snip]

I kind of like the idea of different reset sources (CPU exception, hardware
failure, user initiated) but agree copying the linux architecture is over
the top.

Is there any reason reset() could not take a 'reason' parameter? It could
be a bit-mask with CPU, SOC and arch reserved bits (unhandled exception,
user initiated, panic etc) and board specific bits

Board or arch specific code could handle different reasons however they
please (like logging it in NVRAM prior to restart, gracefully shutting down
multiple CPU's, clearing DMA buffers etc)

All 'hang', 'panic', 'reset' etc code can be simplified into a single code
path (although calling 'reset' to 'hang' is a bit odd)

Just a thought

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear Graeme Russ,

In message <4D8739F6.5040805@gmail.com> you wrote:
>
> I kind of like the idea of different reset sources (CPU exception, hardware
> failure, user initiated) but agree copying the linux architecture is over
> the top.

What's the difference as far as do_reset() is concenred?  It shall
just (hard) reset the system, nothing else.

> Is there any reason reset() could not take a 'reason' parameter? It could
> be a bit-mask with CPU, SOC and arch reserved bits (unhandled exception,
> user initiated, panic etc) and board specific bits

What for? To perform the intended purpose, no parameter is needed.

> Board or arch specific code could handle different reasons however they
> please (like logging it in NVRAM prior to restart, gracefully shutting down
> multiple CPU's, clearing DMA buffers etc)

That would be a layer higher than do_reset() (for example, in
panic()).

> All 'hang', 'panic', 'reset' etc code can be simplified into a single code
> path (although calling 'reset' to 'hang' is a bit odd)

hang() and reset() are intentionally very different things.  A call to
hang() is supposed to hang (surprise, surprise!) infinitely.  It must
not cause a reset.

panic() is a higher software layer. It probably results in calling
reset() in the end.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You don't need a weatherman to know which way the wind blows.
                                                          - Bob Dylan

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Graeme Russ]

On 21/03/11 23:00, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4D8739F6.5040805@gmail.com> you wrote:
>>
>> I kind of like the idea of different reset sources (CPU exception, hardware
>> failure, user initiated) but agree copying the linux architecture is over
>> the top.
> 
> What's the difference as far as do_reset() is concenred?  It shall
> just (hard) reset the system, nothing else.
> 
>> Is there any reason reset() could not take a 'reason' parameter? It could
>> be a bit-mask with CPU, SOC and arch reserved bits (unhandled exception,
>> user initiated, panic etc) and board specific bits
> 
> What for? To perform the intended purpose, no parameter is needed.
> 
>> Board or arch specific code could handle different reasons however they
>> please (like logging it in NVRAM prior to restart, gracefully shutting down
>> multiple CPU's, clearing DMA buffers etc)
> 
> That would be a layer higher than do_reset() (for example, in
> panic()).

Hmmm, but panic() is defined in lib/vsprintf.c with no possibility for it
to be overridden in any arch or board specific way

> 
>> All 'hang', 'panic', 'reset' etc code can be simplified into a single code
>> path (although calling 'reset' to 'hang' is a bit odd)
> 
> hang() and reset() are intentionally very different things.  A call to
> hang() is supposed to hang (surprise, surprise!) infinitely.  It must
> not cause a reset.

As I said, calling reset() to hang is odd :)

> 
> panic() is a higher software layer. It probably results in calling
> reset() in the end.
> 

Unless CONFIG_PANIC_HANG is defined...

Looking into the code

panic():
  - ~130 call sites
  - Implemented in lib/vsprintf.c
  - Calls do_reset() if CONFIG_PANIC_HANG is not defined
  - Calls hang() if CONFIG_PANIC_HANG is defined

hang():
  - ~180 call sites using hang() and hang ()
  - Implemented in arch\lib\board.c
  - ARM, i386, m68k, microblaze, mips,  prints "### ERROR ### Please RESET
the board ###\n" and loops
  - avr32 prints nothing and loops
  - Blackfin can set status LEDs based on #define, prints "### ERROR ###
Please RESET the board ###\n" and triggers a breakpoint if a JTAG debugger
is connected
  - nios2 disables interrupts then does the same as ARM et all
  - powerpc is similar to ARM et al but also calls show_boot_progress(-30)
  - sh - same as ARM et al but prints "Board ERROR\n"
  - sparc - if CONFIG_SHOW_BOOT_PROGRESS defined, same as powerpc, else
same as ARM et al

So hang() could easily be weakly implemented in common\ which would allow
arch and board specific overrides

do_reset():
  - ~70 call sites
  - 39 implemetations
  - Implemented all over the place (arch/cpu/, arch/lib, board/)
  - Only ARM is in arch/lib which could likely be moved to arch/cpu
  - Is U_BOOT_CMD
  - m68k has a number of very similar/identical implementations
  - Is not weak - Cannot be overridden at the board level
  - Ouch, PPC is real ugly:
    #if !defined(CONFIG_PCIPPC2) && \
        !defined(CONFIG_BAB7xx)  && \
        !defined(CONFIG_ELPPC)   && \
        !defined(CONFIG_PPMC7XX)
    /* no generic way to do board reset. simply call soft_reset. */
    int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
    {
    ...
    Boards can thus provide their own do_reset()
  - Wow, m68k/cpu/mcf52x2/cpu.c is even uglier - 7 definitions selectable
    by #ifdef's
  - Because do_reset() is U_BOOT_CMD, practically every call is:
    do_reset (NULL, 0, 0, NULL) - do_bootm() does pass it's args onto
    do_reset()
  - No implementation of do_reset() uses any args anyway

reset():
  - does not exist (as far as I can tell)

So, maybe we could replace do_reset() with a weak reset() function defined
in arch/cpu which can be overridden at the board level. All existing calls
to do_reset() can be converted to simply reset()

The U_BOOT_CMD do_reset() would simply call reset()

Could many of the current calls to do_reset() be replaced with calls to
panic()?

I still like the idea of passing a 'reason' to reset() / panic() - Could we
change panic() to:

void panic(u32 reason, const char *fmt, ...)
{
	va_list	args;
	va_start(args, fmt);
	vprintf(fmt, args);
	putc('\n');
	va_end(args);

	if (reason |= PANIC_FLAG_HANG)
		hang(reason);
	else
		reset(reason);
}

Anywhere in the code that needed to hang has a choice - hang(reason) or
panic(reason | PANIC_FLAG_HANG)

Default implementations of hang() and reset() would just ignore reason.
Board specific code can use reason to do one last boot_progress(), set LED
states etc.

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear Graeme Russ,

In message <4D88909A.80508@gmail.com> you wrote:
>
> > That would be a layer higher than do_reset() (for example, in
> > panic()).
> 
> Hmmm, but panic() is defined in lib/vsprintf.c with no possibility for it
> to be overridden in any arch or board specific way

I guess that could be helped.

> > panic() is a higher software layer. It probably results in calling
> > reset() in the end.
> 
> Unless CONFIG_PANIC_HANG is defined...

> reset():
>   - does not exist (as far as I can tell)

reset() is used as symbol in many arm, mips and sparc start.S files

> I still like the idea of passing a 'reason' to reset() / panic() - Could we
> change panic() to:
...
> Anywhere in the code that needed to hang has a choice - hang(reason) or
> panic(reason | PANIC_FLAG_HANG)

I don't think you resonably decide which to use in common code.

Most calls to panic() appear to come from proprietary flash drivers
anyway - most of which should be dropped as they could use the CFI
driver instead. [And if you look at the actual code, the tests for
these panic()s can easily be computed at compile time, so these are
stupid aniway.]

Others 

Now, assume for things like this:

	panic("No working SDRAM available\n");

or like handling undefined instructions or that - what would be more
useful - to hang() to to reset()? ;-)


Can you please show me a specific case where you would use such
different arguments to panic() in the existing code?

> Default implementations of hang() and reset() would just ignore reason.
> Board specific code can use reason to do one last boot_progress(), set LED
> states etc.

That should be done at a higher software layer.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
CAUTION:  The Mass of This Product Contains the Energy Equivalent  of
85 Million Tons of TNT per Net Ounce of Weight.

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Graeme Russ]

On Wed, Mar 23, 2011 at 12:28 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <4D88909A.80508@gmail.com> you wrote:
>>
>> > That would be a layer higher than do_reset() (for example, in
>> > panic()).
>>
>> Hmmm, but panic() is defined in lib/vsprintf.c with no possibility for it
>> to be overridden in any arch or board specific way
>
> I guess that could be helped.
>
>> > panic() is a higher software layer. It probably results in calling
>> > reset() in the end.
>>
>> Unless CONFIG_PANIC_HANG is defined...
>
>> reset():
>> ? - does not exist (as far as I can tell)
>
> reset() is used as symbol in many arm, mips and sparc start.S files

Good point

>
>> I still like the idea of passing a 'reason' to reset() / panic() - Could we
>> change panic() to:
> ...
>> Anywhere in the code that needed to hang has a choice - hang(reason) or
>> panic(reason | PANIC_FLAG_HANG)
>
> I don't think you resonably decide which to use in common code.

My point was that everything can be piped through panic()

>
> Most calls to panic() appear to come from proprietary flash drivers
> anyway - most of which should be dropped as they could use the CFI
> driver instead. [And if you look at the actual code, the tests for
> these panic()s can easily be computed at compile time, so these are
> stupid aniway.]
>
> Others
>
> Now, assume for things like this:
>
> ? ? ? ?panic("No working SDRAM available\n");
>
> or like handling undefined instructions or that - what would be more
> useful - to hang() to to reset()? ;-)

Replace with:
 ? ? ? ?panic(PANIC_FLAG_HANG, "No working SDRAM available\n");

or:
 ? ? ? ?panic(PANIC_REASON_NO_RAM, "No working SDRAM available\n");

And all of the current do_reset(NULL, 0, 0, NULL) can be changed to:
 ? ? ? ?panic(PANIC_FLAG_RESET, NULL);

or if the print a message before the call to reset then:
 ? ? ? ?panic(PANIC_FLAG_RESET, "Whatever message was printed\n");

And panic() becomes:

void panic(u32 reason, const char *fmt, ...)
{
       if(fmt) {
              va_list args;
              va_start(args, fmt);
              vprintf(fmt, args);
              putc('\n');
              va_end(args);
       }

       if (reason |= PANIC_FLAG_HANG)
               hang(reason);
       else,
               reset(reason);
}

>
>
> Can you please show me a specific case where you would use such
> different arguments to panic() in the existing code?

My reasoning is cleaning up the reset()/hang()/panic() API.

Also, consider devices which do not normally have any device attached to
log serial output, but you may want to log reset/hang reasons for diagnosis
later. Board defined hang() and reset() can log the reason in NVRAM and at
next bootup (with a serial console attached) part of the startup message
could be 'Last Reset Reason'

>
>> Default implementations of hang() and reset() would just ignore reason.
>> Board specific code can use reason to do one last boot_progress(), set LED
>> states etc.
>
> That should be done at a higher software layer.
>

How? For example, if an Ethernet device which the board uses to tftp a file
from fails to initialise, that failure is detected in the common driver
code and as a consequence hang(), reset(), or panic() is called. The driver
can print out a message before calling hang() or reset() (useless if you
have no serial console attached) and by the time any arch or board specific
code gets called, all information regarding the failure has been lost. Why
should a common driver decide if the board should hang or reset? What if
the board has an alternative method of retrieving the file it was going to
tftp (maybe a fail-safe backup in Flash). In which case, the board can
just go

OK, I may be bluring the line towards what might be reasonably handled by
loading an OS (but maybe the latest OS image was being tfp'd) but my point
is that a good reset/panic/hang API gives the _board_ ultimate control over
what do do. Currently, ulitimate control of what to do in a particular
failure scenario is hard-coded by drivers and CPU/SOC/arch specific code
with little to no control offered up to the board. What control there is
has been provided by ugly #ifdef's

I am suggesting an API that goes along the lines of:

 - driver/common/board specific code encounters a failure and calls panic()
   There may be cases where the call site _knows_ a hang or reset must be
   performed and would thus provide PANIC_FLAG_HANG or PANIC_FLAG_RESET
 - panic() prints a message (if provided)
 - panic() calls weak hang or reset functions was default implementations
   in arch/soc/cpu
 - do_reset() from the command line calls straight into reset() with
   PANIC_FLAG_USER_RESET
 - boards can override hang() and reset() in order to provide better
   control of the shutdown processes (release DMA buffers etc) or to
   log the reason in non-volatile storage
 - arch hang() and reset() can be called by the board's override to
   perform shutdown of multi-CPU's etc
 - etc

Regards,

Graeme

[U-Boot] [PATCH 01/21] Define new system_restart() and emergency_restart()

[Wolfgang Denk]

Dear Graeme Russ,

sorry for the delay.

In message <AANLkTikjCJ6TuJ49TRJWHMh3y=OhFjCKMZd=XxNLvuUD@mail.gmail.com> you wrote:
>
> My point was that everything can be piped through panic()

Yes, it can.  But I don't think that makes sense.

> > Can you please show me a specific case where you would use such
> > different arguments to panic() in the existing code?
> 
> My reasoning is cleaning up the reset()/hang()/panic() API.

Then please keep up with good old Unix philosophy: use small building
block, where each of them fulfils a single purpose, and this done
well.

I seriously dislike the idea of a multifunction panic()
implementation.

> Also, consider devices which do not normally have any device attached to
> log serial output, but you may want to log reset/hang reasons for diagnosis
> later. Board defined hang() and reset() can log the reason in NVRAM and at
> next bootup (with a serial console attached) part of the startup message
> could be 'Last Reset Reason'

Please re-read what I wrote.  Things like hang() or reset() are
supposed to hang or reset _only_.  Any logging is another layer.

> How? For example, if an Ethernet device which the board uses to tftp a file
> from fails to initialise, that failure is detected in the common driver
> code and as a consequence hang(), reset(), or panic() is called. The driver
> can print out a message before calling hang() or reset() (useless if you
> have no serial console attached) and by the time any arch or board specific
> code gets called, all information regarding the failure has been lost. Why
> should a common driver decide if the board should hang or reset? What if

OK, you just proed your own argument wrong.  I agree, a driver should
never just hang(), reset(), or panic() as long as there is a
reasonable way to continue normal operation.


> I am suggesting an API that goes along the lines of:

I understand what you are proposing, and I do not want to accept that.
It is IMO a wrong approach. Functions hang(), reset(), or panic() are
the lowest layer of the implementation, they are function promitives
that are useful as is, and they do exactly what you expect them to do,
without any magic stuff.  Feel free to build your own error handling
and repostiong and logging functions on top of them. If they are
generally useful these may then be reused in more code. But don't try
to put any such stuff into the function primitives.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I like your game but we have to change the rules."