Hi Denis, Thank you for the patch.
On Tue, Sep 9, 2025 at 1:06 AM <[email protected]> wrote: > > From: Denis Mukhin <[email protected]> > > Add emulation logic for FCR register. > > Note, that does not hook FIFO interrupt moderation to the FIFO management > code for simplicity. > > Signed-off-by: Denis Mukhin <[email protected]> > --- > Changes since v6: > - dropped UART_IIR_THR handling from UART_FCR_CLTX case > --- > xen/common/emul/vuart/ns16x50.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c > index 137ce08f4e1d..a92df6923aa5 100644 > --- a/xen/common/emul/vuart/ns16x50.c > +++ b/xen/common/emul/vuart/ns16x50.c > @@ -374,6 +374,33 @@ static int ns16x50_io_write8( > regs[UART_IER] = val & UART_IER_MASK; > break; > > + case UART_FCR: /* WO */ > + if ( val & UART_FCR_RSRVD0 ) > + ns16x50_warn(vdev, "FCR: attempt to set reserved bit: %x\n", > + UART_FCR_RSRVD0); > + > + if ( val & UART_FCR_RSRVD1 ) > + ns16x50_warn(vdev, "FCR: attempt to set reserved bit: %x\n", > + UART_FCR_RSRVD1); Do we really need these checks and prints? > + > + if ( val & UART_FCR_CLRX ) > + { > + ns16x50_fifo_rx_reset(vdev); Note from the NS16550A datasheet: Bit 0: This bit must be a 1 when other FCR bits are written to, or they will not be programmed. > + regs[UART_LSR] &= ~UART_LSR_DR; > + } > + > + if ( val & UART_FCR_CLTX ) > + ns16x50_fifo_tx_reset(vdev); > + > + if ( val & UART_FCR_ENABLE ) > + val &= UART_FCR_ENABLE | UART_FCR_DMA | UART_FCR_TRG_MASK; Why can’t we just write back val as is, but with CLTX/CLRX cleared when UART_FCR_ENABLE is set? For example: regs[UART_FCR] = val & ~(UART_FCR_CLTX | UART_FCR_CLRX); > + else > + val = 0; If we take the above into account, I think we shouldn’t change the content of FCR in the case where bit 0 is 0. Also, from the spec: “Resetting FCR0 will clear all bytes in both FIFOs.” Best regards, Mykola > + > + regs[UART_FCR] = val; > + > + break; > + > case UART_LCR: > regs[UART_LCR] = val; > break; > -- > 2.51.0 > >
