On 22:08 Wed 19 Feb     , Philippe Mathieu-Daudé wrote:
> While we model a 16-elements RX FIFO since the PL011 model was
> introduced in commit cdbdb648b7c ("ARM Versatile Platform Baseboard
> emulation"), 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.
> 
> Example of FIFO better used by enabling the pl011 tracing events
> and running the tests/functional/test_aarch64_virt.py tests:
> 
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_receive recv 5 chars
>   pl011_fifo_rx_put RX FIFO push char [0x72] 1/16 depth used
>   pl011_irq_state irq state 1
>   pl011_fifo_rx_put RX FIFO push char [0x6f] 2/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x6f] 3/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x74] 4/16 depth used
>   pl011_fifo_rx_put RX FIFO push char [0x0d] 5/16 depth used
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_write addr 0x038 value 0x00000050 reg IMSC
>   pl011_irq_state irq state 1
>   pl011_can_receive LCR 0x70, RX FIFO used 5/16, can_receive 11 chars
>   pl011_read addr 0x03c value 0x00000030 reg RIS
>   pl011_write addr 0x044 value 0x00000000 reg ICR
>   pl011_irq_state irq state 1
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 4/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x00000072 reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 4/16, can_receive 12 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 3/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x0000006f reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 3/16, can_receive 13 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 2/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x0000006f reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 2/16, can_receive 14 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 1/16
>   pl011_irq_state irq state 1
>   pl011_read addr 0x000 value 0x00000074 reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 1/16, can_receive 15 chars
>   pl011_read addr 0x018 value 0x00000080 reg FR
>   pl011_read_fifo RX FIFO read, used 0/16
>   pl011_irq_state irq state 0
>   pl011_read addr 0x000 value 0x0000000d reg DR
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_read addr 0x018 value 0x00000090 reg FR
>   pl011_read addr 0x03c value 0x00000020 reg RIS
>   pl011_write addr 0x038 value 0x00000050 reg IMSC
>   pl011_irq_state irq state 0
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_can_receive LCR 0x70, RX FIFO used 0/16, can_receive 16 chars
>   pl011_read addr 0x018 value 0x00000090 reg FR
>   pl011_write addr 0x000 value 0x00000072 reg DR
> 
> Inspired-by: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>

Reviewed-by: Luc Michel <luc.mic...@amd.com>

> ---
>  hw/char/pl011.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index 148a7d0dc60..57d293d1e3a 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -489,7 +489,6 @@ static int pl011_can_receive(void *opaque)
>      PL011State *s = (PL011State *)opaque;
>      unsigned fifo_depth = pl011_get_fifo_depth(s);
>      unsigned fifo_available = fifo_depth - s->read_count;
> -    int r = fifo_available ? 1 : 0;
> 
>      if (!(s->cr & CR_UARTEN)) {
>          qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled 
> UART\n");
> @@ -498,7 +497,8 @@ static int pl011_can_receive(void *opaque)
>          qemu_log_mask(LOG_GUEST_ERROR, "PL011 reading data on disabled TX 
> UART\n");
>      }
>      trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, 
> fifo_available);
> -    return r;
> +
> +    return fifo_available;
>  }
> 
>  static void pl011_receive(void *opaque, const uint8_t *buf, int size)
> @@ -513,7 +513,9 @@ static void pl011_receive(void *opaque, const uint8_t 
> *buf, int size)
>          return;
>      }
> 
> -    pl011_fifo_rx_put(opaque, *buf);
> +    for (int i = 0; i < size; i++) {
> +        pl011_fifo_rx_put(opaque, buf[i]);
> +    }
>  }
> 
>  static void pl011_event(void *opaque, QEMUChrEvent event)
> --
> 2.47.1
> 

-- 

Reply via email to