+Graeme
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:08 PM, Wolfgang Denk <w...@denx.de> wrote:
> Dear Simon Glass,
>
> In message
> <CAPnjgZ1=kk9uny2fyk9wj-gcyye5jgwqo_ct8igxjd-ofp_...@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.
Yes I agree, but I can't move them out with the API as it is. If we
move back to a puts() type function then I could do this.
>
>> 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.
It's not great, but let's work out something that is better.
>
> 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?
Only on a fatal error. Unfortunately this idea of 'fatal error' was
lost in the conversion from board_panic_no_console() to
board_pre_console_putc(). I would be keen to move back to that
original plan, so that the idea of a fatal error is captured. In a
fatal error situation there is no prospect of the board working and
the only hope is that we can alert the user.
>
> 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 agres with the sentiment and this is a very ugly corner case, but in
practice people want to know what happened, not just be presented with
a brick.
>
> 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.
OK so how about moving to a puts()-type interface? Then I can remove this.
Regards,
Simon
>
> 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: w...@denx.de
> "Consistency requires you to be as ignorant today as you were a year
> ago." - Bernard Berenson
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
