+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