On Friday 01 July 2022 09:21:34 Stefan Roese wrote:
> On 23.06.22 14:13, Pali Rohár wrote:
> > Commit d293759d55cc ("serial: ns16550: Add support for
> > SPL_DEBUG_UART_BASE") fixed support for setting correct early debug UART
> > base address in SPL.
> >
> > But after this commit, output from Marvell A385 BootROM is truncated or
> > lost and not fully present on serial console.
> >
> > Debugging this issue showed that BootROM just put bytes into UART HW output
> > buffer and does not wait until UART HW transmit all characters. U-Boot
> > ns16550 early debug is initialized very early and during its initialization
> > is resetting UART HW and flushing remaining transmit buffer (which still
> > contains BootROM output).
> >
> > Fix this issue by waiting in init function prior resetting UART HW until
> > TxEmpty bit in UART Line Status Register is set. TxEmpty is set when all
> > remaining bytes from HW buffer are transmitted.
> >
> > Signed-off-by: Pali Rohár <p...@kernel.org>
>
> Just checking, do you need a "Fixes" tag as well here?
Well, ns16550_init() function already contains that waiting for
UART_LSR_TEMT bit. This was introduced in following commit years ago:
https://source.denx.de/u-boot/u-boot/-/commit/cb55b3320014b7f6014416c556fe506efbf0a84b
So probably the "fixes" tag could contain commit which introduced
_debug_uart_init() function. It was in this commit years ago:
https://source.denx.de/u-boot/u-boot/-/commit/21d004368fc8a4da07147c58dfe9a4e16d4ab761
Support for SPL_DEBUG_UART_BASE in my recent commit just caused to hit
issue that in _debug_uart_init() is missing waiting until TxEmpty.
> > ---
> > drivers/serial/ns16550.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> > index 78bfe6281ce3..13418004e2b0 100644
> > --- a/drivers/serial/ns16550.c
> > +++ b/drivers/serial/ns16550.c
> > @@ -328,6 +328,8 @@ static inline void _debug_uart_init(void)
> > struct ns16550 *com_port = (struct ns16550
> > *)CONFIG_VAL(DEBUG_UART_BASE);
> > int baud_divisor;
> > + while (!(serial_din(&com_port->lsr) & UART_LSR_TEMT));
>
> I personally prefer the ";" in a separate line instead. Not sure what
> the official coding style mentions to such constructs.
I have no problem with this change. If you prefer it, feel free to move
";" to next separate line.
> Anyways:
>
> Reviewed-by: Stefan Roese <s...@denx.de>
>
> Thanks,
> Stefan
