[U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init

[Andy Shevchenko]

On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <marex at denx.de> wrote:
> On 08/09/2018 11:13 PM, Adam Ford wrote:
>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt
>> <simon.k.r.goldschmidt at gmail.com> wrote:
>>>
>>> If _debug_uart_putc() is called before _debug_uart_init(), the
>>> ns16550 debug uart driver hangs in a tight loop waiting for the
>>> tx FIFO to get empty.
>>>
>>> As this can happen via a printf sneaking in before the port calls
>>> debug_uart_init(), let's rather ignore characters before the debug
>>> uart is initialized.
>>>
>>> This is done by reading the baudrate divisor and aborting if is zero.

>>>  static inline void _debug_uart_putc(int ch)
>>>  {
>>>         struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE;

>>> +       while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) {
>>> +               if (!NS16550_read_baud_divisor(com_port))
>>
>> Unless there is a change that the read_baud_divisor will change while
>> we're waiting for the character, could we move this check before the
>> while statement?  This would reduce the check for the divisor to 1x
>> and the while statement would only have one comparison to do.  I
>> realize it's rather trivial, but the way I see it, there is no reason
>> to do the while statement at all if the read_baud_divisor fails and
>> there if there is a baud divisor, we should only need to check it
>> once.
>
> This looks like a massive hack -- what about having a flag which says
> that the debug uart was/was not inited somewhere ?

Agree, why not to cache divisor value, for example, instead of doing slow I/O?

-- 
With Best Regards,
Andy Shevchenko