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

[PATCH 5/9] hw/char/bcm2835_aux: Really use RX FIFO depth

Posted by Philippe Mathieu-Daudé 3 days, 8 hours ago

While we model a 8-elements RX FIFO since the PL011 model was
introduced in commit 97398d900ca ("bcm2835_aux: add emulation
of BCM2835 AUX block") 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>
---
 hw/char/bcm2835_aux.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index 73ad5934067..c6e7eccf7dd 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -221,7 +221,7 @@ static int bcm2835_aux_can_receive(void *opaque)
 {
     BCM2835AuxState *s = opaque;
 
-    return s->read_count < BCM2835_AUX_RX_FIFO_LEN;
+    return BCM2835_AUX_RX_FIFO_LEN - s->read_count;
 }
 
 static void bcm2835_aux_put_fifo(void *opaque, uint8_t value)
@@ -243,7 +243,9 @@ static void bcm2835_aux_put_fifo(void *opaque, uint8_t value)
 
 static void bcm2835_aux_receive(void *opaque, const uint8_t *buf, int size)
 {
-    bcm2835_aux_put_fifo(opaque, *buf);
+    for (int i = 0; i < size; i++) {
+        bcm2835_aux_put_fifo(opaque, buf[i]);
+    }
 }
 
 static const MemoryRegionOps bcm2835_aux_ops = {
-- 
2.47.1

Re: [PATCH 5/9] hw/char/bcm2835_aux: Really use RX FIFO depth

Posted by Luc Michel 2 days, 21 hours ago

On 22:08 Wed 19 Feb     , Philippe Mathieu-Daudé wrote:
> While we model a 8-elements RX FIFO since the PL011 model was
> introduced in commit 97398d900ca ("bcm2835_aux: add emulation
> of BCM2835 AUX block") we only read 1 char at a time!

I'm not sure I get why in this patch and the subsequent ones you keep
mentioning the PL011 model while you modify other UARTs. I guess you
mean "the BCM2835 AUX model" here?

In any case:

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

> 
> 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>
> ---
>  hw/char/bcm2835_aux.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
> index 73ad5934067..c6e7eccf7dd 100644
> --- a/hw/char/bcm2835_aux.c
> +++ b/hw/char/bcm2835_aux.c
> @@ -221,7 +221,7 @@ static int bcm2835_aux_can_receive(void *opaque)
>  {
>      BCM2835AuxState *s = opaque;
> 
> -    return s->read_count < BCM2835_AUX_RX_FIFO_LEN;
> +    return BCM2835_AUX_RX_FIFO_LEN - s->read_count;
>  }
> 
>  static void bcm2835_aux_put_fifo(void *opaque, uint8_t value)
> @@ -243,7 +243,9 @@ static void bcm2835_aux_put_fifo(void *opaque, uint8_t value)
> 
>  static void bcm2835_aux_receive(void *opaque, const uint8_t *buf, int size)
>  {
> -    bcm2835_aux_put_fifo(opaque, *buf);
> +    for (int i = 0; i < size; i++) {
> +        bcm2835_aux_put_fifo(opaque, buf[i]);
> +    }
>  }
> 
>  static const MemoryRegionOps bcm2835_aux_ops = {
> --
> 2.47.1
> 

--

Re: [PATCH 5/9] hw/char/bcm2835_aux: Really use RX FIFO depth

Posted by Philippe Mathieu-Daudé 2 days, 21 hours ago

On 20/2/25 09:21, Luc Michel wrote:
> On 22:08 Wed 19 Feb     , Philippe Mathieu-Daudé wrote:
>> While we model a 8-elements RX FIFO since the PL011 model was
>> introduced in commit 97398d900ca ("bcm2835_aux: add emulation
>> of BCM2835 AUX block") we only read 1 char at a time!
> 
> I'm not sure I get why in this patch and the subsequent ones you keep
> mentioning the PL011 model while you modify other UARTs. I guess you
> mean "the BCM2835 AUX model" here?

Oops too much copy/pasting...

> 
> In any case:
> 
> Reviewed-by: Luc Michel <luc.michel@amd.com>

Thanks!

> 
>>
>> 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>
>> ---
>>   hw/char/bcm2835_aux.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)