+Graeme
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:08 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message CAPnjgZ1=KK9UNy2fyk9WJ-gcyYE5JgwqO_cT8igXjD-OFP_h6w@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: wd@denx.de "Consistency requires you to be as ignorant today as you were a year ago." - Bernard Berenson