Skip to contentSkip to conversation

Cadence UART cleanups and Tx path fixes

PATCH In review Peter Crosthwaite 2 participants 13 messages Jan 2, 2014 – Jan 6, 2014

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

v2 cover Cadence UART cleanups and Tx path fixes

When using QEMU in some terminal environments, char back-ends for serial devices can return EAGAIN for non trivial periods. This coupled with use of qemu_chr_fe_write_all() is a leading cause of:

main-loop: WARNING: I/O thread spun for 1000 iterations

This series fixes this for cadence_uart by replacing the open loop tx-write logic with a proper closed-loop that is responsive to the back-end short return.

Various trivial cleanups are at the front of the series. Last patch is the main event.

There is also a major performance increase with removal of the tx timer logic (P7).

As a test, I've been using QEMU in a terminal over trans-pacific SSH. Before this series:

$ time ./qemu-build/arm-softmmu/qemu-system-arm -M xilinx-zynq-a9 ...
main-loop: WARNING: I/O thread spun for 1000 iterations
Booting Linux on physical CPU 0x0
...
PetaLinux v2013.04 (Yocto 1.3) zynq ttyPS0

zynq login: QEMU: Terminated

real    2m26.908s
user    1m41.430s
sys     1m7.116s

And after:

$ time ./qemu-build/arm-softmmu/qemu-system-arm -M xilinx-zynq-a9 ...
Booting Linux on physical CPU 0x0
...
PetaLinux v2013.04 (Yocto 1.3) zynq ttyPS0

zynq login: QEMU: Terminated

real    0m16.149s
user    0m10.357s
sys     0m0.164s

Changed since v1:
Fixed /*< private >*/ spacing (AF review)

Peter Crosthwaite (11):
char/cadence_uart: Mark struct fields as public/private char/cadence_uart: Add missing uart_update_state char/cadence_uart: Fix reset. char/cadence_uart: s/r_fifo/rx_fifo char/cadence_uart: Simplify status generation char/cadence_uart: Define Missing SR/ISR fields char/cadence_uart: Remove TX timer & add TX FIFO state char/cadence_uart: Fix can_receive logic char/cadence_uart: Use the TX fifo for transmission char/cadence_uart: Delete redundant rx rst logic char/cadence_uart: Implement Tx flow control

 hw/char/cadence_uart.c | 153 ++++++++++++++++++++++++++++---------------------
 1 file changed, 87 insertions(+), 66 deletions(-)
Signature · 1 line
1.8.5.2
author
#1 lore raw

PATCH v2 1/11 char/cadence_uart: Mark struct fields as public/private

As per current QOM conventions.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v1:
Fixed spacing (AF review)

 hw/char/cadence_uart.c | 2 ++
 1 file changed, 2 insertions(+)
