[U-Boot] [PATCH v2] Add board_panic_no_console to deal with early critical errors

[Graeme Russ]

Hi Simon,

On Tue, Oct 18, 2011 at 3:45 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Graeme,
>
> On Mon, Oct 17, 2011 at 8:24 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
>> Hi Simon,
>>
>> On Tue, Oct 18, 2011 at 1:16 PM, Simon Glass <sjg at chromium.org> wrote:
>>> This patch adds support for calling panic() before stdio is initialized.
>>> At present this will just hang with little or no indication of what the
>>> problem is.
>>>
>>> A new board_panic_no_console() 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 UARTS and spray the message out if
>>> it possibly can.
>>>
>>> The feature is controlled by a new CONFIG_PRE_CONSOLE_PANIC option.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>

[snip]

>>
>> Hmm, why not:
>>
>>
>>        char str[CONFIG_SYS_PBSIZE];
>>
>>        vsprintf(str, fmt, args);
>>        puts(str);
>>        putc('\n');
>>
>> #ifdef CONFIG_PRE_CONSOLE_PANIC
>>        if (!gd->have_console)
>>                board_panic_no_console(str);
>> #endif
>>
>> Since puts() and putc() will, effectively, be NOPs if !gd->have_console
>
> Thanks for looking at this.
>
> I was trying to avoid any code size increase, which is why I just
> #ifdefed the whole thing (yuk!). The above code increases code size by
> 16 bytes on ARM, due to the extra parameter, etc.
>
> I can't test for sure on upstream/master right now but one thing that
> causes an panic is trying to write to the console before it is ready -
> see the get_console() function in common/console.c. So calling
> puts/putc from within panic might again look for a console and again
> panic (infinite loop).

My squelch pre-console output and CONFIG_PRE_CONSOLE_BUFFER patches have
fixed all that

>> Actually, this could be made generic - board_puts_no_console() and move
>> into console.c - If a board has a way of generating pre-console output
>> then that can be utilised for all pre-console messaging, not just panic
>> messages
>
> Yes it could. However the board panic function could actually be quite
> destructive. For example it might have to sit there for ages flashing
> lights to indicate a panic. It isn't intended as a real printf(). If
> you had one of those ready, you would have used it :-)

True, but console is meant to come up ASAP so pre-console output would (in
theory) be minimal and only in abnormal conditions

>> Now it would be interesting to see what the compiler would do if you
>> dropped CONFIG_PRE_CONSOLE_PANIC and just had in console.c:
>>
>>        if (!gd->have_console)
>>                board_puts_no_console(str);
>>
>>
>> with no board_puts_no_console() - Would it see the empty weak function
>> and optimise the whole lot out?
>
> The compiler can't do this (sort of by definition of 'weak'). The
> linker could, but I don't think it does - you always end up with
> something - I suppose since it doesn't know whether your weak function
> is doing something or not, so doesn't optimise it out. We could use a
> weak function pointer and check if it is zero before calling it (=code
> size). I vaguely recall a linker option which can do something clever
> hear but can't remember what it was.

Ah, I see - Well still, we could do something like the following in
pre_console_putc()
	
static void pre_console_putc(const char c)
{
	char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR;

	buffer[CIRC_BUF_IDX(gd->precon_buf_idx++)] = c;

#ifdef CONFIG_PRE_CONSOLE_PUTC
	if (!gd->have_console)
		board_pre_console_putc(str);
#endif
}

It looks to me like there could be even smoother integration with
CONFIG_PRE_CONSOLE_BUFFER (i.e. allow CONFIG_PRE_CONSOLE_PUTC
with or without CONFIG_PRE_CONSOLE_BUFFER)

> So in summary - please let me know if 16 bytes is worth worrying
> about. If it is fine to increase code size a little, then I will redo
> this patch to clean it up at little as you suggest.

Thats a Wolfgang question, but I think it can be done without the overhead

Regards,

Graeme