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

Dear Simon Glass,

In message 1332188824-5447-2-git-send-email-sjg@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