On Fri, May 1, 2026 at 2:05 AM ~lexbaileylowrisc <[email protected]> wrote: > > From: Lex Bailey <[email protected]> > > The register definitions for the UART device need to be updated to match > the documentation at https://opentitan.org/book/hw/ip/uart/doc/registers.html > > This commit does that, and also switches to using Fifo8 for managing the > transmit and receive buffers. > > Signed-off-by: Lex Bailey <[email protected]> > --- > hw/char/ot_uart.c | 338 ++++++++++++++++++++++---------------- > include/hw/char/ot_uart.h | 18 +- > 2 files changed, 201 insertions(+), 155 deletions(-) > > diff --git a/hw/char/ot_uart.c b/hw/char/ot_uart.c > index 9478af40f9..3fe7c5efe5 100644 > --- a/hw/char/ot_uart.c > +++ b/hw/char/ot_uart.c > @@ -27,6 +27,7 @@ > > #include "qemu/osdep.h" > #include "hw/char/ot_uart.h" > +#include "qemu/fifo8.h" > #include "hw/core/irq.h" > #include "hw/core/qdev-clock.h" > #include "hw/core/qdev-properties.h" > @@ -37,16 +38,22 @@ > #include "qemu/module.h" > > REG32(INTR_STATE, 0x00) > - FIELD(INTR_STATE, TX_WATERMARK, 0, 1) > - FIELD(INTR_STATE, RX_WATERMARK, 1, 1) > - FIELD(INTR_STATE, TX_EMPTY, 2, 1) > - FIELD(INTR_STATE, RX_OVERFLOW, 3, 1) > + SHARED_FIELD(INTR_TX_WATERMARK, 0, 1) > + SHARED_FIELD(INTR_RX_WATERMARK, 1, 1) > + SHARED_FIELD(INTR_TX_DONE, 2, 1) > + SHARED_FIELD(INTR_RX_OVERFLOW, 3, 1) > + SHARED_FIELD(INTR_RX_FRAME_ERR, 4, 1) > + SHARED_FIELD(INTR_RX_BREAK_ERR, 5, 1) > + SHARED_FIELD(INTR_RX_TIMEOUT, 6, 1) > + SHARED_FIELD(INTR_RX_PARITY_ERR, 7, 1) > + SHARED_FIELD(INTR_TX_EMPTY, 8, 1) > REG32(INTR_ENABLE, 0x04) > REG32(INTR_TEST, 0x08) > REG32(ALERT_TEST, 0x0C) > + FIELD(ALERT_TEST, FATAL_FAULT, 0, 1) > REG32(CTRL, 0x10) > - FIELD(CTRL, TX_ENABLE, 0, 1) > - FIELD(CTRL, RX_ENABLE, 1, 1) > + FIELD(CTRL, TX, 0, 1) > + FIELD(CTRL, RX, 1, 1) > FIELD(CTRL, NF, 2, 1) > FIELD(CTRL, SLPBK, 4, 1) > FIELD(CTRL, LLPBK, 5, 1) > @@ -58,43 +65,74 @@ REG32(STATUS, 0x14) > FIELD(STATUS, TXFULL, 0, 1) > FIELD(STATUS, RXFULL, 1, 1) > FIELD(STATUS, TXEMPTY, 2, 1) > + FIELD(STATUS, TXIDLE, 3, 1) > FIELD(STATUS, RXIDLE, 4, 1) > FIELD(STATUS, RXEMPTY, 5, 1) > REG32(RDATA, 0x18) > + FIELD(RDATA, RDATA, 0, 8) > REG32(WDATA, 0x1C) > + FIELD(WDATA, WDATA, 0, 8) > REG32(FIFO_CTRL, 0x20) > FIELD(FIFO_CTRL, RXRST, 0, 1) > FIELD(FIFO_CTRL, TXRST, 1, 1) > FIELD(FIFO_CTRL, RXILVL, 2, 3) > - FIELD(FIFO_CTRL, TXILVL, 5, 2) > + FIELD(FIFO_CTRL, TXILVL, 5, 3) > REG32(FIFO_STATUS, 0x24) > - FIELD(FIFO_STATUS, TXLVL, 0, 5) > - FIELD(FIFO_STATUS, RXLVL, 16, 5) > + FIELD(FIFO_STATUS, TXLVL, 0, 8) > + FIELD(FIFO_STATUS, RXLVL, 16, 8) > REG32(OVRD, 0x28) > + FIELD(OVRD, TXEN, 0, 1) > + FIELD(OVRD, TXVAL, 1, 1) > REG32(VAL, 0x2C) > + FIELD(VAL, RX, 0, 16) > REG32(TIMEOUT_CTRL, 0x30) > + FIELD(TIMEOUT_CTRL, VAL, 0, 24) > + FIELD(TIMEOUT_CTRL, EN, 31, 1) > + > +#define INTR_MASK \ > + (INTR_TX_WATERMARK_MASK | INTR_RX_WATERMARK_MASK | INTR_TX_DONE_MASK | \ > + INTR_RX_OVERFLOW_MASK | INTR_RX_FRAME_ERR_MASK | INTR_RX_BREAK_ERR_MASK > | \ > + INTR_RX_TIMEOUT_MASK | INTR_RX_PARITY_ERR_MASK | INTR_TX_EMPTY_MASK) > + > +#define CTRL_MASK \ > + (R_CTRL_TX_MASK | R_CTRL_RX_MASK | R_CTRL_NF_MASK | R_CTRL_SLPBK_MASK | \ > + R_CTRL_LLPBK_MASK | R_CTRL_PARITY_EN_MASK | R_CTRL_PARITY_ODD_MASK | \ > + R_CTRL_RXBLVL_MASK | R_CTRL_NCO_MASK) > + > +#define OT_UART_NCO_BITS 16 > +#define OT_UART_TX_FIFO_SIZE 128 > +#define OT_UART_RX_FIFO_SIZE 128 > + > +#define R32_OFF(_r_) ((_r_) / sizeof(uint32_t)) > + > +#define R_LAST_REG (R_TIMEOUT_CTRL) > +#define REGS_COUNT (R_LAST_REG + 1u) > +#define REGS_SIZE (REGS_COUNT * sizeof(uint32_t)) > > static void ot_uart_update_irqs(OtUARTState *s) > { > - if (s->uart_intr_state & s->uart_intr_enable & > R_INTR_STATE_TX_WATERMARK_MASK) { > + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] > + & INTR_TX_WATERMARK_MASK) { > qemu_set_irq(s->tx_watermark, 1); > } else { > qemu_set_irq(s->tx_watermark, 0); > } > > - if (s->uart_intr_state & s->uart_intr_enable & > R_INTR_STATE_RX_WATERMARK_MASK) { > + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] > + & INTR_RX_WATERMARK_MASK) { > qemu_set_irq(s->rx_watermark, 1); > } else { > qemu_set_irq(s->rx_watermark, 0); > } > > - if (s->uart_intr_state & s->uart_intr_enable & > R_INTR_STATE_TX_EMPTY_MASK) { > + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] & INTR_TX_EMPTY_MASK) > { > qemu_set_irq(s->tx_empty, 1); > } else { > qemu_set_irq(s->tx_empty, 0); > } > > - if (s->uart_intr_state & s->uart_intr_enable & > R_INTR_STATE_RX_OVERFLOW_MASK) { > + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] > + & INTR_RX_OVERFLOW_MASK) { > qemu_set_irq(s->rx_overflow, 1); > } else { > qemu_set_irq(s->rx_overflow, 0); > @@ -105,122 +143,133 @@ static int ot_uart_can_receive(void *opaque) > { > OtUARTState *s = opaque; > > - if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) > - && !(s->uart_status & R_STATUS_RXFULL_MASK)) { > - return 1; > + if (s->regs[R_CTRL] & R_CTRL_RX_MASK) { > + return (int)fifo8_num_free(&s->rx_fifo); > } > > return 0; > } > > +static uint32_t ot_uart_get_rx_watermark_level(const OtUARTState *s) > +{ > + uint32_t rx_ilvl = FIELD_EX32(s->regs[R_FIFO_CTRL], FIFO_CTRL, RXILVL); > + > + return rx_ilvl < 7 ? (1 << rx_ilvl) : 126; > +} > + > static void ot_uart_receive(void *opaque, const uint8_t *buf, int size) > { > OtUARTState *s = opaque; > - uint8_t rx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_RXILVL_MASK) > - >> R_FIFO_CTRL_RXILVL_SHIFT; > - > - s->uart_rdata = *buf; > + uint32_t rx_watermark_level; > + size_t count = MIN(fifo8_num_free(&s->rx_fifo), (size_t)size); > > - s->uart_status &= ~R_STATUS_RXIDLE_MASK; > - s->uart_status &= ~R_STATUS_RXEMPTY_MASK; > - /* The RXFULL is set after receiving a single byte > - * as the FIFO buffers are not yet implemented. > - */ > - s->uart_status |= R_STATUS_RXFULL_MASK; > - s->rx_level += 1; > + for (int index = 0; index < size; index++) { > + fifo8_push(&s->rx_fifo, buf[index]); > + } > > - if (size > rx_fifo_level) { > - s->uart_intr_state |= R_INTR_STATE_RX_WATERMARK_MASK; > + /* update INTR_STATE */ > + if (count != size) { > + s->regs[R_INTR_STATE] |= INTR_RX_OVERFLOW_MASK; > + } > + rx_watermark_level = ot_uart_get_rx_watermark_level(s); > + if (rx_watermark_level && size >= rx_watermark_level) { > + s->regs[R_INTR_STATE] |= INTR_RX_WATERMARK_MASK; > } > > ot_uart_update_irqs(s); > } > > -static gboolean ot_uart_xmit(void *do_not_use, GIOCondition cond, > - void *opaque) > +static void ot_uart_reset_tx_fifo(OtUARTState *s) > { > - OtUARTState *s = opaque; > - uint8_t tx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_TXILVL_MASK) > - >> R_FIFO_CTRL_TXILVL_SHIFT; > - int ret; > - > - /* instant drain the fifo when there's no back-end */ > - if (!qemu_chr_fe_backend_connected(&s->chr)) { > - s->tx_level = 0; > - return G_SOURCE_REMOVE; > + fifo8_reset(&s->tx_fifo); > + s->regs[R_INTR_STATE] |= INTR_TX_EMPTY_MASK; > + s->regs[R_INTR_STATE] |= INTR_TX_DONE_MASK; > + if (s->tx_watermark_level) { > + s->regs[R_INTR_STATE] |= INTR_TX_WATERMARK_MASK; > + s->tx_watermark_level = 0; > } > +} > > - if (!s->tx_level) { > - s->uart_status &= ~R_STATUS_TXFULL_MASK; > - s->uart_status |= R_STATUS_TXEMPTY_MASK; > - s->uart_intr_state |= R_INTR_STATE_TX_EMPTY_MASK; > - s->uart_intr_state &= ~R_INTR_STATE_TX_WATERMARK_MASK; > - ot_uart_update_irqs(s); > - return G_SOURCE_REMOVE; > +static void ot_uart_reset_rx_fifo(OtUARTState *s) > +{ > + fifo8_reset(&s->rx_fifo); > + s->regs[R_INTR_STATE] &= ~INTR_RX_WATERMARK_MASK; > + s->regs[R_INTR_STATE] &= ~INTR_RX_OVERFLOW_MASK; > + if (FIELD_EX32(s->regs[R_CTRL], CTRL, RX)) { > + qemu_chr_fe_accept_input(&s->chr); > } > +} > > - ret = qemu_chr_fe_write(&s->chr, s->tx_fifo, s->tx_level); > +static uint32_t ot_uart_get_tx_watermark_level(const OtUARTState *s) > +{ > + uint32_t tx_ilvl = FIELD_EX32(s->regs[R_FIFO_CTRL], FIFO_CTRL, TXILVL); > > - if (ret >= 0) { > - s->tx_level -= ret; > - memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_level); > - } > + return tx_ilvl < 7 ? (1 << tx_ilvl) : 64; > +} > > - if (s->tx_level) { > - guint r = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, > - ot_uart_xmit, s); > - if (!r) { > - s->tx_level = 0; > - return G_SOURCE_REMOVE; > - } > +static void ot_uart_xmit(OtUARTState *s) > +{ > + const uint8_t *buf; > + uint32_t size; > + int ret; > + > + if (fifo8_is_empty(&s->tx_fifo)) { > + return; > } > > - /* Clear the TX Full bit */ > - if (s->tx_level != OT_UART_TX_FIFO_SIZE) { > - s->uart_status &= ~R_STATUS_TXFULL_MASK; > + /* instant drain the fifo when there's no back-end */ > + if (!qemu_chr_fe_backend_connected(&s->chr)) { > + ot_uart_reset_tx_fifo(s); > + ot_uart_update_irqs(s); > + return; > } > > - /* Disable the TX_WATERMARK IRQ */ > - if (s->tx_level < tx_fifo_level) { > - s->uart_intr_state &= ~R_INTR_STATE_TX_WATERMARK_MASK; > + /* get a continuous buffer from the FIFO */ > + buf = > + fifo8_peek_bufptr(&s->tx_fifo, fifo8_num_used(&s->tx_fifo), &size); > + /* send as much as possible */ > + ret = qemu_chr_fe_write(&s->chr, buf, (int)size); > + /* if some characters where sent, remove them from the FIFO */ > + if (ret >= 0) { > + fifo8_drop(&s->tx_fifo, ret); > } > > - /* Set TX empty */ > - if (s->tx_level == 0) { > - s->uart_status |= R_STATUS_TXEMPTY_MASK; > - s->uart_intr_state |= R_INTR_STATE_TX_EMPTY_MASK; > + /* update INTR_STATE */ > + if (fifo8_is_empty(&s->tx_fifo)) { > + s->regs[R_INTR_STATE] |= INTR_TX_EMPTY_MASK; > + s->regs[R_INTR_STATE] |= INTR_TX_DONE_MASK; > + } > + if (s->tx_watermark_level && > + fifo8_num_used(&s->tx_fifo) < s->tx_watermark_level) { > + s->regs[R_INTR_STATE] |= INTR_TX_WATERMARK_MASK; > + s->tx_watermark_level = 0; > } > > ot_uart_update_irqs(s); > - return G_SOURCE_REMOVE; > } > > -static void uart_write_tx_fifo(OtUARTState *s, const uint8_t *buf, > - int size) > +static void uart_write_tx_fifo(OtUARTState *s, uint8_t val) > { > uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > - uint8_t tx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_TXILVL_MASK) > - >> R_FIFO_CTRL_TXILVL_SHIFT; > - > - if (size > OT_UART_TX_FIFO_SIZE - s->tx_level) { > - size = OT_UART_TX_FIFO_SIZE - s->tx_level; > + if (fifo8_is_full(&s->tx_fifo)) { > qemu_log_mask(LOG_GUEST_ERROR, "ot_uart: TX FIFO overflow"); > + return; > } > > - memcpy(s->tx_fifo + s->tx_level, buf, size); > - s->tx_level += size; > + fifo8_push(&s->tx_fifo, val); > > - if (s->tx_level > 0) { > - s->uart_status &= ~R_STATUS_TXEMPTY_MASK; > + s->tx_watermark_level = ot_uart_get_tx_watermark_level(s); > + if (fifo8_num_used(&s->tx_fifo) < s->tx_watermark_level) { > + /* > + * TX watermark interrupt is raised when FIFO depth goes from above > + * watermark to below. If we haven't reached watermark, reset cached > + * watermark level > + */ > + s->tx_watermark_level = 0; > }
Don't we need to update the interrupts at this point? Alistair
