[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 4:43 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> On Wed, Mar 21, 2012 at 10:22 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Graeme,
>>
>> On Tue, Mar 20, 2012 at 3:26 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
>>> Hi Simon,
>>>
>
>>>
>>> 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's panic_putc()
>
>> 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
>
> Agreed
>
>> 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.
>
> Ouch!

Indeed.

>
>> 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.
>
> OK, add a function to 'normalise' the pre console buffer by moving the
> characters so the string starts at the beginning of the buffer and then
> add an extra format tot he start of the panic string :) - But now the
> panic buffer needs to be bigger :(
>
> OK, maybe leave it up to the board code to dump the pre-console buffer...

Yes I think it is simpler for now, until we have a better framework.
Both the kernel and U-Boot need early access to information that
either might not arrive, or will only arrive later. This business and
the current hacks in the zImage wrapper are examples of problems that
need solving properly IMO. I am looking at these problems for the
U-Boot SPL case, so at some point this will get nicer in U-Boot. Not
sure about the kernel yet, but hope to avoid zImage for our next
iteration partly because of this nastiness.

>
> I dunno - It all seems a bit 'wrong' somehow

I think you are looking for a unified panic architecture with a
board-specific putc(). That was my previous patch and I just don't
think it works. This new one does solve the problem, avoids Wolfgangs
concerns about dangerous UART output, and is pretty simple to
implement. I believe there is a better way, but we are in the very
early days with device tree, life goes on, and this does work...

Regards,
Simon

>
> Regards,
>
> Graeme