Fix SiFive UART character drop issue and minor refactors

[PATCH 0/4] Fix SiFive UART character drop issue and minor refactors

Posted by frank.chang@sifive.com 2 weeks, 3 days ago

From: Frank Chang <frank.chang@sifive.com>

This patch set fixes the SiFive UART character drop issue introduced
after commit [1], which changed character printing from synchronous to
asynchronous.

Since UART now transmits characters asynchronously, it is possible for
the Tx FIFO to become full, causing new characters to be ignored and
dropped when running Linux. This happens because:

  1. The Linux SiFive UART driver sets the transmit watermark level to 1
     [2], meaning a transmit watermark interrupt is raised whenever a
     character is enqueued into the Tx FIFO.
  2. Upon receiving a transmit watermark interrupt, the Linux driver
     transfers up to a full Tx FIFO's worth of characters from the Linux
     serial transmit buffer [3], without checking the txdata.full flag
     before transferring multiple characters [4].

This patch set updates QEMU to honor the Tx/Rx watermark thresholds and
raise interrupts only when the Tx threshold is exceeded or the Rx
threshold is undercut.

The remaining patches contain minor refactors, including removing an
outdated comment about the Tx FIFO.

[1] 53c1557b230986ab6320a58e1b2c26216ecd86d5
[2] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L1039
[3] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L538
[4] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L291

Frank Chang (4):
  hw/char: sifive_uart: Raise IRQ according to the Tx/Rx watermark
    thresholds
  hw/char: sifive_uart: Avoid pushing Tx FIFO when size is zero
  hw/char: sifive_uart: Remove outdated comment about Tx FIFO
  hw/char: sifive_uart: Add newline to error message

 hw/char/sifive_uart.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

--
2.49.0

Re: [PATCH 0/4] Fix SiFive UART character drop issue and minor refactors

Posted by Alistair Francis 1 week, 6 days ago

On Fri, Sep 12, 2025 at 2:08 AM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> This patch set fixes the SiFive UART character drop issue introduced
> after commit [1], which changed character printing from synchronous to
> asynchronous.
>
> Since UART now transmits characters asynchronously, it is possible for
> the Tx FIFO to become full, causing new characters to be ignored and
> dropped when running Linux. This happens because:
>
>   1. The Linux SiFive UART driver sets the transmit watermark level to 1
>      [2], meaning a transmit watermark interrupt is raised whenever a
>      character is enqueued into the Tx FIFO.
>   2. Upon receiving a transmit watermark interrupt, the Linux driver
>      transfers up to a full Tx FIFO's worth of characters from the Linux
>      serial transmit buffer [3], without checking the txdata.full flag
>      before transferring multiple characters [4].
>
> This patch set updates QEMU to honor the Tx/Rx watermark thresholds and
> raise interrupts only when the Tx threshold is exceeded or the Rx
> threshold is undercut.
>
> The remaining patches contain minor refactors, including removing an
> outdated comment about the Tx FIFO.
>
> [1] 53c1557b230986ab6320a58e1b2c26216ecd86d5
> [2] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L1039
> [3] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L538
> [4] https://github.com/torvalds/linux/blob/master/drivers/tty/serial/sifive.c#L291
>
> Frank Chang (4):
>   hw/char: sifive_uart: Raise IRQ according to the Tx/Rx watermark
>     thresholds
>   hw/char: sifive_uart: Avoid pushing Tx FIFO when size is zero
>   hw/char: sifive_uart: Remove outdated comment about Tx FIFO
>   hw/char: sifive_uart: Add newline to error message

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  hw/char/sifive_uart.c | 36 ++++++++++++++++--------------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
>
> --
> 2.49.0
>
>