Generic cross-architecture system restart support
Subsystems (11)
cover Generic cross-architecture system restart support
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
PATCH 1/21 Define new system_restart() and emergency_restart()
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.
--- 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(-)
api/api.c +2 -1
@@ -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 ;
common/cmd_boot.c +127 -1
140 lines not on this page · raw message
common/cmd_bootm.c +3 -5
@@ -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 (); }/**
common/hush.c +1 -1
@@ -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
common/main.c +1 -1
@@ -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
examples/api/libgenwrap.c +3 -2
@@ -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 ; }
include/_exports.h +1 -1
@@ -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)
include/command.h +13 -0
@@ -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__ */ /*
lib/vsprintf.c +1 -1
@@ -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 }
Signature · 1 line
1.7.2.3
PATCH 1/21 Define new system_restart() and emergency_restart()
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 78
Quoted 10 lines · On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
common/cmd_boot.c +10
-0+__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
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 122
Quoted 9 lines
common/cmd_boot.c +9
-0+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
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 123
Quoted 1 line
common/cmd_boot.c +1
-0+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.
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 139
Quoted 1 line
common/cmd_boot.c +1
-0+printf ("*** SYSTEM RESTART FAILED ***\n" );
puts()
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 162
Quoted 13 lines
common/cmd_boot.c +13
-0+__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
Moffett
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 14 lines · Mike Frysinger
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 78
Quoted 10 lines · On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
common/cmd_boot.c +10
-0+__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.
Quoted 4 lines
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 123
Quoted 1 line
common/cmd_boot.c +1
-0+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.
Quoted 8 lines
Quoted 8 lines
common/cmd_boot.c +8
-0+__attribute__((__weak__)) +int __board_restart (void ) +{ +/* Fallthrough to architecture-specific code */ +return 0 ; +} + +__attribute__((__weak__))
Quoted 8 lines
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 162
Quoted 5 lines
common/cmd_boot.c +5
-0+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
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 7 lines · Kyle D
Quoted 5 lines · Mike Frysinger
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 123
Quoted 1 line · On Monday, March 07, 2011 12:37:22 Kyle Moffett wrote:
common/cmd_boot.c +1
-0+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.
Quoted 7 lines
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 theudelay()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.
Quoted 1 line
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.
Quoted 20 lines
Quoted 8 lines
Quoted 8 lines
common/cmd_boot.c +8
-0+__attribute__((__weak__)) +int __board_restart (void ) +{ +/* Fallthrough to architecture-specific code */ +return 0 ; +} + +__attribute__((__weak__))Quoted 8 lines
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 162
Quoted 5 lines
common/cmd_boot.c +5
-0+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).
Quoted 8 lines
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
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 22 lines · Mike Frysinger
Quoted 7 lines · Kyle D
Quoted 5 lines · Mike Frysinger
Quoted 1 line · 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.
Quoted 7 lines
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 theudelay()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.
Quoted 6 lines
Quoted 1 line
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
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 24 lines · On Monday, March 07, 2011 18:09:25 Graeme Russ wrote:
Quoted 23 lines · Mike Frysinger
Quoted 7 lines · Kyle D
Quoted 5 lines · Mike Frysinger
Quoted 1 line · 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.
Quoted 7 lines
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 theudelay()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.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
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
PATCH 1/21 Define new system_restart() and emergency_restart()
In reply to Kyle MoffettDear Kyle Moffett,
Quoted 6 lines · In message <[email protected]> you…
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?
...
Quoted 3 lines
+/*
+ * 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?
Quoted 2 lines
+ * 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.
Quoted 18 lines
common/cmd_boot.c +18
-0+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 ();
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 141
Quoted 10 lines
common/cmd_boot.c +10
-0+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
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de 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_
Moffett
PATCH 1/21 Define new system_restart() and emergency_restart()
Hi!
Quoted 4 lines · Wolfgang Denk
Quoted 3 lines · In message <[email protected]> you…
In preparation for making system restart use a generic set of hooks for boards and architectures, we define some wrappers and weak stubs.
Quoted 8 lines
Quoted 3 lines
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.
Quoted 7 lines
...
Quoted 3 lines
+/*
+ * 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.
Quoted 6 lines
Quoted 2 lines
+ * 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.
Quoted 23 lines
Quoted 23 lines
common/cmd_boot.c +23
-0+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 );
Quoted 8 lines
common/cmd_boot.c int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 141
Quoted 5 lines
common/cmd_boot.c +5
-0+ +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.
Quoted 2 lines
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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear "Moffett, Kyle D",
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 5 lines
Quoted 4 lines
The new wrapper functions are:
system_restart() - Normal system reboot (IE: user request) emergency_restart() - Critical error response (IE: panic(), etc)
Quoted 8 lines
Quoted 4 lines
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.
Quoted 5 lines
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?
Quoted 3 lines
So
system_restart()is what you use when the system is in a good normal operating condition. Theemergency_restart()is what gets called frompanic()or in other places where a crash has happened.
Why? What's the difference?
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
+ * 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.
Quoted 3 lines
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 9 lines
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
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de If God had a beard, he'd be a UNIX programmer.
Moffett
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 8 lines · Wolfgang Denk
Quoted 6 lines · In message <[email protected]> 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.
Quoted 4 lines
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.
Quoted 17 lines
Quoted 8 lines
Quoted 4 lines
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.
Quoted 5 lines
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.
Quoted 7 lines
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.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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 whateverCan 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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear "Moffett, Kyle D",
Quoted 4 lines · In message <[email protected]> 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.
Quoted 4 lines
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.
...
Quoted 3 lines
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 ?
Quoted 6 lines
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 2 lines
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.
Quoted 23 lines
Quoted 5 lines
Quoted 2 lines
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 whateverCan 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.
Quoted 6 lines
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)
0x0from 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.
Quoted 4 lines
Since the
panic()path is so infrequently used and tested, it's better to be safe andhang()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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Microsoft Multitasking:
several applications can crash at the same time.Moffett
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 11 lines · Wolfgang Denk
Quoted 5 lines · In message <[email protected]> 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.
...
Quoted 3 lines
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.
Quoted 5 lines
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()
Quoted 11 lines
Quoted 6 lines
Quoted 2 lines
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().
Quoted 10 lines
Quoted 6 lines
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)
0x0from 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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear "Moffett, Kyle D",
Quoted 4 lines · In message <[email protected]> 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.
Quoted 2 lines
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.]
Quoted 7 lines
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.
Quoted 11 lines
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.
Quoted 2 lines
Linux has *both* of those cases in the kernel:
sys_reboot()andemergency_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 ;-)
Quoted 1 line
The "jump to
_start" case is very similar to Linuxkexec(). 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.
Quoted 2 lines
(1) Safe reliable run-time handoff from one kernel to another
(2) Emergencypanic()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
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Remember that Beethoven wrote his first symphony in C ...
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 4 lines · Wolfgang Denk
Dear "Moffett, Kyle D",
Quoted 1 line · In message <[email protected]> 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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> 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.
Quoted 3 lines
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.
Quoted 3 lines
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()).
Quoted 2 lines
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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You don't need a weatherman to know which way the wind blows.
- Bob DylanPATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 15 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 4 lines · In message <[email protected]> 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.Quoted 3 lines
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
Quoted 8 lines
What for? To perform the intended purpose, no parameter is needed.
Quoted 3 lines
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, inpanic()).
Hmmm, but panic() is defined in lib/vsprintf.c with no possibility for it to be overridden in any arch or board specific way
Quoted 7 lines
Quoted 3 lines
All 'hang', 'panic', 'reset' etc code can be simplified into a single code path (although calling 'reset' to 'hang' is a bit odd)
hang()andreset()are intentionally very different things. A call tohang()is supposed to hang (surprise, surprise!) infinitely. It must not cause a reset.
As I said, calling reset() to hang is odd :)
Quoted 4 lines
panic()is a higher software layer. It probably results in callingreset()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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear Graeme Russ,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines
That would be a layer higher than
do_reset()(for example, inpanic()).Hmmm, but
panic()is defined inlib/vsprintf.cwith no possibility for it to be overridden in any arch or board specific way
I guess that could be helped.
Quoted 7 lines
Quoted 2 lines
panic()is a higher software layer. It probably results in callingreset()in the end.Unless
CONFIG_PANIC_HANGis 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
Quoted 2 lines
I still like the idea of passing a 'reason' to
reset()/panic()- Could we changepanic()to:
...
Quoted 2 lines
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?
Quoted 3 lines
Default implementations of
hang()andreset()would just ignore reason. Board specific code can use reason to do one lastboot_progress(), set LED states etc.
That should be done at a higher software layer.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de CAUTION: The Mass of This Product Contains the Energy Equivalent of 85 Million Tons of TNT per Net Ounce of Weight.
PATCH 1/21 Define new system_restart() and emergency_restart()
Quoted 15 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines
That would be a layer higher than
do_reset()(for example, inpanic()).Hmmm, but
panic()is defined inlib/vsprintf.cwith no possibility for it to be overridden in any arch or board specific wayI guess that could be helped.
Quoted 3 lines
Quoted 2 lines
panic()is a higher software layer. It probably results in callingreset()in the end.
Quoted 6 lines
Quoted 4 lines
Unless
CONFIG_PANIC_HANGis defined...
reset():
? - does not exist (as far as I can tell)
reset()is used as symbol in many arm, mips and sparcstart.Sfiles
Good point
Quoted 8 lines
Quoted 3 lines
I still like the idea of passing a 'reason' to
reset()/panic()- Could we changepanic()to:...
Quoted 2 lines
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()
Quoted 7 lines
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.]
Quoted 8 lines
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 toreset()? ;-)
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);
}Quoted 4 lines
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'
Quoted 7 lines
Quoted 4 lines
Default implementations of
hang()andreset()would just ignore reason. Board specific code can use reason to do one lastboot_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
PATCH 1/21 Define new system_restart() and emergency_restart()
Dear Graeme Russ,
sorry for the delay.
Quoted 2 lines · In message <[email protected]> you…
My point was that everything can be piped through
panic()
Yes, it can. But I don't think that makes sense.
Quoted 4 lines
Quoted 2 lines
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.
Quoted 5 lines
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()andreset()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.
Quoted 7 lines
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(), orpanic()is called. The driver can print out a message before callinghang()orreset()(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.
Quoted 1 line
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
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "I like your game but we have to change the rules."
PATCH 2/21 Replace do_reset() calls with {system, emergency}_restart()
In reply to Kyle MoffettAll 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.
--- 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(-)
board/dave/PPChameleonEVB/PPChameleonEVB.c +2 -2
@@ -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: " );
board/esd/apc405/apc405.c +2 -2
@@ -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 */
board/esd/ar405/ar405.c +1 -1
@@ -112,7 +112,7 @@ int board_early_init_f (void)udelay (1000 ); }putc ('\n' ); -do_reset (NULL ,0 ,0 ,NULL ); +emergency_restart (); } }
board/esd/ash405/ash405.c +2 -2
@@ -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: " );
board/esd/canbt/canbt.c +1 -1
@@ -108,7 +108,7 @@ int board_early_init_f (void)udelay (1000 ); }putc ('\n' ); -do_reset (NULL ,0 ,0 ,NULL ); +emergency_restart (); }/*
board/esd/cpci405/cpci405.c +3 -3
@@ -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 */
board/esd/cpciiser4/cpciiser4.c +1 -1
@@ -106,7 +106,7 @@ int board_early_init_f (void)udelay (1000 ); }putc ('\n' ); -do_reset (NULL ,0 ,0 ,NULL ); +emergency_restart (); }/*
board/esd/du405/du405.c +1 -1
@@ -106,7 +106,7 @@ int board_early_init_f (void)udelay (1000 ); }putc ('\n' ); -do_reset (NULL ,0 ,0 ,NULL ); +emergency_restart (); }/*
board/esd/hh405/hh405.c +2 -2
@@ -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: " );
board/esd/pci405/pci405.c +2 -2
@@ -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: " );
board/esd/plu405/plu405.c +2 -2
@@ -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: " );
board/esd/tasreg/tasreg.c +2 -2
@@ -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: " );
board/esd/voh405/voh405.c +2 -2
@@ -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: " );
board/esd/wuh405/wuh405.c +2 -2
@@ -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: " );
board/prodrive/pdnb3/pdnb3.c +1 -1
@@ -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: " );
board/sacsng/sacsng.c +1 -1
@@ -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 */
board/zeus/zeus.c +2 -2
@@ -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 ; }
Signature · 1 line
1.7.2.3
PATCH 3/21 arm: Call "panic()" instead of "hang()" for div-by-zero
In reply to Kyle MoffettDivide-by-zero errors should be controlled by the CONFIG_PANIC_HANG configuration variable just like any other kind of trappable U-Boot bug.
--- arch/arm/lib/div0.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/div0.c +3 -3
@@ -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!" ); }
Signature · 1 line
1.7.2.3
PATCH 4/21 arm: Replace unnecessary bad_mode() with panic()
In reply to Kyle MoffettThe 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.
--- arch/arm/lib/interrupts.c | 21 +++++++-------------- 1 files changed, 7 insertions(+), 14 deletions(-)
arch/arm/lib/interrupts.c +7 -14
@@ -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
Signature · 1 line
1.7.2.3
PATCH 5/21 arm: cpux9k2: Remove unnecessary XF_do_reset assignment
In reply to Kyle MoffettThe 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.
--- board/BuS/eb_cpux9k2/cpux9k2.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
board/BuS/eb_cpux9k2/cpux9k2.c +0 -1
@@ -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 );
Signature · 1 line
1.7.2.3
PATCH 6/21 arm: Rename nonstandard board_reset() as at91_board_reset()
In reply to Kyle MoffettThe 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().
--- 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(-)
arch/arm/cpu/arm920t/at91/reset.c +2 -2
@@ -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),
arch/arm/cpu/arm920t/at91rm9200/reset.c +3 -3
@@ -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 */
board/atmel/at91rm9200dk/at91rm9200dk.c +1 -1
@@ -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 ;
Signature · 1 line
1.7.2.3
PATCH 7/21 arm: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/arm/lib/reset.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
arch/arm/lib/reset.c +1 -5
@@ -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 );
Signature · 1 line
1.7.2.3
PATCH 8/21 avr32: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/avr32/cpu/cpu.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/avr32/cpu/cpu.c +1 -1
@@ -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 */
Signature · 1 line
1.7.2.3
PATCH 9/21 blackfin: Replace "bfin_reset_or_hang()" with "panic()"
In reply to Kyle MoffettThe 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.
--- 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(-)
arch/blackfin/cpu/cpu.h +0 -1
@@ -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);
arch/blackfin/cpu/reset.c +1 -18
@@ -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 ; -}
arch/blackfin/cpu/traps.c +1 -1
@@ -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" ); }
Signature · 1 line
1.7.2.3
PATCH 9/21 blackfin: Replace "bfin_reset_or_hang()" with "panic()"
Quoted 4 lines · On Monday, March 07, 2011 12:37:30 Kyle Moffett wrote:
The
bfin_reset_or_hangfunction unnecessarily duplicates thepanic()logic based onCONFIG_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
PATCH 10/21 blackfin: Generic system restart support
In reply to Kyle MoffettThe 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.
--- 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(-)
arch/blackfin/cpu/cpu.h +0 -1
@@ -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);
arch/blackfin/cpu/reset.c +1 -3
@@ -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)); }
board/bf537-stamp/bf537-stamp.c +4 -1
@@ -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
Signature · 1 line
1.7.2.3
PATCH 11/21 i386: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/i386/cpu/cpu.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
arch/i386/cpu/cpu.c +1 -3
@@ -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 );
Signature · 1 line
1.7.2.3
PATCH 11/21 i386: Generic system restart support
Hi Kyle
Quoted 9 lines · Kyle Moffett
The i386 port has its own
reset_cpu()dispatch for its various supported CPU families, so the existingdo_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
Moffett
PATCH 11/21 i386: Generic system restart support
Hi!
Quoted 8 lines · Graeme Russ
Quoted 7 lines · Kyle Moffett
The i386 port has its own
reset_cpu()dispatch for its various supported CPU families, so the existingdo_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.
Quoted 7 lines
Quoted 2 lines
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
PATCH 11/21 i386: Generic system restart support
Hi Kyle
Quoted 19 lines · Kyle D
Hi!
Quoted 8 lines · Graeme Russ
Quoted 7 lines · Kyle Moffett
The i386 port has its own
reset_cpu()dispatch for its various supported CPU families, so the existingdo_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.
Quoted 7 lines
Quoted 2 lines
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
Quoted 6 lines
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
Moffett
PATCH 11/21 i386: Generic system restart support
Quoted 13 lines · Graeme Russ
Quoted 11 lines · Kyle D
Quoted 4 lines · Graeme Russ
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?
Quoted 8 lines
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 etcI 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).
Quoted 2 lines
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
PATCH 11/21 i386: Generic system restart support
Quoted 16 lines · Kyle D
Quoted 15 lines · Graeme Russ
Quoted 11 lines · Kyle D
Quoted 4 lines · Graeme Russ
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.
Quoted 8 lines
Quoted 6 lines
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 etcI 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_EMERGinside the Linux kernel).
OK, makes sense to stick with emergency then
Quoted 10 lines
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()
Quoted 5 lines
=>
do_reset()?[The new, generic version]
? =>system_restart()
? ? ?=>__board_restart()
? ? ?=>__arch_restart()
OK
Quoted 19 lines
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()) ? ? ? ? ? ? ? ?;
}
Quoted 8 lines
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).
Quoted 2 lines
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 justhang().
OK - All sounds good to me. I'll try run it all up on my board 'soon'
Regards,
Graeme
PATCH 12/21 m68k: Generic system restart support
In reply to Kyle MoffettThe 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.
--- 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
arch/m68k/cpu/mcf5227x/cpu.c +1 -1
@@ -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 );
arch/m68k/cpu/mcf523x/cpu.c +1 -1
@@ -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 ;
arch/m68k/cpu/mcf52x2/cpu.c +7 -13
@@ -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 );
arch/m68k/cpu/mcf52x2/cpu.h deleted +0 -33
@@ -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_ */
arch/m68k/cpu/mcf532x/cpu.c +1 -1
@@ -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 );
arch/m68k/cpu/mcf5445x/cpu.c +1 -1
@@ -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 );
arch/m68k/cpu/mcf547x_8x/cpu.c +1 -1
@@ -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 );
Signature · 1 line
1.7.2.3
PATCH 13/21 microblaze: Generic system restart support
In reply to Kyle MoffettThe 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.
--- .../xilinx/microblaze-generic/microblaze-generic.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
board/xilinx/microblaze-generic/microblaze-generic.c +1 -1
@@ -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)) =
Signature · 1 line
1.7.2.3
PATCH 14/21 mips: Generic system restart support
In reply to Kyle MoffettThe 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).
--- 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
arch/mips/cpu/cpu.c +0 -13
@@ -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 ;
arch/mips/include/asm/reboot.h deleted +0 -14
@@ -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 */
board/incaip/incaip.c +2 -2
@@ -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 )
board/micronas/vct/vct.c +1 -1
@@ -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 );
board/purple/purple.c +13 -3
@@ -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)
board/tb0229/tb0229.c +13 -3
@@ -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 )
Signature · 1 line
1.7.2.3
PATCH 15/21 nios2: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/nios2/cpu/cpu.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
arch/nios2/cpu/cpu.c +11 -1
@@ -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 ; +}
Signature · 1 line
1.7.2.3
PATCH 15/21 nios2: Generic system restart support
Hi Kyle,
Quoted 7 lines · 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.
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
Moffett
PATCH 15/21 nios2: Generic system restart support
Hi!
Quoted 11 lines · Scott McNutt
Hi Kyle,
Quoted 7 lines · 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.
Quoted 5 lines
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
PATCH 15/21 nios2: Generic system restart support
Quoted 19 lines · Kyle D
Hi!
Quoted 15 lines · Scott McNutt
Hi Kyle,
Quoted 7 lines · 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.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> ...
Quoted 6 lines
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 stateIf 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
PATCH 16/21 powerpc: Generic system restart support
In reply to Kyle MoffettThe 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.
--- 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(-)
arch/powerpc/cpu/74xx_7xx/cpu.c +1 -6
@@ -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 /* ------------------------------------------------------------------------- */
arch/powerpc/cpu/mpc512x/cpu.c +1 -2
@@ -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 ;
arch/powerpc/cpu/mpc5xx/cpu.c +1 -1
@@ -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 ;
arch/powerpc/cpu/mpc5xxx/cpu.c +1 -2
@@ -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 */
arch/powerpc/cpu/mpc8220/cpu.c +1 -1
@@ -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;
arch/powerpc/cpu/mpc824x/cpu.c +1 -1
@@ -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;
arch/powerpc/cpu/mpc8260/cpu.c +1 -5
@@ -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 */ /* ------------------------------------------------------------------------- */
arch/powerpc/cpu/mpc83xx/cpu.c +1 -4
@@ -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 */
arch/powerpc/cpu/mpc85xx/cpu.c +12 -5
@@ -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
arch/powerpc/cpu/mpc86xx/cpu.c +1 -15
@@ -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);
arch/powerpc/cpu/mpc8xx/cpu.c +1 -29
@@ -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 */ -/* ------------------------------------------------------------------------- */ /*
arch/powerpc/cpu/ppc4xx/cpu.c +1 -7
@@ -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 ; }
board/amcc/yosemite/yosemite.c +2 -1
@@ -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 ; }
board/eltec/bab7xx/bab7xx.c +2 -2
@@ -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 */
board/eltec/elppc/elppc.c +1 -1
@@ -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 );
board/freescale/mpc8610hpcd/mpc8610hpcd.c +2 -1
@@ -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 ; }
board/freescale/mpc8641hpcn/mpc8641hpcn.c +2 -1
@@ -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
board/funkwerk/vovpn-gw/vovpn-gw.c +1 -4
@@ -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)
board/lwmon/lwmon.c +22 -0
@@ -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 ) {
board/lwmon5/lwmon5.c +2 -1
@@ -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 ; }
board/pcippc2/pcippc2.c +1 -1
@@ -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 ();
board/ppmc7xx/ppmc7xx.c +13 -5
@@ -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);
board/sbc8641d/sbc8641d.c +2 -1
@@ -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
include/configs/VoVPN-GW.h +0 -3
@@ -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
include/configs/lwmon5.h +0 -1
@@ -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
include/configs/yosemite.h +0 -1
@@ -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
Signature · 1 line
1.7.2.3
PATCH 17/21 sh: Unify duplicate reset code
In reply to Kyle MoffettAll 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.
--- 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(-)
arch/sh/cpu/sh2/cpu.c +0 -7
@@ -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) {
arch/sh/cpu/sh2/watchdog.c +0 -9
@@ -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 ) - ; -}
arch/sh/cpu/sh3/cpu.c +0 -7
@@ -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) {
arch/sh/cpu/sh3/watchdog.c +0 -9
@@ -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 ) - ; -}
arch/sh/cpu/sh4/cpu.c +0 -7
@@ -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 );
arch/sh/cpu/sh4/watchdog.c +0 -9
@@ -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 ) - ; -}
arch/sh/lib/board.c +10 -0
@@ -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" );
Signature · 1 line
1.7.2.3
PATCH 18/21 sh: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/sh/lib/board.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/sh/lib/board.c +1 -1
@@ -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 ();
Signature · 1 line
1.7.2.3
PATCH 19/21 sparc: Unify duplicate reset code
In reply to Kyle MoffettBoth 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.
--- 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(-)
arch/sparc/cpu/leon2/cpu.c +0 -18
@@ -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) {
arch/sparc/cpu/leon3/cpu.c +0 -19
@@ -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);
arch/sparc/lib/board.c +11 -0
@@ -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 ; +} +/************************************************************************/
Signature · 1 line
1.7.2.3
PATCH 20/21 sparc: Generic system restart support
In reply to Kyle MoffettThe 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.
--- arch/sparc/lib/board.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
arch/sparc/lib/board.c +11 -1
@@ -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 ; +} +/************************************************************************/
Signature · 1 line
1.7.2.3
PATCH 21/21 Remove legacy do_reset() function
In reply to Kyle MoffettAll 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().
--- common/cmd_boot.c | 27 +++------------------------ include/command.h | 1 - 2 files changed, 3 insertions(+), 25 deletions(-)
common/cmd_boot.c +3 -24
@@ -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" ,"" );
include/command.h +0 -1
@@ -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__))
Signature · 1 line
1.7.2.3
PATCH 21/21 Remove legacy do_reset() function
Hi Kyle
Quoted 7 lines · 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 todo_reset()may be removed.In addition the
do_generic_reset()function is renamed to the now-unused namedo_reset().
If it is unused, why not remove it completely?
Regards,
Graeme
Moffett
PATCH 21/21 Remove legacy do_reset() function
Hi!
Quoted 9 lines · Graeme Russ
Quoted 6 lines · 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 todo_reset()may be removed.In addition the
do_generic_reset()function is renamed to the now-unused namedo_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
PATCH 21/21 Remove legacy do_reset() function
Quoted 25 lines · Kyle D
Hi!
Quoted 9 lines · Graeme Russ
Quoted 6 lines · 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 todo_reset()may be removed.In addition the
do_generic_reset()function is renamed to the now-unused namedo_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:
Quoted 7 lines
=>
do_reset()?[Same as olddo_generic_reset()]
? =>system_restart()
? ? ?=>__board_restart()
? ? ?=>__arch_restart()Cheers,
Kyle Moffett
Go it - Thanks
Regards,
Graeme
cover Generic cross-architecture system restart support
In reply to Kyle MoffettHi Kyle,
Quoted 35 lines · 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 onpanic().The new functions to be called from generic code are:
?int
system_restart(void)
?voidemergency_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 olddo_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").
Quoted 7 lines
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
cover Generic cross-architecture system restart support
In reply to Kyle MoffettDear Kyle Moffett,
Quoted 10 lines · In message <[email protected]> you…
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 onpanic().The new functions to be called from generic code are:
int
system_restart(void)
voidemergency_restart(void)
Could you please explain what these functions are supposed to do?
Quoted 4 lines
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 olddo_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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I wish I had a bronze torc for every user who didn't read the manual.
- Terry Pratchett, _The Light Fantastic_