On 08/10/2018 12:35 AM, Andy Shevchenko wrote:
> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <ma...@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.goldschm...@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?
But why do we care about the divisor at all ? The real problem I believe
is that someone can call debug UART print/read functions before it is
inited.
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
