Re: [PATCH v6 4/7] hw/char/pl011: Trace FIFO enablement

[Peter Maydell]

On Mon, 17 Feb 2025 at 14:27, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 8 Feb 2025 at 16:39, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  hw/char/pl011.c      | 4 +++-
> >  hw/char/trace-events | 2 ++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> > index b9c9e5b5983..447f185e2d5 100644
> > --- a/hw/char/pl011.c
> > +++ b/hw/char/pl011.c
> > @@ -148,6 +148,7 @@ static bool pl011_loopback_enabled(PL011State *s)
> >
> >  static bool pl011_is_fifo_enabled(PL011State *s)
> >  {
> > +    trace_pl011_fifo_is_enabled((s->lcr & LCR_FEN) != 0);
> >      return (s->lcr & LCR_FEN) != 0;
>
> Might be neater having a local variable rather than
> repeating the expression twice.

I'll squash in this tweak:

--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -148,8 +148,10 @@ static bool pl011_loopback_enabled(PL011State *s)

 static bool pl011_is_fifo_enabled(PL011State *s)
 {
-    trace_pl011_fifo_is_enabled((s->lcr & LCR_FEN) != 0);
-    return (s->lcr & LCR_FEN) != 0;
+    bool enabled = (s->lcr & LCR_FEN) != 0;
+
+    trace_pl011_fifo_is_enabled(enabled);
+    return enabled;
 }

 static inline unsigned pl011_get_fifo_depth(PL011State *s)

thanks
-- PMM