On 23/1/23 17:09, Evgeny Iakovlev wrote:
On 1/23/2023 16:59, Evgeny Iakovlev wrote:
On 1/23/2023 16:21, Philippe Mathieu-Daudé wrote:
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 <eiakov...@linux.microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@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é <phi...@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.
Hm, no, pl011_can_receive never actually returned the number of bytes.
It's a boolean value as an int. Which is a bug, because
qemu_chr_fe_set_handlers expects it to return the byte count, not a
0/1 value.
I'll fix it then.
Actually, no, i spoke too soon. qemu_chr_fe_set_handlers indeed expects
a number of bytes from pl011_can_receive, but pl011_can_receive also
deliberately gives it 1 and not how many bytes are there in queue
really, because everything else on the receive code path is written with
an assumption to only accept 1 element at a time (see pl011_put_fifo).
That would be great if we had a model actually using the FIFO, but
since nobody complained about it ...
If we want to optimize this part, we would need to change that
assumption. That should better be done as a separate change though,
which i can send later.
No problem, this patch is fine. Thanks!