hw/char/cadence_uart.c +2 -0
index f18db53..a7b2f21 100644
@@ -110,7 +110,9 @@
 #define CADENCE_UART(obj) OBJECT_CHECK(UartState, (obj), TYPE_CADENCE_UART)
 
 typedef struct {
+    /*< private >*/
     SysBusDevice parent_obj;
+    /*< public >*/
 
     MemoryRegion iomem;
     uint32_t r[R_MAX];
Signature · 1 line
1.8.5.2
author
#2 lore raw

PATCH v2 2/11 char/cadence_uart: Add missing uart_update_state

In reply to Peter Crosthwaite

This should be rechecked on bus write accesses as such accesses may change the underlying state that generates the interrupt. Particular relevant for when the guest touches the interrupt status or mask.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 1 +
 1 file changed, 1 insertion(+)
hw/char/cadence_uart.c +1 -0
index a7b2f21..fb9db89 100644
@@ -403,6 +403,7 @@ static void uart_write(void *opaque, hwaddr offset,
         uart_parameters_setup(s);
         break;
     }
+    uart_update_status(s);
 }
 
 static uint64_t uart_read(void *opaque, hwaddr offset,
Signature · 1 line
1.8.5.2
author
#3 lore raw

PATCH v2 3/11 char/cadence_uart: Fix reset.

In reply to Peter Crosthwaite

Don't reset the uart as an init step. Register the reset function as a proper reset fn instead.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
hw/char/cadence_uart.c +4 -3
index fb9db89..7edc119 100644
@@ -431,8 +431,10 @@ static const MemoryRegionOps uart_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void cadence_uart_reset(UartState *s)
+static void cadence_uart_reset(DeviceState *dev)
 {
+    UartState *s = CADENCE_UART(dev);
+
     s->r[R_CR] = 0x00000128;
     s->r[R_IMR] = 0;
     s->r[R_CISR] = 0;

@@ -465,8 +467,6 @@ static int cadence_uart_init(SysBusDevice *dev)
 
     s->chr = qemu_char_get_next_serial();
 
-    cadence_uart_reset(s);
-
     if (s->chr) {
         qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive,
                               uart_event, s);

@@ -508,6 +508,7 @@ static void cadence_uart_class_init(ObjectClass *klass, void *data)
 
     sdc->init = cadence_uart_init;
     dc->vmsd = &vmstate_cadence_uart;
+    dc->reset = cadence_uart_reset;
 }
 
 static const TypeInfo cadence_uart_info = {
Signature · 1 line
1.8.5.2
author
#4 lore raw

PATCH v2 4/11 char/cadence_uart: s/r_fifo/rx_fifo

In reply to Peter Crosthwaite

Rename this field to match the many other uses of "rx". Xilinx docmentation (UG585) also refers to this as "RxFIFO".

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
hw/char/cadence_uart.c +4 -4
index 7edc119..d6abc5b 100644
@@ -116,7 +116,7 @@ typedef struct {
 
     MemoryRegion iomem;
     uint32_t r[R_MAX];
-    uint8_t r_fifo[RX_FIFO_SIZE];
+    uint8_t rx_fifo[RX_FIFO_SIZE];
     uint32_t rx_wpos;
     uint32_t rx_count;
     uint64_t char_tx_time;

@@ -280,7 +280,7 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
         s->r[R_CISR] |= UART_INTR_ROVR;
     } else {
         for (i = 0; i < size; i++) {
-            s->r_fifo[s->rx_wpos] = buf[i];
+            s->rx_fifo[s->rx_wpos] = buf[i];
             s->rx_wpos = (s->rx_wpos + 1) % RX_FIFO_SIZE;
             s->rx_count++;
 

@@ -344,7 +344,7 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
     if (s->rx_count) {
         uint32_t rx_rpos =
                 (RX_FIFO_SIZE + s->rx_wpos - s->rx_count) % RX_FIFO_SIZE;
-        *c = s->r_fifo[rx_rpos];
+        *c = s->rx_fifo[rx_rpos];
         s->rx_count--;
 
         if (!s->rx_count) {

@@ -492,7 +492,7 @@ static const VMStateDescription vmstate_cadence_uart = {
     .post_load = cadence_uart_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(r, UartState, R_MAX),
-        VMSTATE_UINT8_ARRAY(r_fifo, UartState, RX_FIFO_SIZE),
+        VMSTATE_UINT8_ARRAY(rx_fifo, UartState, RX_FIFO_SIZE),
         VMSTATE_UINT32(rx_count, UartState),
         VMSTATE_UINT32(rx_wpos, UartState),
         VMSTATE_TIMER(fifo_trigger_handle, UartState),
Signature · 1 line
1.8.5.2
author
#5 lore raw

PATCH v2 5/11 char/cadence_uart: Simplify status generation

In reply to Peter Crosthwaite

The status register bits are always pure functions of other device state. Move the generation of these bits to the update_status() function to simplify. Makes developing much easier as theres now no need to recheck status bits on all the changes to rx/tx fifo state.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)
hw/char/cadence_uart.c +8 -25
index d6abc5b..ddd7267 100644
@@ -128,6 +128,13 @@ typedef struct {
 
 static void uart_update_status(UartState *s)
 {
+    s->r[R_SR] = 0;
+
+    s->r[R_SR] |= s->rx_count == RX_FIFO_SIZE ? UART_SR_INTR_RFUL : 0;
+    s->r[R_SR] |= !s->rx_count ? UART_SR_INTR_REMPTY : 0;
+    s->r[R_SR] |= s->rx_count >= s->r[R_RTRIG] ? UART_SR_INTR_RTRIG : 0;
+
+    s->r[R_SR] |= UART_SR_INTR_TEMPTY;
     s->r[R_CISR] |= s->r[R_SR] & UART_SR_TO_CISR_MASK;
     qemu_set_irq(s->irq, !!(s->r[R_IMR] & s->r[R_CISR]));
 }

@@ -166,15 +173,10 @@ static void uart_rx_reset(UartState *s)
     if (s->chr) {
         qemu_chr_accept_input(s->chr);
     }
-
-    s->r[R_SR] |= UART_SR_INTR_REMPTY;
-    s->r[R_SR] &= ~UART_SR_INTR_RFUL;
 }
 
 static void uart_tx_reset(UartState *s)
 {
-    s->r[R_SR] |= UART_SR_INTR_TEMPTY;
-    s->r[R_SR] &= ~UART_SR_INTR_TFUL;
 }
 
 static void uart_send_breaks(UartState *s)

@@ -274,8 +276,6 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
         return;
     }
 
-    s->r[R_SR] &= ~UART_SR_INTR_REMPTY;
-
     if (s->rx_count == RX_FIFO_SIZE) {
         s->r[R_CISR] |= UART_INTR_ROVR;
     } else {

@@ -283,15 +283,6 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
             s->rx_fifo[s->rx_wpos] = buf[i];
             s->rx_wpos = (s->rx_wpos + 1) % RX_FIFO_SIZE;
             s->rx_count++;
-
-            if (s->rx_count == RX_FIFO_SIZE) {
-                s->r[R_SR] |= UART_SR_INTR_RFUL;
-                break;
-            }
-
-            if (s->rx_count >= s->r[R_RTRIG]) {
-                s->r[R_SR] |= UART_SR_INTR_RTRIG;
-            }
         }
         timer_mod(s->fifo_trigger_handle, new_rx_time +
                                                 (s->char_tx_time * 4));

@@ -339,26 +330,17 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
         return;
     }
 
-    s->r[R_SR] &= ~UART_SR_INTR_RFUL;
-
     if (s->rx_count) {
         uint32_t rx_rpos =
                 (RX_FIFO_SIZE + s->rx_wpos - s->rx_count) % RX_FIFO_SIZE;
         *c = s->rx_fifo[rx_rpos];
         s->rx_count--;
 
-        if (!s->rx_count) {
-            s->r[R_SR] |= UART_SR_INTR_REMPTY;
-        }
         qemu_chr_accept_input(s->chr);
     } else {
         *c = 0;
-        s->r[R_SR] |= UART_SR_INTR_REMPTY;
     }
 
-    if (s->rx_count < s->r[R_RTRIG]) {
-        s->r[R_SR] &= ~UART_SR_INTR_RTRIG;
-    }
     uart_update_status(s);
 }
 

@@ -447,6 +429,7 @@ static void cadence_uart_reset(DeviceState *dev)
 
     s->rx_count = 0;
     s->rx_wpos = 0;
+    uart_update_status(s);
 }
 
 static int cadence_uart_init(SysBusDevice *dev)
Signature · 1 line
1.8.5.2
author
#6 lore raw

PATCH v2 6/11 char/cadence_uart: Define Missing SR/ISR fields

In reply to Peter Crosthwaite

Some (interrupt) status register bits relating to the TxFIFO path were not defined. Define them. This prepares support for proper Tx data path flow control.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 4 ++++
 1 file changed, 4 insertions(+)
hw/char/cadence_uart.c +4 -0
index ddd7267..216eed7 100644
@@ -34,6 +34,9 @@
 #define UART_SR_INTR_RFUL      0x00000004
 #define UART_SR_INTR_TEMPTY    0x00000008
 #define UART_SR_INTR_TFUL      0x00000010
+/* somewhat awkwardly, TTRIG is misaligned between SR and ISR */
+#define UART_SR_TTRIG          0x00002000
+#define UART_INTR_TTRIG        0x00000400
 /* bits fields in CSR that correlate to CISR. If any of these bits are set in
  * SR, then the same bit in CISR is set high too */
 #define UART_SR_TO_CISR_MASK   0x0000001F

@@ -43,6 +46,7 @@
 #define UART_INTR_PARE         0x00000080
 #define UART_INTR_TIMEOUT      0x00000100
 #define UART_INTR_DMSI         0x00000200
+#define UART_INTR_TOVR         0x00001000
 
 #define UART_SR_RACTIVE    0x00000400
 #define UART_SR_TACTIVE    0x00000800
Signature · 1 line
1.8.5.2
author
#7 lore raw

PATCH v2 7/11 char/cadence_uart: Remove TX timer & add TX FIFO state

In reply to Peter Crosthwaite

This tx timer implementation is flawed. Despite the controller attempting to time the guest visable assertion of the TX-empty status bit (and corresponding interrupt) the controller is still transmitting characters instantaneously. There is also no sense of multiple character delay.

The only side effect of this timer is assertion of tx-empty status. So just remove the timer completely and hold tx-empty as permanently asserted (its reset status). This matches the actual behaviour of instantaneous transmission.

While we are VMSD version bumping, add the tx_fifo as device state to prepare for upcomming TxFIFO flow control. Implement the interrupt generation logic for the TxFIFO occupancy.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 44 +++++++++++++-------------------------------
 1 file changed, 13 insertions(+), 31 deletions(-)
hw/char/cadence_uart.c +13 -31
index 216eed7..3eeadb1 100644
@@ -121,13 +121,14 @@ typedef struct {
     MemoryRegion iomem;
     uint32_t r[R_MAX];
     uint8_t rx_fifo[RX_FIFO_SIZE];
+    uint8_t tx_fifo[TX_FIFO_SIZE];
     uint32_t rx_wpos;
     uint32_t rx_count;
+    uint32_t tx_count;
     uint64_t char_tx_time;
     CharDriverState *chr;
     qemu_irq irq;
     QEMUTimer *fifo_trigger_handle;
-    QEMUTimer *tx_time_handle;
 } UartState;
 
 static void uart_update_status(UartState *s)

@@ -138,8 +139,12 @@ static void uart_update_status(UartState *s)
     s->r[R_SR] |= !s->rx_count ? UART_SR_INTR_REMPTY : 0;
     s->r[R_SR] |= s->rx_count >= s->r[R_RTRIG] ? UART_SR_INTR_RTRIG : 0;
 
-    s->r[R_SR] |= UART_SR_INTR_TEMPTY;
+    s->r[R_SR] |= s->tx_count == TX_FIFO_SIZE ? UART_SR_INTR_TFUL : 0;
+    s->r[R_SR] |= !s->tx_count ? UART_SR_INTR_TEMPTY : 0;
+    s->r[R_SR] |= s->tx_count >= s->r[R_TTRIG] ? UART_SR_TTRIG : 0;
+
     s->r[R_CISR] |= s->r[R_SR] & UART_SR_TO_CISR_MASK;
+    s->r[R_CISR] |= s->r[R_SR] & UART_SR_TTRIG ? UART_INTR_TTRIG : 0;
     qemu_set_irq(s->irq, !!(s->r[R_IMR] & s->r[R_CISR]));
 }
 

@@ -152,24 +157,6 @@ static void fifo_trigger_update(void *opaque)
     uart_update_status(s);
 }
 
