[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation |
Date: |
Mon, 23 Jan 2023 16:21:28 +0100 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 |
On 23/1/23 15:43, Evgeny Iakovlev wrote:
On 1/23/2023 09:14, Philippe Mathieu-Daudé wrote:
On 20/1/23 16:54, Evgeny Iakovlev wrote:
UART should be enabled in general and have RX enabled specifically to be
able to receive data from peripheral device. Same goes for transmitting
data to peripheral device and a TXE flag.
Check if UART CR register has EN and RXE or TXE bits enabled before
trying to receive or transmit data.
Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/char/pl011.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
+static inline bool pl011_can_transmit(PL011State *s)
+{
+ return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_TXE;
+}
+
static void pl011_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
@@ -221,7 +231,9 @@ static void pl011_write(void *opaque, hwaddr offset,
switch (offset >> 2) {
case 0: /* UARTDR */
- /* ??? Check if transmitter is enabled. */
+ if (!pl011_can_transmit(s)) {
+ break;
+ }
ch = value;
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
@@ -292,7 +304,11 @@ static int pl011_can_receive(void *opaque)
PL011State *s = (PL011State *)opaque;
int r;
- r = s->read_count < pl011_get_fifo_depth(s);
+ if (!(s->cr & PL011_CR_UARTEN) || !(s->cr & PL011_CR_RXE)) {
Maybe add pl011_can_receive() similarly to pl011_can_transmit().
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks! There's already a pl011_can_receive though, its the
pl011_can_transmit that's new :)
pl011_can_receive() returns the number of bytes that pl011_receive() can
accept, pl011_can_transmit() returns a boolean.
I was thinking of:
-- >8 --
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index dd20b76609..ea5769a31c 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -221,6 +221,11 @@ static inline bool pl011_can_transmit(PL011State *s)
return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_TXE;
}
+static inline bool pl011_can_receive(PL011State *s)
+{
+ return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_RXE;
+}
+
static void pl011_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
@@ -299,12 +304,12 @@ static void pl011_write(void *opaque, hwaddr offset,
}
}
-static int pl011_can_receive(void *opaque)
+static int pl011_receivable_bytes(void *opaque)
{
PL011State *s = (PL011State *)opaque;
int r;
- if (!(s->cr & PL011_CR_UARTEN) || !(s->cr & PL011_CR_RXE)) {
+ if (!pl011_can_receive(s)) {
r = 0;
} else {
r = s->read_count < pl011_get_fifo_depth(s);
@@ -459,7 +464,7 @@ static void pl011_realize(DeviceState *dev, Error
**errp)
{
PL011State *s = PL011(dev);
- qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
+ qemu_chr_fe_set_handlers(&s->chr, pl011_receivable_bytes,
pl011_receive,
pl011_event, NULL, s, NULL, true);
}
---
with maybe a better name for pl011_receivable_bytes().
- Re: [PATCH v3 1/5] hw/char/pl011: refactor FIFO depth handling code, (continued)
- [PATCH v3 2/5] hw/char/pl011: add post_load hook for backwards-compatibility, Evgeny Iakovlev, 2023/01/20
- [PATCH v3 3/5] hw/char/pl011: implement a reset method, Evgeny Iakovlev, 2023/01/20
- [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation, Evgeny Iakovlev, 2023/01/20
- Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation, Peter Maydell, 2023/01/23
- Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation, Philippe Mathieu-Daudé, 2023/01/23
- Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation, Evgeny Iakovlev, 2023/01/25
[PATCH v3 4/5] hw/char/pl011: better handling of FIFO flags on LCR reset, Evgeny Iakovlev, 2023/01/20