Re: [Qemu-devel] [PATCH v6 1/4] cadence_uart: initial version of device model

2012-02-22 Thread Peter Crosthwaite
On Tue, Feb 21, 2012 at 4:58 AM, Peter Maydell  wrote:
> On 20 February 2012 01:45, Peter A. G. Crosthwaite
>  wrote:
>> Implemented cadence UART serial controller
>
> Is there any publicly available documentation for this? Google
> only found a minimum-info datasheet...

No, Silicon vendor still has it under NDA. We are pushing for the release.

>
>> Signed-off-by: Peter A. G. Crosthwaite 
>> Signed-off-by: John Linn 
>> ---
>> ---
>> 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 000..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 .
>> + */
>> +
>> +#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.

OK

> In fact since this is used exactly once in the whole file
> I think I'd just drop it completely.
>

Keeps it consistent with how I handle it in other device models

> (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     0x0001
>> +#define UART_INTR_REMPTY    0x0002
>> +#define UART_INTR_RFUL      0x0004
>> +#define UART_INTR_TEMPTY    0x0008
>> +#define UART_INTR_TFUL      0x0010
>> +#define UART_INTR_ROVR      0x0020
>> +#define UART_INTR_FRAME     0x0040
>> +#define UART_INTR_PARE      0x0080
>> +#define UART_INTR_TIMEOUT   0x0100
>> +#define UART_INTR_DMSI      0x0200
>> +#define UART_INTR_TTRIG     0x0400
>> +#define UART_INTR_TNFUL     0x0800
>> +#define UART_INTR_TOVR      0x1000
>> +
>> +#define UART_CSR_RTRIG      0x0001
>> +#define UART_CSR_REMPTY     0x0002
>> +#define UART_CSR_RFUL       0x0004
>> +#define UART_CSR_TEMPTY     0x0008
>> +#define UART_CSR_TFUL       0x0010
>> +#define UART_CSR_ROVR       0x0020
>> +#define UART_CSR_FRAME      0x0040
>> +#define UART_CSR_PARE       0x0080
>> +#define UART_CSR_TIMEOUT    0x0100
>> +#define UART_CSR_DMSI       0x0200
>> +#define UART_CSR_RACTIVE    0x0400
>> +#define UART_CSR_TACTIVE    0x0800
>> +#define UART_CSR_FDELT      0x1000
>> +#define UART_CSR_TTRIG      0x2000
>> +#define UART_CSR_TNFUL      0x4000
>> +
>> +#define UART_CR_STOPBRK     0x0100
>> +#define UART_CR_STARTBRK    0x0080
>> +#define UART_CR_RST_TO      0x0040
>> +#define UART_CR_TX_DIS      0x0020
>> +#define UART_CR_TX_EN       0x0010
>> +#define UART_CR_RX_DIS      0x0008
>> +#define UART_CR_RX_EN       0x0004
>> +#define UART_CR_TXRST       0x0002
>> +#define UART_CR_RXRST       0x0001
>> +
>> +#define UART_MR_CLKS            0x0001
>> +#define UART_MR_CHRL            0x0006
>> +#define UART_MR_PAR             0x0038
>> +#define UART_MR_NBSTOP          0x00C0
>> +#define UART_MR_CHMODE      

Re: [Qemu-devel] [PATCH v6 1/4] cadence_uart: initial version of device model

2012-02-20 Thread Peter Maydell
On 20 February 2012 01:45, Peter A. G. Crosthwaite
 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 
> Signed-off-by: John Linn 
> ---
> ---
> 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 000..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 .
> + */
> +
> +#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     0x0001
> +#define UART_INTR_REMPTY    0x0002
> +#define UART_INTR_RFUL      0x0004
> +#define UART_INTR_TEMPTY    0x0008
> +#define UART_INTR_TFUL      0x0010
> +#define UART_INTR_ROVR      0x0020
> +#define UART_INTR_FRAME     0x0040
> +#define UART_INTR_PARE      0x0080
> +#define UART_INTR_TIMEOUT   0x0100
> +#define UART_INTR_DMSI      0x0200
> +#define UART_INTR_TTRIG     0x0400
> +#define UART_INTR_TNFUL     0x0800
> +#define UART_INTR_TOVR      0x1000
> +
> +#define UART_CSR_RTRIG      0x0001
> +#define UART_CSR_REMPTY     0x0002
> +#define UART_CSR_RFUL       0x0004
> +#define UART_CSR_TEMPTY     0x0008
> +#define UART_CSR_TFUL       0x0010
> +#define UART_CSR_ROVR       0x0020
> +#define UART_CSR_FRAME      0x0040
> +#define UART_CSR_PARE       0x0080
> +#define UART_CSR_TIMEOUT    0x0100
> +#define UART_CSR_DMSI       0x0200
> +#define UART_CSR_RACTIVE    0x0400
> +#define UART_CSR_TACTIVE    0x0800
> +#define UART_CSR_FDELT      0x1000
> +#define UART_CSR_TTRIG      0x2000
> +#define UART_CSR_TNFUL      0x4000
> +
> +#define UART_CR_STOPBRK     0x0100
> +#define UART_CR_STARTBRK    0x0080
> +#define UART_CR_RST_TO      0x0040
> +#define UART_CR_TX_DIS      0x0020
> +#define UART_CR_TX_EN       0x0010
> +#define UART_CR_RX_DIS      0x0008
> +#define UART_CR_RX_EN       0x0004
> +#define UART_CR_TXRST       0x0002
> +#define UART_CR_RXRST       0x0001
> +
> +#define UART_MR_CLKS            0x0001
> +#define UART_MR_CHRL            0x0006
> +#define UART_MR_PAR             0x0038
> +#define UART_MR_NBSTOP          0x00C0
> +#define UART_MR_CHMODE          0x0300
> +#define UART_MR_UCLKEN          0x0400
> +#define UART_MR_IRMODE          0x0800
> +
> +#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