On Mon, 17 Feb 2025 at 14:29, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Sat, 8 Feb 2025 at 16:39, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
> >
> > When transmission is disabled, characters are still queued
> > to the FIFO which eventually overruns. Report that error
> > condition in the status register.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> > ---
> > hw/char/pl011.c | 20 ++++++++++++++++++++
> > hw/char/trace-events | 2 ++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> > index 447f185e2d5..ef39ab666a2 100644
> > --- a/hw/char/pl011.c
> > +++ b/hw/char/pl011.c
> > @@ -61,6 +61,9 @@ DeviceState *pl011_create(hwaddr addr, qemu_irq irq,
> > Chardev *chr)
> > /* Data Register, UARTDR */
> > #define DR_BE (1 << 10)
> >
> > +/* Receive Status Register/Error Clear Register, UARTRSR/UARTECR */
> > +#define RSR_OE (1 << 3)
> > +
> > /* Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC */
> > #define INT_OE (1 << 10)
> > #define INT_BE (1 << 9)
> > @@ -158,6 +161,16 @@ static inline unsigned pl011_get_fifo_depth(PL011State
> > *s)
> > return pl011_is_fifo_enabled(s) ? PL011_FIFO_DEPTH : 1;
> > }
> >
> > +static bool pl011_is_tx_fifo_full(PL011State *s)
> > +{
> > + if (pl011_is_fifo_enabled(s)) {
> > + trace_pl011_fifo_tx_is_full("FIFO", fifo8_is_full(&s->xmit_fifo));
> > + return fifo8_is_full(&s->xmit_fifo);
> > + }
> > + trace_pl011_fifo_tx_is_full("CHAR", !fifo8_is_empty(&s->xmit_fifo));
> > + return !fifo8_is_empty(&s->xmit_fifo);
>
> More repetition of expressions, but anyway
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
Here I propose to squash in this tweak:
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -165,12 +165,13 @@ static inline unsigned pl011_get_fifo_depth(PL011State *s)
static bool pl011_is_tx_fifo_full(PL011State *s)
{
- if (pl011_is_fifo_enabled(s)) {
- trace_pl011_fifo_tx_is_full("FIFO", fifo8_is_full(&s->xmit_fifo));
- return fifo8_is_full(&s->xmit_fifo);
- }
- trace_pl011_fifo_tx_is_full("CHAR", !fifo8_is_empty(&s->xmit_fifo));
- return !fifo8_is_empty(&s->xmit_fifo);
+ bool fifo_enabled = pl011_is_fifo_enabled(s);
+ bool tx_fifo_full = fifo_enabled ?
+ fifo8_is_full(&s->xmit_fifo) : !fifo8_is_empty(&s->xmit_fifo);
+
+ trace_pl011_fifo_tx_is_full(fifo_enabled ? "FIFO" : "CHAR",
+ tx_fifo_full);
+ return tx_fifo_full;
}
thanks
-- PMM
