Re: [PATCH v2 2/2] xen/ns16550: drain TX before reinitializing

[Zhang Zheng <zhangzheng@xxxxxxxxxxx>]

On Mon, Oct 12, 2026 at 03:00:19PM +0200, Jan Beulich wrote:
> On 12.10.2026 12:20, Oleksii Kurochko wrote:
> > On 10/10/26 5:14 AM, Zhang Zheng wrote:
> >> --- a/xen/drivers/char/ns16550.c
> >> +++ b/xen/drivers/char/ns16550.c
> >> @@ -427,6 +427,10 @@ static void __init cf_check 
> >> ns16550_init_preirq(struct serial_port *port)
> >>   #endif
> >>       }
> >>   
> >> +    /* Preserve output queued by firmware before taking over the UART. */
> > я'
> > Could the comment say why the wait is needed at this point? Something
> > like:
> > 
> >      /*
> >       * Output written before ns16550 takes over (e.g. by firmware or the
> >       * early console) may still be in the transmitter. Let it drain before
> >       * ns16550_setup_preirq() reprograms the line settings and resets the
> >       * FIFOs.
> >       */
> > 
> >> +    while ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT) )
> >> +        cpu_relax();
> >> +
> > 
> > IMO waiting for TEMT rather than THRE is the right choice: besides
> > resetting the FIFOs, ns16550_setup_preirq() reprograms the divisor and
> > LCR, which would corrupt a character still being shifted out. (Linux
> > waits for UART_LSR_BOTH_EMPTY, i.e. THRE | TEMT, but on a 16550 TEMT
> > already implies THRE.)
> 
> Then again we have the .lsr_mask field. Not using it here would then call
> for justification in the description.
> 
> Jan

I will clarify this distinction in the commit message.

Zhang