On 21/03/11 23:00, Wolfgang Denk wrote: > Dear Graeme Russ, > > In message <4d8739f6.5040...@gmail.com> you wrote: >> >> I kind of like the idea of different reset sources (CPU exception, hardware >> failure, user initiated) but agree copying the linux architecture is over >> the top. > > What's the difference as far as do_reset() is concenred? It shall > just (hard) reset the system, nothing else. > >> Is there any reason reset() could not take a 'reason' parameter? It could >> be a bit-mask with CPU, SOC and arch reserved bits (unhandled exception, >> user initiated, panic etc) and board specific bits > > What for? To perform the intended purpose, no parameter is needed. > >> Board or arch specific code could handle different reasons however they >> please (like logging it in NVRAM prior to restart, gracefully shutting down >> multiple CPU's, clearing DMA buffers etc) > > That would be a layer higher than do_reset() (for example, in > panic()).
Hmmm, but panic() is defined in lib/vsprintf.c with no possibility for it to be overridden in any arch or board specific way > >> All 'hang', 'panic', 'reset' etc code can be simplified into a single code >> path (although calling 'reset' to 'hang' is a bit odd) > > hang() and reset() are intentionally very different things. A call to > hang() is supposed to hang (surprise, surprise!) infinitely. It must > not cause a reset. As I said, calling reset() to hang is odd :) > > panic() is a higher software layer. It probably results in calling > reset() in the end. > Unless CONFIG_PANIC_HANG is defined... Looking into the code panic(): - ~130 call sites - Implemented in lib/vsprintf.c - Calls do_reset() if CONFIG_PANIC_HANG is not defined - Calls hang() if CONFIG_PANIC_HANG is defined hang(): - ~180 call sites using hang() and hang () - Implemented in arch\lib\board.c - ARM, i386, m68k, microblaze, mips, prints "### ERROR ### Please RESET the board ###\n" and loops - avr32 prints nothing and loops - Blackfin can set status LEDs based on #define, prints "### ERROR ### Please RESET the board ###\n" and triggers a breakpoint if a JTAG debugger is connected - nios2 disables interrupts then does the same as ARM et all - powerpc is similar to ARM et al but also calls show_boot_progress(-30) - sh - same as ARM et al but prints "Board ERROR\n" - sparc - if CONFIG_SHOW_BOOT_PROGRESS defined, same as powerpc, else same as ARM et al So hang() could easily be weakly implemented in common\ which would allow arch and board specific overrides do_reset(): - ~70 call sites - 39 implemetations - Implemented all over the place (arch/cpu/, arch/lib, board/) - Only ARM is in arch/lib which could likely be moved to arch/cpu - Is U_BOOT_CMD - m68k has a number of very similar/identical implementations - Is not weak - Cannot be overridden at the board level - Ouch, PPC is real ugly: #if !defined(CONFIG_PCIPPC2) && \ !defined(CONFIG_BAB7xx) && \ !defined(CONFIG_ELPPC) && \ !defined(CONFIG_PPMC7XX) /* no generic way to do board reset. simply call soft_reset. */ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ... Boards can thus provide their own do_reset() - Wow, m68k/cpu/mcf52x2/cpu.c is even uglier - 7 definitions selectable by #ifdef's - Because do_reset() is U_BOOT_CMD, practically every call is: do_reset (NULL, 0, 0, NULL) - do_bootm() does pass it's args onto do_reset() - No implementation of do_reset() uses any args anyway reset(): - does not exist (as far as I can tell) So, maybe we could replace do_reset() with a weak reset() function defined in arch/cpu which can be overridden at the board level. All existing calls to do_reset() can be converted to simply reset() The U_BOOT_CMD do_reset() would simply call reset() Could many of the current calls to do_reset() be replaced with calls to panic()? I still like the idea of passing a 'reason' to reset() / panic() - Could we change panic() to: void panic(u32 reason, const char *fmt, ...) { va_list args; va_start(args, fmt); vprintf(fmt, args); putc('\n'); va_end(args); if (reason |= PANIC_FLAG_HANG) hang(reason); else reset(reason); } Anywhere in the code that needed to hang has a choice - hang(reason) or panic(reason | PANIC_FLAG_HANG) Default implementations of hang() and reset() would just ignore reason. Board specific code can use reason to do one last boot_progress(), set LED states etc. Regards, Graeme _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot