[v2] hw/char: Improve RX FIFO depth uses

[PATCH v2 6/9] hw/char/imx_serial: Really use RX FIFO depth

Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago

While we model a 32-elements RX FIFO since the IMX serial
model was introduced in commit 988f2442971 ("hw/char/imx_serial:
Implement receive FIFO and ageing timer") we only read 1 char
at a time!

Have the IOCanReadHandler handler return how many elements are
available, and use that in the IOReadHandler handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Luc Michel <luc.michel@amd.com>
---
 hw/char/imx_serial.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index 38b4865157e..6f14f8403a9 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -386,7 +386,8 @@ static void imx_serial_write(void *opaque, hwaddr offset,
 static int imx_can_receive(void *opaque)
 {
     IMXSerialState *s = (IMXSerialState *)opaque;
-    return s->ucr2 & UCR2_RXEN && fifo32_num_used(&s->rx_fifo) < FIFO_SIZE;
+
+    return s->ucr2 & UCR2_RXEN ? fifo32_num_free(&s->rx_fifo) : 0;
 }
 
 static void imx_put_data(void *opaque, uint32_t value)
@@ -417,7 +418,10 @@ static void imx_receive(void *opaque, const uint8_t *buf, int size)
     IMXSerialState *s = (IMXSerialState *)opaque;
 
     s->usr2 |= USR2_WAKE;
-    imx_put_data(opaque, *buf);
+
+    for (int i = 0; i < size; i++) {
+        imx_put_data(opaque, buf[i]);
+    }
 }
 
 static void imx_event(void *opaque, QEMUChrEvent event)
-- 
2.47.1

Re: [PATCH v2 6/9] hw/char/imx_serial: Really use RX FIFO depth

Posted by Bernhard Beschow 11 months, 2 weeks ago


Am 20. Februar 2025 09:28:59 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>While we model a 32-elements RX FIFO since the IMX serial
>model was introduced in commit 988f2442971 ("hw/char/imx_serial:
>Implement receive FIFO and ageing timer") we only read 1 char
>at a time!
>
>Have the IOCanReadHandler handler return how many elements are
>available, and use that in the IOReadHandler handler.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Reviewed-by: Luc Michel <luc.michel@amd.com>

Tested-by: Bernhard Beschow <shentey@gmail.com>

>---
> hw/char/imx_serial.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
>index 38b4865157e..6f14f8403a9 100644
>--- a/hw/char/imx_serial.c
>+++ b/hw/char/imx_serial.c
>@@ -386,7 +386,8 @@ static void imx_serial_write(void *opaque, hwaddr offset,
> static int imx_can_receive(void *opaque)
> {
>     IMXSerialState *s = (IMXSerialState *)opaque;
>-    return s->ucr2 & UCR2_RXEN && fifo32_num_used(&s->rx_fifo) < FIFO_SIZE;
>+
>+    return s->ucr2 & UCR2_RXEN ? fifo32_num_free(&s->rx_fifo) : 0;
> }
> 
> static void imx_put_data(void *opaque, uint32_t value)
>@@ -417,7 +418,10 @@ static void imx_receive(void *opaque, const uint8_t *buf, int size)
>     IMXSerialState *s = (IMXSerialState *)opaque;
> 
>     s->usr2 |= USR2_WAKE;
>-    imx_put_data(opaque, *buf);
>+
>+    for (int i = 0; i < size; i++) {
>+        imx_put_data(opaque, buf[i]);
>+    }
> }
> 
> static void imx_event(void *opaque, QEMUChrEvent event)

Re: [PATCH v2 6/9] hw/char/imx_serial: Really use RX FIFO depth

Posted by Richard Henderson 11 months, 2 weeks ago

On 2/20/25 01:28, Philippe Mathieu-Daudé wrote:
> While we model a 32-elements RX FIFO since the IMX serial
> model was introduced in commit 988f2442971 ("hw/char/imx_serial:
> Implement receive FIFO and ageing timer") we only read 1 char
> at a time!
> 
> Have the IOCanReadHandler handler return how many elements are
> available, and use that in the IOReadHandler handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Luc Michel <luc.michel@amd.com>
> ---
>   hw/char/imx_serial.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~