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

[Graeme Russ]

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

Good point

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

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

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

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

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

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

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

And panic() becomes:

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

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

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

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

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

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

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

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

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

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

Regards,

Graeme