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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 17/2/25 15:39, Peter Maydell wrote:
> 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;
>   }

Thank you :)