| Message ID | 20250220092903.3726-10-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/char: Improve RX FIFO depth uses | expand |
On 2/20/25 01:29, Philippe Mathieu-Daudé wrote: > In the IOCanReadHandler sh_serial_can_receive(), if the Serial > Control Register 'Receive Enable' bit is set (bit 4), then we > return a size of (1 << 4) which happens to be equal to 16, so > effectively SH_RX_FIFO_LENGTH. > > The IOReadHandler, sh_serial_receive1() takes care to receive > multiple chars, but if the FIFO is partly filled, we only process > the number of free slots in the FIFO, discarding the other chars! > > Fix by returning how many elements the FIFO can queue in the > IOCanReadHandler, so we don't have to process more than that in > the IOReadHandler, thus not discarding anything. > > Remove the now unnecessary check on 's->rx_cnt < SH_RX_FIFO_LENGTH' > in IOReadHandler, reducing the block indentation. > > Fixes: 63242a007a1 ("SH4: Serial controller improvement") > Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org> > Reviewed-by: Luc Michel<luc.michel@amd.com> > --- > hw/char/sh_serial.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c index 247aeb071ac..41c8175a638 100644 --- a/hw/char/sh_serial.c +++ b/hw/char/sh_serial.c @@ -320,7 +320,7 @@ static uint64_t sh_serial_read(void *opaque, hwaddr offs, static int sh_serial_can_receive(SHSerialState *s) { - return s->scr & (1 << 4); + return s->scr & (1 << 4) ? SH_RX_FIFO_LENGTH - s->rx_head : 0; } static void sh_serial_receive_break(SHSerialState *s) @@ -353,22 +353,20 @@ static void sh_serial_receive1(void *opaque, const uint8_t *buf, int size) if (s->feat & SH_SERIAL_FEAT_SCIF) { int i; for (i = 0; i < size; i++) { - if (s->rx_cnt < SH_RX_FIFO_LENGTH) { - s->rx_fifo[s->rx_head++] = buf[i]; - if (s->rx_head == SH_RX_FIFO_LENGTH) { - s->rx_head = 0; - } - s->rx_cnt++; - if (s->rx_cnt >= s->rtrg) { - s->flags |= SH_SERIAL_FLAG_RDF; - if (s->scr & (1 << 6) && s->rxi) { - timer_del(&s->fifo_timeout_timer); - qemu_set_irq(s->rxi, 1); - } - } else { - timer_mod(&s->fifo_timeout_timer, - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 15 * s->etu); + s->rx_fifo[s->rx_head++] = buf[i]; + if (s->rx_head == SH_RX_FIFO_LENGTH) { + s->rx_head = 0; + } + s->rx_cnt++; + if (s->rx_cnt >= s->rtrg) { + s->flags |= SH_SERIAL_FLAG_RDF; + if (s->scr & (1 << 6) && s->rxi) { + timer_del(&s->fifo_timeout_timer); + qemu_set_irq(s->rxi, 1); } + } else { + timer_mod(&s->fifo_timeout_timer, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 15 * s->etu); } } } else {
In the IOCanReadHandler sh_serial_can_receive(), if the Serial Control Register 'Receive Enable' bit is set (bit 4), then we return a size of (1 << 4) which happens to be equal to 16, so effectively SH_RX_FIFO_LENGTH. The IOReadHandler, sh_serial_receive1() takes care to receive multiple chars, but if the FIFO is partly filled, we only process the number of free slots in the FIFO, discarding the other chars! Fix by returning how many elements the FIFO can queue in the IOCanReadHandler, so we don't have to process more than that in the IOReadHandler, thus not discarding anything. Remove the now unnecessary check on 's->rx_cnt < SH_RX_FIFO_LENGTH' in IOReadHandler, reducing the block indentation. Fixes: 63242a007a1 ("SH4: Serial controller improvement") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Luc Michel <luc.michel@amd.com> --- hw/char/sh_serial.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)