From: Alistair Francis <alistair23@gmail.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Alistair Francis" <alistair@alistair23.me>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
"Andrew Baumann" <Andrew.Baumann@microsoft.com>,
qemu-arm <qemu-arm@nongnu.org>,
"Marc-Andre Lureau" <marcandre.lureau@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH 1/3] hw/char: Add the BCM2835 miniuart
Date: Fri, 11 Oct 2019 15:05:56 -0700 [thread overview]
Message-ID: <CAKmqyKM-7caCkev3U2x_ZhCJc-zgRk5SN1QkfHAxCQk1C44cZw@mail.gmail.com> (raw)
In-Reply-To: <20191007170646.14961-2-f4bug@amsat.org>
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
>
>
next prev parent reply other threads:[~2019-10-11 22:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-07 17:06 [PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block Philippe Mathieu-Daudé
2019-10-07 17:06 ` [PATCH 1/3] hw/char: Add the BCM2835 miniuart Philippe Mathieu-Daudé
2019-10-11 22:05 ` Alistair Francis [this message]
2019-10-07 17:06 ` [PATCH 2/3] hw/char/bcm2835_aux: Use the BCM2835 miniuart block Philippe Mathieu-Daudé
2019-10-07 17:06 ` [PATCH 3/3] hw: Move BCM2835 AUX device from hw/char/ to hw/misc/ Philippe Mathieu-Daudé
2019-10-07 21:09 ` [PATCH 0/3] hw/arm/raspi: Split the UART block from the AUX block no-reply
2019-10-17 16:23 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKM-7caCkev3U2x_ZhCJc-zgRk5SN1QkfHAxCQk1C44cZw@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=Andrew.Baumann@microsoft.com \
--cc=alistair@alistair23.me \
--cc=f4bug@amsat.org \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).