On 20 February 2012 01:45, Peter A. G. Crosthwaite <peter.crosthwa...@petalogix.com> wrote: > Implemented cadence UART serial controller
Is there any publicly available documentation for this? Google only found a minimum-info datasheet... > Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwa...@petalogix.com> > Signed-off-by: John Linn <john.l...@xilinx.com> > --- > --- > changed from v4: > fixed FSF addess > changed device_init -> type_init > changes from v1: > converted register file to array > added vmsd state save/load support > removed read side effects from CISR register > > Makefile.target | 1 + > hw/cadence_uart.c | 559 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 560 insertions(+), 0 deletions(-) > create mode 100644 hw/cadence_uart.c > > diff --git a/Makefile.target b/Makefile.target > index 651500e..868be15 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -343,6 +343,7 @@ endif > obj-arm-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o > obj-arm-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o > pl190.o > obj-arm-y += versatile_pci.o > +obj-arm-y += cadence_uart.o > obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o > obj-arm-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o > obj-arm-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o > diff --git a/hw/cadence_uart.c b/hw/cadence_uart.c > new file mode 100644 > index 0000000..a7e461f > --- /dev/null > +++ b/hw/cadence_uart.c > @@ -0,0 +1,559 @@ > +/* > + * Device model for Cadence UART > + * > + * Copyright (c) 2010 Xilinx Inc. > + * Copyright (c) 2012 Peter A.G. Crosthwaite > (peter.crosthwa...@petalogix.com) > + * Copyright (c) 2012 PetaLogix Pty Ltd. > + * Written by Haibing Ma > + * M.Habib > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include "sysbus.h" > +#include "qemu-char.h" > +#include "qemu-timer.h" > + > +#ifdef CADENCE_UART_ERR_DEBUG > +#define qemu_debug(...) do { \ > + fprintf(stderr, ": %s: ", __func__); \ > + fprintf(stderr, ## __VA_ARGS__); \ > + fflush(stderr); \ > + } while (0); > +#else > + #define qemu_debug(...) > +#endif Don't use lowercase for macros; also qemu_debug() is a bad choice as it's liable to get used by generic code. In fact since this is used exactly once in the whole file I think I'd just drop it completely. (Ideally we'd support "debug register accesses to this device" by letting you interpose a MemoryRegion that traced addresses and values. Sadly not implemented yet ;-)) > +#define UART_INTR_RTRIG 0x00000001 > +#define UART_INTR_REMPTY 0x00000002 > +#define UART_INTR_RFUL 0x00000004 > +#define UART_INTR_TEMPTY 0x00000008 > +#define UART_INTR_TFUL 0x00000010 > +#define UART_INTR_ROVR 0x00000020 > +#define UART_INTR_FRAME 0x00000040 > +#define UART_INTR_PARE 0x00000080 > +#define UART_INTR_TIMEOUT 0x00000100 > +#define UART_INTR_DMSI 0x00000200 > +#define UART_INTR_TTRIG 0x00000400 > +#define UART_INTR_TNFUL 0x00000800 > +#define UART_INTR_TOVR 0x00001000 > + > +#define UART_CSR_RTRIG 0x00000001 > +#define UART_CSR_REMPTY 0x00000002 > +#define UART_CSR_RFUL 0x00000004 > +#define UART_CSR_TEMPTY 0x00000008 > +#define UART_CSR_TFUL 0x00000010 > +#define UART_CSR_ROVR 0x00000020 > +#define UART_CSR_FRAME 0x00000040 > +#define UART_CSR_PARE 0x00000080 > +#define UART_CSR_TIMEOUT 0x00000100 > +#define UART_CSR_DMSI 0x00000200 > +#define UART_CSR_RACTIVE 0x00000400 > +#define UART_CSR_TACTIVE 0x00000800 > +#define UART_CSR_FDELT 0x00001000 > +#define UART_CSR_TTRIG 0x00002000 > +#define UART_CSR_TNFUL 0x00004000 > + > +#define UART_CR_STOPBRK 0x00000100 > +#define UART_CR_STARTBRK 0x00000080 > +#define UART_CR_RST_TO 0x00000040 > +#define UART_CR_TX_DIS 0x00000020 > +#define UART_CR_TX_EN 0x00000010 > +#define UART_CR_RX_DIS 0x00000008 > +#define UART_CR_RX_EN 0x00000004 > +#define UART_CR_TXRST 0x00000002 > +#define UART_CR_RXRST 0x00000001 > + > +#define UART_MR_CLKS 0x00000001 > +#define UART_MR_CHRL 0x00000006 > +#define UART_MR_PAR 0x00000038 > +#define UART_MR_NBSTOP 0x000000C0 > +#define UART_MR_CHMODE 0x00000300 > +#define UART_MR_UCLKEN 0x00000400 > +#define UART_MR_IRMODE 0x00000800 > + > +#define UART_PARITY_ODD 0x001 > +#define UART_PARITY_EVEN 0x000 > +#define UART_DATA_BITS_6 0x003 > +#define UART_DATA_BITS_7 0x002 > +#define UART_STOP_BITS_1 0x003 > +#define UART_STOP_BITS_2 0x002 > +#define RX_FIFO_SIZE 16 > +#define TX_FIFO_SIZE 16 > +#define UARK_INPUT_CLK 50000000 > + > +#define NORMAL_MODE 0 > +#define ECHO_MODE 1 > +#define LOCAL_LOOPBACK 2 > +#define REMOTE_LOOPBACK 3 > + > +#define R_CR (0x00/4) > +#define R_MR (0x04/4) > +#define R_IER (0x08/4) > +#define R_IDR (0x0C/4) > +#define R_IMR (0x10/4) > +#define R_CISR (0x14/4) > +#define R_BRGR (0x18/4) > +#define R_RTOR (0x1C/4) > +#define R_RTRIG (0x20/4) > +#define R_MCR (0x24/4) > +#define R_MSR (0x28/4) > +#define R_CSR (0x2C/4) > +#define R_TX_RX (0x30/4) > +#define R_BDIV (0x34/4) > +#define R_FDEL (0x38/4) > +#define R_PMIN (0x3C/4) > +#define R_PWID (0x40/4) > +#define R_TTRIG (0x44/4) > + > +#define R_MAX (R_TTRIG + 1) > + > +typedef struct { > + SysBusDevice busdev; > + MemoryRegion iomem; > + uint32_t r[R_MAX]; > + uint8_t r_fifo[RX_FIFO_SIZE]; > + uint32_t rx_wpos; > + uint32_t rx_count; > + uint64_t char_tx_time; > + CharDriverState *chr; > + qemu_irq irq; > + struct QEMUTimer *fifo_trigger_handle; > + struct QEMUTimer *tx_time_handle; > +} UartState; > + > +static void uart_update_status(UartState *s) > +{ > + qemu_set_irq(s->irq, !!(s->r[R_IMR] & s->r[R_CISR])); > +} > + > +static void fifo_trigger_update(void *opaque) > +{ > + UartState *s = (UartState *)opaque; > + > + s->r[R_CSR] |= UART_CSR_TIMEOUT; > + s->r[R_CISR] |= UART_INTR_TIMEOUT; > + > + uart_update_status(s); > +} > + > +static void uart_tx_redo(UartState *s) > +{ > + uint64_t new_tx_time = qemu_get_clock_ns(vm_clock); > + > + qemu_mod_timer(s->tx_time_handle, new_tx_time + s->char_tx_time); > + > + s->r[R_CSR] |= UART_CSR_TEMPTY; > + s->r[R_CISR] |= UART_INTR_TEMPTY; > + > + uart_update_status(s); > +} > + > +static void uart_tx_write(void *opaque) > +{ > + UartState *s = (UartState *)opaque; > + > + uart_tx_redo(s); > +} > + > +static void uart_rx_reset(UartState *s) > +{ > + s->rx_wpos = 0; > + s->rx_count = 0; > + > + s->r[R_CSR] |= UART_CSR_REMPTY; > + s->r[R_CSR] &= ~UART_CSR_RFUL; > + s->r[R_CSR] &= ~UART_CSR_ROVR; > + s->r[R_CSR] &= ~UART_CSR_TIMEOUT; > + > + s->r[R_CISR] &= ~UART_INTR_REMPTY; > + s->r[R_CISR] &= ~UART_INTR_RFUL; > + s->r[R_CISR] &= ~UART_INTR_ROVR; > + s->r[R_CISR] &= ~UART_INTR_TIMEOUT; > +} > + > +static void uart_tx_reset(UartState *s) > +{ > + s->r[R_CSR] |= UART_CSR_TEMPTY; > + s->r[R_CSR] &= ~UART_CSR_TFUL; > + > + s->r[R_CISR] &= ~UART_INTR_TEMPTY; > + s->r[R_CISR] &= ~UART_INTR_TFUL; > +} > + > +static void uart_send_breaks(UartState *s) > +{ > + int break_enabled = 1; > + > + qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_BREAK, > + &break_enabled); > +} > + > +static inline uint32_t mask_and_right_justify(uint32_t value, uint32_t mask) > +{ > + while (!mask & 0x1) { > + mask >>= 1; > + value >>= 1; > + } > + return value & mask; > +} This doesn't do what you think it does. (Hint, operator precedence.) I think that plus the fact it's not how we handle this in the rest of qemu is good enough to suggest that you just do this the way other device models do, and shift/mask by a constant amount. > + > +static void uart_parameters_setup(UartState *s) > +{ > + QEMUSerialSetParams ssp; > + unsigned int baud_rate, packet_size; > + > + baud_rate = (s->r[R_MR] & UART_MR_CLKS) ? > + UARK_INPUT_CLK / 8 : UARK_INPUT_CLK; > + > + ssp.speed = baud_rate / (s->r[R_BRGR] * (s->r[R_BDIV] + 1)); > + packet_size = 1; > + > + switch (mask_and_right_justify(s->r[R_MR], UART_MR_PAR)) { > + case UART_PARITY_EVEN: > + ssp.parity = 'E'; > + packet_size++; > + break; > + case UART_PARITY_ODD: > + ssp.parity = 'O'; > + packet_size++; > + break; > + default: > + ssp.parity = 'N'; > + break; > + } > + > + switch (mask_and_right_justify(s->r[R_MR], UART_MR_CHRL)) { > + case UART_DATA_BITS_6: > + ssp.data_bits = 6; > + break; > + case UART_DATA_BITS_7: > + ssp.data_bits = 7; > + break; > + default: > + ssp.data_bits = 8; > + break; > + } > + > + switch (mask_and_right_justify(s->r[R_MR], UART_MR_NBSTOP)) { > + case UART_STOP_BITS_1: > + ssp.stop_bits = 1; > + break; > + default: > + ssp.stop_bits = 2; > + break; > + } > + > + packet_size += ssp.data_bits + ssp.stop_bits; > + s->char_tx_time = (get_ticks_per_sec() / ssp.speed) * packet_size; Stray double space. > + qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp); > +} > + > +static void uart_stop_breaks(UartState *s) > +{ > + int break_enabled = 0; > + qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_BREAK, > + &break_enabled); > +} Why not put this next to uart_send_breaks() ? > + > +static int uart_can_receive(void *opaque) > +{ > + UartState *s = (UartState *)opaque; > + > + return RX_FIFO_SIZE - s->rx_count; > +} > + > +static void uart_ctrl_update(UartState *s, uint32_t value) > +{ When this is called by uart_write() we've already done s->r[offset] = value, but this code seems to assume that s->r[R_CR] and value are the old and new values. Who's right? > + if (value & UART_CR_TXRST) { > + uart_tx_reset(s); > + } > + > + if (value & UART_CR_RXRST) { > + uart_rx_reset(s); > + } > + > + s->r[R_CR] &= ~(UART_CR_TXRST | UART_CR_RXRST); > + > + if ((value & UART_CR_TX_EN) && !(s->r[R_CR] & UART_CR_TX_DIS)) { > + uart_tx_redo(s); > + } > + > + if (value & UART_CR_STARTBRK) { > + if (!(s->r[R_CR] & UART_CR_STOPBRK)) { > + uart_send_breaks(s); > + } > + } > + if (value & UART_CR_STARTBRK) { > + uart_stop_breaks(s); ...if the STARTBRK bit is set we stop sending breaks?? Why does the hardware have separate STARTBRK and STOPBRK bits? What happens if they're both set? > + } > +} > + > +static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size) > +{ > + UartState *s = (UartState *)opaque; > + uint64_t new_rx_time = qemu_get_clock_ns(vm_clock); > + int i; > + > + if ((s->r[R_CR] & UART_CR_RX_DIS) || !(s->r[R_CR] & UART_CR_RX_EN)) { > + return; > + } > + > + s->r[R_CSR] &= ~UART_CSR_REMPTY; > + s->r[R_CISR] &= ~UART_INTR_REMPTY; > + > + if (s->rx_count == RX_FIFO_SIZE) { > + s->r[R_CISR] |= UART_INTR_ROVR; > + s->r[R_CSR] |= UART_CSR_ROVR; > + } else { > + for (i = 0; i < size; i++) { > + s->r_fifo[s->rx_wpos] = buf[i]; > + s->rx_wpos = (s->rx_wpos + 1) % RX_FIFO_SIZE; > + s->rx_count++; > + > + if (s->rx_count == RX_FIFO_SIZE) { > + s->r[R_CSR] |= UART_CSR_RFUL; > + s->r[R_CISR] |= UART_INTR_RFUL; > + break; > + } > + > + if (s->rx_count >= s->r[R_RTRIG]) { > + s->r[R_CISR] |= UART_INTR_RTRIG; > + s->r[R_CSR] |= UART_CSR_RTRIG; > + } > + } > + qemu_mod_timer(s->fifo_trigger_handle, new_rx_time + > + (s->char_tx_time * 4)); > + } > + uart_update_status(s); > +} > + > +static void uart_write_tx_fifo(UartState *s, unsigned char *c) > +{ > + unsigned char ch = *c; > + > + if ((s->r[R_CR] & UART_CR_TX_DIS) || !(s->r[R_CR] & UART_CR_TX_EN)) { > + return; > + } > + > + while (!qemu_chr_fe_write(s->chr, &ch, 1)) { ...why not just pass c? > + } Is looping like this really a good idea? None of the other uarts seem to do it... > +} > + > +static void uart_receive(void *opaque, const uint8_t *buf, int size) > +{ > + UartState *s = (UartState *)opaque; > + uint32_t ch_mode = mask_and_right_justify(s->r[R_MR], UART_MR_CHMODE); > + > + if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) { > + uart_write_rx_fifo(opaque, buf, size); > + } > + if (ch_mode == REMOTE_LOOPBACK || ch_mode == ECHO_MODE) { > + uart_write_tx_fifo(s, (unsigned char *)buf); > + } Is it really correct that in loopback mode we write the whole buffer to the RX fifo but only one byte of it to the TX fifo? Why not make the write_tx_fifo prototype take a const uint8_t* like read_tx_fifo so we don't have to have a cast here? > +} > + > +static void uart_event(void *opaque, int event) > +{ > + UartState *s = (UartState *)opaque; > + uint8_t buf = '\0'; > + > + if (event == CHR_EVENT_BREAK) { > + uart_write_rx_fifo(opaque, &buf, 1); > + } > + > + uart_update_status(s); > +} > + > +static void uart_read_rx_fifo(UartState *s, uint32_t *c) > +{ > + if ((s->r[R_CR] & UART_CR_RX_DIS) || !(s->r[R_CR] & UART_CR_RX_EN)) { > + return; > + } > + > + s->r[R_CSR] &= ~UART_CSR_RFUL; > + s->r[R_CSR] &= ~UART_CSR_ROVR; > + s->r[R_CISR] &= ~UART_INTR_ROVR; > + s->r[R_CISR] &= ~UART_INTR_RFUL; Are you sure this is right? I can't tell without a data sheet, but usually if there's a status register and an interrupt status register then the status register tracks current status but the interrupt status register latches it and is only cleared by explicit interrupt acknowledge. > + > + if (s->rx_count) { > + uint32_t rx_rpos = > + (RX_FIFO_SIZE + s->rx_wpos - s->rx_count) % RX_FIFO_SIZE; > + *c = s->r_fifo[rx_rpos]; > + s->rx_count--; > + > + if (!s->rx_count) { > + s->r[R_CISR] |= UART_INTR_REMPTY; > + s->r[R_CSR] |= UART_CSR_REMPTY; > + } > + } else { > + *c = 0; > + s->r[R_CISR] |= UART_INTR_REMPTY; > + s->r[R_CSR] |= UART_CSR_REMPTY; > + } > + > + if (s->rx_count < s->r[R_RTRIG]) { > + s->r[R_CSR] &= ~UART_CSR_RTRIG; > + s->r[R_CISR] &= ~UART_INTR_RTRIG; > + } > + uart_update_status(s); > +} > + > +static void uart_write(void *opaque, target_phys_addr_t offset, > + uint64_t value, unsigned size) > +{ > + UartState *s = (UartState *)opaque; > + > + qemu_debug(" offset:%x data:%08x\n", offset, (unsigned)value); > + offset >>= 2; > + switch (offset) { > + case R_IER: /* ier (wts imr) */ > + s->r[R_IMR] |= value; > + break; > + case R_IDR: /* idr (wtc imr) */ > + s->r[R_IMR] &= ~value; > + break; > + case R_IMR: /* imr (read only) */ > + break; > + case R_CISR: /* cisr (wtc) */ > + s->r[R_CISR] &= ~value; > + break; > + case R_TX_RX: /* UARTDR */ > + switch (mask_and_right_justify(s->r[R_MR], UART_MR_CHMODE)) { > + case NORMAL_MODE: > + uart_write_tx_fifo(s, (unsigned char *) &value); > + break; > + case LOCAL_LOOPBACK: > + uart_write_rx_fifo(opaque, (unsigned char *) &value, 1); > + break; > + } > + break; > + default: > + s->r[offset] = value; > + } > + > + switch (offset) { > + case R_CR: > + uart_ctrl_update(s, value); > + break; > + case R_MR: > + uart_parameters_setup(s); > + break; > + } > +} As noted earlier, the handling of R_CR is suspect here. That would only leave R_MR in this second switch, so if I were you I'd just fold it all into the first switch (ie set s->[R_MR] by hand rather than trying to reuse the default: case). > + > +static uint64_t uart_read(void *opaque, target_phys_addr_t offset, > + unsigned size) > +{ > + UartState *s = (UartState *)opaque; > + uint32_t c = 0; > + > + offset >>= 2; > + if (offset > R_MAX) { > + return 0; > + } else if (offset == R_TX_RX) { > + uart_read_rx_fifo(s, &c); > + return c; > + } > + return s->r[offset]; > +} > + > +static const MemoryRegionOps uart_ops = { > + .read = uart_read, > + .write = uart_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static int cadence_uart_init(SysBusDevice *dev) > +{ > + UartState *s = FROM_SYSBUS(UartState, dev); > + > + memory_region_init_io(&s->iomem, &uart_ops, s, "uart", 0x1000); > + sysbus_init_mmio(dev, &s->iomem); > + sysbus_init_irq(dev, &s->irq); > + > + s->fifo_trigger_handle = qemu_new_timer_ns(vm_clock, > + (QEMUTimerCB *)fifo_trigger_update, s); > + > + s->tx_time_handle = qemu_new_timer_ns(vm_clock, > + (QEMUTimerCB *)uart_tx_write, s); > + > + s->char_tx_time = (get_ticks_per_sec() / 9600) * 10; > + > + s->chr = qemu_char_get_next_serial(); > + > + s->r[R_CR] = 0x00000128; > + s->r[R_IMR] = 0; > + s->r[R_CISR] = 0; > + s->r[R_RTRIG] = 0x00000020; > + s->r[R_BRGR] = 0x0000000F; > + s->r[R_TTRIG] = 0x00000020; > + > + uart_rx_reset(s); > + uart_tx_reset(s); > + > + s->rx_count = 0; > + s->rx_wpos = 0; This sort of reset code should go in a reset function, not the init function. > + > + if (s->chr) { > + qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive, > + uart_event, s); > + } > + > + return 0; > +} > + > +static int cadence_uart_post_load(void *opaque, int version_id) > +{ > + UartState *s = opaque; > + > + uart_parameters_setup(s); > + uart_update_status(s); > + return 0; > +} > + > +static const VMStateDescription vmstate_cadence_uart = { > + .name = "cadence_uart", > + .version_id = 3, > + .minimum_version_id = 2, > + .minimum_version_id_old = 2, ...who are we trying to be backward compatible with here? > + .post_load = cadence_uart_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32_ARRAY(r, UartState, R_MAX), > + VMSTATE_UINT8_ARRAY(r_fifo, UartState, RX_FIFO_SIZE), > + VMSTATE_UINT32(rx_count, UartState), > + VMSTATE_UINT32(rx_wpos, UartState), > + VMSTATE_TIMER(fifo_trigger_handle, UartState), > + VMSTATE_TIMER(tx_time_handle, UartState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void cadence_uart_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); > + > + sdc->init = cadence_uart_init; > + dc->vmsd = &vmstate_cadence_uart; > +} > + > +static TypeInfo cadence_uart_info = { > + .name = "cadence_uart", > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(UartState), > + .class_init = cadence_uart_class_init, > +}; > + > +static void cadence_uart_register_types(void) > +{ > + type_register_static(&cadence_uart_info); > +} > + > +type_init(cadence_uart_register_types) > -- > 1.7.3.2 -- PMM