[PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block

[PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block

[Philippe Mathieu-Daudé]

The BCM2838 has many more peripherals than his little brother,
the BCM2837. With the raspi4, the Linux kernel takes more steps
to configure the various MUXed devices. At some point it started
to bug me, so I plan to add a dummy simple BCM2835_SPI block.
It is cleaner to add it as a separate device than mixed with the
AUX block. As a first step, split the UART block out.

Since this part is self-contained and my raspi4 branch is getting
too big, I'm sending it as a single series.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  hw/char: Add the BCM2835 miniuart
  hw/char/bcm2835_aux: Use the BCM2835 miniuart block
  hw: Move BCM2835 AUX device from hw/char/ to hw/misc/

 hw/char/Makefile.objs                   |   2 +-
 hw/char/bcm2835_aux.c                   | 317 -----------------------
 hw/char/bcm2835_miniuart.c              | 327 ++++++++++++++++++++++++
 hw/char/trace-events                    |   4 +
 hw/misc/Makefile.objs                   |   1 +
 hw/misc/bcm2835_aux.c                   | 189 ++++++++++++++
 hw/misc/trace-events                    |   4 +
 include/hw/arm/bcm2835_peripherals.h    |   2 +-
 include/hw/char/bcm2835_miniuart.h      |  37 +++
 include/hw/{char => misc}/bcm2835_aux.h |  10 +-
 10 files changed, 567 insertions(+), 326 deletions(-)
 delete mode 100644 hw/char/bcm2835_aux.c
 create mode 100644 hw/char/bcm2835_miniuart.c
 create mode 100644 hw/misc/bcm2835_aux.c
 create mode 100644 include/hw/char/bcm2835_miniuart.h
 rename include/hw/{char => misc}/bcm2835_aux.h (73%)

-- 
2.21.0

[PATCH 1/3] hw/char: Add the BCM2835 miniuart

[Philippe Mathieu-Daudé]

The miniuart code is already present in the multi-device
hw/char/bcm2835_aux.c. Simply extracting it does not generate
patch easy to review. Instead, add it again, rename the function
names accordingly, use the "hw/registerfields.h" API.
This is roughtly a copy of commit 97398d900caacc.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/char/Makefile.objs              |   1 +
 hw/char/bcm2835_miniuart.c         | 327 +++++++++++++++++++++++++++++
 hw/char/trace-events               |   4 +
 include/hw/char/bcm2835_miniuart.h |  37 ++++
 4 files changed, 369 insertions(+)
 create mode 100644 hw/char/bcm2835_miniuart.c
 create mode 100644 include/hw/char/bcm2835_miniuart.h

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 02d8a66925..5bd93bde9f 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -22,6 +22,7 @@ obj-$(CONFIG_DIGIC) += digic-uart.o
 obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
 obj-$(CONFIG_RASPI) += bcm2835_aux.o
 
+common-obj-$(CONFIG_RASPI) += bcm2835_miniuart.o
 common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
diff --git a/hw/char/bcm2835_miniuart.c b/hw/char/bcm2835_miniuart.c
new file mode 100644
index 0000000000..0e99cecce7
--- /dev/null
+++ b/hw/char/bcm2835_miniuart.c
@@ -0,0 +1,327 @@
+/*
+ * BCM2835 (Raspberry Pi) mini UART block.
+ *
+ * Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ * Based on pl011.c.
+ *
+ * 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
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/char/bcm2835_miniuart.h"
+#include "hw/qdev-properties.h"
+#include "hw/registerfields.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+REG32(MU_IO,        0x00)
+REG32(MU_IER,       0x04)
+REG32(MU_IIR,       0x08)
+REG32(MU_LCR,       0x0c)
+REG32(MU_MCR,       0x10)
+REG32(MU_LSR,       0x14)
+REG32(MU_MSR,       0x18)
+REG32(MU_SCRATCH,   0x1c)
+REG32(MU_CNTL,      0x20)
+REG32(MU_STAT,      0x24)
+REG32(MU_BAUD,      0x28)
+
+/* bits in IER/IIR registers */
+#define RX_INT  0x1
+#define TX_INT  0x2
+
+static void bcm2835_miniuart_update(BCM2835MiniUartState *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 bool is_16650(hwaddr offset)
+{
+    return offset < A_MU_CNTL;
+}
+
+static uint64_t bcm2835_miniuart_read(void *opaque, hwaddr offset,
+                                      unsigned size)
+{
+    BCM2835MiniUartState *s = opaque;
+    uint32_t c, res = 0;
+
+    switch (offset) {
+    case A_MU_IO:
+        /* "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_MINIUART_RX_FIFO_LEN) {
+                s->read_pos = 0;
+            }
+        }
+        qemu_chr_fe_accept_input(&s->chr);
+        bcm2835_miniuart_update(s);
+        res = c;
+        break;
+
+    case A_MU_IER:
+        /* "DLAB bit set means access baudrate register" is NYI */
+        res = 0xc0 | s->ier; /* FIFO enables always read 1 */
+        break;
+
+    case A_MU_IIR:
+        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;
+        }
+        break;
+
+    case A_MU_LCR:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_LCR_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_MCR:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MCR_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_LSR:
+        res = 0x60; /* tx idle, empty */
+        if (s->read_count != 0) {
+            res |= 0x1;
+        }
+        break;
+
+    case A_MU_MSR:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MSR_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_SCRATCH:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_SCRATCH unsupported\n", __func__);
+        break;
+
+    case A_MU_CNTL:
+        res = 0x3; /* tx, rx enabled */
+        break;
+
+    case A_MU_STAT:
+        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_MINIUART_RX_FIFO_LEN);
+            res |= ((uint32_t)s->read_count) << 16; /* rx fifo fill level */
+        }
+        break;
+
+    case A_MU_BAUD:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_BAUD_REG unsupported\n", __func__);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                      __func__, offset);
+        break;
+    }
+
+    if (is_16650(offset)) {
+        trace_serial_ioport_read((offset & 0x1f) >> 2, res);
+    } else {
+        trace_bcm2835_miniuart_read(offset, res);
+    }
+
+    return res;
+}
+
+static void bcm2835_miniuart_write(void *opaque, hwaddr offset, uint64_t value,
+                                   unsigned size)
+{
+    BCM2835MiniUartState *s = opaque;
+    unsigned char ch;
+
+    if (is_16650(offset)) {
+        trace_serial_ioport_write((offset & 0x1f) >> 2, value);
+    } else {
+        trace_bcm2835_miniuart_write(offset, value);
+    }
+
+    switch (offset) {
+    case A_MU_IO:
+        /* "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 A_MU_IER:
+        /* "DLAB bit set means access baudrate register" is NYI */
+        s->ier = value & (TX_INT | RX_INT);
+        bcm2835_miniuart_update(s);
+        break;
+
+    case A_MU_IIR:
+        if (value & 0x2) {
+            s->read_count = 0;
+        }
+        break;
+
+    case A_MU_LCR:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_LCR_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_MCR:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MCR_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_SCRATCH:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_SCRATCH unsupported\n", __func__);
+        break;
+
+    case A_MU_CNTL:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_CNTL_REG unsupported\n", __func__);
+        break;
+
+    case A_MU_BAUD:
+        qemu_log_mask(LOG_UNIMP, "%s: A_MU_BAUD_REG unsupported\n", __func__);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
+                      __func__, offset);
+    }
+
+    bcm2835_miniuart_update(s);
+}
+
+static int bcm2835_miniuart_can_receive(void *opaque)
+{
+    BCM2835MiniUartState *s = opaque;
+
+    return s->read_count < BCM2835_MINIUART_RX_FIFO_LEN;
+}
+
+static void bcm2835_miniuart_put_fifo(void *opaque, uint8_t value)
+{
+    BCM2835MiniUartState *s = opaque;
+    int slot;
+
+    slot = s->read_pos + s->read_count;
+    if (slot >= BCM2835_MINIUART_RX_FIFO_LEN) {
+        slot -= BCM2835_MINIUART_RX_FIFO_LEN;
+    }
+    s->read_fifo[slot] = value;
+    s->read_count++;
+    if (s->read_count == BCM2835_MINIUART_RX_FIFO_LEN) {
+        /* buffer full */
+    }
+    bcm2835_miniuart_update(s);
+}
+
+static void bcm2835_miniuart_receive(void *opaque, const uint8_t *buf, int size)
+{
+    bcm2835_miniuart_put_fifo(opaque, *buf);
+}
+
+static const MemoryRegionOps bcm2835_miniuart_ops = {
+    .read = bcm2835_miniuart_read,
+    .write = bcm2835_miniuart_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static const VMStateDescription vmstate_bcm2835_aux = {
+    .name = TYPE_BCM2835_MINIUART,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(read_fifo, BCM2835MiniUartState,
+                            BCM2835_MINIUART_RX_FIFO_LEN),
+        VMSTATE_UINT8(read_pos, BCM2835MiniUartState),
+        VMSTATE_UINT8(read_count, BCM2835MiniUartState),
+        VMSTATE_UINT8(ier, BCM2835MiniUartState),
+        VMSTATE_UINT8(iir, BCM2835MiniUartState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void bcm2835_miniuart_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    BCM2835MiniUartState *s = BCM2835_MINIUART(obj);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &bcm2835_miniuart_ops, s,
+                          TYPE_BCM2835_MINIUART, 0x40);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+}
+
+static void bcm2835_miniuart_realize(DeviceState *dev, Error **errp)
+{
+    BCM2835MiniUartState *s = BCM2835_MINIUART(dev);
+
+    qemu_chr_fe_set_handlers(&s->chr, bcm2835_miniuart_can_receive,
+                             bcm2835_miniuart_receive, NULL, NULL,
+                             s, NULL, true);
+}
+
+static Property bcm2835_miniuart_props[] = {
+    DEFINE_PROP_CHR("chardev", BCM2835MiniUartState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void bcm2835_miniuart_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = bcm2835_miniuart_realize;
+    dc->vmsd = &vmstate_bcm2835_aux;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    dc->props = bcm2835_miniuart_props;
+}
+
+static const TypeInfo bcm2835_miniuart_info = {
+    .name          = TYPE_BCM2835_MINIUART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(BCM2835MiniUartState),
+    .instance_init = bcm2835_miniuart_init,
+    .class_init    = bcm2835_miniuart_class_init,
+};
+
+static void bcm2835_miniuart_register_types(void)
+{
+    type_register_static(&bcm2835_miniuart_info);
+}
+
+type_init(bcm2835_miniuart_register_types)
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 2ce7f2f998..f1e6dd9918 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -1,5 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# bcm2835_miniuart.c
+bcm2835_miniuart_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+bcm2835_miniuart_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+
 # parallel.c
 parallel_ioport_read(const char *desc, uint16_t addr, uint8_t value) "read [%s] addr 0x%02x val 0x%02x"
 parallel_ioport_write(const char *desc, uint16_t addr, uint8_t value) "write [%s] addr 0x%02x val 0x%02x"
diff --git a/include/hw/char/bcm2835_miniuart.h b/include/hw/char/bcm2835_miniuart.h
new file mode 100644
index 0000000000..54d3b622ed
--- /dev/null
+++ b/include/hw/char/bcm2835_miniuart.h
@@ -0,0 +1,37 @@
+/*
+ * BCM2835 (Raspberry Pi) mini UART block.
+ *
+ * Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_CHAR_BCM2835_MINIUART_H
+#define HW_CHAR_BCM2835_MINIUART_H
+
+#include "chardev/char-fe.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+
+#define TYPE_BCM2835_MINIUART "bcm2835-miniuart"
+#define BCM2835_MINIUART(obj) \
+            OBJECT_CHECK(BCM2835MiniUartState, (obj), TYPE_BCM2835_MINIUART)
+
+#define BCM2835_MINIUART_RX_FIFO_LEN 8
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+    CharBackend chr;
+    qemu_irq irq;
+
+    uint8_t read_fifo[BCM2835_MINIUART_RX_FIFO_LEN];
+    uint8_t read_pos, read_count;
+    uint8_t ier, iir;
+} BCM2835MiniUartState;
+
+#endif
-- 
2.21.0

[PATCH 2/3] hw/char/bcm2835_aux: Use the BCM2835 miniuart block

[Philippe Mathieu-Daudé]

In the previous commit we extracted the miniuart block. Time to
use it. It is mapped at offset 0x40 in the AUX block, and uses
the first IRQ.

Due to incomplete Clock Tree support, Linux kernel fails at
initializing the UART clock and disable this block. For now Disable
this feature with the 'aux_enable_supported' variable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/char/bcm2835_aux.c         | 280 +++++++++-------------------------
 hw/char/trace-events          |   4 +
 include/hw/char/bcm2835_aux.h |  10 +-
 3 files changed, 83 insertions(+), 211 deletions(-)

diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index 3f855196e3..a1ca9741d6 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -2,19 +2,10 @@
  * 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
+ * The following features/registers are unimplemented:
  *  - Extra control
  *  - Baudrate
  *  - SPI interfaces
@@ -24,189 +15,77 @@
 #include "hw/char/bcm2835_aux.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "hw/registerfields.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
+#include "trace.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_CNTL_REG 0x60
-#define AUX_MU_STAT_REG 0x64
-#define AUX_MU_BAUD_REG 0x68
+REG32(AUX_IRQ,      0x00)
+REG32(AUX_ENABLE,   0x04)
 
-/* bits in IER/IIR registers */
-#define RX_INT  0x1
-#define TX_INT  0x2
+static const bool aux_enable_supported = false;
 
 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);
+    qemu_set_irq(s->irq, !!(s->reg[R_AUX_IRQ] & s->reg[R_AUX_ENABLE]));
+}
+
+static void bcm2835_aux_set_irq(void *opaque, int irq, int level)
+{
+    BCM2835AuxState *s = opaque;
+
+    s->reg[R_AUX_IRQ] = deposit32(s->reg[R_AUX_IRQ], irq, 1, !!level);
+
+    bcm2835_aux_update(s);
 }
 
 static uint64_t bcm2835_aux_read(void *opaque, hwaddr offset, unsigned size)
 {
-    BCM2835AuxState *s = opaque;
-    uint32_t c, res;
+    BCM2835AuxState *s = BCM2835_AUX(opaque);
+    uint32_t res = 0;
 
     switch (offset) {
-    case AUX_IRQ:
-        return s->iir != 0;
+    case A_AUX_IRQ:
+        res = s->reg[R_AUX_IRQ];
+        break;
 
-    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 */
-        }
-        return res;
-
-    case AUX_MU_BAUD_REG:
-        qemu_log_mask(LOG_UNIMP, "%s: AUX_MU_BAUD_REG unsupported\n", __func__);
-        return 0;
+    case A_AUX_ENABLE:
+        res = s->reg[R_AUX_ENABLE];
+        break;
 
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
                       __func__, offset);