-static void uart_tx_redo(UartState *s)
-{
-    uint64_t new_tx_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
-    timer_mod(s->tx_time_handle, new_tx_time + s->char_tx_time);
-
-    s->r[R_SR] |= UART_SR_INTR_TEMPTY;
-
-    uart_update_status(s);
-}
-
-static void uart_tx_write(void *opaque)
-{
-    UartState *s = (UartState *)opaque;
-
-    uart_tx_redo(s);
-}
-
 static void uart_rx_reset(UartState *s)
 {
     s->rx_wpos = 0;

@@ -181,6 +168,7 @@ static void uart_rx_reset(UartState *s)
 
 static void uart_tx_reset(UartState *s)
 {
+    s->tx_count = 0;
 }
 
 static void uart_send_breaks(UartState *s)

@@ -261,10 +249,6 @@ static void uart_ctrl_update(UartState *s)
 
     s->r[R_CR] &= ~(UART_CR_TXRST | UART_CR_RXRST);
 
-    if ((s->r[R_CR] & UART_CR_TX_EN) && !(s->r[R_CR] & UART_CR_TX_DIS)) {
-            uart_tx_redo(s);
-    }
-
     if (s->r[R_CR] & UART_CR_STARTBRK && !(s->r[R_CR] & UART_CR_STOPBRK)) {
         uart_send_breaks(s);
     }

@@ -447,9 +431,6 @@ static int cadence_uart_init(SysBusDevice *dev)
     s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
             (QEMUTimerCB *)fifo_trigger_update, s);
 
