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