-        return 0;
+        break;
     }
+    trace_bcm2835_aux_read(offset, res);
+
+    return res;
 }
 
 static void bcm2835_aux_write(void *opaque, hwaddr offset, uint64_t value,
                               unsigned size)
 {
-    BCM2835AuxState *s = opaque;
-    unsigned char ch;
+    BCM2835AuxState *s = BCM2835_AUX(opaque);
 
+    trace_bcm2835_aux_write(offset, value);
     switch (offset) {
-    case AUX_ENABLES:
-        if (value != 1) {
-            qemu_log_mask(LOG_UNIMP, "%s: unsupported attempt to enable SPI "
-                          "or disable UART\n", __func__);
+    case A_AUX_ENABLE:
+        if (value <= 1) {
+            if (aux_enable_supported) {
+                memory_region_set_enabled(
+                        sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart), 0),
+                        value);
+            }
+        } else {
+            qemu_log_mask(LOG_UNIMP, "%s: unsupported attempt to enable SPI:"
+                                     " 0x%"PRIx64"\n",
+                          __func__, 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__);
-        break;
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
                       __func__, offset);
@@ -215,35 +94,6 @@ static void bcm2835_aux_write(void *opaque, hwaddr offset, uint64_t value,
     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 = {
     .read = bcm2835_aux_read,
     .write = bcm2835_aux_write,
@@ -254,15 +104,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,18 +120,45 @@ static void bcm2835_aux_init(Object *obj)
                           TYPE_BCM2835_AUX, 0x100);
     sysbus_init_mmio(sbd, &s->iomem);
     sysbus_init_irq(sbd, &s->irq);
+
+    qdev_init_gpio_in_named(DEVICE(obj), bcm2835_aux_set_irq, "aux-irq", 3);
+
+    sysbus_init_child_obj(obj, "miniuart", &s->uart, sizeof(s->uart),
+                          TYPE_BCM2835_MINIUART);
+    object_property_add_alias(obj, "chardev", OBJECT(&s->uart), "chardev",
+                              &error_abort);
 }
 
 static void bcm2835_aux_realize(DeviceState *dev, Error **errp)
 {
     BCM2835AuxState *s = BCM2835_AUX(dev);
+    Error *err = NULL;
 
-    qemu_chr_fe_set_handlers(&s->chr, bcm2835_aux_can_receive,
-                             bcm2835_aux_receive, NULL, NULL, s, NULL, true);
+    object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    memory_region_add_subregion(&s->iomem, 0x40,
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart), 0));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart), 0,
+                qdev_get_gpio_in_named(dev, "aux-irq", 0));
+}
+
+static void bcm2835_aux_reset(DeviceState *dev)
+{
+    BCM2835AuxState *s = BCM2835_AUX(dev);
+
+    s->reg[R_AUX_IRQ] = s->reg[R_AUX_ENABLE] = 0;
+
+    if (aux_enable_supported) {
+        memory_region_set_enabled(
+                    sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart), 0),
+                    false);
+    }
 }
 
 static Property bcm2835_aux_props[] = {
-    DEFINE_PROP_CHR("chardev", BCM2835AuxState, chr),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -296,8 +167,9 @@ static void bcm2835_aux_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = bcm2835_aux_realize;
+    dc->reset = bcm2835_aux_reset;
     dc->vmsd = &vmstate_bcm2835_aux;
-    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->props = bcm2835_aux_props;
 }
 
