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.
>>
>> Tested on socfpga_cyclone5_socrates.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
>> ---
>> v2:
>> - this patch is new in v2 of the series. It replaces the printf/debug
>> change in reset_manager_gen5.c from v1
>>
>> drivers/serial/ns16550.c | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index 9c80090aa7..475075c03c 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -266,12 +266,26 @@ static inline void _debug_uart_init(void)
>> serial_dout(&com_port->lcr, UART_LCRVAL);
>> }
>>
>> +static inline int NS16550_read_baud_divisor(struct NS16550 *com_port)
>> +{
>> + int ret;
>> +
>> + serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
>> + ret = serial_din(&com_port->dll) & 0xff;
>> + ret |= (serial_din(&com_port->dlm) & 0xff) << 8;
>> + serial_dout(&com_port->lcr, UART_LCRVAL);
>> +
>> + return ret;
>> +}
>> +
>> 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))
>> - ;
>> + 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 ?
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
