Re: [PATCH v6 10/15] emul/ns16x50: implement THR register

[dmukhin@xen.org]

On Fri, Sep 05, 2025 at 06:59:30PM -0700, Stefano Stabellini wrote:
> On Fri, 5 Sep 2025, dmukhin@xen.org wrote:
> > From: Denis Mukhin <dmukhin@ford.com> 
[..]
> > @@ -439,6 +523,16 @@ static int ns16x50_io_read8(
> >  
> >          case UART_IIR: /* RO */
> >              val = ns16x50_iir_get(vdev);
> > +
> > +            /*
> > +             * Since there's no baud rate emulation, transmits are immediate
> > +             * to the guest. Clear IIR scratch location to make sure there
> > +             * will be interrupt generated once guest re-enabled ETHREI in
> > +             * IER.
> > +             */
> > +            if ( val & UART_IIR_THR )
> > +                regs[NS16X50_REGS_NUM + UART_IIR] &= ~UART_IIR_THR;
> 
> Why clear UART_IIR_THR here?
> 
> UART_IIR_THR should be set if the out buffer is not full and should not
> be set of the out buffer is full?

Now that the THR/FCR register emulation _may_ clear UART_IIR_THR, clearing
UART_IIR_THR here is not needed. 

Thanks for the catch!

> 
> Given that the only function adding to out is ns16x50_fifo_tx_putchar,
> and given that ns16x50_fifo_tx_putchar clears the out buffer when full
> by calling ns16x50_fifo_tx_flush if ns16x50_fifo_tx_full, then basically
> we can keep UART_IIR_THR set all the time?