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

[dmukhin@xxxxxxx]

On Sun, Sep 07, 2025 at 07:50:34PM -0700, dmukhin@xxxxxxx wrote:
> On Fri, Sep 05, 2025 at 06:59:30PM -0700, Stefano Stabellini wrote:
> > On Fri, 5 Sep 2025, dmukhin@xxxxxxx wrote:
> > > From: Denis Mukhin <dmukhin@xxxxxxxx> 
> [..]
> > > @@ -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!

Clarification: I reworked the code to report UART_IIR_THR based on
ns16x50_fifo_tx_full() status.

> 
> > 
> > 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?
>