On Wed, Mar 23, 2011 at 12:28 AM, Wolfgang Denk <w...@denx.de> wrote: > Dear Graeme Russ, > > In message <4d88909a.80...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot