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

[Adam Ford]

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.
>
> Tested on socfpga_cyclone5_socrates.
>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at 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.

just my two-cents.

adam
> +                       return;
> +       }
>         serial_dout(&com_port->thr, ch);
>  }
>
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot