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

[Wolfgang Denk]

Dear Simon Glass,

In message <1332188824-5447-2-git-send-email-sjg at chromium.org> you 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.

In addition to the more constructive comments made already by Grame, I
object against this patch because I dislike the whole concept.

> +- Pre-console panic():
> +		Prior to the console being initialised, console output is
> +		normally silently discarded. This can be annoying if a
> +		panic() happens in this time. One reason for this might be

True.  This is the reason why it has always been an important design
rule for U-Boot to initialize the console port as soon as possible.

> +		that you have CONFIG_OF_CONTROL defined but have not
> +		provided a valid device tree for U-Boot. In general U-Boot's
> +		console code cannot resolve this problem, since the console
> +		has not been set up, and it may not be known which UART is
> +		the console anyway (for example if this information is in
> +		the device tree).

Please make up your mind:  either you CAN initialize the console, then
you can use it to output messages in a regular way, or you CANNOT
initialize it, in which case you cannot print anything.  There is no
third option.

> +		The solution is to define a function called
> +		board_pre_console_panic() for your board, which alerts the
> +		user however it can. Hopefuly this will involve displaying
> +		a message on available UARTs, or perhaps at least flashing

If you have "available UARTs", you could use this as console, right?

In the previous discussion you explained the situation differently -
you were talking about _any_ UARTs that might be present on the
hardware, without caring about twhat these might actually be used for.

I explained that such an approach is highly dangerous.  I do not want
you toi set a precedent for such stle and code.

> +		an LED. The function should be very careful to only output
> +		to UARTs that are known to be unused for peripheral
> +		interfaces, and change GPIOs that will have no ill effect
> +		on the board. That said, it is fine to do something
> +		destructive that will prevent the board from continuing to
> +		boot, since it isn't going to...

Sorry, but this is bogus. Either you know the console port, or you
don't.  If there is a free UART, it might be attached to custome
hardware you have no idea about.  Ditto for GPIOs.  Please do not
meddle with device state in arbitrary ways.  If there are LED ports on
that board that are intended to signalize status information then it
makes sense to use these - but do not use any other ports that might
be present on the hardware.

> +		The function will need to output serial data directly,
> +		since the console code is not functional yet. Note that if

This is broken design.  If you can initialize the UART as console,
then doi so and use it in the regular way.

> +		the panic happens early enough, then it is possible that
> +		board_init_f() (or even arch_cpu_init() on ARM) has not
> +		been called yet. You should init all clocks, GPIOs, etc.
> +		that are needed to get the string out. Baud rates will need
> +		to default to something sensible.

Again, this is broken design.  We cannot try to catch errors sooner
and sooner and soner, and if needed initialization steps have not been
executed yet, provide additional code for emergency initialization.
We have regular console code, and now we have pre_console_*() stuff,
and soon we will have pre_pre_pre_pre_pre_pre_pre_pre_console_*()
stuff ? NAK.

Just stick to the design principles, and make sure there is as few
stuff that could possibly go wrong before console initialization as
possible.  Than all this crap (excuse me, but I don;t find a better
word) will not be needed.


I also dislike the modifications to the common code.


I think you are trying to solve an unsolvable problem.  If you cannot
accept that the board gets stuck without printing anything on the
debug console port because there are situations when you don't know
which port this is, then you simply should _define_ and _document_ a
single port as debug console port.  Then initialize and use this in
the regular way.  If later information (like from a loaded device
tree) means the console gets switched to anothe rport, that's OK.
That's what Linux will do as well if you assign another console port
there.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Gods don't like people not doing much work. People  who  aren't  busy
all the time might start to _think_.  - Terry Pratchett, _Small Gods_