Hi Graeme,
On Tue, Mar 20, 2012 at 3:26 PM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Simon,
On Tue, Mar 20, 2012 at 7:27 AM, Simon Glass sjg@chromium.org wrote:
This patch adds support for console output in the event of a panic() before the console is inited. The main purpose of this is to deal with a very early panic() which would otherwise cause a silent hang.
A new board_pre_console_panic() function is added to the board API. If provided by the board it will be called in the event of a panic before the console is ready. This function should turn on all available UARTs and output the string (plus a newline) if it possibly can.
Signed-off-by: Simon Glass sjg@chromium.org
void panic(const char *fmt, ...) {
- char str[CONFIG_SYS_PBSIZE];
va_list args;
va_start(args, fmt);
- vprintf(fmt, args);
- putc('\n');
- vsnprintf(str, sizeof(str), fmt, args);
va_end(args);
- if (gd->have_console) {
- puts(str);
- putc('\n');
- } else {
- board_pre_console_panic(str);
- }
Would there be benefit in including an option to dump the pre-console buffer (if one is enabled) - I think it's contents could be rather useful in debugging what went wrong...
And something is nagging at me that the API is just 'not right'. I don't know exactly why, but I really don't like how this is plumbed
panic() calls putc() which, if we do not have a console (and we won't in the case of the early panic case we are dealing with), will be put into the pre console buffer (if enabled)
So having panic() call a board specific function to dump the pre console buffer after the vprintf() / putc() would achieve the same result (but you need pre console buffer enabled)
So, if CONFIG_PRE_CONSOLE_BUFFER is defined, we could simply add a call to dump_pre_console_buffer() at the end of panic(). But we already have a function to do that - print_pre_console_buffer(). If we added an argument to print_pre_console_buffer() which is a pointer to a 'putc()' type function (NULL meaning the standard putc()) and that function lived in the board files then life would be good. We could also add a call to a setup function so we are not setting up the UARTS every putc (not that performance is a priority at this stage, but some boards might quibble about hitting certain registers continuously)
But what to do if pre console buffer is not defined? The panic message needs to be sent directly to the 'panic UARTS'...
OK, so what about in panic(): - If gd->have_console is not set: o call the board specific setup_panic_uarts() o call print_pre_console_buffer() passing panic_putc() o call panic_putc() for all characters in str[] - If gd->have_console is set: o call putc() for all characters in str[]
setup_panic_uarts() and panic_putc() are overriden in the board files
I think this is where we got to last time.
The act of calling this pre-console panic function is destructive - it may hang the board and output data to UARTs.
I think I understand the scheme you propose. But setup_panic_uarts() puts the board into a funny state (e.g. may set up or change clocks and pinmux). You then need a board_pre_console_putc() to actually output the characters - you don't mention that. That was the patch that I reverted :-( Yes I understand that you can separate out the UART init from the part that outputs the characters, but does that really help?
To put it another way, I think we need to stick with the idea that this is a panic, not a normal situation. That means that return from the board panic output function may be difficult or impossible, and so a putc() arrangement is not a good idea.
For example, I have another board on which there are two possible oscillator clock settings - and we don't know which to use, and the arch clock code has not yet been called! In that case I want the board_pre_console_panic() function to output the string using both options, so that one will produce a message for the user (the other will likely produce just a few characters of garbage because the baud rate is wrong). If we output only a single character then the garbage and good characters will be interspersed.
So, can we get away from the idea that this is a reliable and stable way of outputting characters before the console is ready? If you want the pre-console output, I'm sure we can provide a way of accessing it which the board pre-console panic function can use.
Regards, Simon
Regards,
Graeme