| Message ID | 20250208163911.54522-5-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/char/pl011: Implement TX (async) FIFO to avoid blocking the main loop | expand |
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. Anyway Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
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
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 :)
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; } @@ -464,8 +465,9 @@ static void pl011_write(void *opaque, hwaddr offset, pl011_trace_baudrate_change(s); break; case 11: /* UARTLCR_H */ - /* Reset the FIFO state on FIFO enable or disable */ if ((s->lcr ^ value) & LCR_FEN) { + /* Reset the FIFO state on FIFO enable or disable */ + trace_pl011_fifo_enable(value & LCR_FEN); pl011_reset_rx_fifo(s); pl011_reset_tx_fifo(s); } diff --git a/hw/char/trace-events b/hw/char/trace-events index 3d07866be5c..dd635ac6012 100644 --- a/hw/char/trace-events +++ b/hw/char/trace-events @@ -63,6 +63,8 @@ pl011_read(uint32_t addr, uint32_t value, const char *regname) "addr 0x%03x valu pl011_read_fifo(int read_count) "FIFO read, read_count now %d" pl011_write(uint32_t addr, uint32_t value, const char *regname) "addr 0x%03x value 0x%08x reg %s" pl011_can_receive(uint32_t lcr, int read_count, int r) "LCR 0x%08x read_count %d returning %d" +pl011_fifo_enable(bool enable) "enable:%u" +pl011_fifo_is_enabled(bool enabled) "enabled:%u" pl011_fifo_rx_put(uint32_t c, int read_count) "new char 0x%02x read_count now %d" pl011_fifo_rx_full(void) "RX FIFO now full, RXFF set" pl011_fifo_tx_put(uint8_t byte) "TX FIFO push char [0x%02x]"
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(-)