| Message ID | 20250219210841.94797-6-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/char: Improve RX FIFO depth uses | expand |
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 > --
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(-)
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 = {
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(-)