-    s->tx_time_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-            (QEMUTimerCB *)uart_tx_write, s);
-
     s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
 
     s->chr = qemu_char_get_next_serial();

@@ -473,17 +454,18 @@ static int cadence_uart_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_cadence_uart = {
     .name = "cadence_uart",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
     .post_load = cadence_uart_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(r, UartState, R_MAX),
         VMSTATE_UINT8_ARRAY(rx_fifo, UartState, RX_FIFO_SIZE),
+        VMSTATE_UINT8_ARRAY(tx_fifo, UartState, RX_FIFO_SIZE),
         VMSTATE_UINT32(rx_count, UartState),
+        VMSTATE_UINT32(tx_count, UartState),
         VMSTATE_UINT32(rx_wpos, UartState),
         VMSTATE_TIMER(fifo_trigger_handle, UartState),
-        VMSTATE_TIMER(tx_time_handle, UartState),
         VMSTATE_END_OF_LIST()
     }
 };
Signature · 1 line
1.8.5.2
author
#8 lore raw

PATCH v2 8/11 char/cadence_uart: Fix can_receive logic

In reply to Peter Crosthwaite

The can_receive logic was only taking into account the RxFIFO occupancy. RxFIFO population is only used for the echo and normal modes however. Improve the logic to correctly return the true number of receivable characters based on the current mode:

