| Message ID | 1499771839-32518-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > Implement a model of the simple "APB UART" provided in > the Cortex-M System Design Kit (CMSDK). > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/char/Makefile.objs | 1 + > include/hw/char/cmsdk-apb-uart.h | 78 ++++++++ > hw/char/cmsdk-apb-uart.c | 390 +++++++++++++++++++++++++++++++++++++++ > default-configs/arm-softmmu.mak | 2 + > hw/char/trace-events | 9 + > 5 files changed, 480 insertions(+) > create mode 100644 include/hw/char/cmsdk-apb-uart.h > create mode 100644 hw/char/cmsdk-apb-uart.c > > diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs > index 55fcb68..1bcd37e 100644 > --- a/hw/char/Makefile.objs > +++ b/hw/char/Makefile.objs > @@ -19,6 +19,7 @@ obj-$(CONFIG_DIGIC) += digic-uart.o > obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o > obj-$(CONFIG_RASPI) += bcm2835_aux.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 > common-obj-$(CONFIG_GRLIB) += grlib_apbuart.o > diff --git a/include/hw/char/cmsdk-apb-uart.h b/include/hw/char/cmsdk-apb-uart.h > new file mode 100644 > index 0000000..c41fba9 > --- /dev/null > +++ b/include/hw/char/cmsdk-apb-uart.h > @@ -0,0 +1,78 @@ > +/* > + * ARM CMSDK APB UART emulation > + * > + * Copyright (c) 2017 Linaro Limited > + * Written by Peter Maydell > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 or > + * (at your option) any later version. > + */ > + > +#ifndef CMSDK_APB_UART_H > +#define CMSDK_APB_UART_H > + > +#include "hw/sysbus.h" > +#include "chardev/char-fe.h" > + > +#define TYPE_CMSDK_APB_UART "cmsdk-apb-uart" > +#define CMSDK_APB_UART(obj) OBJECT_CHECK(CMSDKAPBUART, (obj), \ > + TYPE_CMSDK_APB_UART) > + > +typedef struct { > + /*< private >*/ > + SysBusDevice parent_obj; > + > + /*< public >*/ > + MemoryRegion iomem; > + CharBackend chr; > + qemu_irq txint; > + qemu_irq rxint; > + qemu_irq txovrint; > + qemu_irq rxovrint; > + qemu_irq uartint; > + guint watch_tag; > + uint32_t pclk_frq; > + > + uint32_t state; > + uint32_t ctrl; > + uint32_t intstatus; > + uint32_t bauddiv; > + /* This UART has no FIFO, only a 1-character buffer for each of Tx and Rx */ > + uint8_t txbuf; > + uint8_t rxbuf; > +} CMSDKAPBUART; This should be CamelCase. > + > +/** > + * cmsdk_apb_uart_create - convenience function to create TYPE_CMSDK_APB_UART > + * @addr: location in system memory to map registers > + * @chr: Chardev backend to connect UART to, or NULL if no backend > + * @pclk_frq: frequency in Hz of the PCLK clock (used for calculating baud rate) > + */ > +static inline DeviceState *cmsdk_apb_uart_create(hwaddr addr, > + qemu_irq txint, > + qemu_irq rxint, > + qemu_irq txovrint, > + qemu_irq rxovrint, > + qemu_irq uartint, > + Chardev *chr, > + uint32_t pclk_frq) > +{ > + DeviceState *dev; > + SysBusDevice *s; > + > + dev = qdev_create(NULL, TYPE_CMSDK_APB_UART); > + s = SYS_BUS_DEVICE(dev); > + qdev_prop_set_chr(dev, "chardev", chr); > + qdev_prop_set_uint32(dev, "pclk-frq", pclk_frq); > + qdev_init_nofail(dev); > + sysbus_mmio_map(s, 0, addr); > + sysbus_connect_irq(s, 0, txint); > + sysbus_connect_irq(s, 1, rxint); > + sysbus_connect_irq(s, 2, txovrint); > + sysbus_connect_irq(s, 3, rxovrint); > + sysbus_connect_irq(s, 4, uartint); > + return dev; > +} > + > +#endif > diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c > new file mode 100644 > index 0000000..a2f55db > --- /dev/null > +++ b/hw/char/cmsdk-apb-uart.c > @@ -0,0 +1,390 @@ > +/* > + * ARM CMSDK APB UART emulation > + * > + * Copyright (c) 2017 Linaro Limited > + * Written by Peter Maydell > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 or > + * (at your option) any later version. > + */ > + > +/* This is a model of the "APB UART" which is part of the Cortex-M > + * System Design Kit (CMSDK) and documented in the Cortex-M System > + * Design Kit Technical Reference Manual (ARM DDI0479C): > + * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit I can't find the spec from this site. Is it possible to link directly to the guides? I have found a few dead links from some of these marketing focused site. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/log.h" > +#include "qapi/error.h" > +#include "trace.h" > +#include "hw/sysbus.h" > +#include "hw/registerfields.h" > +#include "chardev/char-fe.h" > +#include "chardev/char-serial.h" > +#include "hw/char/cmsdk-apb-uart.h" > + > +REG32(DATA, 0) > +REG32(STATE, 4) > + FIELD(STATE, TXFULL, 0, 1) > + FIELD(STATE, RXFULL, 1, 1) > + FIELD(STATE, TXOVERRUN, 2, 1) > + FIELD(STATE, RXOVERRUN, 3, 1) > +REG32(CTRL, 8) > + FIELD(CTRL, TX_EN, 0, 1) > + FIELD(CTRL, RX_EN, 1, 1) > + FIELD(CTRL, TX_INTEN, 2, 1) > + FIELD(CTRL, RX_INTEN, 3, 1) > + FIELD(CTRL, TXO_INTEN, 4, 1) > + FIELD(CTRL, RXO_INTEN, 5, 1) > + FIELD(CTRL, HSTEST, 6, 1) > +REG32(INTSTATUS, 0xc) > + FIELD(INTSTATUS, TX, 0, 1) > + FIELD(INTSTATUS, RX, 1, 1) > + FIELD(INTSTATUS, TXO, 2, 1) > + FIELD(INTSTATUS, RXO, 3, 1) > +REG32(BAUDDIV, 0x10) > +REG32(PID4, 0xFD0) > +REG32(PID5, 0xFD4) > +REG32(PID6, 0xFD8) > +REG32(PID7, 0xFDC) > +REG32(PID0, 0xFE0) > +REG32(PID1, 0xFE4) > +REG32(PID2, 0xFE8) > +REG32(PID3, 0xFEC) > +REG32(CID0, 0xFF0) > +REG32(CID1, 0xFF4) > +REG32(CID2, 0xFF8) > +REG32(CID3, 0xFFC) > + > +/* PID/CID values */ > +static const int uart_id[] = { > + 0x04, 0x00, 0x00, 0x00, /* PID4..PID7 */ > + 0x21, 0xb8, 0x1b, 0x00, /* PID0..PID3 */ > + 0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */ > +}; > + > +static void uart_update_parameters(CMSDKAPBUART *s) > +{ > + QEMUSerialSetParams ssp; > + > + /* This UART is always 8N1 but the baud rate is programmable. > + * The minimum permitted bauddiv setting is 16, so we just ignore > + * settings below that (usually this means the device has just > + * been reset and not yet programmed). > + */ > + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { > + return; > + } This seems like it should deserve a guest error print. > + > + ssp.data_bits = 8; > + ssp.parity = 'N'; > + ssp.stop_bits = 1; > + ssp.speed = s->pclk_frq / s->bauddiv; > + qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp); > + trace_cmsdk_apb_uart_set_params(ssp.speed); > +} > + > +static void cmsdk_apb_uart_update(CMSDKAPBUART *s) > +{ > + /* update outbound irqs, including handling the way the rxo and txo > + * interrupt status bits are just logical AND of the overrun bit in > + * STATE and the overrun interrupt enable bit in CTRL. > + */ > + uint32_t omask = (R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK); > + s->intstatus &= ~omask; > + s->intstatus |= (s->state & (s->ctrl >> 2) & omask); > + > + qemu_set_irq(s->txint, !!(s->intstatus & R_INTSTATUS_TX_MASK)); > + qemu_set_irq(s->rxint, !!(s->intstatus & R_INTSTATUS_RX_MASK)); > + qemu_set_irq(s->txovrint, !!(s->intstatus & R_INTSTATUS_TXO_MASK)); > + qemu_set_irq(s->rxovrint, !!(s->intstatus & R_INTSTATUS_RXO_MASK)); > + qemu_set_irq(s->uartint, !!(s->intstatus)); > +} > + > +static int uart_can_receive(void *opaque) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + > + /* We can take a char if RX is enabled and the buffer is empty */ > + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { > + return 1; > + } > + return 0; > +} > + > +static void uart_receive(void *opaque, const uint8_t *buf, int size) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + > + trace_cmsdk_apb_uart_receive(*buf); > + > + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { > + /* Just drop the character on the floor */ > + return; Doesn't this also deserve a guest error print. > + } > + > + if (s->state & R_STATE_RXFULL_MASK) { > + s->state |= R_STATE_RXOVERRUN_MASK; > + } > + > + s->rxbuf = *buf; > + s->state |= R_STATE_RXFULL_MASK; > + if (s->ctrl & R_CTRL_RX_INTEN_MASK) { > + s->intstatus |= R_INTSTATUS_RX_MASK; > + } > + cmsdk_apb_uart_update(s); > +} > + > +static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + uint64_t r; > + > + switch (offset) { > + case A_DATA: > + r = s->rxbuf; > + s->state &= ~R_STATE_RXFULL_MASK; > + cmsdk_apb_uart_update(s); > + break; > + case A_STATE: > + r = s->state; > + break; > + case A_CTRL: > + r = s->ctrl; > + break; > + case A_INTSTATUS: > + r = s->intstatus; > + break; > + case A_BAUDDIV: > + r = s->bauddiv; > + break; You used pasrt of the register API but not the second part. This seems like a greaet device to use the register API on. > + case A_PID4 ... A_CID3: > + r = uart_id[offset / 4 - A_PID4]; I think this is the one you already found in the cover letter. > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "CMSDK APB UART read: bad offset %x\n", (int) offset); > + r = 0; > + break; > + } > + trace_cmsdk_apb_uart_read(offset, r, size); > + return r; > +} > + > +/* Try to send tx data, and arrange to be called back later if > + * we can't (ie the char backend is busy/blocking). > + */ > +static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + int ret; > + > + s->watch_tag = 0; > + > + if (!(s->ctrl & R_CTRL_TX_EN_MASK) || !(s->state & R_STATE_TXFULL_MASK)) { > + return FALSE; > + } > + > + ret = qemu_chr_fe_write(&s->chr, &s->txbuf, 1); > + if (ret <= 0) { > + s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, > + uart_transmit, s); > + if (!s->watch_tag) { > + /* Most common reason to be here is "no chardev backend": > + * just insta-drain the buffer, so the serial output > + * goes into a void, rather than blocking the guest. > + */ > + goto buffer_drained; > + } > + /* Transmit pending */ > + trace_cmsdk_apb_uart_tx_pending(); > + return FALSE; > + } > + > +buffer_drained: > + /* Character successfully sent */ > + trace_cmsdk_apb_uart_tx(s->txbuf); > + s->state &= ~R_STATE_TXFULL_MASK; > + /* Going from TXFULL set to clear triggers the tx interrupt */ > + if (s->ctrl & R_CTRL_TX_INTEN_MASK) { > + s->intstatus |= R_INTSTATUS_TX_MASK; > + } > + cmsdk_apb_uart_update(s); > + return FALSE; > +} > + > +static void uart_cancel_transmit(CMSDKAPBUART *s) > +{ > + if (s->watch_tag) { > + g_source_remove(s->watch_tag); > + s->watch_tag = 0; > + } > +} > + > +static void uart_write(void *opaque, hwaddr offset, uint64_t value, > + unsigned size) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + > + trace_cmsdk_apb_uart_write(offset, value, size); > + > + switch (offset) { > + case A_DATA: > + { > + s->txbuf = value; > + if (s->state & R_STATE_TXFULL_MASK) { > + /* Buffer already full -- note the overrun and let the > + * existing pending transmit callback handle the new char. > + */ > + s->state |= R_STATE_TXOVERRUN_MASK; > + cmsdk_apb_uart_update(s); > + } else { > + s->state |= R_STATE_TXFULL_MASK; > + uart_transmit(NULL, G_IO_OUT, s); > + } > + break; > + } Why is this case inside braces? Thanks, Alistair
On 11 July 2017 at 16:12, Alistair Francis <alistair23@gmail.com> wrote: > On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> Implement a model of the simple "APB UART" provided in >> the Cortex-M System Design Kit (CMSDK). >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> +} CMSDKAPBUART; > > This should be CamelCase. Yes, but CamelCase where all the words are all-uppercase (as with CMSDK, APB and UART) is indistinguishable from all-uppercase. (Compare the way we say "PCIIORegion" rather than "PciIoRegion".) >> +/* This is a model of the "APB UART" which is part of the Cortex-M >> + * System Design Kit (CMSDK) and documented in the Cortex-M System >> + * Design Kit Technical Reference Manual (ARM DDI0479C): >> + * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit > > I can't find the spec from this site. Is it possible to link directly > to the guides? I have found a few dead links from some of these > marketing focused site. It's the link with the big blue button "Cortex-M System Design Kit TRM". I didn't want to link directly to the TRM, because that is (currently) an infocenter.arm.com URL which may eventually go away as content migrates to developer.arm.com. (The DDI document reference number is unique and sufficient to find the document in future even in the face of broken links.) >> +static void uart_update_parameters(CMSDKAPBUART *s) >> +{ >> + QEMUSerialSetParams ssp; >> + >> + /* This UART is always 8N1 but the baud rate is programmable. >> + * The minimum permitted bauddiv setting is 16, so we just ignore >> + * settings below that (usually this means the device has just >> + * been reset and not yet programmed). >> + */ >> + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { >> + return; >> + } > > This seems like it should deserve a guest error print. As the comment says, that would cause us to print a spurious warning every time the device was reset. >> +static int uart_can_receive(void *opaque) >> +{ >> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >> + >> + /* We can take a char if RX is enabled and the buffer is empty */ >> + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { >> + return 1; >> + } >> + return 0; >> +} >> + >> +static void uart_receive(void *opaque, const uint8_t *buf, int size) >> +{ >> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >> + >> + trace_cmsdk_apb_uart_receive(*buf); >> + >> + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { >> + /* Just drop the character on the floor */ >> + return; > > Doesn't this also deserve a guest error print. It's a can't-happen case because uart_can_receive() won't return 1 if the EN bit is clear. Checking again here is perhaps unnecessary paranoia, but it's not a guest error. >> + } >> + >> + if (s->state & R_STATE_RXFULL_MASK) { >> + s->state |= R_STATE_RXOVERRUN_MASK; >> + } >> + >> + s->rxbuf = *buf; >> + s->state |= R_STATE_RXFULL_MASK; >> + if (s->ctrl & R_CTRL_RX_INTEN_MASK) { >> + s->intstatus |= R_INTSTATUS_RX_MASK; >> + } >> + cmsdk_apb_uart_update(s); >> +} >> + >> +static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size) >> +{ >> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >> + uint64_t r; >> + >> + switch (offset) { >> + case A_DATA: >> + r = s->rxbuf; >> + s->state &= ~R_STATE_RXFULL_MASK; >> + cmsdk_apb_uart_update(s); >> + break; >> + case A_STATE: >> + r = s->state; >> + break; >> + case A_CTRL: >> + r = s->ctrl; >> + break; >> + case A_INTSTATUS: >> + r = s->intstatus; >> + break; >> + case A_BAUDDIV: >> + r = s->bauddiv; >> + break; > > You used pasrt of the register API but not the second part. This seems > like a greaet device to use the register API on. I really don't like the register API. The field definition convenience macros are fine, but I prefer to define devices with read and write functions with switches, I think it's clearer, and it's easier to see where you want to put a breakpoint to be able to step through register reads and writes, and so on. >> + case A_PID4 ... A_CID3: >> + r = uart_id[offset / 4 - A_PID4]; > > I think this is the one you already found in the cover letter. Yep. (Same in both timer and uart.) >> + switch (offset) { >> + case A_DATA: >> + { >> + s->txbuf = value; >> + if (s->state & R_STATE_TXFULL_MASK) { >> + /* Buffer already full -- note the overrun and let the >> + * existing pending transmit callback handle the new char. >> + */ >> + s->state |= R_STATE_TXOVERRUN_MASK; >> + cmsdk_apb_uart_update(s); >> + } else { >> + s->state |= R_STATE_TXFULL_MASK; >> + uart_transmit(NULL, G_IO_OUT, s); >> + } >> + break; >> + } > > Why is this case inside braces? I probably had a local variable in there at one point which I ended up not needing. I'll delete the braces. thanks -- PMM
On Tue, Jul 11, 2017 at 5:33 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 11 July 2017 at 16:12, Alistair Francis <alistair23@gmail.com> wrote: >> On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> Implement a model of the simple "APB UART" provided in >>> the Cortex-M System Design Kit (CMSDK). >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >>> +} CMSDKAPBUART; >> >> This should be CamelCase. > > Yes, but CamelCase where all the words are all-uppercase > (as with CMSDK, APB and UART) is indistinguishable from > all-uppercase. (Compare the way we say "PCIIORegion" rather > than "PciIoRegion".) Good point, I feel like it just looks strange being all caps though. > >>> +/* This is a model of the "APB UART" which is part of the Cortex-M >>> + * System Design Kit (CMSDK) and documented in the Cortex-M System >>> + * Design Kit Technical Reference Manual (ARM DDI0479C): >>> + * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit >> >> I can't find the spec from this site. Is it possible to link directly >> to the guides? I have found a few dead links from some of these >> marketing focused site. > > It's the link with the big blue button "Cortex-M System Design Kit > TRM". I didn't want to link directly to the TRM, because that is > (currently) an infocenter.arm.com URL which may eventually go > away as content migrates to developer.arm.com. (The DDI document > reference number is unique and sufficient to find the document > in future even in the face of broken links.) > >>> +static void uart_update_parameters(CMSDKAPBUART *s) >>> +{ >>> + QEMUSerialSetParams ssp; >>> + >>> + /* This UART is always 8N1 but the baud rate is programmable. >>> + * The minimum permitted bauddiv setting is 16, so we just ignore >>> + * settings below that (usually this means the device has just >>> + * been reset and not yet programmed). >>> + */ >>> + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { >>> + return; >>> + } >> >> This seems like it should deserve a guest error print. > > As the comment says, that would cause us to print a spurious > warning every time the device was reset. I just see this being hard to debug. What about if not printing if baud rate is 0 but guest error otherwise? Or can this not be called from reset? > >>> +static int uart_can_receive(void *opaque) >>> +{ >>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>> + >>> + /* We can take a char if RX is enabled and the buffer is empty */ >>> + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { >>> + return 1; >>> + } >>> + return 0; >>> +} >>> + >>> +static void uart_receive(void *opaque, const uint8_t *buf, int size) >>> +{ >>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>> + >>> + trace_cmsdk_apb_uart_receive(*buf); >>> + >>> + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { >>> + /* Just drop the character on the floor */ >>> + return; >> >> Doesn't this also deserve a guest error print. > > It's a can't-happen case because uart_can_receive() won't > return 1 if the EN bit is clear. Checking again here is > perhaps unnecessary paranoia, but it's not a guest error. Ah good point. Maybe that is worth stating? Thanks, Alistair > >>> + } >>> + >>> + if (s->state & R_STATE_RXFULL_MASK) { >>> + s->state |= R_STATE_RXOVERRUN_MASK; >>> + } >>> + >>> + s->rxbuf = *buf; >>> + s->state |= R_STATE_RXFULL_MASK; >>> + if (s->ctrl & R_CTRL_RX_INTEN_MASK) { >>> + s->intstatus |= R_INTSTATUS_RX_MASK; >>> + } >>> + cmsdk_apb_uart_update(s); >>> +} >>> + >>> +static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size) >>> +{ >>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>> + uint64_t r; >>> + >>> + switch (offset) { >>> + case A_DATA: >>> + r = s->rxbuf; >>> + s->state &= ~R_STATE_RXFULL_MASK; >>> + cmsdk_apb_uart_update(s); >>> + break; >>> + case A_STATE: >>> + r = s->state; >>> + break; >>> + case A_CTRL: >>> + r = s->ctrl; >>> + break; >>> + case A_INTSTATUS: >>> + r = s->intstatus; >>> + break; >>> + case A_BAUDDIV: >>> + r = s->bauddiv; >>> + break; >> >> You used pasrt of the register API but not the second part. This seems >> like a greaet device to use the register API on. > > I really don't like the register API. The field definition > convenience macros are fine, but I prefer to define devices > with read and write functions with switches, I think it's > clearer, and it's easier to see where you want to put a breakpoint > to be able to step through register reads and writes, and so on. > >>> + case A_PID4 ... A_CID3: >>> + r = uart_id[offset / 4 - A_PID4]; >> >> I think this is the one you already found in the cover letter. > > Yep. (Same in both timer and uart.) > >>> + switch (offset) { >>> + case A_DATA: >>> + { >>> + s->txbuf = value; >>> + if (s->state & R_STATE_TXFULL_MASK) { >>> + /* Buffer already full -- note the overrun and let the >>> + * existing pending transmit callback handle the new char. >>> + */ >>> + s->state |= R_STATE_TXOVERRUN_MASK; >>> + cmsdk_apb_uart_update(s); >>> + } else { >>> + s->state |= R_STATE_TXFULL_MASK; >>> + uart_transmit(NULL, G_IO_OUT, s); >>> + } >>> + break; >>> + } >> >> Why is this case inside braces? > > I probably had a local variable in there at one point which > I ended up not needing. I'll delete the braces. > > thanks > -- PMM
On 11 July 2017 at 16:40, Alistair Francis <alistair23@gmail.com> wrote: > On Tue, Jul 11, 2017 at 5:33 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 11 July 2017 at 16:12, Alistair Francis <alistair23@gmail.com> wrote: >>> On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> Implement a model of the simple "APB UART" provided in >>>> the Cortex-M System Design Kit (CMSDK). >>>> >>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> >>>> +} CMSDKAPBUART; >>> >>> This should be CamelCase. >> >> Yes, but CamelCase where all the words are all-uppercase >> (as with CMSDK, APB and UART) is indistinguishable from >> all-uppercase. (Compare the way we say "PCIIORegion" rather >> than "PciIoRegion".) > > Good point, I feel like it just looks strange being all caps though. Me too, but I didn't have any better naming ideas. >>>> +static void uart_update_parameters(CMSDKAPBUART *s) >>>> +{ >>>> + QEMUSerialSetParams ssp; >>>> + >>>> + /* This UART is always 8N1 but the baud rate is programmable. >>>> + * The minimum permitted bauddiv setting is 16, so we just ignore >>>> + * settings below that (usually this means the device has just >>>> + * been reset and not yet programmed). >>>> + */ >>>> + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { >>>> + return; >>>> + } >>> >>> This seems like it should deserve a guest error print. >> >> As the comment says, that would cause us to print a spurious >> warning every time the device was reset. > > I just see this being hard to debug. What about if not printing if > baud rate is 0 but guest error otherwise? > > Or can this not be called from reset? The thing is that the guest error really here is "enabled TX but didn't program the baud rate". So if you want to flag it up you'd want to do it in uart_write() at the point where CTRL.TX_EN is written to 1, not in this function. I decided I didn't really care enough to check and report that (we often don't bother to track every possible guest mistake). It won't make any difference to QEMU anyway except in the vanishingly rare case that the user connects QEMU up to a real serial port. I very nearly didn't bother to implement the set-params logic at all (we don't have it on the pl011 and nobody's ever complained). I can add the qemu_log on "TX_EN written as 1 and baud_div out of range" if you think it's helpful though. >>>> +static int uart_can_receive(void *opaque) >>>> +{ >>>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>>> + >>>> + /* We can take a char if RX is enabled and the buffer is empty */ >>>> + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { >>>> + return 1; >>>> + } >>>> + return 0; >>>> +} >>>> + >>>> +static void uart_receive(void *opaque, const uint8_t *buf, int size) >>>> +{ >>>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>>> + >>>> + trace_cmsdk_apb_uart_receive(*buf); >>>> + >>>> + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { >>>> + /* Just drop the character on the floor */ >>>> + return; >>> >>> Doesn't this also deserve a guest error print. >> >> It's a can't-happen case because uart_can_receive() won't >> return 1 if the EN bit is clear. Checking again here is >> perhaps unnecessary paranoia, but it's not a guest error. > > Ah good point. Maybe that is worth stating? Mmm, something like: /* In fact uart_can_receive() ensures that we can't be * called unless RX is enabled and the buffer is empty, * but we include this logic as documentation of what the * hardware does if a character arrives in these circumstances. */ (Or we could delete this and the RXFULL check below it entirely.) thanks -- PMM
On 07/11/2017 12:12 PM, Alistair Francis wrote: > On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: [...] >> +#include "qemu/osdep.h" >> +#include "qemu/log.h" >> +#include "qapi/error.h" >> +#include "trace.h" >> +#include "hw/sysbus.h" >> +#include "hw/registerfields.h" [...] >> +REG32(DATA, 0) >> +REG32(STATE, 4) >> + FIELD(STATE, TXFULL, 0, 1) >> + FIELD(STATE, RXFULL, 1, 1) >> + FIELD(STATE, TXOVERRUN, 2, 1) >> + FIELD(STATE, RXOVERRUN, 3, 1) > > You used pasrt of the register API but not the second part. This seems > like a greaet device to use the register API on. He did not used the Register API (declared in hw/register.h) but only the Register Fields API ;)
Hi Peter, There is probably no need yet, but we could model a generic APB slave with this code, as an overlapping subregion (device top 0x30 bytes). For now only PID0 differs, but since all ID are 8bit the device could even instantiate the subregion with some u64 PID, 32 CID args. (thinking about watchdog and dualtimers). Regards, Phil. On 07/11/2017 08:17 AM, Peter Maydell wrote: > +REG32(PID4, 0xFD0) > +REG32(PID5, 0xFD4) > +REG32(PID6, 0xFD8) > +REG32(PID7, 0xFDC) > +REG32(PID0, 0xFE0) > +REG32(PID1, 0xFE4) > +REG32(PID2, 0xFE8) > +REG32(PID3, 0xFEC) > +REG32(CID0, 0xFF0) > +REG32(CID1, 0xFF4) > +REG32(CID2, 0xFF8) > +REG32(CID3, 0xFFC) > + > +/* PID/CID values */ > +static const int uart_id[] = { > + 0x04, 0x00, 0x00, 0x00, /* PID4..PID7 */ > + 0x21, 0xb8, 0x1b, 0x00, /* PID0..PID3 */ > + 0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */ > +}; > + [...] > +static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + uint64_t r; > + > + switch (offset) { [...] > + case A_PID4 ... A_CID3: > + r = uart_id[offset / 4 - A_PID4]; > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "CMSDK APB UART read: bad offset %x\n", (int) offset); > + r = 0; > + break; > + } > + trace_cmsdk_apb_uart_read(offset, r, size); > + return r; > +} > + [...] > +static void uart_write(void *opaque, hwaddr offset, uint64_t value, > + unsigned size) > +{ > + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); > + > + trace_cmsdk_apb_uart_write(offset, value, size); > + > + switch (offset) { [...] > + case A_PID4 ... A_CID3: > + qemu_log_mask(LOG_GUEST_ERROR, > + "CMSDK APB UART write: write to RO offset 0x%x\n", > + (int)offset); > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "CMSDK APB UART write: bad offset 0x%x\n", (int) offset); > + break; > + } > +}
On 11 July 2017 at 21:42, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Peter, > > There is probably no need yet, but we could model a generic APB slave with > this code, as an overlapping subregion (device top 0x30 bytes). > > For now only PID0 differs, but since all ID are 8bit the device could even > instantiate the subregion with some u64 PID, 32 CID args. > > (thinking about watchdog and dualtimers). If you look at the pl* primecell devices you'll see that's not historically how we've done them. I think it would be more code in each device to instantiate a generic device to handle the ID register section and a container to put it in than just to deal with the IDs by hand... thanks -- PMM
On Tue, Jul 11, 2017 at 6:45 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 11 July 2017 at 16:40, Alistair Francis <alistair23@gmail.com> wrote: >> On Tue, Jul 11, 2017 at 5:33 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 11 July 2017 at 16:12, Alistair Francis <alistair23@gmail.com> wrote: >>>> On Tue, Jul 11, 2017 at 1:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>>> Implement a model of the simple "APB UART" provided in >>>>> the Cortex-M System Design Kit (CMSDK). >>>>> >>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> >>>>> +} CMSDKAPBUART; >>>> >>>> This should be CamelCase. >>> >>> Yes, but CamelCase where all the words are all-uppercase >>> (as with CMSDK, APB and UART) is indistinguishable from >>> all-uppercase. (Compare the way we say "PCIIORegion" rather >>> than "PciIoRegion".) >> >> Good point, I feel like it just looks strange being all caps though. > > Me too, but I didn't have any better naming ideas. > >>>>> +static void uart_update_parameters(CMSDKAPBUART *s) >>>>> +{ >>>>> + QEMUSerialSetParams ssp; >>>>> + >>>>> + /* This UART is always 8N1 but the baud rate is programmable. >>>>> + * The minimum permitted bauddiv setting is 16, so we just ignore >>>>> + * settings below that (usually this means the device has just >>>>> + * been reset and not yet programmed). >>>>> + */ >>>>> + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { >>>>> + return; >>>>> + } >>>> >>>> This seems like it should deserve a guest error print. >>> >>> As the comment says, that would cause us to print a spurious >>> warning every time the device was reset. >> >> I just see this being hard to debug. What about if not printing if >> baud rate is 0 but guest error otherwise? >> >> Or can this not be called from reset? > > The thing is that the guest error really here is "enabled TX > but didn't program the baud rate". So if you want to flag it > up you'd want to do it in uart_write() at the point where > CTRL.TX_EN is written to 1, not in this function. I decided I > didn't really care enough to check and report that (we often > don't bother to track every possible guest mistake). It won't > make any difference to QEMU anyway except in the vanishingly > rare case that the user connects QEMU up to a real serial port. > I very nearly didn't bother to implement the set-params logic > at all (we don't have it on the pl011 and nobody's ever complained). > > I can add the qemu_log on "TX_EN written as 1 and baud_div out > of range" if you think it's helpful though. I think it makes sense to add. It's one line of code and can possibly be very helpful for debugging. > >>>>> +static int uart_can_receive(void *opaque) >>>>> +{ >>>>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>>>> + >>>>> + /* We can take a char if RX is enabled and the buffer is empty */ >>>>> + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { >>>>> + return 1; >>>>> + } >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void uart_receive(void *opaque, const uint8_t *buf, int size) >>>>> +{ >>>>> + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); >>>>> + >>>>> + trace_cmsdk_apb_uart_receive(*buf); >>>>> + >>>>> + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { >>>>> + /* Just drop the character on the floor */ >>>>> + return; >>>> >>>> Doesn't this also deserve a guest error print. >>> >>> It's a can't-happen case because uart_can_receive() won't >>> return 1 if the EN bit is clear. Checking again here is >>> perhaps unnecessary paranoia, but it's not a guest error. >> >> Ah good point. Maybe that is worth stating? > > Mmm, something like: > /* In fact uart_can_receive() ensures that we can't be > * called unless RX is enabled and the buffer is empty, > * but we include this logic as documentation of what the > * hardware does if a character arrives in these circumstances. > */ > > (Or we could delete this and the RXFULL check below it > entirely.) I like the comment, it makes sure it is really clear. Thanks, Alistair > > thanks > -- PMM
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs index 55fcb68..1bcd37e 100644 --- a/hw/char/Makefile.objs +++ b/hw/char/Makefile.objs @@ -19,6 +19,7 @@ obj-$(CONFIG_DIGIC) += digic-uart.o obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o obj-$(CONFIG_RASPI) += bcm2835_aux.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 common-obj-$(CONFIG_GRLIB) += grlib_apbuart.o diff --git a/include/hw/char/cmsdk-apb-uart.h b/include/hw/char/cmsdk-apb-uart.h new file mode 100644 index 0000000..c41fba9 --- /dev/null +++ b/include/hw/char/cmsdk-apb-uart.h @@ -0,0 +1,78 @@ +/* + * ARM CMSDK APB UART emulation + * + * Copyright (c) 2017 Linaro Limited + * Written by Peter Maydell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or + * (at your option) any later version. + */ + +#ifndef CMSDK_APB_UART_H +#define CMSDK_APB_UART_H + +#include "hw/sysbus.h" +#include "chardev/char-fe.h" + +#define TYPE_CMSDK_APB_UART "cmsdk-apb-uart" +#define CMSDK_APB_UART(obj) OBJECT_CHECK(CMSDKAPBUART, (obj), \ + TYPE_CMSDK_APB_UART) + +typedef struct { + /*< private >*/ + SysBusDevice parent_obj; + + /*< public >*/ + MemoryRegion iomem; + CharBackend chr; + qemu_irq txint; + qemu_irq rxint; + qemu_irq txovrint; + qemu_irq rxovrint; + qemu_irq uartint; + guint watch_tag; + uint32_t pclk_frq; + + uint32_t state; + uint32_t ctrl; + uint32_t intstatus; + uint32_t bauddiv; + /* This UART has no FIFO, only a 1-character buffer for each of Tx and Rx */ + uint8_t txbuf; + uint8_t rxbuf; +} CMSDKAPBUART; + +/** + * cmsdk_apb_uart_create - convenience function to create TYPE_CMSDK_APB_UART + * @addr: location in system memory to map registers + * @chr: Chardev backend to connect UART to, or NULL if no backend + * @pclk_frq: frequency in Hz of the PCLK clock (used for calculating baud rate) + */ +static inline DeviceState *cmsdk_apb_uart_create(hwaddr addr, + qemu_irq txint, + qemu_irq rxint, + qemu_irq txovrint, + qemu_irq rxovrint, + qemu_irq uartint, + Chardev *chr, + uint32_t pclk_frq) +{ + DeviceState *dev; + SysBusDevice *s; + + dev = qdev_create(NULL, TYPE_CMSDK_APB_UART); + s = SYS_BUS_DEVICE(dev); + qdev_prop_set_chr(dev, "chardev", chr); + qdev_prop_set_uint32(dev, "pclk-frq", pclk_frq); + qdev_init_nofail(dev); + sysbus_mmio_map(s, 0, addr); + sysbus_connect_irq(s, 0, txint); + sysbus_connect_irq(s, 1, rxint); + sysbus_connect_irq(s, 2, txovrint); + sysbus_connect_irq(s, 3, rxovrint); + sysbus_connect_irq(s, 4, uartint); + return dev; +} + +#endif diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c new file mode 100644 index 0000000..a2f55db --- /dev/null +++ b/hw/char/cmsdk-apb-uart.c @@ -0,0 +1,390 @@ +/* + * ARM CMSDK APB UART emulation + * + * Copyright (c) 2017 Linaro Limited + * Written by Peter Maydell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or + * (at your option) any later version. + */ + +/* This is a model of the "APB UART" which is part of the Cortex-M + * System Design Kit (CMSDK) and documented in the Cortex-M System + * Design Kit Technical Reference Manual (ARM DDI0479C): + * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qapi/error.h" +#include "trace.h" +#include "hw/sysbus.h" +#include "hw/registerfields.h" +#include "chardev/char-fe.h" +#include "chardev/char-serial.h" +#include "hw/char/cmsdk-apb-uart.h" + +REG32(DATA, 0) +REG32(STATE, 4) + FIELD(STATE, TXFULL, 0, 1) + FIELD(STATE, RXFULL, 1, 1) + FIELD(STATE, TXOVERRUN, 2, 1) + FIELD(STATE, RXOVERRUN, 3, 1) +REG32(CTRL, 8) + FIELD(CTRL, TX_EN, 0, 1) + FIELD(CTRL, RX_EN, 1, 1) + FIELD(CTRL, TX_INTEN, 2, 1) + FIELD(CTRL, RX_INTEN, 3, 1) + FIELD(CTRL, TXO_INTEN, 4, 1) + FIELD(CTRL, RXO_INTEN, 5, 1) + FIELD(CTRL, HSTEST, 6, 1) +REG32(INTSTATUS, 0xc) + FIELD(INTSTATUS, TX, 0, 1) + FIELD(INTSTATUS, RX, 1, 1) + FIELD(INTSTATUS, TXO, 2, 1) + FIELD(INTSTATUS, RXO, 3, 1) +REG32(BAUDDIV, 0x10) +REG32(PID4, 0xFD0) +REG32(PID5, 0xFD4) +REG32(PID6, 0xFD8) +REG32(PID7, 0xFDC) +REG32(PID0, 0xFE0) +REG32(PID1, 0xFE4) +REG32(PID2, 0xFE8) +REG32(PID3, 0xFEC) +REG32(CID0, 0xFF0) +REG32(CID1, 0xFF4) +REG32(CID2, 0xFF8) +REG32(CID3, 0xFFC) + +/* PID/CID values */ +static const int uart_id[] = { + 0x04, 0x00, 0x00, 0x00, /* PID4..PID7 */ + 0x21, 0xb8, 0x1b, 0x00, /* PID0..PID3 */ + 0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */ +}; + +static void uart_update_parameters(CMSDKAPBUART *s) +{ + QEMUSerialSetParams ssp; + + /* This UART is always 8N1 but the baud rate is programmable. + * The minimum permitted bauddiv setting is 16, so we just ignore + * settings below that (usually this means the device has just + * been reset and not yet programmed). + */ + if (s->bauddiv < 16 || s->bauddiv > s->pclk_frq) { + return; + } + + ssp.data_bits = 8; + ssp.parity = 'N'; + ssp.stop_bits = 1; + ssp.speed = s->pclk_frq / s->bauddiv; + qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp); + trace_cmsdk_apb_uart_set_params(ssp.speed); +} + +static void cmsdk_apb_uart_update(CMSDKAPBUART *s) +{ + /* update outbound irqs, including handling the way the rxo and txo + * interrupt status bits are just logical AND of the overrun bit in + * STATE and the overrun interrupt enable bit in CTRL. + */ + uint32_t omask = (R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK); + s->intstatus &= ~omask; + s->intstatus |= (s->state & (s->ctrl >> 2) & omask); + + qemu_set_irq(s->txint, !!(s->intstatus & R_INTSTATUS_TX_MASK)); + qemu_set_irq(s->rxint, !!(s->intstatus & R_INTSTATUS_RX_MASK)); + qemu_set_irq(s->txovrint, !!(s->intstatus & R_INTSTATUS_TXO_MASK)); + qemu_set_irq(s->rxovrint, !!(s->intstatus & R_INTSTATUS_RXO_MASK)); + qemu_set_irq(s->uartint, !!(s->intstatus)); +} + +static int uart_can_receive(void *opaque) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + + /* We can take a char if RX is enabled and the buffer is empty */ + if (s->ctrl & R_CTRL_RX_EN_MASK && !(s->state & R_STATE_RXFULL_MASK)) { + return 1; + } + return 0; +} + +static void uart_receive(void *opaque, const uint8_t *buf, int size) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + + trace_cmsdk_apb_uart_receive(*buf); + + if (!(s->ctrl & R_CTRL_RX_EN_MASK)) { + /* Just drop the character on the floor */ + return; + } + + if (s->state & R_STATE_RXFULL_MASK) { + s->state |= R_STATE_RXOVERRUN_MASK; + } + + s->rxbuf = *buf; + s->state |= R_STATE_RXFULL_MASK; + if (s->ctrl & R_CTRL_RX_INTEN_MASK) { + s->intstatus |= R_INTSTATUS_RX_MASK; + } + cmsdk_apb_uart_update(s); +} + +static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + uint64_t r; + + switch (offset) { + case A_DATA: + r = s->rxbuf; + s->state &= ~R_STATE_RXFULL_MASK; + cmsdk_apb_uart_update(s); + break; + case A_STATE: + r = s->state; + break; + case A_CTRL: + r = s->ctrl; + break; + case A_INTSTATUS: + r = s->intstatus; + break; + case A_BAUDDIV: + r = s->bauddiv; + break; + case A_PID4 ... A_CID3: + r = uart_id[offset / 4 - A_PID4]; + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, + "CMSDK APB UART read: bad offset %x\n", (int) offset); + r = 0; + break; + } + trace_cmsdk_apb_uart_read(offset, r, size); + return r; +} + +/* Try to send tx data, and arrange to be called back later if + * we can't (ie the char backend is busy/blocking). + */ +static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + int ret; + + s->watch_tag = 0; + + if (!(s->ctrl & R_CTRL_TX_EN_MASK) || !(s->state & R_STATE_TXFULL_MASK)) { + return FALSE; + } + + ret = qemu_chr_fe_write(&s->chr, &s->txbuf, 1); + if (ret <= 0) { + s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, + uart_transmit, s); + if (!s->watch_tag) { + /* Most common reason to be here is "no chardev backend": + * just insta-drain the buffer, so the serial output + * goes into a void, rather than blocking the guest. + */ + goto buffer_drained; + } + /* Transmit pending */ + trace_cmsdk_apb_uart_tx_pending(); + return FALSE; + } + +buffer_drained: + /* Character successfully sent */ + trace_cmsdk_apb_uart_tx(s->txbuf); + s->state &= ~R_STATE_TXFULL_MASK; + /* Going from TXFULL set to clear triggers the tx interrupt */ + if (s->ctrl & R_CTRL_TX_INTEN_MASK) { + s->intstatus |= R_INTSTATUS_TX_MASK; + } + cmsdk_apb_uart_update(s); + return FALSE; +} + +static void uart_cancel_transmit(CMSDKAPBUART *s) +{ + if (s->watch_tag) { + g_source_remove(s->watch_tag); + s->watch_tag = 0; + } +} + +static void uart_write(void *opaque, hwaddr offset, uint64_t value, + unsigned size) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + + trace_cmsdk_apb_uart_write(offset, value, size); + + switch (offset) { + case A_DATA: + { + s->txbuf = value; + if (s->state & R_STATE_TXFULL_MASK) { + /* Buffer already full -- note the overrun and let the + * existing pending transmit callback handle the new char. + */ + s->state |= R_STATE_TXOVERRUN_MASK; + cmsdk_apb_uart_update(s); + } else { + s->state |= R_STATE_TXFULL_MASK; + uart_transmit(NULL, G_IO_OUT, s); + } + break; + } + case A_STATE: + /* Bits 0 and 1 are read only; bits 2 and 3 are W1C */ + s->state &= ~(value & 0xc); + cmsdk_apb_uart_update(s); + break; + case A_CTRL: + s->ctrl = value & 0x7f; + cmsdk_apb_uart_update(s); + break; + case A_INTSTATUS: + /* All bits are W1C. Clearing the overrun interrupt bits really + * clears the overrun status bits in the STATE register (which + * is then reflected into the intstatus value by the update function). + */ + s->state &= ~(value & 0xc); + cmsdk_apb_uart_update(s); + break; + case A_BAUDDIV: + s->bauddiv = value & 0xFFFFF; + uart_update_parameters(s); + break; + case A_PID4 ... A_CID3: + qemu_log_mask(LOG_GUEST_ERROR, + "CMSDK APB UART write: write to RO offset 0x%x\n", + (int)offset); + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, + "CMSDK APB UART write: bad offset 0x%x\n", (int) offset); + break; + } +} + +static const MemoryRegionOps uart_ops = { + .read = uart_read, + .write = uart_write, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void cmsdk_apb_uart_reset(DeviceState *dev) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(dev); + + trace_cmsdk_apb_uart_reset(); + uart_cancel_transmit(s); + s->state = 0; + s->ctrl = 0; + s->intstatus = 0; + s->bauddiv = 0; + s->txbuf = 0; + s->rxbuf = 0; +} + +static void cmsdk_apb_uart_init(Object *obj) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + CMSDKAPBUART *s = CMSDK_APB_UART(obj); + + memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000); + sysbus_init_mmio(sbd, &s->iomem); + sysbus_init_irq(sbd, &s->txint); + sysbus_init_irq(sbd, &s->rxint); + sysbus_init_irq(sbd, &s->txovrint); + sysbus_init_irq(sbd, &s->rxovrint); + sysbus_init_irq(sbd, &s->uartint); +} + +static void cmsdk_apb_uart_realize(DeviceState *dev, Error **errp) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(dev); + + if (s->pclk_frq == 0) { + error_setg(errp, "CMSDK APB UART: pclk-frq property must be set"); + return; + } + + /* This UART has no flow control, so we do not need to register + * an event handler to deal with CHR_EVENT_BREAK. + */ + qemu_chr_fe_set_handlers(&s->chr, uart_can_receive, uart_receive, + NULL, s, NULL, true); +} + +static int cmsdk_apb_uart_post_load(void *opaque, int version_id) +{ + CMSDKAPBUART *s = CMSDK_APB_UART(opaque); + + /* If we have a pending character, arrange to resend it. */ + if (s->state & R_STATE_TXFULL_MASK) { + s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, + uart_transmit, s); + } + uart_update_parameters(s); + return 0; +} + +static const VMStateDescription cmsdk_apb_uart_vmstate = { + .name = "cmsdk-apb-uart", + .version_id = 1, + .minimum_version_id = 1, + .post_load = cmsdk_apb_uart_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT32(state, CMSDKAPBUART), + VMSTATE_UINT32(ctrl, CMSDKAPBUART), + VMSTATE_UINT32(intstatus, CMSDKAPBUART), + VMSTATE_UINT32(bauddiv, CMSDKAPBUART), + VMSTATE_UINT8(txbuf, CMSDKAPBUART), + VMSTATE_UINT8(rxbuf, CMSDKAPBUART), + VMSTATE_END_OF_LIST() + } +}; + +static Property cmsdk_apb_uart_properties[] = { + DEFINE_PROP_CHR("chardev", CMSDKAPBUART, chr), + DEFINE_PROP_UINT32("pclk-frq", CMSDKAPBUART, pclk_frq, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void cmsdk_apb_uart_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = cmsdk_apb_uart_realize; + dc->vmsd = &cmsdk_apb_uart_vmstate; + dc->reset = cmsdk_apb_uart_reset; + dc->props = cmsdk_apb_uart_properties; +} + +static const TypeInfo cmsdk_apb_uart_info = { + .name = TYPE_CMSDK_APB_UART, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(CMSDKAPBUART), + .instance_init = cmsdk_apb_uart_init, + .class_init = cmsdk_apb_uart_class_init, +}; + +static void cmsdk_apb_uart_register_types(void) +{ + type_register_static(&cmsdk_apb_uart_info); +} + +type_init(cmsdk_apb_uart_register_types); diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index bfd2a88..d128e9d 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -96,6 +96,8 @@ CONFIG_STM32F2XX_ADC=y CONFIG_STM32F2XX_SPI=y CONFIG_STM32F205_SOC=y +CONFIG_CMSDK_APB_UART=y + CONFIG_VERSATILE_PCI=y CONFIG_VERSATILE_I2C=y diff --git a/hw/char/trace-events b/hw/char/trace-events index 7fd48bb..daf4ee4 100644 --- a/hw/char/trace-events +++ b/hw/char/trace-events @@ -56,3 +56,12 @@ pl011_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x" pl011_can_receive(uint32_t lcr, int read_count, int r) "LCR %08x read_count %d returning %d" pl011_put_fifo(uint32_t c, int read_count) "new char 0x%x read_count now %d" pl011_put_fifo_full(void) "FIFO now full, RXFF set" + +# hw/char/cmsdk_apb_uart.c +cmsdk_apb_uart_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB UART read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" +cmsdk_apb_uart_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB UART write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" +cmsdk_apb_uart_reset(void) "CMSDK APB UART: reset" +cmsdk_apb_uart_receive(uint8_t c) "CMSDK APB UART: got character 0x%x from backend" +cmsdk_apb_uart_tx_pending(void) "CMSDK APB UART: character send to backend pending" +cmsdk_apb_uart_tx(uint8_t c) "CMSDK APB UART: character 0x%x sent to backend" +cmsdk_apb_uart_set_params(int speed) "CMSDK APB UART: params set to %d 8N1"
Implement a model of the simple "APB UART" provided in the Cortex-M System Design Kit (CMSDK). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/char/Makefile.objs | 1 + include/hw/char/cmsdk-apb-uart.h | 78 ++++++++ hw/char/cmsdk-apb-uart.c | 390 +++++++++++++++++++++++++++++++++++++++ default-configs/arm-softmmu.mak | 2 + hw/char/trace-events | 9 + 5 files changed, 480 insertions(+) create mode 100644 include/hw/char/cmsdk-apb-uart.h create mode 100644 hw/char/cmsdk-apb-uart.c