From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36142) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTeQS-0001VX-44 for qemu-devel@nongnu.org; Thu, 24 Nov 2011 13:53:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTeQP-0002YK-9A for qemu-devel@nongnu.org; Thu, 24 Nov 2011 13:53:35 -0500 Received: from mail-qw0-f52.google.com ([209.85.216.52]:63869) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTeQP-0002YE-66 for qemu-devel@nongnu.org; Thu, 24 Nov 2011 13:53:33 -0500 Received: by qadz2 with SMTP id z2so3014425qad.4 for ; Thu, 24 Nov 2011 10:53:32 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: Date: Thu, 24 Nov 2011 18:53:32 +0000 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 1/4] imx.31 and KZM board support: UART support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Chubb Cc: =?UTF-8?Q?Andreas_F=C3=A4rber?= , qemu-devel@nongnu.org On 22 November 2011 04:32, Peter Chubb wrote: > > Implement the FreeScale i.MX UART. =C2=A0This uart is used in a variety o= f > SoCs, including some by Motorola, as well as in the FreeScale i.MX > series. > > Signed-off-by: Hans Jang > Signed-off-by: Adam Clench > Signed-off-by: Peter Chubb > --- > =C2=A0hw/imx_serial.c | =C2=A0307 +++++++++++++++++++++++++++++++++++++++= +++++++++++++++++ > =C2=A01 file changed, 307 insertions(+) > =C2=A0create mode 100644 hw/imx_serial.c > > Index: qemu-working/hw/imx_serial.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- /dev/null =C2=A0 1970-01-01 00:00:00.000000000 +0000 > +++ qemu-working/hw/imx_serial.c =C2=A0 =C2=A0 =C2=A0 =C2=A02011-11-22 14= :47:09.242035743 +1100 > @@ -0,0 +1,307 @@ > +/* > + * IMX31 UARTS > + * > + * Copyright (c) 2008 OKL > + * Written by Hans > + * > + * This work is licensed under the terms of the GNU GPL, version 2. =C2= =A0See > + * the COPYING file in the top-level directory. Really v2 only? (you will need an ack from the copyright holder to say '2 or later' here, though). > + * This is a `bare-bones' implementation of the IMX series serial ports. > + * TODO: > + * =C2=A0-- implement FIFOs. =C2=A0The real hardware has 32 word transmi= t > + * =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 and receive FIFOs I assume readbuff is currently standing in as a single word fifo? Might be worth a comment. (You'll break save/restore compatibility when you add the fifos properly but I don't think we need to worry about that.) > + * =C2=A0-- implement DMA > + * =C2=A0-- implement BAUD-rate and modem lines, for when the backend > + * =C2=A0 =C2=A0 is a real serial device. > + */ > + > +#include "hw.h" > +#include "sysbus.h" > +#include "qemu-char.h" > + > +//#define DEBUG_SERIAL 1 > + > +#ifdef DEBUG_SERIAL > +#define DPRINTF(fmt, args...) \ > +do { printf("imx_serial: " fmt , ##args); } while (0) > +#else > +#define DPRINTF(fmt, args...) do {} while (0) > +#endif > + > +/* > + * Print a message at most ten times. > + */ > +#define scream(fmt, args...) \ > + =C2=A0 =C2=A0do { \ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0static int printable =3D 10;\ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (printable--) { \ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, fmt, ##args); = \ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} \ > + =C2=A0 =C2=A0} while (0) Drop this. > + > + > +typedef struct { > + =C2=A0 =C2=A0SysBusDevice busdev; > + =C2=A0 =C2=A0MemoryRegion iomem; > + =C2=A0 =C2=A0int32_t readbuff; > + > + =C2=A0 =C2=A0uint32_t usr1; > + =C2=A0 =C2=A0uint32_t usr2; > + =C2=A0 =C2=A0uint32_t ucr1; > + =C2=A0 =C2=A0uint32_t uts1; > + > + =C2=A0 =C2=A0uint32_t ubrm; > + =C2=A0 =C2=A0uint32_t ubrc; > + > + =C2=A0 =C2=A0qemu_irq irq; > + =C2=A0 =C2=A0CharDriverState *chr; > +} imx_state; > + > +static const VMStateDescription vmstate_imx_serial =C2=A0=3D { > + =C2=A0 =C2=A0.name =3D "imx-serial", > + =C2=A0 =C2=A0.version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id_old =3D 1, > + =C2=A0 =C2=A0.fields =3D (VMStateField []) { checkpatch complains about this space before the []. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(usr1, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(usr2, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(ucr1, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(uts1, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(ubrm, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(ubrc, imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_END_OF_LIST() This list seems to be missing readbuff. > + =C2=A0 =C2=A0}, > +}; > + > + > +#define URXD_CHARRDY =C2=A0 =C2=A0(1<<15) =C2=A0 /* character read is va= lid */ > + > +#define USR1_TRDY =C2=A0 =C2=A0 =C2=A0 (1<<13) =C2=A0 /* Xmitter ready *= / > +#define USR1_RRDY =C2=A0 =C2=A0 =C2=A0 (1<<9) =C2=A0 =C2=A0/* receiver r= eady */ > + > +#define USR2_TXFE =C2=A0 =C2=A0 =C2=A0 (1<<14) =C2=A0 /* Transmit FIFO e= mpty */ > +#define USR2_RDR =C2=A0 =C2=A0 =C2=A0 =C2=A0(1<<0) =C2=A0 =C2=A0/* Recei= ve data ready */ > +#define USR2_TXDC =C2=A0 =C2=A0 =C2=A0 (1<<3) =C2=A0 =C2=A0/* Transmissi= on complete */ > + > +#define UCR1_UARTEN =C2=A0 =C2=A0 (1<<0) > +#define UCR1_RRDYEN =C2=A0 =C2=A0 (1<<9) > +#define UCR1_TRDYEN =C2=A0 =C2=A0 (1<<13) > +#define UCR1_TXMPTYEN =C2=A0 (1<<6) Is there any significance to the order of these? > +#define UTS1_TXEMPTY =C2=A0 =C2=A0(1<<6) > +#define UTS1_RXEMPTY =C2=A0 =C2=A0(1<<5) > +#define UTS1_TXFULL =C2=A0 =C2=A0 (1<<4) > +#define UTS1_RXFULL =C2=A0 =C2=A0 (1<<3) > + > +static void imx_update(imx_state *s) > +{ > + =C2=A0 =C2=A0uint32_t flags; > + > + =C2=A0 =C2=A0flags =3D ((s->usr1 & s->ucr1)) & (USR1_TRDY|USR1_RRDY); > + =C2=A0 =C2=A0if (!(s->ucr1 & UCR1_TXMPTYEN)) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0flags &=3D ~USR1_TRDY; > + =C2=A0 =C2=A0} > + > + =C2=A0 =C2=A0qemu_set_irq(s->irq, !!flags); > +} > + > +static void imx_serial_reset(imx_state *s) > +{ > + =C2=A0 =C2=A0s->usr1 =3D USR1_TRDY; > + =C2=A0 =C2=A0s->usr2 =3D USR2_TXFE | USR2_TXDC; > + =C2=A0 =C2=A0s->uts1 =3D UTS1_RXEMPTY; > + =C2=A0 =C2=A0s->ubrm =3D 0; > + =C2=A0 =C2=A0s->ubrc =3D 0; > + =C2=A0 =C2=A0s->readbuff =3D 0; > +} > + > +static uint64_t imx_serial_read(void *opaque, target_phys_addr_t offset, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned size) > +{ > + =C2=A0 =C2=A0imx_state *s =3D (imx_state *)opaque; > + =C2=A0 =C2=A0uint32_t c; > + > + =C2=A0 =C2=A0DPRINTF("read(offset=3D%x)\n", offset >> 2); > + =C2=A0 =C2=A0switch (offset >> 2) { > + =C2=A0 =C2=A0case 0x0: /* URXD */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0c =3D s->readbuff; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->usr1 &=3D ~USR1_RRDY; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->usr2 &=3D ~USR2_RDR; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->uts1 |=3D UTS1_RXEMPTY; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_chr_accept_input(s->chr); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return c | URXD_CHARRDY; > + > + =C2=A0 =C2=A0case 0x20: /* UCR1 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->ucr1; > + > + =C2=A0 =C2=A0case 0x21: /* UCR2 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 1; /* reset complete */ > + > + =C2=A0 =C2=A0case 0x25: /* USR1 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->usr1; Why do we need to call imx_update() in the read function? Surely it should be called only where we update the registers which affect interrupt status? > + > + =C2=A0 =C2=A0case 0x26: /* USR2 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->usr2; > + > + Stray blank line. (There are a few of these in the file.) > + =C2=A0 =C2=A0case 0x2A: /* BRM Modulator */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->ubrm; > + > + =C2=A0 =C2=A0case 0x2B: /* Baud Rate Count */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->ubrc; > + > + =C2=A0 =C2=A0case 0x2d: /* UTS1 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return s->uts1; > + > + > + =C2=A0 =C2=A0case 0x22: /* UCR3 */ > + =C2=A0 =C2=A0case 0x23: /* UCR4 */ > + =C2=A0 =C2=A0case 0x24: /* UFCR */ > + =C2=A0 =C2=A0case 0x29: /* BRM Incremental */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0x0; /* TODO */ > + > + =C2=A0 =C2=A0default: > + =C2=A0 =C2=A0 =C2=A0 =C2=A0scream("imx_serial_read: bad offset: 0x%x\n"= , (int)offset); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0; > + =C2=A0 =C2=A0} > +} > + > + > +static void imx_serial_write(void *opaque, target_phys_addr_t offset, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0uint64_t value, unsigned size) > +{ > + =C2=A0 =C2=A0imx_state *s =3D (imx_state *)opaque; > + =C2=A0 =C2=A0unsigned char ch; > + > + =C2=A0 =C2=A0DPRINTF("write(offset=3D%x, value =3D %x)\n", offset >> 2,= (unsigned int)value); > + =C2=A0 =C2=A0switch (offset >> 2) { > + =C2=A0 =C2=A0case 0x10: /* UTXD */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0ch =3D value; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->chr) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_chr_fe_write(s->chr, &ch,= 1); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->usr1 &=3D ~USR1_TRDY; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->usr1 |=3D USR1_TRDY; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0case 0x20: /* UCR1 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->ucr1 =3D value; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0DPRINTF("write(ucr1=3D%x)\n", (unsigned int)= value); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_update(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0case 0x21: /* UCR2 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!(value & 1)) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_serial_reset(s); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0case 0x26: /* USR2 */ > + =C2=A0 =C2=A0 =C2=A0 /* > + =C2=A0 =C2=A0 =C2=A0 =C2=A0* Writing 1 to some bits clears them; all ot= her > + =C2=A0 =C2=A0 =C2=A0 =C2=A0* values are ignored > + =C2=A0 =C2=A0 =C2=A0 =C2=A0*/ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0value &=3D (1<<15) | (1<<13) | (1<<12) | (1<= <11) | (1<<10)| > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(1<<8) | (1<<7) | (1<<6) | (1<= <4) | (1<<2) | (1<<1); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->usr2 &=3D ~value; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Linux expects to see what it writes here.= */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* We don't currently alter the baud rate */ > + =C2=A0 =C2=A0case 0x29: /* UBIR */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->ubrc =3D value; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0case 0x2a: /* UBRM */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->ubrm =3D value; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0case 0x2d: /* UTS1 */ > + =C2=A0 =C2=A0case 0x22: /* UCR3 */ > + =C2=A0 =C2=A0case 0x23: /* UCR4 */ > + =C2=A0 =C2=A0case 0x24: /* UFCR */ > + =C2=A0 =C2=A0case 0x25: /* USR1 */ > + =C2=A0 =C2=A0case 0x2c: /* BIPR1 */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* TODO */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + > + =C2=A0 =C2=A0default: > + =C2=A0 =C2=A0 =C2=A0 =C2=A0scream("imx_serial_write: Bad offset 0x%x\n"= , (int)offset); > + =C2=A0 =C2=A0} > +} > + > +static int imx_can_receive(void *opaque) > +{ > + =C2=A0 =C2=A0imx_state *s =3D (imx_state *)opaque; > + =C2=A0 =C2=A0return !(s->usr1 & USR1_RRDY); > +} > + > +static void imx_put_data(void *opaque, uint32_t value) > +{ > + =C2=A0 =C2=A0imx_state *s =3D (imx_state *)opaque; > + > + =C2=A0 =C2=A0s->usr1 |=3D USR1_RRDY; > + =C2=A0 =C2=A0s->usr2 |=3D USR2_RDR; > + =C2=A0 =C2=A0s->uts1 &=3D ~UTS1_RXEMPTY; > + =C2=A0 =C2=A0s->readbuff =3D value; > + =C2=A0 =C2=A0imx_update(s); > +} > + > +static void imx_receive(void *opaque, const uint8_t *buf, int size) > +{ > + =C2=A0 =C2=A0imx_put_data(opaque, *buf); > +} > + > +static void imx_event(void *opaque, int event) > +{ > + =C2=A0 =C2=A0if (event =3D=3D CHR_EVENT_BREAK) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_put_data(opaque, 0x400); > + =C2=A0 =C2=A0} > +} > + > + > +static const struct MemoryRegionOps imx_serial_ops =3D { > + =C2=A0 =C2=A0.read =3D imx_serial_read, > + =C2=A0 =C2=A0.write =3D imx_serial_write, > + =C2=A0 =C2=A0.endianness =3D DEVICE_NATIVE_ENDIAN, > +}; > + > +static int imx_serial_init(SysBusDevice *dev) > +{ > + =C2=A0 =C2=A0imx_state *s =3D FROM_SYSBUS(imx_state, dev); > + > + =C2=A0 =C2=A0memory_region_init_io(&s->iomem, &imx_serial_ops, s, "imx-= serial", 0x1000); > + =C2=A0 =C2=A0sysbus_init_mmio_region(dev, &s->iomem); > + =C2=A0 =C2=A0sysbus_init_irq(dev, &s->irq); > + =C2=A0 =C2=A0s->chr =3D qdev_init_chardev(&dev->qdev); > + =C2=A0 =C2=A0imx_serial_reset(s); If you put imx_serial_reset in a SysBusDeviceInfo struct as the .qdev.reset pointer you don't need to call it from your init function. > + =C2=A0 =C2=A0/* > + =C2=A0 =C2=A0 * enable the uart on boot, so messages from the linux dec= ompresser > + =C2=A0 =C2=A0 * are visible > + =C2=A0 =C2=A0 */ > + =C2=A0 =C2=A0s->ucr1 =3D UCR1_UARTEN; Is this matching the hardware's behaviour on cold boot vs warm boot, or is it just a random hack? > + =C2=A0 =C2=A0if (s->chr) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_chr_add_handlers(s->chr, imx_can_receiv= e, imx_receive, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0imx_event, s); > + =C2=A0 =C2=A0} > + =C2=A0 =C2=A0vmstate_register(&dev->qdev, -1, &vmstate_imx_serial, s); Put this in SysBusDeviceInfo .qdev.vmsd, don't call vmstate_register(). > + =C2=A0 =C2=A0return 0; > +} > + > + > +static void imx_serial_register_devices(void) > +{ > + =C2=A0 =C2=A0DPRINTF("imx_serial_register_devices\n"); > + =C2=A0 =C2=A0sysbus_register_dev("imx_serial", sizeof(imx_state), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0imx_serial_init); Use sysbus_register_withprop() so you can pass it a SysBusDeviceInfo. -- PMM