Normal mode: RxFIFO vacancy.
Remote loopback: TxFIFO vacancy.
Echo mode: The min of the TxFIFO and RxFIFO vacancies. Local Loopback: Return non-zero (to implement droppage)

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
hw/char/cadence_uart.c +9 -1
index 3eeadb1..3bcaf29 100644
@@ -233,8 +233,16 @@ static void uart_parameters_setup(UartState *s)
 static int uart_can_receive(void *opaque)
 {
     UartState *s = (UartState *)opaque;
+    int ret = MAX(RX_FIFO_SIZE, TX_FIFO_SIZE);
+    uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
 
-    return RX_FIFO_SIZE - s->rx_count;
+    if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
+        ret = MIN(ret, RX_FIFO_SIZE - s->rx_count);
+    }
+    if (ch_mode == REMOTE_LOOPBACK || ch_mode == ECHO_MODE) {
+        ret = MIN(ret, TX_FIFO_SIZE - s->tx_count);
+    }
+    return ret;
 }
 
 static void uart_ctrl_update(UartState *s)
Signature · 1 line
1.8.5.2
author
#9 lore raw

PATCH v2 9/11 char/cadence_uart: Use the TX fifo for transmission

In reply to Peter Crosthwaite

Populate the TxFIFO with the Tx data before sending. Prepares support for proper Tx flow control implementation.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
hw/char/cadence_uart.c +16 -1
index 3bcaf29..be32126 100644
@@ -292,7 +292,22 @@ static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
         return;
     }
 
