From: Frank Chang <frank.chang@sifive.com>
Implement txctrl.txen and rxctrl.rxen as follows:
* txctrl.txen
The txen bit controls whether the Tx channel is active. When cleared,
transmission of Tx FIFO contents is suppressed, and the txd pin is
driven high.
* rxctrl.rxen:
The rxen bit controls whether the Rx channel is active. When cleared,
the state of the rxd pin is ignored, and no characters will be
enqueued into the Rx FIFO.
Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
cleared.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
hw/char/sifive_uart.c | 27 ++++++++++++++++++++-------
include/hw/char/sifive_uart.h | 2 ++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index af17cf9a6ce..3ce6a4ee76a 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
return G_SOURCE_REMOVE;
}
+ /* Don't pop the FIFO if transmit is disabled. */
+ if (!SIFIVE_UART_TXEN(s->txctrl)) {
+ return G_SOURCE_REMOVE;
+ }
+
/* Don't pop the FIFO in case the write fails */
characters = fifo8_peek_bufptr(&s->tx_fifo,
fifo8_num_used(&s->tx_fifo), &numptr);
@@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
return G_SOURCE_REMOVE;
}
-static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
- int size)
+static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
{
uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ if (!timer_pending(s->fifo_trigger_handle)) {
+ timer_mod(s->fifo_trigger_handle, current_time +
+ TX_INTERRUPT_TRIGGER_DELAY_NS);
+ }
+}
+
+static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
+ int size)
+{
if (size > fifo8_num_free(&s->tx_fifo)) {
size = fifo8_num_free(&s->tx_fifo);
qemu_log_mask(LOG_GUEST_ERROR, "sifive_uart: TX FIFO overflow.\n");
@@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
}
- if (!timer_pending(s->fifo_trigger_handle)) {
- timer_mod(s->fifo_trigger_handle, current_time +
- TX_INTERRUPT_TRIGGER_DELAY_NS);
- }
+ sifive_uart_trigger_tx_fifo(s);
}
static uint64_t
@@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
return;
case SIFIVE_UART_TXCTRL:
s->txctrl = val64;
+ if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
+ sifive_uart_trigger_tx_fifo(s);
+ }
return;
case SIFIVE_UART_RXCTRL:
s->rxctrl = val64;
@@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
{
SiFiveUARTState *s = opaque;
- return s->rx_fifo_len < sizeof(s->rx_fifo);
+ return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
}
static void sifive_uart_event(void *opaque, QEMUChrEvent event)
diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
index 414564b0266..c78d9bd1fc6 100644
--- a/include/hw/char/sifive_uart.h
+++ b/include/hw/char/sifive_uart.h
@@ -51,6 +51,8 @@ enum {
#define SIFIVE_UART_TXFIFO_FULL 0x80000000
+#define SIFIVE_UART_TXEN(txctrl) (txctrl & 0x1)
+#define SIFIVE_UART_RXEN(rxctrl) (rxctrl & 0x1)
#define SIFIVE_UART_GET_TXCNT(txctrl) ((txctrl >> 16) & 0x7)
#define SIFIVE_UART_GET_RXCNT(rxctrl) ((rxctrl >> 16) & 0x7)
--
2.43.0On Thu, Mar 12, 2026 at 11:31:58AM +0800, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Implement txctrl.txen and rxctrl.rxen as follows:
>
> * txctrl.txen
> The txen bit controls whether the Tx channel is active. When cleared,
> transmission of Tx FIFO contents is suppressed, and the txd pin is
> driven high.
>
> * rxctrl.rxen:
> The rxen bit controls whether the Rx channel is active. When cleared,
> the state of the rxd pin is ignored, and no characters will be
> enqueued into the Rx FIFO.
>
> Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
> cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
> cleared.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
> hw/char/sifive_uart.c | 27 ++++++++++++++++++++-------
> include/hw/char/sifive_uart.h | 2 ++
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index af17cf9a6ce..3ce6a4ee76a 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
> return G_SOURCE_REMOVE;
> }
>
> + /* Don't pop the FIFO if transmit is disabled. */
> + if (!SIFIVE_UART_TXEN(s->txctrl)) {
> + return G_SOURCE_REMOVE;
> + }
> +
> /* Don't pop the FIFO in case the write fails */
> characters = fifo8_peek_bufptr(&s->tx_fifo,
> fifo8_num_used(&s->tx_fifo), &numptr);
> @@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
> return G_SOURCE_REMOVE;
> }
>
> -static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> - int size)
> +static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
> {
> uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> + if (!timer_pending(s->fifo_trigger_handle)) {
> + timer_mod(s->fifo_trigger_handle, current_time +
> + TX_INTERRUPT_TRIGGER_DELAY_NS);
> + }
> +}
> +
> +static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> + int size)
> +{
> if (size > fifo8_num_free(&s->tx_fifo)) {
> size = fifo8_num_free(&s->tx_fifo);
> qemu_log_mask(LOG_GUEST_ERROR, "sifive_uart: TX FIFO overflow.\n");
> @@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
> }
>
> - if (!timer_pending(s->fifo_trigger_handle)) {
> - timer_mod(s->fifo_trigger_handle, current_time +
> - TX_INTERRUPT_TRIGGER_DELAY_NS);
> - }
> + sifive_uart_trigger_tx_fifo(s);
> }
>
> static uint64_t
> @@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
> return;
> case SIFIVE_UART_TXCTRL:
> s->txctrl = val64;
A sifive_uart_update_irq() call is also needed here.
When txctrl is written, the watermark (txcnt) may
change, or txen may toggle(you can see SiFive FE310-G002
datasheet), both of which can alter the txwm pending state.
Patch 3/4 adds this call for RXCTRL with the rationale "the
user may activate the Rx channel or change the Rx FIFO
watermark level" -- the same reasoning applies to TXCTRL.
Without this, there's a window where the IRQ line
doesn't reflect the actual pending state after a
txctrl write that changes the watermark.
Everything else looks good to me.
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Thanks,
Chao
> + if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
> + sifive_uart_trigger_tx_fifo(s);
> + }
> return;
> case SIFIVE_UART_RXCTRL:
> s->rxctrl = val64;
> @@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
> {
> SiFiveUARTState *s = opaque;
>
> - return s->rx_fifo_len < sizeof(s->rx_fifo);
> + return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
> }
>
> static void sifive_uart_event(void *opaque, QEMUChrEvent event)
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index 414564b0266..c78d9bd1fc6 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -51,6 +51,8 @@ enum {
>
> #define SIFIVE_UART_TXFIFO_FULL 0x80000000
>
> +#define SIFIVE_UART_TXEN(txctrl) (txctrl & 0x1)
> +#define SIFIVE_UART_RXEN(rxctrl) (rxctrl & 0x1)
macro arguments should be parenthesized to avoid precedence surprises.
e.g. if the argument is ever an expression like (a | b), & binds tighter
than |. I suggest:
#define SIFIVE_UART_TXEN(txctrl) ((txctrl) & 0x1)
#define SIFIVE_UART_RXEN(rxctrl) ((rxctrl) & 0x1)
The existing GET_TXCNT / GET_RXCNT macros have the
same issue, so this could be a follow-up cleanup.
> #define SIFIVE_UART_GET_TXCNT(txctrl) ((txctrl >> 16) & 0x7)
> #define SIFIVE_UART_GET_RXCNT(rxctrl) ((rxctrl >> 16) & 0x7)
>
> --
> 2.43.0
>
>
On Thu, Mar 12, 2026 at 1:33 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Implement txctrl.txen and rxctrl.rxen as follows:
>
> * txctrl.txen
> The txen bit controls whether the Tx channel is active. When cleared,
> transmission of Tx FIFO contents is suppressed, and the txd pin is
> driven high.
>
> * rxctrl.rxen:
> The rxen bit controls whether the Rx channel is active. When cleared,
> the state of the rxd pin is ignored, and no characters will be
> enqueued into the Rx FIFO.
>
> Therefore, the Tx FIFO should not be dequeued when txctrl.txen is
> cleared, and the Rx FIFO should not be enqueued when rxctrl.rxen is
> cleared.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/char/sifive_uart.c | 27 ++++++++++++++++++++-------
> include/hw/char/sifive_uart.h | 2 ++
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index af17cf9a6ce..3ce6a4ee76a 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -78,6 +78,11 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
> return G_SOURCE_REMOVE;
> }
>
> + /* Don't pop the FIFO if transmit is disabled. */
> + if (!SIFIVE_UART_TXEN(s->txctrl)) {
> + return G_SOURCE_REMOVE;
> + }
> +
> /* Don't pop the FIFO in case the write fails */
> characters = fifo8_peek_bufptr(&s->tx_fifo,
> fifo8_num_used(&s->tx_fifo), &numptr);
> @@ -106,11 +111,19 @@ static gboolean sifive_uart_xmit(void *do_not_use, GIOCondition cond,
> return G_SOURCE_REMOVE;
> }
>
> -static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> - int size)
> +static void sifive_uart_trigger_tx_fifo(SiFiveUARTState *s)
> {
> uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
> + if (!timer_pending(s->fifo_trigger_handle)) {
> + timer_mod(s->fifo_trigger_handle, current_time +
> + TX_INTERRUPT_TRIGGER_DELAY_NS);
> + }
> +}
> +
> +static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> + int size)
> +{
> if (size > fifo8_num_free(&s->tx_fifo)) {
> size = fifo8_num_free(&s->tx_fifo);
> qemu_log_mask(LOG_GUEST_ERROR, "sifive_uart: TX FIFO overflow.\n");
> @@ -124,10 +137,7 @@ static void sifive_uart_write_tx_fifo(SiFiveUARTState *s, const uint8_t *buf,
> s->txfifo |= SIFIVE_UART_TXFIFO_FULL;
> }
>
> - if (!timer_pending(s->fifo_trigger_handle)) {
> - timer_mod(s->fifo_trigger_handle, current_time +
> - TX_INTERRUPT_TRIGGER_DELAY_NS);
> - }
> + sifive_uart_trigger_tx_fifo(s);
> }
>
> static uint64_t
> @@ -184,6 +194,9 @@ sifive_uart_write(void *opaque, hwaddr addr,
> return;
> case SIFIVE_UART_TXCTRL:
> s->txctrl = val64;
> + if (SIFIVE_UART_TXEN(s->txctrl) && !fifo8_is_empty(&s->tx_fifo)) {
> + sifive_uart_trigger_tx_fifo(s);
> + }
> return;
> case SIFIVE_UART_RXCTRL:
> s->rxctrl = val64;
> @@ -231,7 +244,7 @@ static int sifive_uart_can_rx(void *opaque)
> {
> SiFiveUARTState *s = opaque;
>
> - return s->rx_fifo_len < sizeof(s->rx_fifo);
> + return SIFIVE_UART_RXEN(s->rxctrl) && (s->rx_fifo_len < sizeof(s->rx_fifo));
> }
>
> static void sifive_uart_event(void *opaque, QEMUChrEvent event)
> diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
> index 414564b0266..c78d9bd1fc6 100644
> --- a/include/hw/char/sifive_uart.h
> +++ b/include/hw/char/sifive_uart.h
> @@ -51,6 +51,8 @@ enum {
>
> #define SIFIVE_UART_TXFIFO_FULL 0x80000000
>
> +#define SIFIVE_UART_TXEN(txctrl) (txctrl & 0x1)
> +#define SIFIVE_UART_RXEN(rxctrl) (rxctrl & 0x1)
> #define SIFIVE_UART_GET_TXCNT(txctrl) ((txctrl >> 16) & 0x7)
> #define SIFIVE_UART_GET_RXCNT(rxctrl) ((rxctrl >> 16) & 0x7)
>
> --
> 2.43.0
>
>
© 2016 - 2026 Red Hat, Inc.