[U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ1=KK9UNy2fyk9WJ-gcyYE5JgwqO_cT8igXjD-OFP_h6w at mail.gmail.com> you wrote:
> 
> >> +void board_pre_console_putc(int ch)
> >> +{
> > ...
> >> +     for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
> >> +             NS16550_t regs = (NS16550_t)*uart_addr;
> >> +
> >> +             NS16550_init(regs, divisor);
> >> +             NS16550_putc(regs, ch);
> >> +             if (ch == '\n')
> >> +                     NS16550_putc(regs, '\r');
> >> +             NS16550_drain(regs);
> >
> > Why is this needed for every output character?
> >
> > Actually, why is it needed at all?
>
> Of course in this case the init could be done for each UART at the
> start of the function rather than in the loop, by looping through the
> UARTs twice.

Both the init() and the drain() should never be used in a character
output loop.

> After requests on the list for general purpose pre-console output
> function (the purpose of which I didn't necessary see) I changed this
> to a putc() mechanism. This means that we need to set up the UARTs
> each time it is called. We can't really add a flag to global data
> since this might be called before that is even set up. There might be
> a better way, but I'm not sure what it is.
>
> For SPL I would like to be able use this same mechanism to call
> panic() when something goes wrong, and use the same mechanism to get a
> message to the user. Again, this avoids a bricked unit with no failure
> indication.

I understand your intention, but I disagree with the design, and with
the implementation.

I think outputting data to all "potential" console ports is a really
bad thing, as you cannot know how your users are using the hardware.
They may have attached hardware to the UARTs, and the data you send to
the port causes a mis-function of the device.  I guess you don't add
to your documentation a warning like: select one port as console, and
leave allother ports unused, because we may send random date to all
ports any time?

If you do not know which UART port to use, then the only consequence
can be not to use any UART prt at all.  Use a LED with blink codes or
whatever your hardwar ehas, but do not mess with random ports.

I also cannot understand why you think you must init() and drain() the
UART for each character printed - ok, the latter is probably a
consequence of re-initializing the port for each character.
Eventually this will be not needed once you get rid of the re-init.

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
"Consistency requires you to be as ignorant today as you were a  year
ago."                                              - Bernard Berenson