-    qemu_chr_fe_write_all(s->chr, buf, size);
+    if (size > TX_FIFO_SIZE - s->tx_count) {
+        size = TX_FIFO_SIZE - s->tx_count;
+        /*
+         * This can only be a guest error via a bad tx fifo register push,
+         * as can_receive() should stop remote loop and echo modes ever getting
+         * us to here.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR, "cadence_uart: TxFIFO overflow");
+        s->r[R_CISR] |= UART_INTR_ROVR;
+    }
+
+    memcpy(s->tx_fifo + s->tx_count, buf, size);
+    s->tx_count += size;
+
+    qemu_chr_fe_write_all(s->chr, s->tx_fifo, s->tx_count);
+    s->tx_count = 0;
 }
 
 static void uart_receive(void *opaque, const uint8_t *buf, int size)
Signature · 1 line
1.8.5.2
author
#10 lore raw

PATCH v2 10/11 char/cadence_uart: Delete redundant rx rst logic

In reply to Peter Crosthwaite

uart_rx_reset() called immediately above already does this. Remove.

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 2 --
 1 file changed, 2 deletions(-)
hw/char/cadence_uart.c +0 -2
index be32126..8a9ef81 100644
@@ -438,8 +438,6 @@ static void cadence_uart_reset(DeviceState *dev)
     uart_rx_reset(s);
     uart_tx_reset(s);
 
-    s->rx_count = 0;
-    s->rx_wpos = 0;
     uart_update_status(s);
 }
Signature · 1 line
1.8.5.2
author
#11 lore raw

PATCH v2 11/11 char/cadence_uart: Implement Tx flow control

In reply to Peter Crosthwaite

If the UART back-end blocks, buffer in the Tx FIFO to try again later. This stops the IO-thread busy waiting on char back-ends (which causes all sorts of performance problems).

Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/char/cadence_uart.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)
hw/char/cadence_uart.c +29 -2
index 8a9ef81..1012f1a 100644
@@ -286,6 +286,34 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
     uart_update_status(s);
 }
 
+static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
+                                  void *opaque)
+{
+    UartState *s = opaque;
+    int ret;
+
+    /* instant drain the fifo when there's no back-end */
+    if (!s->chr) {
+        s->tx_count = 0;
+    }
+
+    if (!s->tx_count) {
+        return FALSE;
+    }
+
+    ret = qemu_chr_fe_write(s->chr, s->tx_fifo, s->tx_count);
+    s->tx_count -= ret;
+    memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count);
+
+    if (s->tx_count) {
+        int r = qemu_chr_fe_add_watch(s->chr, G_IO_OUT, cadence_uart_xmit, s);
+        assert(r);
+    }
+
+    uart_update_status(s);
+    return FALSE;
+}
+
 static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
 {
     if ((s->r[R_CR] & UART_CR_TX_DIS) || !(s->r[R_CR] & UART_CR_TX_EN)) {

@@ -306,8 +334,7 @@ static void uart_write_tx_fifo(UartState *s, const uint8_t *buf, int size)
     memcpy(s->tx_fifo + s->tx_count, buf, size);
     s->tx_count += size;
 
-    qemu_chr_fe_write_all(s->chr, s->tx_fifo, s->tx_count);
-    s->tx_count = 0;
+    cadence_uart_xmit(NULL, G_IO_OUT, s);
 }
 
 static void uart_receive(void *opaque, const uint8_t *buf, int size)
Signature · 1 line
1.8.5.2
#12 lore raw
In reply to Peter Crosthwaite
Quoted 10 lines · Peter Crosthwaite

When using QEMU in some terminal environments, char back-ends for serial devices can return EAGAIN for non trivial periods. This coupled with use of qemu_chr_fe_write_all() is a leading cause of:

main-loop: WARNING: I/O thread spun for 1000 iterations

This series fixes this for cadence_uart by replacing the open loop tx-write logic with a proper closed-loop that is responsive to the back-end short return.

Thanks; applied all to target-arm.next.

-- PMM