[{"id":3675621,"web_url":"http://patchwork.ozlabs.org/comment/3675621/","msgid":"","list_archive_url":null,"date":"2026-04-10T03:04:25","subject":"Re: [PATCH qemu v2 5/7] ot_uart: implement missing features","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:49 AM ~lexbaileylowrisc\n wrote:\n>\n> From: Lex Bailey \n>\n> Adds in the missing features:\n> - oversampling\n> - loopback\n> - TX override\n> - RX Fifo control\n> - Alert handler test (will only be usable once the alert handler block is also added)\n\nThese should be split up a bit more, this is still a huge patch\n\nAlistair\n\n>\n> Signed-off-by: Lex Bailey \n> ---\n> hw/char/ot_uart.c | 491 +++++++++++++++++++++++---------------\n> include/hw/char/ot_uart.h | 19 +-\n> 2 files changed, 307 insertions(+), 203 deletions(-)\n>\n> diff --git a/hw/char/ot_uart.c b/hw/char/ot_uart.c\n> index 923aab12af..37f2ca06ba 100644\n> --- a/hw/char/ot_uart.c\n> +++ b/hw/char/ot_uart.c\n> @@ -28,8 +28,10 @@\n> #include \"qemu/osdep.h\"\n> #include \"hw/char/ot_uart.h\"\n> #include \"qemu/fifo8.h\"\n> +#include \"qapi/error.h\"\n> +#include \"chardev/char-fe.h\"\n> +#include \"hw/core/cpu.h\"\n> #include \"hw/core/irq.h\"\n> -#include \"hw/core/qdev-clock.h\"\n> #include \"hw/core/qdev-properties.h\"\n> #include \"hw/core/qdev-properties-system.h\"\n> #include \"hw/core/registerfields.h\"\n> @@ -114,6 +116,28 @@ REG32(TIMEOUT_CTRL, 0x30)\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> +#define REG_NAME(_reg_) \\\n> + ((((_reg_) < REGS_COUNT) && REG_NAMES[_reg_]) ? REG_NAMES[_reg_] : \"?\")\n> +\n> +#define REG_NAME_ENTRY(_reg_) [R_##_reg_] = stringify(_reg_)\n> +static const char *REG_NAMES[REGS_COUNT] = {\n> + /* clang-format off */\n> + REG_NAME_ENTRY(INTR_STATE),\n> + REG_NAME_ENTRY(INTR_ENABLE),\n> + REG_NAME_ENTRY(INTR_TEST),\n> + REG_NAME_ENTRY(ALERT_TEST),\n> + REG_NAME_ENTRY(CTRL),\n> + REG_NAME_ENTRY(STATUS),\n> + REG_NAME_ENTRY(RDATA),\n> + REG_NAME_ENTRY(WDATA),\n> + REG_NAME_ENTRY(FIFO_CTRL),\n> + REG_NAME_ENTRY(FIFO_STATUS),\n> + REG_NAME_ENTRY(OVRD),\n> + REG_NAME_ENTRY(VAL),\n> + REG_NAME_ENTRY(TIMEOUT_CTRL),\n> + /* clang-format on */\n> +};\n> +#undef REG_NAME_ENTRY\n>\n> static void ot_uart_update_irqs(OtUARTState *s)\n> {\n> @@ -125,6 +149,21 @@ static void ot_uart_update_irqs(OtUARTState *s)\n> }\n> }\n>\n> +static bool ot_uart_is_sys_loopack_enabled(const OtUARTState *s)\n> +{\n> + return FIELD_EX32(s->regs[R_CTRL], CTRL, SLPBK);\n> +}\n> +\n> +static bool ot_uart_is_tx_enabled(const OtUARTState *s)\n> +{\n> + return FIELD_EX32(s->regs[R_CTRL], CTRL, TX);\n> +}\n> +\n> +static bool ot_uart_is_rx_enabled(const OtUARTState *s)\n> +{\n> + return FIELD_EX32(s->regs[R_CTRL], CTRL, RX);\n> +}\n> +\n> static int ot_uart_can_receive(void *opaque)\n> {\n> OtUARTState *s = opaque;\n> @@ -149,6 +188,11 @@ static void ot_uart_receive(void *opaque, const uint8_t *buf, int size)\n> uint32_t rx_watermark_level;\n> size_t count = MIN(fifo8_num_free(&s->rx_fifo), (size_t)size);\n>\n> + if (size && !s->toggle_break) {\n> + /* no longer breaking, so emulate idle in oversampled VAL register */\n> + s->in_break = false;\n> + }\n> +\n> for (int index = 0; index < size; index++) {\n> fifo8_push(&s->rx_fifo, buf[index]);\n> }\n> @@ -203,21 +247,37 @@ static void ot_uart_xmit(OtUARTState *s)\n> return;\n> }\n>\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> + if (ot_uart_is_sys_loopack_enabled(s)) {\n> + /* system loopback mode, just forward to RX FIFO */\n> + uint32_t count = fifo8_num_used(&s->tx_fifo);\n> + buf = fifo8_pop_bufptr(&s->tx_fifo, count, &size);\n> + ot_uart_receive(s, buf, (int)size);\n> + count -= size;\n> + /*\n> + * there may be more data to send if data wraps around the end of TX\n> + * FIFO\n> + */\n> + if (count) {\n> + buf = fifo8_pop_bufptr(&s->tx_fifo, count, &size);\n> + ot_uart_receive(s, buf, (int)size);\n> + }\n> + } else {\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> - /* 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> + /* 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>\n> /* update INTR_STATE */\n> @@ -253,7 +313,7 @@ static void uart_write_tx_fifo(OtUARTState *s, uint8_t val)\n> s->tx_watermark_level = 0;\n> }\n>\n> - if (FIELD_EX32(s->regs[R_CTRL], CTRL, TX)) {\n> + if (ot_uart_is_tx_enabled(s)) {\n> ot_uart_xmit(s);\n> }\n> }\n> @@ -269,8 +329,6 @@ static void ot_uart_reset_enter(Object *obj, ResetType type)\n>\n> memset(&s->regs[0], 0, sizeof(s->regs));\n>\n> - s->regs[R_STATUS] = 0x0000003c;\n> -\n> s->tx_watermark_level = 0;\n> for (unsigned index = 0; index < ARRAY_SIZE(s->irqs); index++) {\n> qemu_set_irq(s->irqs[index], 0);\n> @@ -278,226 +336,265 @@ static void ot_uart_reset_enter(Object *obj, ResetType type)\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> -\n> - s->char_tx_time = (NANOSECONDS_PER_SECOND / 230400) * 10;\n> + /*\n> + * do not reset `s->in_break`, as that tracks whether we are currently\n> + * receiving a break condition over UART RX from some device talking\n> + * to OpenTitan, which should survive resets. The QEMU CharDev only\n> + * supports transient break events and not the notion of holding the\n> + * UART in break, so remembering breaks like this is required to\n> + * support mocking of break conditions in the oversampled `VAL` reg.\n> + */\n> + if (s->in_break) {\n> + /* ignore CTRL.RXBLVL as we have no notion of break \"time\" */\n> + s->regs[R_INTR_STATE] |= INTR_RX_BREAK_ERR_MASK;\n> + }\n>\n> ot_uart_update_irqs(s);\n> }\n>\n> -static uint64_t ot_uart_get_baud(OtUARTState *s)\n> +static void ot_uart_event_handler(void *opaque, QEMUChrEvent event)\n> +{\n> + OtUARTState *s = opaque;\n> +\n> + if (event == CHR_EVENT_BREAK) {\n> + if (!s->in_break || !s->oversample_break) {\n> + /* ignore CTRL.RXBLVL as we have no notion of break \"time\" */\n> + s->regs[R_INTR_STATE] |= INTR_RX_BREAK_ERR_MASK;\n> + ot_uart_update_irqs(s);\n> + /* emulate break in the oversampled VAL register */\n> + s->in_break = true;\n> + } else if (s->toggle_break) {\n> + /* emulate toggling break off in the oversampled VAL register */\n> + s->in_break = false;\n> + }\n> + }\n> +}\n> +\n> +static uint8_t ot_uart_read_rx_fifo(OtUARTState *s)\n> +{\n> + uint8_t val;\n> +\n> + if (!(s->regs[R_CTRL] & R_CTRL_RX_MASK)) {\n> + return 0;\n> + }\n> +\n> + if (fifo8_is_empty(&s->rx_fifo)) {\n> + return 0;\n> + }\n> +\n> + val = fifo8_pop(&s->rx_fifo);\n> +\n> + if (ot_uart_is_rx_enabled(s) && !ot_uart_is_sys_loopack_enabled(s)) {\n> + qemu_chr_fe_accept_input(&s->chr);\n> + }\n> +\n> + return val;\n> +}\n> +\n> +static gboolean ot_uart_watch_cb(void *do_not_use, GIOCondition cond,\n> + void *opaque)\n> {\n> - uint64_t baud;\n> + OtUARTState *s = opaque;\n>\n> - baud = ((s->regs[R_CTRL] & R_CTRL_NCO_MASK) >> 16);\n> - baud *= clock_get_hz(s->f_clk);\n> - baud >>= 20;\n> + s->watch_tag = 0;\n> + ot_uart_xmit(s);\n>\n> - return baud;\n> + return FALSE;\n> +}\n> +\n> +static void ot_uart_clock_input(void *opaque, int irq, int level)\n> +{\n> + OtUARTState *s = opaque;\n> +\n> + g_assert(irq == 0);\n> +\n> + s->pclk = (unsigned)level;\n> +\n> + /* TODO: disable UART transfer when PCLK is 0 */\n> }\n>\n> static uint64_t ot_uart_read(void *opaque, hwaddr addr, unsigned int size)\n> {\n> OtUARTState *s = opaque;\n> - uint64_t retvalue = 0;\n> + uint32_t val32;\n>\n> - switch (addr >> 2) {\n> + hwaddr reg = R32_OFF(addr);\n> + switch (reg) {\n> case R_INTR_STATE:\n> - retvalue = s->regs[R_INTR_STATE];\n> - break;\n> case R_INTR_ENABLE:\n> - retvalue = s->regs[R_INTR_ENABLE];\n> - break;\n> - case R_INTR_TEST:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: wdata is write only\\n\", __func__);\n> - break;\n> -\n> case R_CTRL:\n> - retvalue = s->regs[R_CTRL];\n> + case R_FIFO_CTRL:\n> + val32 = s->regs[reg];\n> break;\n> case R_STATUS:\n> - retvalue = s->regs[R_STATUS];\n> - break;\n> -\n> - case R_RDATA:\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->regs[R_STATUS] &= ~R_STATUS_RXFULL_MASK;\n> - if (s->rx_level == 0) {\n> - s->regs[R_STATUS] |= R_STATUS_RXIDLE_MASK;\n> - s->regs[R_STATUS] |= R_STATUS_RXEMPTY_MASK;\n> - }\n> + /* assume that UART always report RXIDLE */\n> + val32 = R_STATUS_RXIDLE_MASK;\n> + /* report RXEMPTY or RXFULL */\n> + switch (fifo8_num_used(&s->rx_fifo)) {\n> + case 0:\n> + val32 |= R_STATUS_RXEMPTY_MASK;\n> + break;\n> + case OT_UART_RX_FIFO_SIZE:\n> + val32 |= R_STATUS_RXFULL_MASK;\n> + break;\n> + default:\n> + break;\n> + }\n> + /* report TXEMPTY+TXIDLE or TXFULL */\n> + switch (fifo8_num_used(&s->tx_fifo)) {\n> + case 0:\n> + val32 |= R_STATUS_TXEMPTY_MASK | R_STATUS_TXIDLE_MASK;\n> + break;\n> + case OT_UART_TX_FIFO_SIZE:\n> + val32 |= R_STATUS_TXFULL_MASK;\n> + break;\n> + default:\n> + break;\n> + }\n> + if (!ot_uart_is_tx_enabled(s)) {\n> + val32 |= R_STATUS_TXIDLE_MASK;\n> + }\n> + if (!ot_uart_is_rx_enabled(s)) {\n> + val32 |= R_STATUS_RXIDLE_MASK;\n> }\n> break;\n> - case R_WDATA:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: wdata is write only\\n\", __func__);\n> - break;\n> -\n> - case R_FIFO_CTRL:\n> - retvalue = s->regs[R_FIFO_CTRL];\n> + case R_RDATA:\n> + val32 = (uint32_t)ot_uart_read_rx_fifo(s);\n> break;\n> case R_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> -\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: RX fifos are not supported\\n\", __func__);\n> - break;\n> -\n> - case R_OVRD:\n> - retvalue = s->regs[R_OVRD];\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: ovrd is not supported\\n\", __func__);\n> + val32 =\n> + (fifo8_num_used(&s->rx_fifo) & 0xffu) << R_FIFO_STATUS_RXLVL_SHIFT;\n> + val32 |=\n> + (fifo8_num_used(&s->tx_fifo) & 0xffu) << R_FIFO_STATUS_TXLVL_SHIFT;\n> break;\n> case R_VAL:\n> - retvalue = s->regs[R_VAL];\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: val is not supported\\n\", __func__);\n> + /*\n> + * This is not trivially implemented due to the QEMU UART\n> + * interface. There is no way to reliably sample or oversample\n> + * given our emulated interface, but some software might poll the\n> + * value of this register to determine break conditions.\n> + *\n> + * As such, default to reporting 16 of the last sample received\n> + * instead. This defaults to 16 idle high samples (as a stop bit is\n> + * always the last received), except for when the `oversample-break`\n> + * property is set and a break condition is received over UART RX,\n> + * where we then show 16 low samples until the next valid UART\n> + * transmission is received (or break is toggled off with the\n> + * `toggle-break` property enabled). This will not be accurate, but\n> + * should be sufficient to support basic software flows that\n> + * essentially use UART break as a strapping mechanism.\n> + */\n> + val32 = (s->in_break && s->oversample_break) ? 0u : UINT16_MAX;\n> + qemu_log_mask(LOG_UNIMP, \"%s: VAL only shows idle%s\\n\", __func__,\n> + (s->oversample_break ? \"/break\" : \"\"));\n> break;\n> + case R_OVRD:\n> case R_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> + val32 = s->regs[reg];\n> + qemu_log_mask(LOG_UNIMP, \"%s: %s is not supported\\n\", __func__,\n> + REG_NAME(reg));\n> + break;\n> + case R_ALERT_TEST:\n> + case R_INTR_TEST:\n> + case R_WDATA:\n> + qemu_log_mask(LOG_GUEST_ERROR, \"%s: W/O register 0x%02x (%s)\\n\",\n> + __func__, (uint32_t)addr, REG_NAME(reg));\n> + val32 = 0;\n> break;\n> default:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: Bad offset 0x%\"HWADDR_PRIx\"\\n\", __func__, addr);\n> - return 0;\n> + qemu_log_mask(LOG_GUEST_ERROR, \"%s: Bad offset 0x%02x\\n\", __func__,\n> + (uint32_t)addr);\n> + val32 = 0;\n> + break;\n> }\n>\n> - return retvalue;\n> + return (uint64_t)val32;\n> }\n>\n> static void ot_uart_write(void *opaque, hwaddr addr, uint64_t val64,\n> unsigned int size)\n> {\n> OtUARTState *s = opaque;\n> - uint32_t value = val64;\n> + uint32_t val32 = val64;\n> +\n> + hwaddr reg = R32_OFF(addr);\n>\n> - switch (addr >> 2) {\n> + switch (reg) {\n> case R_INTR_STATE:\n> - /* Write 1 clear */\n> - s->regs[R_INTR_STATE] &= ~value;\n> + val32 &= INTR_MASK;\n> + s->regs[R_INTR_STATE] &= ~val32; /* RW1C */\n> ot_uart_update_irqs(s);\n> break;\n> case R_INTR_ENABLE:\n> - s->regs[R_INTR_ENABLE] = value;\n> + val32 &= INTR_MASK;\n> + s->regs[R_INTR_ENABLE] = val32;\n> ot_uart_update_irqs(s);\n> break;\n> case R_INTR_TEST:\n> - s->regs[R_INTR_STATE] |= value;\n> + val32 &= INTR_MASK;\n> + s->regs[R_INTR_STATE] |= val32;\n> ot_uart_update_irqs(s);\n> break;\n> -\n> + case R_ALERT_TEST:\n> + val32 &= R_ALERT_TEST_FATAL_FAULT_MASK;\n> + s->regs[reg] = val32;\n> + /* This will also set an IRQ once the alert handler is added */\n> + break;\n> case R_CTRL:\n> - s->regs[R_CTRL] = value;\n> -\n> - if (value & R_CTRL_NF_MASK) {\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_NF is not supported\\n\", __func__);\n> - }\n> - if (value & R_CTRL_SLPBK_MASK) {\n> + if (val32 & ~CTRL_SUP_MASK) {\n> qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_SLPBK is not supported\\n\", __func__);\n> + \"%s: UART_CTRL feature not supported: 0x%08x\\n\",\n> + __func__, val32 & ~CTRL_SUP_MASK);\n> }\n> - if (value & R_CTRL_LLPBK_MASK) {\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_LLPBK is not supported\\n\", __func__);\n> - }\n> - if (value & R_CTRL_PARITY_EN_MASK) {\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_PARITY_EN is not supported\\n\",\n> - __func__);\n> - }\n> - if (value & R_CTRL_PARITY_ODD_MASK) {\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_PARITY_ODD is not supported\\n\",\n> - __func__);\n> - }\n> - if (value & R_CTRL_RXBLVL_MASK) {\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: UART_CTRL_RXBLVL is not supported\\n\", __func__);\n> + uint32_t prev = s->regs[R_CTRL];\n> + s->regs[R_CTRL] = val32 & CTRL_MASK;\n> + uint32_t change = prev ^ s->regs[R_CTRL];\n> + if ((change & R_CTRL_RX_MASK) && ot_uart_is_rx_enabled(s) &&\n> + !ot_uart_is_sys_loopack_enabled(s)) {\n> + qemu_chr_fe_accept_input(&s->chr);\n> }\n> - if (value & R_CTRL_NCO_MASK) {\n> - uint64_t baud = ot_uart_get_baud(s);\n> -\n> - s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;\n> + if ((change & R_CTRL_TX_MASK) && ot_uart_is_tx_enabled(s)) {\n> + /* try sending pending data from TX FIFO if any */\n> + ot_uart_xmit(s);\n> }\n> break;\n> - case R_STATUS:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: status is read only\\n\", __func__);\n> - break;\n> -\n> - case R_RDATA:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: rdata is read only\\n\", __func__);\n> - break;\n> case R_WDATA:\n> - uart_write_tx_fifo(s, value);\n> + uart_write_tx_fifo(s, (uint8_t)(val32 & R_WDATA_WDATA_MASK));\n> break;\n> -\n> case R_FIFO_CTRL:\n> - s->regs[R_FIFO_CTRL] = value;\n> -\n> - if (value & R_FIFO_CTRL_RXRST_MASK) {\n> - s->rx_level = 0;\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: RX fifos are not supported\\n\", __func__);\n> + s->regs[R_FIFO_CTRL] =\n> + val32 & (R_FIFO_CTRL_RXILVL_MASK | R_FIFO_CTRL_TXILVL_MASK);\n> + if (val32 & R_FIFO_CTRL_RXRST_MASK) {\n> + ot_uart_reset_rx_fifo(s);\n> + ot_uart_update_irqs(s);\n> }\n> - if (value & R_FIFO_CTRL_TXRST_MASK) {\n> - s->tx_level = 0;\n> + if (val32 & R_FIFO_CTRL_TXRST_MASK) {\n> + ot_uart_reset_tx_fifo(s);\n> + ot_uart_update_irqs(s);\n> }\n> break;\n> - case R_FIFO_STATUS:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: fifo_status is read only\\n\", __func__);\n> - break;\n> -\n> case R_OVRD:\n> - s->regs[R_OVRD] = value;\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: ovrd is not supported\\n\", __func__);\n> - break;\n> - case R_VAL:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: val is read only\\n\", __func__);\n> + if (val32 & R_OVRD_TXEN_MASK) {\n> + qemu_log_mask(LOG_UNIMP, \"%s: OVRD.TXEN is not supported\\n\",\n> + __func__);\n> + }\n> + s->regs[R_OVRD] = val32 & R_OVRD_TXVAL_MASK;\n> break;\n> case R_TIMEOUT_CTRL:\n> - s->regs[R_TIMEOUT_CTRL] = value;\n> - qemu_log_mask(LOG_UNIMP,\n> - \"%s: timeout_ctrl is not supported\\n\", __func__);\n> + s->regs[R_TIMEOUT_CTRL] =\n> + val32 & (R_TIMEOUT_CTRL_EN_MASK | R_TIMEOUT_CTRL_VAL_MASK);\n> + break;\n> + case R_STATUS:\n> + case R_RDATA:\n> + case R_FIFO_STATUS:\n> + case R_VAL:\n> + qemu_log_mask(LOG_GUEST_ERROR, \"%s: R/O register 0x%02x (%s)\\n\",\n> + __func__, (uint32_t)addr, REG_NAME(reg));\n> break;\n> default:\n> - qemu_log_mask(LOG_GUEST_ERROR,\n> - \"%s: Bad offset 0x%\"HWADDR_PRIx\"\\n\", __func__, addr);\n> - }\n> -}\n> -\n> -static void ot_uart_clk_update(void *opaque, ClockEvent event)\n> -{\n> - OtUARTState *s = opaque;\n> -\n> - /* recompute uart's speed on clock change */\n> - uint64_t baud = ot_uart_get_baud(s);\n> -\n> - s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;\n> -}\n> -\n> -static void fifo_trigger_update(void *opaque)\n> -{\n> - OtUARTState *s = opaque;\n> -\n> - if (s->regs[R_CTRL] & R_CTRL_TX_MASK) {\n> - ot_uart_xmit(s);\n> + qemu_log_mask(LOG_GUEST_ERROR, \"%s: Bad offset 0x%02x\\n\", __func__,\n> + (uint32_t)addr);\n> + break;\n> }\n> }\n>\n> @@ -519,15 +616,12 @@ 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 = 2,\n> - .minimum_version_id = 2,\n> + .version_id = 3,\n> + .minimum_version_id = 3,\n> .post_load = ot_uart_post_load,\n> .fields = (const VMStateField[]) {\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_ARRAY(regs, OtUARTState, REGS_COUNT, 1, vmstate_info_uint32,\n> uint32_t),\n> VMSTATE_END_OF_LIST()\n> @@ -536,22 +630,38 @@ static const VMStateDescription vmstate_ot_uart = {\n>\n> static const Property ot_uart_properties[] = {\n> DEFINE_PROP_CHR(\"chardev\", OtUARTState, chr),\n> + DEFINE_PROP_BOOL(\"oversample-break\", OtUARTState, oversample_break, false),\n> + DEFINE_PROP_BOOL(\"toggle-break\", OtUARTState, toggle_break, false),\n> };\n>\n> +static int ot_uart_fe_change(void *opaque)\n> +{\n> + OtUARTState *s = opaque;\n> +\n> + qemu_chr_fe_set_handlers(&s->chr, ot_uart_can_receive, ot_uart_receive,\n> + ot_uart_event_handler, ot_uart_fe_change, s, NULL,\n> + true);\n> +\n> + if (s->watch_tag > 0) {\n> + g_source_remove(s->watch_tag);\n> + /* NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) */\n> + s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,\n> + ot_uart_watch_cb, s);\n> + }\n> +\n> + return 0;\n> +}\n> +\n> static void ot_uart_init(Object *obj)\n> {\n> OtUARTState *s = OT_UART(obj);\n>\n> - s->f_clk = qdev_init_clock_in(DEVICE(obj), \"f_clock\",\n> - ot_uart_clk_update, s, ClockUpdate);\n> - clock_set_hz(s->f_clk, OT_UART_CLOCK);\n> -\n> for (unsigned index = 0; index < OT_UART_IRQ_NUM; index++) {\n> sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irqs[index]);\n> }\n>\n> - memory_region_init_io(&s->mmio, obj, &ot_uart_ops, s,\n> - TYPE_OT_UART, 0x400);\n> + memory_region_init_io(&s->mmio, obj, &ot_uart_ops, s, TYPE_OT_UART,\n> + REGS_SIZE);\n> sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);\n>\n> /*\n> @@ -567,15 +677,14 @@ static void ot_uart_realize(DeviceState *dev, Error **errp)\n> {\n> OtUARTState *s = OT_UART(dev);\n>\n> - s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,\n> - fifo_trigger_update, s);\n> + qdev_init_gpio_in_named(DEVICE(s), &ot_uart_clock_input, \"clock-in\", 1);\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> + qemu_chr_fe_set_handlers(&s->chr, ot_uart_can_receive, ot_uart_receive,\n> + ot_uart_event_handler, ot_uart_fe_change, s, NULL,\n> + true);\n> }\n>\n> static void ot_uart_class_init(ObjectClass *klass, const void *data)\n> diff --git a/include/hw/char/ot_uart.h b/include/hw/char/ot_uart.h\n> index a2c5ff8b33..512684afd6 100644\n> --- a/include/hw/char/ot_uart.h\n> +++ b/include/hw/char/ot_uart.h\n> @@ -28,11 +28,8 @@\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_CLOCK 50000000 /* 50MHz clock */\n> -\n> #define TYPE_OT_UART \"ot-uart\"\n> OBJECT_DECLARE_TYPE(OtUARTState, OtUARTClass, OT_UART)\n>\n> @@ -44,22 +41,20 @@ struct OtUARTState {\n> MemoryRegion mmio;\n> qemu_irq irqs[9];\n>\n> - uint32_t tx_level;\n> -\n> - uint32_t rx_level;\n> -\n> - QEMUTimer *fifo_trigger_handle;\n> - uint64_t char_tx_time;\n> -\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> + bool in_break;\n> + guint watch_tag;\n> + unsigned pclk; /* Current input clock */\n> + const char *clock_src_name; /* IRQ name once connected */\n>\n> - Clock *f_clk;\n> -\n> + DeviceState *clock_src;\n> CharFrontend chr;\n> + bool oversample_break; /* Should mock break in the oversampled VAL reg? */\n> + bool toggle_break; /* Are incoming breaks temporary or toggled? */\n> };\n>\n> struct OtUARTClass {\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=dVkNzbqS;\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 4fsM9m6WBDz20HT\n\tfor ; Fri, 10 Apr 2026 13:05:16 +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 1wB2B0-0005BC-Pg; Thu, 09 Apr 2026 23:04:58 -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 1wB2Az-0005Ac-95\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 23:04:57 -0400","from mail-ed1-x532.google.com ([2a00:1450:4864:20::532])\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 1wB2Aw-0008NG-Cd\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 23:04:57 -0400","by mail-ed1-x532.google.com with SMTP id\n 4fb4d7f45d1cf-66bb6eb28acso2899184a12.0\n for ; Thu, 09 Apr 2026 20:04:53 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; t=1775790293; cv=none;\n d=google.com; s=arc-20240605;\n b=hG479K7DeG9FHTCfAY5xaV9oCk7e8puBwcHZS6/uZf8OQb1/0gSncd2yb/8WqjTDEa\n mcyMGonngi/lS/O0O/6DMeEZUBnvnzUaBNJ9nlIOF1AJTxdCfGv+vxlB+gH/bW9VnNEb\n u/Rnxoru8IkhJTj56mkYs7uz8mBTic8eV+nP2BV29oUp0d5IfzXwbQvW3mN8xt84IORH\n 2qKDEa1YFRm3JQ6LfiZXP/RkWseUVcsS/vGCTYAVPYs2hyHYtiQEq+h0f84WskaeOLUC\n miPPtjDPl2g93KkSA1wPO0X5DD8X6AOnRLwP+5N0OC3dJzsEu8bS32UuP+57afDHX0dm\n cbSQ==","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=/bbgEJfaIkIowVYcS0htMfGzYrz1l9lMq7r4KDKlQ3w=;\n fh=qF9ugW4szhnyS0qe8D5bFnruRtXcqvkgvX/fNNkd/yw=;\n b=ThEDQFld+dJHAirz8g2nX6UFzvdQ2hdpH5YpkSgvifW84Ywc9nl72YkS1JHcJm55Cz\n 8Y/NmvavFavmDag7rESo6zazkp2xbg91CgocLVDhv0DVUMsF2/wTQWc+ECPCtHad3jch\n UJouwH887LucOrH0Rm6mdep5GWgbHSPdEKjPhjvTLl2QeL9tidAb4RL6lqsJKNRk3dSI\n gulO9o0EAuZWd3zqp45T2r7wKkiiBsHcnQBvlNsy2knUwGOMmmbk1WqcGobQJeMCzqDP\n DgpC2ZECnFpoYmn9LjGzYlJPSq6wHjZyDnPzCNydv8Ge945z3anR3rWQZPdsJXhbtk62\n EB8A==; 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=1775790293; x=1776395093; 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=/bbgEJfaIkIowVYcS0htMfGzYrz1l9lMq7r4KDKlQ3w=;\n b=dVkNzbqSHPeIqUjaYmw9anNQ4jz7Os2hfltIWonHu3bElvCzRZ3xngsvwxsSZ9NdOC\n 7pOGhaURc3lhb8neF7jNSV1lWhLrhvBaAV248MxxRBVo4AiMIp0kr/2EUfmk61TvXZkd\n c38r6zFX9vBRRbe1aSAzFcxfhloOsymRorVU3pauEGA3OzVYDTcxBMW1ZHVkXXQkJrxl\n hUIoqpS4gLGASWcB6yHVgmJrOqJuB6Gx9Mueq+rtuD582/ApC2Ok6/Xx71QVQA2c57jI\n RPJQSOdtVRY96Gc8MVO20gSmfFyseNtYSc+WnIuJz/AtvAH9sZ/sQtkENFJKbzcrvV0n\n D+HA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1775790293; x=1776395093;\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=/bbgEJfaIkIowVYcS0htMfGzYrz1l9lMq7r4KDKlQ3w=;\n b=BzRBTvmMctcatCcCdzKXSvNgG6XQrMn2fHsrNSEW29UWLg5MOsPMhvqyOD1SBz+p8c\n AxO1h61CytSHqlBpiG3Vq/tnjALNSSyJIHA0bJqXRbGLeJH4+LTMTLUsYoR2r6D+pZRX\n kPQSKRhg/2w7N+05/fqZgjZ/sUVa9Htgbh44jj4WrWpv5VxFyirwnUxbpm6Au7p9UHH+\n xXI0l1Rh1megskIB/e3rix0Wawykw55KBbzALRrt/LvSGwnF+YUnEL+CaWk8dTbT6LBp\n vQKymLJd7U34vGa0qZHdRqOxOYmTAaTTeUUDvCBYYNUNnK/B681wzQFsI8WGDT2iRQ1P\n /0zw==","X-Forwarded-Encrypted":"i=1;\n AJvYcCUUSvBi8TQ6hIYTZhbKofKuJOQELGhcRmjMmWuUyd89ByyJbyF7PCpx/eNKHRoDNmwdKfsoEBwdWTaX@nongnu.org","X-Gm-Message-State":"AOJu0Ywnl+7QUGH6fBGrbPKPCVH5t9YHE6lvEwwoq4feKWKdEDoZC/lx\n bkLLPwWOqsbFj1JBSjiAuke4yMLfraq1Ymx7KphJ9cdCc4MMnzGspq3MFjnXThQBhOUVgfP7zP9\n LSjQ442dNLhWBfqXMxSeg2WTA54INLTI=","X-Gm-Gg":"AeBDiesCRFprhZOHbPenwID71BMYyqTMC6Zq7r+0OPaDSekfPEd7HFLSLAoH9rAElo7\n STMX175x/Og4pxONUorXyMfdLbGfXHaZvPRX5eRJYqTulVf/Zxd4mSKPiZ5bAT8V0fLyyC8i3wD\n SSvCEszoNY0yaZe4/jSmmSP4Jfrw9r68LTAtVDfhuqafDYWLQ4aGTu7v03LOd5Ro/IXIP4UD1ra\n GuzTYDICSC5kUeq9Em16PV110CCmxGN9CyuuDQG3a6q0YWo4vGuvOWcykgQ7Fc+ptobGBeatbdY\n z1NXNbHjCe5RHa90xGmG9zIQgMpoqUIpi3IxeA==","X-Received":"by 2002:a17:907:1c24:b0:b9d:30b1:66ff with SMTP id\n a640c23a62f3a-b9d728229dcmr89700466b.43.1775790292364; Thu, 09 Apr 2026\n 20:04:52 -0700 (PDT)","MIME-Version":"1.0","References":"<177564643888.23414.7922925369077631439-0@git.sr.ht>\n <177564643888.23414.7922925369077631439-5@git.sr.ht>","In-Reply-To":"<177564643888.23414.7922925369077631439-5@git.sr.ht>","From":"Alistair Francis ","Date":"Fri, 10 Apr 2026 13:04:25 +1000","X-Gm-Features":"AQROBzDuNSwmvU5QzCGJ3HeDsrM75uHKYnn5L5PYwDsZHX40MrkHeB37S31lJpw","Message-ID":"\n ","Subject":"Re: [PATCH qemu v2 5/7] ot_uart: implement missing features","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::532;\n envelope-from=alistair23@gmail.com; helo=mail-ed1-x532.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"}}]