| Message ID | 1299519462-25320-12-git-send-email-Kyle.D.Moffett@boeing.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Kyle On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote: > The i386 port has its own reset_cpu() dispatch for its various supported > CPU families, so the existing do_reset() function is simply altered to > use the new prototype for __arch_restart(). > > In addition, the debug message and delay are duplicated from the generic > code, so they are removed. > > This reset code will probably work even when the CPU is in a bad state, > so no separate __arch_emergency_restart() function is required. This part does not make much sense - If the CPU is in 'a bad state' then it will probably be lights out anyway. As I understand it, an emergency restart is a restart not initiated by the user (divide by zero, unhandled exception etc), in which case i386 will make use of it Regards, Graeme
Hi! On Mar 07, 2011, at 16:54, Graeme Russ wrote: > On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote: >> The i386 port has its own reset_cpu() dispatch for its various supported >> CPU families, so the existing do_reset() function is simply altered to >> use the new prototype for __arch_restart(). >> >> In addition, the debug message and delay are duplicated from the generic >> code, so they are removed. >> >> This reset code will probably work even when the CPU is in a bad state, >> so no separate __arch_emergency_restart() function is required. > > This part does not make much sense - If the CPU is in 'a bad state' then > it will probably be lights out anyway. As I understand it, an emergency > restart is a restart not initiated by the user (divide by zero, unhandled > exception etc), in which case i386 will make use of it I was considering unhandled exceptions, etc. to be "a bad state" :-D. Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default. Hopefully I am making sense now? Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand? Cheers, Kyle Moffett
Hi Kyle On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote: > Hi! > > On Mar 07, 2011, at 16:54, Graeme Russ wrote: >> On Tue, Mar 8, 2011 at 4:37 AM, Kyle Moffett <Kyle.D.Moffett@boeing.com> wrote: >>> The i386 port has its own reset_cpu() dispatch for its various supported >>> CPU families, so the existing do_reset() function is simply altered to >>> use the new prototype for __arch_restart(). >>> >>> In addition, the debug message and delay are duplicated from the generic >>> code, so they are removed. >>> >>> This reset code will probably work even when the CPU is in a bad state, >>> so no separate __arch_emergency_restart() function is required. >> >> This part does not make much sense - If the CPU is in 'a bad state' then >> it will probably be lights out anyway. As I understand it, an emergency >> restart is a restart not initiated by the user (divide by zero, unhandled >> exception etc), in which case i386 will make use of it > > I was considering unhandled exceptions, etc. to be "a bad state" :-D. > > Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default. > > Hopefully I am making sense now? Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand? > I understand what you are doing now, thanks. I think you can scrap this part of the description and I will have i386 start using __arch_emergency_restart() for 'Internal U-Boot Errors' such as divide by zero, unhandled exception, general protection faults etc I don't particularly like the 'emergency' naming - It's like if we don't do it things will blow up :) I think 'automatic' might be a closer term Is there anywhere yet where the code paths for the emergency and non emergency variants differ in the way they end up resetting the board? Regards, Graeme
On Mar 07, 2011, at 17:26, Graeme Russ wrote: > On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote: >> On Mar 07, 2011, at 16:54, Graeme Russ wrote: >>> This part does not make much sense - If the CPU is in 'a bad state' then >>> it will probably be lights out anyway. As I understand it, an emergency >>> restart is a restart not initiated by the user (divide by zero, unhandled >>> exception etc), in which case i386 will make use of it >> >> I was considering unhandled exceptions, etc. to be "a bad state" :-D. >> >> Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default. >> >> Hopefully I am making sense now? Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand? > > I understand what you are doing now, thanks. > > I think you can scrap this part of the description and I will have i386 > start using __arch_emergency_restart() for 'Internal U-Boot Errors' such > as divide by zero, unhandled exception, general protection faults etc > > I don't particularly like the 'emergency' naming - It's like if we don't > do it things will blow up :) I think 'automatic' might be a closer term The name "emergency_restart()" was borrowed from the Linux kernel; the kernel shuts down disks and network interfaces prior to a regular restart but not during an emergency restart. The best analogy is to the "EMERG" log level (KERN_EMERG inside the Linux kernel). Furthermore, you should *not* directly call __arch_emergency_restart(), that is simply the architecture hook you provide to the generic code. Your exception handlers should call "emergency_restart()" to allow board-specific code to override the reboot, for example by triggering an onboard watchdog to reset other hardware. EG: => some_i386_trap_handler() => emergency_restart() => __board_emergency_restart() => __arch_emergency_restart() => do_reset() [The new, generic version] => system_restart() => __board_restart() => __arch_restart() The __{board,arch}_restart(), etc are just predefined weak functions that can be overridden by the implementation. For example, my HWW-1U-1A board file effectively does this: int __board_restart(void) { while (poll_external_software()) { if (ctrlc()) return -1; } return 0; } void __board_emergency_restart(void) { while (poll_external_software_uninterruptible()) ; } During a normal restart, I allow the polling to be interrupted with Ctrl-C, but during an emergency restart I don't. In both cases, my function just *returns* to allow the default MPC85xx code to actually perform the hardware-level restart (by poking at the "reset request" bit in a CPU register). > Is there anywhere yet where the code paths for the emergency and non > emergency variants differ in the way they end up resetting the board? There are several U-Boot board ports whose do_reset() functions (now called "__board_restart()") just perform an unconditional jump to "_start" or a FLASH "soft-reset" address. If the system has experienced an unexpected exception or other problem then that is not safe, and it would be better to just hang(). Cheers, Kyle Moffett
On Tue, Mar 8, 2011 at 9:57 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote: > On Mar 07, 2011, at 17:26, Graeme Russ wrote: >> On Tue, Mar 8, 2011 at 9:06 AM, Moffett, Kyle D <Kyle.D.Moffett@boeing.com> wrote: >>> On Mar 07, 2011, at 16:54, Graeme Russ wrote: >>>> This part does not make much sense - If the CPU is in 'a bad state' then >>>> it will probably be lights out anyway. As I understand it, an emergency >>>> restart is a restart not initiated by the user (divide by zero, unhandled >>>> exception etc), in which case i386 will make use of it >>> >>> I was considering unhandled exceptions, etc. to be "a bad state" :-D. >>> >>> Maybe I didn't explain it well enough in the patch summary, but basically the default for "__arch_emergency_restart()" is to just call "__arch_restart()". Since the i386 "__arch_restart()" function should work fine even when U-Boot is in trouble, that architecture does not need to override the default. >>> >>> Hopefully I am making sense now? Should I reword it from "when the CPU is in a bad state" to "when U-Boot is in trouble", or is there something else that would be easier to understand? >> >> I understand what you are doing now, thanks. >> >> I think you can scrap this part of the description and I will have i386 >> start using __arch_emergency_restart() for 'Internal U-Boot Errors' such >> as divide by zero, unhandled exception, general protection faults etc >> >> I don't particularly like the 'emergency' naming - It's like if we don't >> do it things will blow up :) I think 'automatic' might be a closer term > > The name "emergency_restart()" was borrowed from the Linux kernel; the kernel shuts down disks and network interfaces prior to a regular restart but not during an emergency restart. The best analogy is to the "EMERG" log level (KERN_EMERG inside the Linux kernel). OK, makes sense to stick with emergency then > > Furthermore, you should *not* directly call __arch_emergency_restart(), that is simply the architecture hook you provide to the generic code. Your exception handlers should call "emergency_restart()" to allow board-specific code to override the reboot, for example by triggering an onboard watchdog to reset other hardware. > > EG: > > => some_i386_trap_handler() > => emergency_restart() > => __board_emergency_restart() > => __arch_emergency_restart() > > => do_reset() [The new, generic version] > => system_restart() > => __board_restart() > => __arch_restart() > OK > The __{board,arch}_restart(), etc are just predefined weak functions that can be overridden by the implementation. > > For example, my HWW-1U-1A board file effectively does this: > > int __board_restart(void) > { > while (poll_external_software()) { > if (ctrlc()) > return -1; > } > return 0; > } > > void __board_emergency_restart(void) > { > while (poll_external_software_uninterruptible()) > ; > } > > During a normal restart, I allow the polling to be interrupted with Ctrl-C, but during an emergency restart I don't. In both cases, my function just *returns* to allow the default MPC85xx code to actually perform the hardware-level restart (by poking at the "reset request" bit in a CPU register). > > >> Is there anywhere yet where the code paths for the emergency and non >> emergency variants differ in the way they end up resetting the board? > > There are several U-Boot board ports whose do_reset() functions (now called "__board_restart()") just perform an unconditional jump to "_start" or a FLASH "soft-reset" address. If the system has experienced an unexpected exception or other problem then that is not safe, and it would be better to just hang(). > OK - All sounds good to me. I'll try run it all up on my board 'soon' Regards, Graeme
diff --git a/arch/i386/cpu/cpu.c b/arch/i386/cpu/cpu.c index 2339cd4..b1454ba 100644 --- a/arch/i386/cpu/cpu.c +++ b/arch/i386/cpu/cpu.c @@ -122,10 +122,8 @@ int x86_cpu_init_r(void) } int cpu_init_r(void) __attribute__((weak, alias("x86_cpu_init_r"))); -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +int __arch_restart(void) { - printf ("resetting ...\n"); - udelay(50000); /* wait 50 ms */ disable_interrupts(); reset_cpu(0);
The i386 port has its own reset_cpu() dispatch for its various supported CPU families, so the existing do_reset() function is simply altered to use the new prototype for __arch_restart(). In addition, the debug message and delay are duplicated from the generic code, so they are removed. This reset code will probably work even when the CPU is in a bad state, so no separate __arch_emergency_restart() function is required. Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com> Cc: Graeme Russ <graeme.russ@gmail.com> --- arch/i386/cpu/cpu.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)