[Qemu-devel] [RFC PATCH 2/2] hw/char/bcm2835_aux: Provide full 16550 UART support

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The BCM2835 AUX UART is compatible with the 16650 model.
Since a such model is already available and tested, reuse
it.

TODO:
- The serial model hardcodes a 16B FIFO (16550A) while
  this model is a 8B FIFO
- Various 16550A register bits are not implemented, thus
  require to be properly masked
- How to keep migration working?
- How to use a chardev property?

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/bcm2835_peripherals.c  |   2 -
 hw/char/bcm2835_aux.c         | 211 +++-------------------------------
 include/hw/char/bcm2835_aux.h |   7 +-
 3 files changed, 20 insertions(+), 200 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 8984e2e91f..dbe396fd40 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -167,8 +167,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
         qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
                                INTERRUPT_UART));
     /* AUX / UART1 */
-    qdev_prop_set_chr(DEVICE(&s->aux), "chardev", serial_hd(1));
-
     object_property_set_bool(OBJECT(&s->aux), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index 3f855196e3..10154b9530 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -2,149 +2,56 @@
  * BCM2835 (Raspberry Pi / Pi 2) Aux block (mini UART and SPI).
  * Copyright (c) 2015, Microsoft
  * Written by Andrew Baumann
- * Based on pl011.c, copyright terms below:
- *
- * Arm PrimeCell PL011 UART
- *
- * Copyright (c) 2006 CodeSourcery.
- * Written by Paul Brook
  *
  * This code is licensed under the GPL.
  *
  * At present only the core UART functions (data path for tx/rx) are
  * implemented. The following features/registers are unimplemented:
- *  - Line/modem control
- *  - Scratch register
  *  - Extra control
- *  - Baudrate
  *  - SPI interfaces
  */
 
 #include "qemu/osdep.h"
 #include "hw/char/bcm2835_aux.h"
-#include "hw/irq.h"
-#include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
-#include "qemu/module.h"
+#include "sysemu/sysemu.h"
 
 #define AUX_IRQ         0x0
 #define AUX_ENABLES     0x4
-#define AUX_MU_IO_REG   0x40
-#define AUX_MU_IER_REG  0x44
-#define AUX_MU_IIR_REG  0x48
-#define AUX_MU_LCR_REG  0x4c
-#define AUX_MU_MCR_REG  0x50
-#define AUX_MU_LSR_REG  0x54
-#define AUX_MU_MSR_REG  0x58
-#define AUX_MU_SCRATCH  0x5c
+#define AUX_MU_REGS     0x40
 #define AUX_MU_CNTL_REG 0x60
 #define AUX_MU_STAT_REG 0x64
 #define AUX_MU_BAUD_REG 0x68
 
-/* bits in IER/IIR registers */
-#define RX_INT  0x1
-#define TX_INT  0x2
-
-static void bcm2835_aux_update(BCM2835AuxState *s)
-{
-    /* signal an interrupt if either:
-     * 1. rx interrupt is enabled and we have a non-empty rx fifo, or
-     * 2. the tx interrupt is enabled (since we instantly drain the tx fifo)
-     */
-    s->iir = 0;
-    if ((s->ier & RX_INT) && s->read_count != 0) {
-        s->iir |= RX_INT;
-    }
-    if (s->ier & TX_INT) {
-        s->iir |= TX_INT;
-    }
-    qemu_set_irq(s->irq, s->iir != 0);
-}
-
 static uint64_t bcm2835_aux_read(void *opaque, hwaddr offset, unsigned size)
 {
     BCM2835AuxState *s = opaque;
-    uint32_t c, res;
+    uint32_t res;
 
     switch (offset) {
     case AUX_IRQ:
-        return s->iir != 0;
+        return s->serial->iir != 0;
 
     case AUX_ENABLES:
         return 1; /* mini UART permanently enabled */
 
-    case AUX_MU_IO_REG:
-        /* "DLAB bit set means access baudrate register" is NYI */
-        c = s->read_fifo[s->read_pos];
-        if (s->read_count > 0) {
-            s->read_count--;
-            if (++s->read_pos == BCM2835_AUX_RX_FIFO_LEN) {
-                s->read_pos = 0;
-            }
-        }
-        qemu_chr_fe_accept_input(&s->chr);
-        bcm2835_aux_update(s);
-        return c;
-
-    case AUX_MU_IER_REG:
-        /* "DLAB bit set means access baudrate register" is NYI */
-        return 0xc0 | s->ier; /* FIFO enables always read 1 */
-
-    case AUX_MU_IIR_REG:
-        res = 0xc0; /* FIFO enables */
-        /* The spec is unclear on what happens when both tx and rx
-         * interrupts are active, besides that this cannot occur. At
-         * present, we choose to prioritise the rx interrupt, since
-         * the tx fifo is always empty. */
-        if (s->read_count != 0) {
-            res |= 0x4;
-        } else {
-            res |= 0x2;
-        }
-        if (s->iir == 0) {
-            res |= 0x1;
-        }
-        return res;
-
-    case AUX_MU_LCR_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_LCR_REG unsupported\n", __func__);
-        return 0;
-
-    case AUX_MU_MCR_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_MCR_REG unsupported\n", __func__);
-        return 0;
-
-    case AUX_MU_LSR_REG:
-        res = 0x60; /* tx idle, empty */
-        if (s->read_count != 0) {
-            res |= 0x1;
-        }
-        return res;
-
-    case AUX_MU_MSR_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_MSR_REG unsupported\n", __func__);
-        return 0;
-
-    case AUX_MU_SCRATCH:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_SCRATCH unsupported\n", __func__);
-        return 0;
-
     case AUX_MU_CNTL_REG:
         return 0x3; /* tx, rx enabled */
 
     case AUX_MU_STAT_REG:
         res = 0x30e; /* space in the output buffer, empty tx fifo, idle tx/rx */
-        if (s->read_count > 0) {
-            res |= 0x1; /* data in input buffer */
-            assert(s->read_count < BCM2835_AUX_RX_FIFO_LEN);
-            res |= ((uint32_t)s->read_count) << 16; /* rx fifo fill level */
-        }
+        res |= fifo8_num_used(&s->serial->xmit_fifo) << 24;
+        res |= fifo8_num_used(&s->serial->recv_fifo) << 16;
+        res |= fifo8_is_empty(&s->serial->xmit_fifo) << 8;
+        res |= fifo8_is_full(&s->serial->xmit_fifo) << 5;
+        res |= fifo8_is_full(&s->serial->recv_fifo) << 4;
+        res |= !fifo8_is_full(&s->serial->xmit_fifo) << 1;
+        res |= !fifo8_is_empty(&s->serial->recv_fifo) << 0;
         return res;
 
     case AUX_MU_BAUD_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_BAUD_REG unsupported\n", __func__);
-        return 0;
+        return s->serial->divider;
 
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
@@ -157,7 +64,6 @@ static void bcm2835_aux_write(void *opaque, hwaddr offset, uint64_t value,
                               unsigned size)
 {
     BCM2835AuxState *s = opaque;
-    unsigned char ch;
 
     switch (offset) {
     case AUX_ENABLES:
@@ -167,81 +73,18 @@ static void bcm2835_aux_write(void *opaque, hwaddr offset, uint64_t value,
         }
         break;
 
-    case AUX_MU_IO_REG:
-        /* "DLAB bit set means access baudrate register" is NYI */
-        ch = value;
-        /* XXX this blocks entire thread. Rewrite to use
-         * qemu_chr_fe_write and background I/O callbacks */
-        qemu_chr_fe_write_all(&s->chr, &ch, 1);
-        break;
-
-    case AUX_MU_IER_REG:
-        /* "DLAB bit set means access baudrate register" is NYI */
-        s->ier = value & (TX_INT | RX_INT);
-        bcm2835_aux_update(s);
-        break;
-
-    case AUX_MU_IIR_REG:
-        if (value & 0x2) {
-            s->read_count = 0;
-        }
-        break;
-
-    case AUX_MU_LCR_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_LCR_REG unsupported\n", __func__);
-        break;
-
-    case AUX_MU_MCR_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_MCR_REG unsupported\n", __func__);
-        break;
-
-    case AUX_MU_SCRATCH:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_SCRATCH unsupported\n", __func__);
-        break;
-
     case AUX_MU_CNTL_REG:
         qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_CNTL_REG unsupported\n", __func__);
         break;
 
     case AUX_MU_BAUD_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_BAUD_REG unsupported\n", __func__);
+        serial_set_divider(s->serial, value);
         break;
 
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
                       __func__, offset);
     }
-
-    bcm2835_aux_update(s);
-}
-
-static int bcm2835_aux_can_receive(void *opaque)
-{
-    BCM2835AuxState *s = opaque;
-
-    return s->read_count < BCM2835_AUX_RX_FIFO_LEN;
-}
-
-static void bcm2835_aux_put_fifo(void *opaque, uint8_t value)
-{
-    BCM2835AuxState *s = opaque;
-    int slot;
-
-    slot = s->read_pos + s->read_count;
-    if (slot >= BCM2835_AUX_RX_FIFO_LEN) {
-        slot -= BCM2835_AUX_RX_FIFO_LEN;
-    }
-    s->read_fifo[slot] = value;
-    s->read_count++;
-    if (s->read_count == BCM2835_AUX_RX_FIFO_LEN) {
-        /* buffer full */
-    }
-    bcm2835_aux_update(s);
-}
-
-static void bcm2835_aux_receive(void *opaque, const uint8_t *buf, int size)
-{
-    bcm2835_aux_put_fifo(opaque, *buf);
 }
 
 static const MemoryRegionOps bcm2835_aux_ops = {
@@ -254,15 +97,9 @@ static const MemoryRegionOps bcm2835_aux_ops = {
 
 static const VMStateDescription vmstate_bcm2835_aux = {
     .name = TYPE_BCM2835_AUX,
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT8_ARRAY(read_fifo, BCM2835AuxState,
-                            BCM2835_AUX_RX_FIFO_LEN),
-        VMSTATE_UINT8(read_pos, BCM2835AuxState),
-        VMSTATE_UINT8(read_count, BCM2835AuxState),
-        VMSTATE_UINT8(ier, BCM2835AuxState),
-        VMSTATE_UINT8(iir, BCM2835AuxState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -276,29 +113,17 @@ static void bcm2835_aux_init(Object *obj)
                           TYPE_BCM2835_AUX, 0x100);
     sysbus_init_mmio(sbd, &s->iomem);
     sysbus_init_irq(sbd, &s->irq);
+
+    s->serial = serial_mm_init(&s->iomem, AUX_MU_REGS, 2, s->irq, 2419200,
+                               serial_hd(1), DEVICE_LITTLE_ENDIAN);
 }
 
-static void bcm2835_aux_realize(DeviceState *dev, Error **errp)
-{
-    BCM2835AuxState *s = BCM2835_AUX(dev);
-
-    qemu_chr_fe_set_handlers(&s->chr, bcm2835_aux_can_receive,
-                             bcm2835_aux_receive, NULL, NULL, s, NULL, true);
-}
-
-static Property bcm2835_aux_props[] = {
-    DEFINE_PROP_CHR("chardev", BCM2835AuxState, chr),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void bcm2835_aux_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    dc->realize = bcm2835_aux_realize;
     dc->vmsd = &vmstate_bcm2835_aux;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-    dc->props = bcm2835_aux_props;
 }
 
 static const TypeInfo bcm2835_aux_info = {
diff --git a/include/hw/char/bcm2835_aux.h b/include/hw/char/bcm2835_aux.h
index cdbf7e3e37..f534bccf5d 100644
--- a/include/hw/char/bcm2835_aux.h
+++ b/include/hw/char/bcm2835_aux.h
@@ -9,6 +9,7 @@
 #define BCM2835_AUX_H
 
 #include "hw/sysbus.h"
+#include "hw/char/serial.h"
 #include "chardev/char-fe.h"
 
 #define TYPE_BCM2835_AUX "bcm2835-aux"
@@ -22,12 +23,8 @@ typedef struct {
     /*< public >*/
 
     MemoryRegion iomem;
-    CharBackend chr;
     qemu_irq irq;
-
-    uint8_t read_fifo[BCM2835_AUX_RX_FIFO_LEN];
-    uint8_t read_pos, read_count;
-    uint8_t ier, iir;
+    SerialState *serial;
 } BCM2835AuxState;
 
 #endif
-- 
2.20.1