[U-Boot] [PATCH 2/5] Add board_panic_no_console() to deal with early critical errors

[Simon Glass]

Hi Graeme,

On Tue, Mar 20, 2012 at 3:26 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Mar 20, 2012 at 7:27 AM, Simon Glass <sjg at 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 at 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