[{"id":3675616,"web_url":"http://patchwork.ozlabs.org/comment/3675616/","msgid":"","list_archive_url":null,"date":"2026-04-10T03:02:08","subject":"Re: [PATCH qemu v2 3/7] ot_uart: update register defs, switch to\n Fifo8 for tx/rx buffers","submitter":{"id":64571,"url":"http://patchwork.ozlabs.org/api/people/64571/","name":"Alistair Francis","email":"alistair23@gmail.com"},"content":"On Thu, Apr 9, 2026 at 5:52 AM ~lexbaileylowrisc\n wrote:\n>\n> From: Lex Bailey \n>\n> The register definitions for the UART device need to be updated to match\n> the documentation at https://opentitan.org/book/hw/ip/uart/doc/registers.html\n>\n> This commit does that, and also switches to using Fifo8 for managing the\n> transmit and receive buffers.\n>\n> Signed-off-by: Lex Bailey \n> ---\n> hw/char/ot_uart.c | 332 ++++++++++++++++++++++----------------\n> include/hw/char/ot_uart.h | 18 +--\n> 2 files changed, 196 insertions(+), 154 deletions(-)\n>\n> diff --git a/hw/char/ot_uart.c b/hw/char/ot_uart.c\n> index 2cf0d73cf5..3bf3295b1b 100644\n> --- a/hw/char/ot_uart.c\n> +++ b/hw/char/ot_uart.c\n> @@ -27,6 +27,7 @@\n>\n> #include \"qemu/osdep.h\"\n> #include \"hw/char/ot_uart.h\"\n> +#include \"qemu/fifo8.h\"\n> #include \"hw/core/irq.h\"\n> #include \"hw/core/qdev-clock.h\"\n> #include \"hw/core/qdev-properties.h\"\n> @@ -36,17 +37,24 @@\n> #include \"qemu/log.h\"\n> #include \"qemu/module.h\"\n>\n> +/* clang-format off */\n\nLet's remove these\n\n> REG32(INTR_STATE, 0x00)\n> - FIELD(INTR_STATE, TX_WATERMARK, 0, 1)\n> - FIELD(INTR_STATE, RX_WATERMARK, 1, 1)\n> - FIELD(INTR_STATE, TX_EMPTY, 2, 1)\n> - FIELD(INTR_STATE, RX_OVERFLOW, 3, 1)\n> + SHARED_FIELD(INTR_TX_WATERMARK, 0, 1)\n> + SHARED_FIELD(INTR_RX_WATERMARK, 1, 1)\n> + SHARED_FIELD(INTR_TX_DONE, 2, 1)\n> + SHARED_FIELD(INTR_RX_OVERFLOW, 3, 1)\n> + SHARED_FIELD(INTR_RX_FRAME_ERR, 4, 1)\n> + SHARED_FIELD(INTR_RX_BREAK_ERR, 5, 1)\n> + SHARED_FIELD(INTR_RX_TIMEOUT, 6, 1)\n> + SHARED_FIELD(INTR_RX_PARITY_ERR, 7, 1)\n> + SHARED_FIELD(INTR_TX_EMPTY, 8, 1)\n> REG32(INTR_ENABLE, 0x04)\n> REG32(INTR_TEST, 0x08)\n> REG32(ALERT_TEST, 0x0C)\n> + FIELD(ALERT_TEST, FATAL_FAULT, 0, 1)\n> REG32(CTRL, 0x10)\n> - FIELD(CTRL, TX_ENABLE, 0, 1)\n> - FIELD(CTRL, RX_ENABLE, 1, 1)\n> + FIELD(CTRL, TX, 0, 1)\n> + FIELD(CTRL, RX, 1, 1)\n> FIELD(CTRL, NF, 2, 1)\n> FIELD(CTRL, SLPBK, 4, 1)\n> FIELD(CTRL, LLPBK, 5, 1)\n> @@ -58,43 +66,78 @@ REG32(STATUS, 0x14)\n> FIELD(STATUS, TXFULL, 0, 1)\n> FIELD(STATUS, RXFULL, 1, 1)\n> FIELD(STATUS, TXEMPTY, 2, 1)\n> + FIELD(STATUS, TXIDLE, 3, 1)\n> FIELD(STATUS, RXIDLE, 4, 1)\n> FIELD(STATUS, RXEMPTY, 5, 1)\n> REG32(RDATA, 0x18)\n> + FIELD(RDATA, RDATA, 0, 8)\n> REG32(WDATA, 0x1C)\n> + FIELD(WDATA, WDATA, 0, 8)\n> REG32(FIFO_CTRL, 0x20)\n> FIELD(FIFO_CTRL, RXRST, 0, 1)\n> FIELD(FIFO_CTRL, TXRST, 1, 1)\n> FIELD(FIFO_CTRL, RXILVL, 2, 3)\n> - FIELD(FIFO_CTRL, TXILVL, 5, 2)\n> + FIELD(FIFO_CTRL, TXILVL, 5, 3)\n> REG32(FIFO_STATUS, 0x24)\n> - FIELD(FIFO_STATUS, TXLVL, 0, 5)\n> - FIELD(FIFO_STATUS, RXLVL, 16, 5)\n> + FIELD(FIFO_STATUS, TXLVL, 0, 8)\n> + FIELD(FIFO_STATUS, RXLVL, 16, 8)\n> REG32(OVRD, 0x28)\n> + FIELD(OVRD, TXEN, 0, 1)\n> + FIELD(OVRD, TXVAL, 1, 1)\n> REG32(VAL, 0x2C)\n> + FIELD(VAL, RX, 0, 16)\n> REG32(TIMEOUT_CTRL, 0x30)\n> + FIELD(TIMEOUT_CTRL, VAL, 0, 24)\n> + FIELD(TIMEOUT_CTRL, EN, 31, 1)\n> +/* clang-format on */\n> +\n> +#define INTR_MASK \\\n> + (INTR_TX_WATERMARK_MASK | INTR_RX_WATERMARK_MASK | INTR_TX_DONE_MASK | \\\n> + INTR_RX_OVERFLOW_MASK | INTR_RX_FRAME_ERR_MASK | INTR_RX_BREAK_ERR_MASK | \\\n> + INTR_RX_TIMEOUT_MASK | INTR_RX_PARITY_ERR_MASK | INTR_TX_EMPTY_MASK)\n> +\n> +#define CTRL_MASK \\\n> + (R_CTRL_TX_MASK | R_CTRL_RX_MASK | R_CTRL_NF_MASK | R_CTRL_SLPBK_MASK | \\\n> + R_CTRL_LLPBK_MASK | R_CTRL_PARITY_EN_MASK | R_CTRL_PARITY_ODD_MASK | \\\n> + R_CTRL_RXBLVL_MASK | R_CTRL_NCO_MASK)\n> +\n> +#define CTRL_SUP_MASK \\\n> + (R_CTRL_RX_MASK | R_CTRL_TX_MASK | R_CTRL_SLPBK_MASK | R_CTRL_NCO_MASK)\n> +\n> +#define OT_UART_NCO_BITS 16\n> +#define OT_UART_TX_FIFO_SIZE 128\n> +#define OT_UART_RX_FIFO_SIZE 128\n> +\n> +#define R32_OFF(_r_) ((_r_) / sizeof(uint32_t))\n> +\n> +#define R_LAST_REG (R_TIMEOUT_CTRL)\n> +#define REGS_COUNT (R_LAST_REG + 1u)\n> +#define REGS_SIZE (REGS_COUNT * sizeof(uint32_t))\n>\n> static void ot_uart_update_irqs(OtUARTState *s)\n> {\n> - if (s->uart_intr_state & s->uart_intr_enable & R_INTR_STATE_TX_WATERMARK_MASK) {\n> + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE]\n> + & INTR_TX_WATERMARK_MASK) {\n> qemu_set_irq(s->tx_watermark, 1);\n> } else {\n> qemu_set_irq(s->tx_watermark, 0);\n> }\n>\n> - if (s->uart_intr_state & s->uart_intr_enable & R_INTR_STATE_RX_WATERMARK_MASK) {\n> + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE]\n> + & INTR_RX_WATERMARK_MASK) {\n> qemu_set_irq(s->rx_watermark, 1);\n> } else {\n> qemu_set_irq(s->rx_watermark, 0);\n> }\n>\n> - if (s->uart_intr_state & s->uart_intr_enable & R_INTR_STATE_TX_EMPTY_MASK) {\n> + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE] & INTR_TX_EMPTY_MASK) {\n> qemu_set_irq(s->tx_empty, 1);\n> } else {\n> qemu_set_irq(s->tx_empty, 0);\n> }\n>\n> - if (s->uart_intr_state & s->uart_intr_enable & R_INTR_STATE_RX_OVERFLOW_MASK) {\n> + if (s->regs[R_INTR_STATE] & s->regs[R_INTR_ENABLE]\n> + & INTR_RX_OVERFLOW_MASK) {\n> qemu_set_irq(s->rx_overflow, 1);\n> } else {\n> qemu_set_irq(s->rx_overflow, 0);\n> @@ -105,126 +148,133 @@ static int ot_uart_can_receive(void *opaque)\n> {\n> OtUARTState *s = opaque;\n>\n> - if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK)\n> - && !(s->uart_status & R_STATUS_RXFULL_MASK)) {\n> - return 1;\n> + if (s->regs[R_CTRL] & R_CTRL_RX_MASK) {\n> + return (int)fifo8_num_free(&s->rx_fifo);\n> }\n>\n> return 0;\n> }\n>\n> +static uint32_t ot_uart_get_rx_watermark_level(const OtUARTState *s)\n> +{\n> + uint32_t rx_ilvl = FIELD_EX32(s->regs[R_FIFO_CTRL], FIFO_CTRL, RXILVL);\n> +\n> + return rx_ilvl < 7 ? (1 << rx_ilvl) : 126;\n> +}\n> +\n> static void ot_uart_receive(void *opaque, const uint8_t *buf, int size)\n> {\n> OtUARTState *s = opaque;\n> - uint8_t rx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_RXILVL_MASK)\n> - >> R_FIFO_CTRL_RXILVL_SHIFT;\n> -\n> - s->uart_rdata = *buf;\n> + uint32_t rx_watermark_level;\n> + size_t count = MIN(fifo8_num_free(&s->rx_fifo), (size_t)size);\n>\n> - s->uart_status &= ~R_STATUS_RXIDLE_MASK;\n> - s->uart_status &= ~R_STATUS_RXEMPTY_MASK;\n> - /* The RXFULL is set after receiving a single byte\n> - * as the FIFO buffers are not yet implemented.\n> - */\n> - s->uart_status |= R_STATUS_RXFULL_MASK;\n> - s->rx_level += 1;\n> + for (int index = 0; index < size; index++) {\n> + fifo8_push(&s->rx_fifo, buf[index]);\n> + }\n>\n> - if (size > rx_fifo_level) {\n> - s->uart_intr_state |= R_INTR_STATE_RX_WATERMARK_MASK;\n> + /* update INTR_STATE */\n> + if (count != size) {\n> + s->regs[R_INTR_STATE] |= INTR_RX_OVERFLOW_MASK;\n> + }\n> + rx_watermark_level = ot_uart_get_rx_watermark_level(s);\n> + if (rx_watermark_level && size >= rx_watermark_level) {\n> + s->regs[R_INTR_STATE] |= INTR_RX_WATERMARK_MASK;\n> }\n>\n> ot_uart_update_irqs(s);\n> }\n>\n> -static gboolean ot_uart_xmit(void *do_not_use, GIOCondition cond,\n> - void *opaque)\n> +static void ot_uart_reset_tx_fifo(OtUARTState *s)\n> {\n> - OtUARTState *s = opaque;\n> - uint8_t tx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_TXILVL_MASK)\n> - >> R_FIFO_CTRL_TXILVL_SHIFT;\n> - int ret;\n> -\n> - /* instant drain the fifo when there's no back-end */\n> - if (!qemu_chr_fe_backend_connected(&s->chr)) {\n> - s->tx_level = 0;\n> - return G_SOURCE_REMOVE;\n> + fifo8_reset(&s->tx_fifo);\n> + s->regs[R_INTR_STATE] |= INTR_TX_EMPTY_MASK;\n> + s->regs[R_INTR_STATE] |= INTR_TX_DONE_MASK;\n> + if (s->tx_watermark_level) {\n> + s->regs[R_INTR_STATE] |= INTR_TX_WATERMARK_MASK;\n> + s->tx_watermark_level = 0;\n> }\n> +}\n>\n> - if (!s->tx_level) {\n> - s->uart_status &= ~R_STATUS_TXFULL_MASK;\n> - s->uart_status |= R_STATUS_TXEMPTY_MASK;\n> - s->uart_intr_state |= R_INTR_STATE_TX_EMPTY_MASK;\n> - s->uart_intr_state &= ~R_INTR_STATE_TX_WATERMARK_MASK;\n> - ot_uart_update_irqs(s);\n> - return G_SOURCE_REMOVE;\n> +static void ot_uart_reset_rx_fifo(OtUARTState *s)\n> +{\n> + fifo8_reset(&s->rx_fifo);\n> + s->regs[R_INTR_STATE] &= ~INTR_RX_WATERMARK_MASK;\n> + s->regs[R_INTR_STATE] &= ~INTR_RX_OVERFLOW_MASK;\n> + if (FIELD_EX32(s->regs[R_CTRL], CTRL, RX)) {\n> + qemu_chr_fe_accept_input(&s->chr);\n> }\n> +}\n>\n> - ret = qemu_chr_fe_write(&s->chr, s->tx_fifo, s->tx_level);\n> +static uint32_t ot_uart_get_tx_watermark_level(const OtUARTState *s)\n> +{\n> + uint32_t tx_ilvl = FIELD_EX32(s->regs[R_FIFO_CTRL], FIFO_CTRL, TXILVL);\n>\n> - if (ret >= 0) {\n> - s->tx_level -= ret;\n> - memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_level);\n> - }\n> + return tx_ilvl < 7 ? (1 << tx_ilvl) : 64;\n> +}\n>\n> - if (s->tx_level) {\n> - guint r = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,\n> - ot_uart_xmit, s);\n> - if (!r) {\n> - s->tx_level = 0;\n> - return G_SOURCE_REMOVE;\n> - }\n> +static void ot_uart_xmit(OtUARTState *s)\n> +{\n> + const uint8_t *buf;\n> + uint32_t size;\n> + int ret;\n> +\n> + if (fifo8_is_empty(&s->tx_fifo)) {\n> + return;\n> }\n>\n> - /* Clear the TX Full bit */\n> - if (s->tx_level != OT_UART_TX_FIFO_SIZE) {\n> - s->uart_status &= ~R_STATUS_TXFULL_MASK;\n> + /* instant drain the fifo when there's no back-end */\n> + if (!qemu_chr_fe_backend_connected(&s->chr)) {\n> + ot_uart_reset_tx_fifo(s);\n> + ot_uart_update_irqs(s);\n> + return;\n> }\n>\n> - /* Disable the TX_WATERMARK IRQ */\n> - if (s->tx_level < tx_fifo_level) {\n> - s->uart_intr_state &= ~R_INTR_STATE_TX_WATERMARK_MASK;\n> + /* get a continuous buffer from the FIFO */\n> + buf =\n> + fifo8_peek_bufptr(&s->tx_fifo, fifo8_num_used(&s->tx_fifo), &size);\n> + /* send as much as possible */\n> + ret = qemu_chr_fe_write(&s->chr, buf, (int)size);\n> + /* if some characters where sent, remove them from the FIFO */\n> + if (ret >= 0) {\n> + fifo8_drop(&s->tx_fifo, ret);\n> }\n>\n> - /* Set TX empty */\n> - if (s->tx_level == 0) {\n> - s->uart_status |= R_STATUS_TXEMPTY_MASK;\n> - s->uart_intr_state |= R_INTR_STATE_TX_EMPTY_MASK;\n> + /* update INTR_STATE */\n> + if (fifo8_is_empty(&s->tx_fifo)) {\n> + s->regs[R_INTR_STATE] |= INTR_TX_EMPTY_MASK;\n> + s->regs[R_INTR_STATE] |= INTR_TX_DONE_MASK;\n> + }\n> + if (s->tx_watermark_level &&\n> + fifo8_num_used(&s->tx_fifo) < s->tx_watermark_level) {\n> + s->regs[R_INTR_STATE] |= INTR_TX_WATERMARK_MASK;\n> + s->tx_watermark_level = 0;\n> }\n>\n> ot_uart_update_irqs(s);\n> - return G_SOURCE_REMOVE;\n> }\n>\n> -static void uart_write_tx_fifo(OtUARTState *s, const uint8_t *buf,\n> - int size)\n> +static void uart_write_tx_fifo(OtUARTState *s, uint8_t val)\n> {\n> - uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);\n> - uint8_t tx_fifo_level = (s->uart_fifo_ctrl & R_FIFO_CTRL_TXILVL_MASK)\n> - >> R_FIFO_CTRL_TXILVL_SHIFT;\n> -\n> - if (size > OT_UART_TX_FIFO_SIZE - s->tx_level) {\n> - size = OT_UART_TX_FIFO_SIZE - s->tx_level;\n> + if (fifo8_is_full(&s->tx_fifo)) {\n> qemu_log_mask(LOG_GUEST_ERROR, \"ot_uart: TX FIFO overflow\");\n> + return;\n> }\n>\n> - memcpy(s->tx_fifo + s->tx_level, buf, size);\n> - s->tx_level += size;\n> -\n> - if (s->tx_level > 0) {\n> - s->uart_status &= ~R_STATUS_TXEMPTY_MASK;\n> - }\n> + fifo8_push(&s->tx_fifo, val);\n>\n> - if (s->tx_level >= tx_fifo_level) {\n> - s->uart_intr_state |= R_INTR_STATE_TX_WATERMARK_MASK;\n> - ot_uart_update_irqs(s);\n> + s->tx_watermark_level = ot_uart_get_tx_watermark_level(s);\n> + if (fifo8_num_used(&s->tx_fifo) < s->tx_watermark_level) {\n> + /*\n> + * TX watermark interrupt is raised when FIFO depth goes from above\n> + * watermark to below. If we haven't reached watermark, reset cached\n> + * watermark level\n> + */\n> + s->tx_watermark_level = 0;\n> }\n>\n> - if (s->tx_level == OT_UART_TX_FIFO_SIZE) {\n> - s->uart_status |= R_STATUS_TXFULL_MASK;\n> + if (FIELD_EX32(s->regs[R_CTRL], CTRL, TX)) {\n> + ot_uart_xmit(s);\n> }\n> -\n> - timer_mod(s->fifo_trigger_handle, current_time +\n> - (s->char_tx_time * 4));\n\nWhat about the timer?\n\nAlistair\n\n> }\n>\n> static void ot_uart_reset_enter(Object *obj, ResetType type)\n> @@ -236,17 +286,13 @@ static void ot_uart_reset_enter(Object *obj, ResetType type)\n> c->parent_phases.enter(obj, type);\n> }\n>\n> - s->uart_intr_state = 0x00000000;\n> - s->uart_intr_state = 0x00000000;\n> - s->uart_intr_enable = 0x00000000;\n> - s->uart_ctrl = 0x00000000;\n> - s->uart_status = 0x0000003c;\n> - s->uart_rdata = 0x00000000;\n> - s->uart_fifo_ctrl = 0x00000000;\n> - s->uart_fifo_status = 0x00000000;\n> - s->uart_ovrd = 0x00000000;\n> - s->uart_val = 0x00000000;\n> - s->uart_timeout_ctrl = 0x00000000;\n> + memset(&s->regs[0], 0, sizeof(s->regs));\n> +\n> + s->regs[R_STATUS] = 0x0000003c;\n> +\n> + s->tx_watermark_level = 0;\n> + ot_uart_reset_tx_fifo(s);\n> + ot_uart_reset_rx_fifo(s);\n>\n> s->tx_level = 0;\n> s->rx_level = 0;\n> @@ -260,7 +306,7 @@ static uint64_t ot_uart_get_baud(OtUARTState *s)\n> {\n> uint64_t baud;\n>\n> - baud = ((s->uart_ctrl & R_CTRL_NCO_MASK) >> 16);\n> + baud = ((s->regs[R_CTRL] & R_CTRL_NCO_MASK) >> 16);\n> baud *= clock_get_hz(s->f_clk);\n> baud >>= 20;\n>\n> @@ -274,10 +320,10 @@ static uint64_t ot_uart_read(void *opaque, hwaddr addr, unsigned int size)\n>\n> switch (addr >> 2) {\n> case R_INTR_STATE:\n> - retvalue = s->uart_intr_state;\n> + retvalue = s->regs[R_INTR_STATE];\n> break;\n> case R_INTR_ENABLE:\n> - retvalue = s->uart_intr_enable;\n> + retvalue = s->regs[R_INTR_ENABLE];\n> break;\n> case R_INTR_TEST:\n> qemu_log_mask(LOG_GUEST_ERROR,\n> @@ -285,22 +331,22 @@ static uint64_t ot_uart_read(void *opaque, hwaddr addr, unsigned int size)\n> break;\n>\n> case R_CTRL:\n> - retvalue = s->uart_ctrl;\n> + retvalue = s->regs[R_CTRL];\n> break;\n> case R_STATUS:\n> - retvalue = s->uart_status;\n> + retvalue = s->regs[R_STATUS];\n> break;\n>\n> case R_RDATA:\n> - retvalue = s->uart_rdata;\n> - if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) && (s->rx_level > 0)) {\n> + retvalue = s->regs[R_RDATA];\n> + if ((s->regs[R_CTRL] & R_CTRL_RX_MASK) && (s->rx_level > 0)) {\n> qemu_chr_fe_accept_input(&s->chr);\n>\n> s->rx_level -= 1;\n> - s->uart_status &= ~R_STATUS_RXFULL_MASK;\n> + s->regs[R_STATUS] &= ~R_STATUS_RXFULL_MASK;\n> if (s->rx_level == 0) {\n> - s->uart_status |= R_STATUS_RXIDLE_MASK;\n> - s->uart_status |= R_STATUS_RXEMPTY_MASK;\n> + s->regs[R_STATUS] |= R_STATUS_RXIDLE_MASK;\n> + s->regs[R_STATUS] |= R_STATUS_RXEMPTY_MASK;\n> }\n> }\n> break;\n> @@ -310,10 +356,10 @@ static uint64_t ot_uart_read(void *opaque, hwaddr addr, unsigned int size)\n> break;\n>\n> case R_FIFO_CTRL:\n> - retvalue = s->uart_fifo_ctrl;\n> + retvalue = s->regs[R_FIFO_CTRL];\n> break;\n> case R_FIFO_STATUS:\n> - retvalue = s->uart_fifo_status;\n> + retvalue = s->regs[R_FIFO_STATUS];\n>\n> retvalue |= (s->rx_level & 0x1F) << R_FIFO_STATUS_RXLVL_SHIFT;\n> retvalue |= (s->tx_level & 0x1F) << R_FIFO_STATUS_TXLVL_SHIFT;\n> @@ -323,17 +369,17 @@ static uint64_t ot_uart_read(void *opaque, hwaddr addr, unsigned int size)\n> break;\n>\n> case R_OVRD:\n> - retvalue = s->uart_ovrd;\n> + retvalue = s->regs[R_OVRD];\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: ovrd is not supported\\n\", __func__);\n> break;\n> case R_VAL:\n> - retvalue = s->uart_val;\n> + retvalue = s->regs[R_VAL];\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: val is not supported\\n\", __func__);\n> break;\n> case R_TIMEOUT_CTRL:\n> - retvalue = s->uart_timeout_ctrl;\n> + retvalue = s->regs[R_TIMEOUT_CTRL];\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: timeout_ctrl is not supported\\n\", __func__);\n> break;\n> @@ -355,20 +401,20 @@ static void ot_uart_write(void *opaque, hwaddr addr, uint64_t val64,\n> switch (addr >> 2) {\n> case R_INTR_STATE:\n> /* Write 1 clear */\n> - s->uart_intr_state &= ~value;\n> + s->regs[R_INTR_STATE] &= ~value;\n> ot_uart_update_irqs(s);\n> break;\n> case R_INTR_ENABLE:\n> - s->uart_intr_enable = value;\n> + s->regs[R_INTR_ENABLE] = value;\n> ot_uart_update_irqs(s);\n> break;\n> case R_INTR_TEST:\n> - s->uart_intr_state |= value;\n> + s->regs[R_INTR_STATE] |= value;\n> ot_uart_update_irqs(s);\n> break;\n>\n> case R_CTRL:\n> - s->uart_ctrl = value;\n> + s->regs[R_CTRL] = value;\n>\n> if (value & R_CTRL_NF_MASK) {\n> qemu_log_mask(LOG_UNIMP,\n> @@ -412,11 +458,11 @@ static void ot_uart_write(void *opaque, hwaddr addr, uint64_t val64,\n> \"%s: rdata is read only\\n\", __func__);\n> break;\n> case R_WDATA:\n> - uart_write_tx_fifo(s, (uint8_t *) &value, 1);\n> + uart_write_tx_fifo(s, value);\n> break;\n>\n> case R_FIFO_CTRL:\n> - s->uart_fifo_ctrl = value;\n> + s->regs[R_FIFO_CTRL] = value;\n>\n> if (value & R_FIFO_CTRL_RXRST_MASK) {\n> s->rx_level = 0;\n> @@ -433,7 +479,7 @@ static void ot_uart_write(void *opaque, hwaddr addr, uint64_t val64,\n> break;\n>\n> case R_OVRD:\n> - s->uart_ovrd = value;\n> + s->regs[R_OVRD] = value;\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: ovrd is not supported\\n\", __func__);\n> break;\n> @@ -442,7 +488,7 @@ static void ot_uart_write(void *opaque, hwaddr addr, uint64_t val64,\n> \"%s: val is read only\\n\", __func__);\n> break;\n> case R_TIMEOUT_CTRL:\n> - s->uart_timeout_ctrl = value;\n> + s->regs[R_TIMEOUT_CTRL] = value;\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: timeout_ctrl is not supported\\n\", __func__);\n> break;\n> @@ -466,8 +512,8 @@ static void fifo_trigger_update(void *opaque)\n> {\n> OtUARTState *s = opaque;\n>\n> - if (s->uart_ctrl & R_CTRL_TX_ENABLE_MASK) {\n> - ot_uart_xmit(NULL, G_IO_OUT, s);\n> + if (s->regs[R_CTRL] & R_CTRL_TX_MASK) {\n> + ot_uart_xmit(s);\n> }\n> }\n>\n> @@ -489,25 +535,17 @@ static int ot_uart_post_load(void *opaque, int version_id)\n>\n> static const VMStateDescription vmstate_ot_uart = {\n> .name = TYPE_OT_UART,\n> - .version_id = 1,\n> - .minimum_version_id = 1,\n> + .version_id = 2,\n> + .minimum_version_id = 2,\n> .post_load = ot_uart_post_load,\n> .fields = (const VMStateField[]) {\n> - VMSTATE_UINT8_ARRAY(tx_fifo, OtUARTState,\n> - OT_UART_TX_FIFO_SIZE),\n> + VMSTATE_STRUCT(tx_fifo, OtUARTState, 1, vmstate_fifo8, Fifo8),\n> + VMSTATE_STRUCT(rx_fifo, OtUARTState, 1, vmstate_fifo8, Fifo8),\n> VMSTATE_UINT32(tx_level, OtUARTState),\n> VMSTATE_UINT64(char_tx_time, OtUARTState),\n> VMSTATE_TIMER_PTR(fifo_trigger_handle, OtUARTState),\n> - VMSTATE_UINT32(uart_intr_state, OtUARTState),\n> - VMSTATE_UINT32(uart_intr_enable, OtUARTState),\n> - VMSTATE_UINT32(uart_ctrl, OtUARTState),\n> - VMSTATE_UINT32(uart_status, OtUARTState),\n> - VMSTATE_UINT32(uart_rdata, OtUARTState),\n> - VMSTATE_UINT32(uart_fifo_ctrl, OtUARTState),\n> - VMSTATE_UINT32(uart_fifo_status, OtUARTState),\n> - VMSTATE_UINT32(uart_ovrd, OtUARTState),\n> - VMSTATE_UINT32(uart_val, OtUARTState),\n> - VMSTATE_UINT32(uart_timeout_ctrl, OtUARTState),\n> + VMSTATE_ARRAY(regs, OtUARTState, REGS_COUNT, 1, vmstate_info_uint32,\n> + uint32_t),\n> VMSTATE_END_OF_LIST()\n> }\n> };\n> @@ -532,6 +570,13 @@ static void ot_uart_init(Object *obj)\n> memory_region_init_io(&s->mmio, obj, &ot_uart_ops, s,\n> TYPE_OT_UART, 0x400);\n> sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);\n> +\n> + /*\n> + * This array has a fixed size in the header. This assertion is used to\n> + * check that it is consistent with the definition in this file. This is\n> + * ostensibly a runtime check, but may be optimised away by the compiler.\n> + */\n> + assert(REGS_SIZE == sizeof(s->regs));\n> }\n>\n> static void ot_uart_realize(DeviceState *dev, Error **errp)\n> @@ -541,6 +586,9 @@ static void ot_uart_realize(DeviceState *dev, Error **errp)\n> s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,\n> fifo_trigger_update, s);\n>\n> + fifo8_create(&s->tx_fifo, OT_UART_TX_FIFO_SIZE);\n> + fifo8_create(&s->rx_fifo, OT_UART_RX_FIFO_SIZE);\n> +\n> qemu_chr_fe_set_handlers(&s->chr, ot_uart_can_receive,\n> ot_uart_receive, NULL, NULL,\n> s, NULL, true);\n> diff --git a/include/hw/char/ot_uart.h b/include/hw/char/ot_uart.h\n> index fee2128f90..f489612700 100644\n> --- a/include/hw/char/ot_uart.h\n> +++ b/include/hw/char/ot_uart.h\n> @@ -26,11 +26,11 @@\n> #define HW_OT_UART_H\n>\n> #include \"hw/core/sysbus.h\"\n> +#include \"qemu/fifo8.h\"\n> #include \"chardev/char-fe.h\"\n> #include \"qemu/timer.h\"\n> #include \"qom/object.h\"\n>\n> -#define OT_UART_TX_FIFO_SIZE 16\n> #define OT_UART_CLOCK 50000000 /* 50MHz clock */\n>\n> #define TYPE_OT_UART \"ot-uart\"\n> @@ -43,7 +43,6 @@ struct OtUARTState {\n> /* */\n> MemoryRegion mmio;\n>\n> - uint8_t tx_fifo[OT_UART_TX_FIFO_SIZE];\n> uint32_t tx_level;\n>\n> uint32_t rx_level;\n> @@ -51,16 +50,11 @@ struct OtUARTState {\n> QEMUTimer *fifo_trigger_handle;\n> uint64_t char_tx_time;\n>\n> - uint32_t uart_intr_state;\n> - uint32_t uart_intr_enable;\n> - uint32_t uart_ctrl;\n> - uint32_t uart_status;\n> - uint32_t uart_rdata;\n> - uint32_t uart_fifo_ctrl;\n> - uint32_t uart_fifo_status;\n> - uint32_t uart_ovrd;\n> - uint32_t uart_val;\n> - uint32_t uart_timeout_ctrl;\n> + uint32_t regs[13]; /* Length must be updated if regs added or removed */\n> +\n> + Fifo8 tx_fifo;\n> + Fifo8 rx_fifo;\n> + uint32_t tx_watermark_level;\n>\n> Clock *f_clk;\n>\n> --\n> 2.49.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=g1wWrQ8v;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org\n (client-ip=209.51.188.17; helo=lists.gnu.org;\n envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from lists.gnu.org (lists1p.gnu.org [209.51.188.17])\n\t(using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fsM801h15z1yGb\n\tfor ; Fri, 10 Apr 2026 13:03:42 +1000 (AEST)","from localhost ([::1] helo=lists1p.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.90_1)\n\t(envelope-from )\n\tid 1wB28o-000444-Ib; Thu, 09 Apr 2026 23:02:42 -0400","from eggs.gnu.org ([2001:470:142:3::10])\n by lists1p.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from )\n id 1wB28m-00043M-AU\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 23:02:40 -0400","from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b])\n by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)\n (Exim 4.90_1) (envelope-from )\n id 1wB28j-0007EW-Dx\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 23:02:40 -0400","by mail-ej1-x62b.google.com with SMTP id\n a640c23a62f3a-b93698bb57aso303310266b.0\n for ; Thu, 09 Apr 2026 20:02:37 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; t=1775790156; cv=none;\n d=google.com; s=arc-20240605;\n b=NF2ZT4pUrss2WaaCmrpoYiaVPojNyckm9d/nOoadWFAu9S3n8ga7OmakfrdL5qkrD0\n cho0kRyvn8BN3pbXLHR56KvGGRSYvOOzs4wfQ1AiGABVEF3gR/xOhWx3C0QZOXl0Ox3r\n 9DpNNra04IiKwxuoXYm59oRRnPB5+wmqyiFkgylDLP85AO1s5Iu65ReqWLmRhpEiSSQQ\n CrTFoPO1kZi6amqo4SdYXBDw4pwEu04Z+h+5cAzIuxQpJKsql+l31gQKtcKW93ju34Ea\n dNs7OeefsBjBw5nbNNo6WimxAcO/XKJ5QteLWtNnOgCAF0qnSSD7RzhNryzh/5K/VxPS\n h4dg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n s=arc-20240605;\n h=content-transfer-encoding:cc:to:subject:message-id:date:from\n :in-reply-to:references:mime-version:dkim-signature;\n bh=ToBpNTsy8LQOEgY8+uv6QTPSril3IfYYkFbZ2WaPU78=;\n fh=ERTM5XTiHyFBA9zeoTdaB1nbCf1LrfIqZBj8ZlakxMA=;\n b=TYWud4qhxFD7h+xfC17PzVtyoujoKmPTZvpCz94MsOVn9M/eKTY+yY3SSnB6CGAHy8\n vJhP3QLQhinxYuFWI6HRglhYCmD73RL3vsul3DQubvVDXC1ZLU7FL2QGsxK7lVEIWuZs\n 8is2wTMX54ey0aqA83OYby8RIswq127RC7znMQ76vydlEZgsQBdX0MdHy+ZbYVAlHmal\n 3I/dM9Gssk42s5E5pYdWa+IndU2ci6egX9x8GOg7H6hdgO9+uNeNeyJ5rK/GcDXF2Q0N\n oD09LOdt0/hf/Mg2QVmLoEAQmuyvHFFPS7xsC8wojcKygp7kJFqxUgaOO63FVbxpswJw\n L7PA==; darn=nongnu.org","ARC-Authentication-Results":"i=1; mx.google.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20251104; t=1775790156; x=1776394956; darn=nongnu.org;\n h=content-transfer-encoding:cc:to:subject:message-id:date:from\n :in-reply-to:references:mime-version:from:to:cc:subject:date\n :message-id:reply-to;\n bh=ToBpNTsy8LQOEgY8+uv6QTPSril3IfYYkFbZ2WaPU78=;\n b=g1wWrQ8vyapAnd0uekqHRFFeoQsMySOwUg9+tcc1soi4X2Bs40wzJvL8T7pNS3si0J\n RVkIOog30JzWqKJ13VrKKgVEOfZEWM4VPlBzCsquqc3oWWRkk4zzZIGEA08VREZK8Oby\n 2io4RHtOMkuDeXB+0lkErXDCXrR9OToHXLueH9K1bZ6rPgPPZSqkNrkwGix7WWaYx7yz\n EPLZg8q+yt3/pLaAXW+XfePwu/UGzHuxEzNhG5rgiw/xhhjWNeh7WczuJnGOlWzURGah\n Uc4s92gzAqUB7bJvFv+HkN2xWdCVI9ISmws21WhUAkK4fCT8hWdzkJdoLJJmeuXYOKWW\n SVgg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1775790156; x=1776394956;\n h=content-transfer-encoding:cc:to:subject:message-id:date:from\n :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from\n :to:cc:subject:date:message-id:reply-to;\n bh=ToBpNTsy8LQOEgY8+uv6QTPSril3IfYYkFbZ2WaPU78=;\n b=m4kpp16uL/zt/IwrqHMf34HLKlfiE+FEWLUcB6l++oYmIRa0r021Ko1LM8FGi6XDzI\n Xe4xMCL32trNu2yaPchxw6I3px7d2hTAd+ifhbuHmcC+l4H9S0/wHwCQi3uvLvq9YFNH\n 2BaJR9sLVsm/7cx9ntwzt/COeLCHqqv85kjepUqxfgR2LRlCbYcaZ3gEocnNZ/O2FtJQ\n AJmwy1be8YC+gSwwQ/EKOWAwYwP3ZbOMYHR9Jm4UKjnurv+zlfqV49ZlWfGOqclvruUZ\n HW0H6xPLQqTJfbMPvJoANAMEHbiB45AXth/jM46+HtsKJVbtIgHu+Yp1RH2dXIwRDy11\n hIaw==","X-Forwarded-Encrypted":"i=1;\n AJvYcCXpirdrEFBL9sK1beU6vRNT4JYLoD21ckrlcn4HPMjMK9xpJWwo2g1IZPvxMYBlLghZnGfDyxhUP7Ay@nongnu.org","X-Gm-Message-State":"AOJu0Yxu0IaSBrqVjseBCaOYIqDdUAPaOfq9mlSvxV6RZrBbjzV7jpD9\n hAE2JSxUtZe/9B0+SmojYE/Ut+U2O8CM1RcBw0ks4/uigg8ZnWMFpB4RpFi94lu+nVUmvt3LwUF\n 5+Va1xLPx495XUiuCkAEIt42O2j+bQaU=","X-Gm-Gg":"AeBDietlfyjqwrwcLYRtsaBuYZNgIKdoyCMFqmhEMDQvtkDEntO/0gP1AnYdmhgg0LP\n evd4btaAJSYnVcn7n1Wefqq6TWlgUjLEpuxeILbTmNN/kqm7DjMg81syxweuil1BDdO4Ks76acd\n M5G9yYEeOF/v5iko+CdpcvJ+Qs1DC6BKoGmQvbGW2KbCZuu73udNntWlvMkVp4POJ3g4a76+jwx\n FKwMm4Rr5kcA5WfllxNoQg9y5VrG05h7BnhdU7xFrmsebB/haJnRQURZ/xOGxvM9QtUHs1hZgmo\n aSwHWaFsIKyXHzDyOiadefaztMWSzQ6w0h0p7A==","X-Received":"by 2002:a17:907:d1b:b0:b9d:424c:bb0a with SMTP id\n a640c23a62f3a-b9d76dce539mr47487066b.22.1775790155343; Thu, 09 Apr 2026\n 20:02:35 -0700 (PDT)","MIME-Version":"1.0","References":"<177564643888.23414.7922925369077631439-0@git.sr.ht>\n <177564643888.23414.7922925369077631439-3@git.sr.ht>","In-Reply-To":"<177564643888.23414.7922925369077631439-3@git.sr.ht>","From":"Alistair Francis ","Date":"Fri, 10 Apr 2026 13:02:08 +1000","X-Gm-Features":"AQROBzDbvh03L_9ZwSzO2eChENxjRkBEhiKd4gnzsweAlIKxDqV9KacmRrLkJdo","Message-ID":"\n ","Subject":"Re: [PATCH qemu v2 3/7] ot_uart: update register defs, switch to\n Fifo8 for tx/rx buffers","To":"\"~lexbaileylowrisc\" ","Cc":"qemu-riscv@nongnu.org, Alistair Francis ,\n Paolo Bonzini ,\n =?utf-8?q?Marc-Andr=C3=A9_Lureau?= ,\n Palmer Dabbelt , Weiwei Li ,\n Daniel Henrique Barboza ,\n Liu Zhiwei ,\n Chao Liu , qemu-devel@nongnu.org,\n Amit Kumar-Hermosillo , nabihestefan@google.com","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Received-SPF":"pass client-ip=2a00:1450:4864:20::62b;\n envelope-from=alistair23@gmail.com; helo=mail-ej1-x62b.google.com","X-Spam_score_int":"-17","X-Spam_score":"-1.8","X-Spam_bar":"-","X-Spam_report":"(-1.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,\n DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,\n FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001,\n RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001,\n SPF_PASS=-0.001 autolearn=ham autolearn_force=no","X-Spam_action":"no action","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"qemu development ","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org"}}]