Re: [PATCH] hw/char: sifive_uart: Implement txctrl.txen and rxctrl.rxen

[Frank Chang <frank.chang@sifive.com>]

I found other SiFive UART bugs that need to be fixed.
This commit is squashed and replaced by another patchset:
https://patchew.org/QEMU/20251209160117.1239596-1-frank.chang@sifive.com/

Regards,
Frank Chang

<frank.chang@sifive.com> 於 2025年11月14日週五 下午4:44寫道：
>
> 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 e7357d585a1..4a54dd52a1e 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 6486c3f4a5d..e216cacf693 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
>
>