[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 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>
> ---
> Changes in v2:
> - Made this feature conditional on CONFIG_PRE_CONSOLE_PANIC
>
>  README           |   11 +++++++++++
>  include/common.h |    8 ++++++++
>  lib/vsprintf.c   |   29 +++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/README b/README
> index eb9ade9..0748a6f 100644
> --- a/README
> +++ b/README
> @@ -634,6 +634,17 @@ The following options need to be configured:
>                'Sane' compilers will generate smaller code if
>                CONFIG_PRE_CON_BUF_SZ is a power of 2
>
> +- Pre-console Panic:
> +               Prior to the console being initialised, since console output
> +               is silently discarded, a panic() will cause no output and no
> +               indication of what is wrong. However, if the
> +               CONFIG_PRE_CONSOLE_PANIC option is defined, then U-Boot will
> +               call board_panic_no_console() in this case, passing the panic
> +               message. This function should try to output the message if
> +               possible, perhaps on all available UARTs (it will need to do
> +               this directly, since the console code is not functional yet).
> +               Another option is to flash some LEDs to indicate a panic.
> +
>  - Boot Delay:  CONFIG_BOOTDELAY - in seconds
>                Delay before automatically booting the default image;
>                set to -1 to disable autoboot.
> diff --git a/include/common.h b/include/common.h
> index 4c3e3a6..acc4030 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -277,6 +277,14 @@ int        last_stage_init(void);
>  extern ulong monitor_flash_len;
>  int mac_read_from_eeprom(void);
>
> +/*
> + * Called during a panic() when no console is available. The board should do
> + * its best to get a message to the user any way it can. This function should
> + * return if it can, in which case the system will either hang or reset.
> + * See panic().
> + */
> +void board_panic_no_console(const char *str);
> +
>  /* common/flash.c */
>  void flash_perror (int);
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 79dead3..56a2aef 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -24,6 +24,8 @@
>  # define NUM_TYPE long long
>  #define noinline __attribute__((noinline))
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  const char hex_asc[] = "0123456789abcdef";
>  #define hex_asc_lo(x)   hex_asc[((x) & 0x0f)]
>  #define hex_asc_hi(x)   hex_asc[((x) & 0xf0) >> 4]
> @@ -714,12 +716,39 @@ int sprintf(char * buf, const char *fmt, ...)
>        return i;
>  }
>
> +#ifdef CONFIG_PRE_CONSOLE_PANIC
> +/* Provide a default function for when no console is available */
> +static void __board_panic_no_console(const char *msg)
> +{
> +       /* Just return since we can't access the console */
> +}
> +
> +void board_panic_no_console(const char *msg) __attribute__((weak,
> +                                       alias("__board_panic_no_console")));
> +#endif
> +
>  void panic(const char *fmt, ...)
>  {
>        va_list args;
> +
>        va_start(args, fmt);

No need to add this extra blank line

> +#ifdef CONFIG_PRE_CONSOLE_PANIC
> +       {
> +               char str[CONFIG_SYS_PBSIZE];
> +
> +               /* TODO)sjg): Move to vsnprintf() when available */
> +               vsprintf(str, fmt, args);
> +               if (gd->have_console) {
> +                       puts(str);
> +                       putc('\n');
> +               } else {
> +                       board_panic_no_console(str);
> +               }
> +       }
> +#else
>        vprintf(fmt, args);
>        putc('\n');
> +#endif
>        va_end(args);
>  #if defined (CONFIG_PANIC_HANG)
>        hang();

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

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

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?

Regards,

Graeme