diff --git a/hw/char/trace-events b/hw/char/trace-events
index f1e6dd9918..0c86a907df 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -1,5 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# bcm2835_aux.c
+bcm2835_aux_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+bcm2835_aux_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+
 # bcm2835_miniuart.c
 bcm2835_miniuart_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
 bcm2835_miniuart_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
diff --git a/include/hw/char/bcm2835_aux.h b/include/hw/char/bcm2835_aux.h
index cdbf7e3e37..a7b3a6ac60 100644
--- a/include/hw/char/bcm2835_aux.h
+++ b/include/hw/char/bcm2835_aux.h
@@ -9,25 +9,21 @@
 #define BCM2835_AUX_H
 
 #include "hw/sysbus.h"
-#include "chardev/char-fe.h"
+#include "hw/char/bcm2835_miniuart.h"
 
 #define TYPE_BCM2835_AUX "bcm2835-aux"
 #define BCM2835_AUX(obj) OBJECT_CHECK(BCM2835AuxState, (obj), TYPE_BCM2835_AUX)
 
-#define BCM2835_AUX_RX_FIFO_LEN 8
-
 typedef struct {
     /*< private >*/
     SysBusDevice parent_obj;
     /*< 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;
+    uint32_t reg[2];
+    BCM2835MiniUartState uart;
 } BCM2835AuxState;
 
 #endif
-- 
2.21.0

[PATCH 3/3] hw: Move BCM2835 AUX device from hw/char/ to hw/misc/

[Philippe Mathieu-Daudé]

The BCM2835 AUX device is a MUX between one UART block and two SPI
blocks. Move it to the hw/misc/ folder.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/char/Makefile.objs                   | 1 -
 hw/char/trace-events                    | 4 ----
 hw/misc/Makefile.objs                   | 1 +
 hw/{char => misc}/bcm2835_aux.c         | 2 +-
 hw/misc/trace-events                    | 4 ++++
 include/hw/arm/bcm2835_peripherals.h    | 2 +-
 include/hw/{char => misc}/bcm2835_aux.h | 0
 7 files changed, 7 insertions(+), 7 deletions(-)
 rename hw/{char => misc}/bcm2835_aux.c (99%)
 rename include/hw/{char => misc}/bcm2835_aux.h (100%)

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 5bd93bde9f..d6b8ce5e60 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -20,7 +20,6 @@ obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
 obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
-obj-$(CONFIG_RASPI) += bcm2835_aux.o
 
 common-obj-$(CONFIG_RASPI) += bcm2835_miniuart.o
 common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 0c86a907df..f1e6dd9918 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -1,9 +1,5 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
-# bcm2835_aux.c
-bcm2835_aux_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
-bcm2835_aux_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
-
 # bcm2835_miniuart.c
 bcm2835_miniuart_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
 bcm2835_miniuart_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index a150680966..9ffeee7f46 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -50,6 +50,7 @@ common-obj-$(CONFIG_OMAP) += omap_gpmc.o
 common-obj-$(CONFIG_OMAP) += omap_l4.o
 common-obj-$(CONFIG_OMAP) += omap_sdrc.o
 common-obj-$(CONFIG_OMAP) += omap_tap.o
+common-obj-$(CONFIG_RASPI) += bcm2835_aux.o
 common-obj-$(CONFIG_RASPI) += bcm2835_mbox.o
 common-obj-$(CONFIG_RASPI) += bcm2835_property.o
 common-obj-$(CONFIG_RASPI) += bcm2835_rng.o
diff --git a/hw/char/bcm2835_aux.c b/hw/misc/bcm2835_aux.c
similarity index 99%
rename from hw/char/bcm2835_aux.c
rename to hw/misc/bcm2835_aux.c
index a1ca9741d6..83698e2ece 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/misc/bcm2835_aux.c
@@ -12,7 +12,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/char/bcm2835_aux.h"
+#include "hw/misc/bcm2835_aux.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/registerfields.h"
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 74276225f8..0756d58162 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -1,5 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# bcm2835_aux.c
+bcm2835_aux_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+bcm2835_aux_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
+
 # eccmemctl.c
 ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
 ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index 6b17f6a382..7aae515d7e 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -13,7 +13,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/char/pl011.h"
-#include "hw/char/bcm2835_aux.h"
+#include "hw/misc/bcm2835_aux.h"
 #include "hw/display/bcm2835_fb.h"
 #include "hw/dma/bcm2835_dma.h"
 #include "hw/intc/bcm2835_ic.h"
diff --git a/include/hw/char/bcm2835_aux.h b/include/hw/misc/bcm2835_aux.h
similarity index 100%
rename from include/hw/char/bcm2835_aux.h
rename to include/hw/misc/bcm2835_aux.h
-- 
2.21.0

Re: [PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block

[no-reply]

Patchew URL: https://patchew.org/QEMU/20191007170646.14961-1-f4bug@amsat.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block
Message-id: 20191007170646.14961-1-f4bug@amsat.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
3ee4459 hw: Move BCM2835 AUX device from hw/char/ to hw/misc/
4109add hw/char/bcm2835_aux: Use the BCM2835 miniuart block
c19904e hw/char: Add the BCM2835 miniuart

=== OUTPUT BEGIN ===
1/3 Checking commit c19904eab80f (hw/char: Add the BCM2835 miniuart)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 380 lines checked

Patch 1/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit 4109addacc9f (hw/char/bcm2835_aux: Use the BCM2835 miniuart block)
ERROR: do not initialise statics to 0 or NULL
#73: FILE: hw/char/bcm2835_aux.c:28:
+static const bool aux_enable_supported = false;

total: 1 errors, 0 warnings, 397 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 3ee4459660f9 (hw: Move BCM2835 AUX device from hw/char/ to hw/misc/)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53: 
rename from hw/char/bcm2835_aux.c

total: 0 errors, 1 warnings, 48 lines checked

Patch 3/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191007170646.14961-1-f4bug@amsat.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 1/3] hw/char: Add the BCM2835 miniuart

[Alistair Francis]

On Mon, Oct 7, 2019 at 10:18 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The miniuart code is already present in the multi-device
> hw/char/bcm2835_aux.c. Simply extracting it does not generate
> patch easy to review. Instead, add it again, rename the function
> names accordingly, use the "hw/registerfields.h" API.
> This is roughtly a copy of commit 97398d900caacc.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/char/Makefile.objs              |   1 +
>  hw/char/bcm2835_miniuart.c         | 327 +++++++++++++++++++++++++++++
>  hw/char/trace-events               |   4 +
>  include/hw/char/bcm2835_miniuart.h |  37 ++++
>  4 files changed, 369 insertions(+)
>  create mode 100644 hw/char/bcm2835_miniuart.c
>  create mode 100644 include/hw/char/bcm2835_miniuart.h
>
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index 02d8a66925..5bd93bde9f 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -22,6 +22,7 @@ obj-$(CONFIG_DIGIC) += digic-uart.o
>  obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
>  obj-$(CONFIG_RASPI) += bcm2835_aux.o
>
> +common-obj-$(CONFIG_RASPI) += bcm2835_miniuart.o
>  common-obj-$(CONFIG_CMSDK_APB_UART) += cmsdk-apb-uart.o
>  common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
>  common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
> diff --git a/hw/char/bcm2835_miniuart.c b/hw/char/bcm2835_miniuart.c
> new file mode 100644
> index 0000000000..0e99cecce7
> --- /dev/null
> +++ b/hw/char/bcm2835_miniuart.c
> @@ -0,0 +1,327 @@
> +/*
> + * BCM2835 (Raspberry Pi) mini UART block.
> + *
> + * Copyright (c) 2015, Microsoft
> + * Written by Andrew Baumann
> + * Based on pl011.c.
> + *
> + * 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
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/char/bcm2835_miniuart.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/registerfields.h"
> +#include "migration/vmstate.h"
> +#include "trace.h"
> +
> +REG32(MU_IO,        0x00)
> +REG32(MU_IER,       0x04)
> +REG32(MU_IIR,       0x08)
> +REG32(MU_LCR,       0x0c)
> +REG32(MU_MCR,       0x10)
> +REG32(MU_LSR,       0x14)
> +REG32(MU_MSR,       0x18)
> +REG32(MU_SCRATCH,   0x1c)
> +REG32(MU_CNTL,      0x20)
> +REG32(MU_STAT,      0x24)
> +REG32(MU_BAUD,      0x28)
> +
> +/* bits in IER/IIR registers */
> +#define RX_INT  0x1
> +#define TX_INT  0x2
> +
> +static void bcm2835_miniuart_update(BCM2835MiniUartState *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 bool is_16650(hwaddr offset)
> +{
> +    return offset < A_MU_CNTL;
> +}
> +
> +static uint64_t bcm2835_miniuart_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    BCM2835MiniUartState *s = opaque;
> +    uint32_t c, res = 0;
> +
> +    switch (offset) {
> +    case A_MU_IO:
> +        /* "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_MINIUART_RX_FIFO_LEN) {
> +                s->read_pos = 0;
> +            }
> +        }
> +        qemu_chr_fe_accept_input(&s->chr);
> +        bcm2835_miniuart_update(s);
> +        res = c;
> +        break;
> +
> +    case A_MU_IER:
> +        /* "DLAB bit set means access baudrate register" is NYI */
> +        res = 0xc0 | s->ier; /* FIFO enables always read 1 */
> +        break;
> +
> +    case A_MU_IIR:
> +        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;
> +        }
> +        break;
> +
> +    case A_MU_LCR:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_LCR_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_MCR:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MCR_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_LSR:
> +        res = 0x60; /* tx idle, empty */
> +        if (s->read_count != 0) {
> +            res |= 0x1;
> +        }
> +        break;
> +
> +    case A_MU_MSR:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MSR_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_SCRATCH:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_SCRATCH unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_CNTL:
> +        res = 0x3; /* tx, rx enabled */
> +        break;
> +
> +    case A_MU_STAT:
> +        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_MINIUART_RX_FIFO_LEN);
> +            res |= ((uint32_t)s->read_count) << 16; /* rx fifo fill level */
> +        }
> +        break;
> +
> +    case A_MU_BAUD:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_BAUD_REG unsupported\n", __func__);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
> +                      __func__, offset);
> +        break;
> +    }
> +
> +    if (is_16650(offset)) {
> +        trace_serial_ioport_read((offset & 0x1f) >> 2, res);
> +    } else {
> +        trace_bcm2835_miniuart_read(offset, res);
> +    }
> +
> +    return res;
> +}
> +
> +static void bcm2835_miniuart_write(void *opaque, hwaddr offset, uint64_t value,
> +                                   unsigned size)
> +{
> +    BCM2835MiniUartState *s = opaque;
> +    unsigned char ch;
> +
> +    if (is_16650(offset)) {
> +        trace_serial_ioport_write((offset & 0x1f) >> 2, value);
> +    } else {
> +        trace_bcm2835_miniuart_write(offset, value);
> +    }
> +
> +    switch (offset) {
> +    case A_MU_IO:
> +        /* "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 A_MU_IER:
> +        /* "DLAB bit set means access baudrate register" is NYI */
> +        s->ier = value & (TX_INT | RX_INT);
> +        bcm2835_miniuart_update(s);
> +        break;
> +
> +    case A_MU_IIR:
> +        if (value & 0x2) {
> +            s->read_count = 0;
> +        }
> +        break;
> +
> +    case A_MU_LCR:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_LCR_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_MCR:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_MCR_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_SCRATCH:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_SCRATCH unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_CNTL:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_CNTL_REG unsupported\n", __func__);
> +        break;
> +
> +    case A_MU_BAUD:
> +        qemu_log_mask(LOG_UNIMP, "%s: A_MU_BAUD_REG unsupported\n", __func__);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset %"HWADDR_PRIx"\n",
> +                      __func__, offset);
> +    }
> +
> +    bcm2835_miniuart_update(s);
> +}
> +
> +static int bcm2835_miniuart_can_receive(void *opaque)
> +{
> +    BCM2835MiniUartState *s = opaque;
> +
> +    return s->read_count < BCM2835_MINIUART_RX_FIFO_LEN;
> +}
> +
> +static void bcm2835_miniuart_put_fifo(void *opaque, uint8_t value)
> +{
> +    BCM2835MiniUartState *s = opaque;
> +    int slot;
> +
> +    slot = s->read_pos + s->read_count;
> +    if (slot >= BCM2835_MINIUART_RX_FIFO_LEN) {
> +        slot -= BCM2835_MINIUART_RX_FIFO_LEN;
> +    }
> +    s->read_fifo[slot] = value;
> +    s->read_count++;
> +    if (s->read_count == BCM2835_MINIUART_RX_FIFO_LEN) {
> +        /* buffer full */
> +    }
> +    bcm2835_miniuart_update(s);
> +}
> +
> +static void bcm2835_miniuart_receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    bcm2835_miniuart_put_fifo(opaque, *buf);
> +}
> +
> +static const MemoryRegionOps bcm2835_miniuart_ops = {
> +    .read = bcm2835_miniuart_read,
> +    .write = bcm2835_miniuart_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};
> +
> +static const VMStateDescription vmstate_bcm2835_aux = {
> +    .name = TYPE_BCM2835_MINIUART,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8_ARRAY(read_fifo, BCM2835MiniUartState,
> +                            BCM2835_MINIUART_RX_FIFO_LEN),
> +        VMSTATE_UINT8(read_pos, BCM2835MiniUartState),
> +        VMSTATE_UINT8(read_count, BCM2835MiniUartState),
> +        VMSTATE_UINT8(ier, BCM2835MiniUartState),
> +        VMSTATE_UINT8(iir, BCM2835MiniUartState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void bcm2835_miniuart_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    BCM2835MiniUartState *s = BCM2835_MINIUART(obj);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &bcm2835_miniuart_ops, s,
> +                          TYPE_BCM2835_MINIUART, 0x40);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +}
> +
> +static void bcm2835_miniuart_realize(DeviceState *dev, Error **errp)
> +{
> +    BCM2835MiniUartState *s = BCM2835_MINIUART(dev);
> +
> +    qemu_chr_fe_set_handlers(&s->chr, bcm2835_miniuart_can_receive,
> +                             bcm2835_miniuart_receive, NULL, NULL,
> +                             s, NULL, true);
> +}
> +
> +static Property bcm2835_miniuart_props[] = {
> +    DEFINE_PROP_CHR("chardev", BCM2835MiniUartState, chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void bcm2835_miniuart_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = bcm2835_miniuart_realize;
> +    dc->vmsd = &vmstate_bcm2835_aux;
> +    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +    dc->props = bcm2835_miniuart_props;
> +}
> +
> +static const TypeInfo bcm2835_miniuart_info = {
> +    .name          = TYPE_BCM2835_MINIUART,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(BCM2835MiniUartState),
> +    .instance_init = bcm2835_miniuart_init,
> +    .class_init    = bcm2835_miniuart_class_init,
> +};
> +
> +static void bcm2835_miniuart_register_types(void)
> +{
> +    type_register_static(&bcm2835_miniuart_info);
> +}
> +
> +type_init(bcm2835_miniuart_register_types)
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index 2ce7f2f998..f1e6dd9918 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -1,5 +1,9 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>
> +# bcm2835_miniuart.c
> +bcm2835_miniuart_read(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
> +bcm2835_miniuart_write(uint64_t addr, uint32_t value) "read addr 0x%"PRIx64" value 0x%x"
> +
>  # parallel.c
>  parallel_ioport_read(const char *desc, uint16_t addr, uint8_t value) "read [%s] addr 0x%02x val 0x%02x"
>  parallel_ioport_write(const char *desc, uint16_t addr, uint8_t value) "write [%s] addr 0x%02x val 0x%02x"
> diff --git a/include/hw/char/bcm2835_miniuart.h b/include/hw/char/bcm2835_miniuart.h
> new file mode 100644
> index 0000000000..54d3b622ed
> --- /dev/null
> +++ b/include/hw/char/bcm2835_miniuart.h
> @@ -0,0 +1,37 @@
> +/*
> + * BCM2835 (Raspberry Pi) mini UART block.
> + *
> + * Copyright (c) 2015, Microsoft
> + * Written by Andrew Baumann
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_CHAR_BCM2835_MINIUART_H
> +#define HW_CHAR_BCM2835_MINIUART_H
> +
> +#include "chardev/char-fe.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +
> +#define TYPE_BCM2835_MINIUART "bcm2835-miniuart"
> +#define BCM2835_MINIUART(obj) \
> +            OBJECT_CHECK(BCM2835MiniUartState, (obj), TYPE_BCM2835_MINIUART)
> +
> +#define BCM2835_MINIUART_RX_FIFO_LEN 8
> +
> +typedef struct {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +    CharBackend chr;
> +    qemu_irq irq;
> +
> +    uint8_t read_fifo[BCM2835_MINIUART_RX_FIFO_LEN];
> +    uint8_t read_pos, read_count;
> +    uint8_t ier, iir;
> +} BCM2835MiniUartState;
> +
> +#endif
> --
> 2.21.0
>
>

Re: [PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block

[Peter Maydell]

On Mon, 7 Oct 2019 at 18:06, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The BCM2838 has many more peripherals than his little brother,
> the BCM2837. With the raspi4, the Linux kernel takes more steps
> to configure the various MUXed devices. At some point it started
> to bug me, so I plan to add a dummy simple BCM2835_SPI block.
> It is cleaner to add it as a separate device than mixed with the
> AUX block. As a first step, split the UART block out.
>
> Since this part is self-contained and my raspi4 branch is getting
> too big, I'm sending it as a single series.

If you squash patches 1 and 2 together and avoid doing
things like renaming all the register offset constant
names (or, for stuff like fixing up comment syntax and
checkpatch nits, do them in a preparatory patch), then it
becomes somewhat easier to review, because then you can use
git diff --color-moved and can see easily that code has
only been moved to the other file without any accidental
extra changes.

thanks
-- PMM