
Hi Marek,
On Thu, 10 Jul 2014 14:37:18 +0200 Marek Vasut marex@denx.de wrote:
On Thursday, July 10, 2014 at 01:31:01 PM, Masahiro Yamada wrote:
Hi Marek,
Hi!
On Thu, 10 Jul 2014 12:14:35 +0200
Marek Vasut marex@denx.de wrote:
+static void uniphier_serial_putc(struct uniphier_serial *port, const char c) +{
- if (c == '\n')
uniphier_serial_putc(port, '\r');
- while (!(readb(&port->lsr) & UART_LSR_THRE))
;
I think in this function, you can avoid such completely unbounded loop.
[...]
Why? Could you give me more detailed explanation?
In case the LSR_THRE bit would never get cleared, this loop would keep spinning forever. On the other hand, if there was some kind of a timeout, U-Boot would be able to continue doing at least something when exiting this loop by timing out. Though you're right that something would most likely have gone really bonkers if this LSR_THRE never got cleared.
I guess you mentioned the case where the flow control with CTS/RTS results in the dead lock.
Either that or plain hardware failure.
I am not sure if we need to be so careful.
Is there any other UART driver using timeout check?
I don't know. If you feel this is not needed, then please just ignore my suggestion. I'm just pushing the usual "let's not do unbounded loops where reasonable" thing here ;-)
(What would U-Boot continue to do in case console cannot display the error message?)
It can still boot kernel, even if it cannot print on console.
Finally, I decided to keep this as is because:
[1] Our on-chip UART do not have the flow control feature. It uses only TX/RX signals.
[2] So, the eternal loop occurrs only when the hardware is broken. In this case, the situation is really fatal and I don't think it is reasonable to continue to do something.
[3] I checked some (but not all) other serial drivers but I could not find ones using timeout and break thing.
Best Regards Masahiro Yamada