Re: [Qemu-devel] [PATCH] hw/misc: Add support for ADC controller in Xilinx Zynq 7000

2015-09-09 Thread Guenter Roeck

Hi Edgar,

On 09/08/2015 11:39 AM, Edgar E. Iglesias wrote:

On Wed, Aug 12, 2015 at 02:33:47PM -0700, Guenter Roeck wrote:

Add support for the Xilinx XADC core used in Zynq 7000.

References:
- Zynq-7000 All Programmable SoC Technical Reference Manual
- 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC
   Dual 12-Bit 1 MSPS Analog-to-Digital Converter

Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree
files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration
multi_v7_defconfig.



Adding Alistair to CC.
A few comments inline.




Signed-off-by: Guenter Roeck 
---
  hw/arm/xilinx_zynq.c  |   5 +
  hw/misc/Makefile.objs |   1 +
  hw/misc/zynq_xadc.c   | 305 ++
  3 files changed, 311 insertions(+)
  create mode 100644 hw/misc/zynq_xadc.c



[ ... ]


+
+static void zynq_xadc_reset(DeviceState *d)
+{
+_zynq_xadc_reset(ZYNQ_XADC(d));


Historically we've tried to avoid symbolnames starting with _.
You could rename or maybe stick to one reset func?


Turns out using one function was easy, after figuring out how to use
the available macros.


+
+switch (reg) {
+case CFG:
+s->regs[CFG] = val;
+break;
+case INTSTS:
+s->regs[INTSTS] &= ~val;
+zynq_xadc_update_ints(s);
+break;
+case INTMSK:
+s->regs[INTMSK] = val & 0x003ff;
+zynq_xadc_update_ints(s);
+break;
+case CFIFO:
+xadc_reg = (val >> 16) & 0x007f;
+xadc_cmd = (val >> 26) & 0x0f;
+xadc_data = val & 0x;


We have extract* functions to make these field extractions a bit more
readable. For example see extract32 in include/qemu/bitops.h.


Done, thanks for the hint.

Should I wait for more feedback or resubmit ?

Thanks,
Guenter




Re: [Qemu-devel] [PATCH] hw/misc: Add support for ADC controller in Xilinx Zynq 7000

2015-09-09 Thread Peter Crosthwaite
On Wed, Aug 12, 2015 at 2:33 PM, Guenter Roeck  wrote:
> Add support for the Xilinx XADC core used in Zynq 7000.
>
> References:
> - Zynq-7000 All Programmable SoC Technical Reference Manual
> - 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC
>   Dual 12-Bit 1 MSPS Analog-to-Digital Converter
>
> Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree
> files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration
> multi_v7_defconfig.
>
> Signed-off-by: Guenter Roeck 
> ---
>  hw/arm/xilinx_zynq.c  |   5 +
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/zynq_xadc.c   | 305 
> ++
>  3 files changed, 311 insertions(+)
>  create mode 100644 hw/misc/zynq_xadc.c
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index a4e7b5c..195e9f1 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -225,6 +225,11 @@ static void zynq_init(MachineState *machine)
>  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
>  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
>
> +dev = qdev_create(NULL, "xilinx,zynq_xadc");

See comment below about type def in header, which will remove this
literal string.

> +qdev_init_nofail(dev);
> +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
> +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
> +
>  dev = qdev_create(NULL, "pl330");
>  qdev_prop_set_uint8(dev, "num_chnls",  8);
>  qdev_prop_set_uint8(dev, "num_periph_req",  4);
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 4aa76ff..5f76f05 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
>  obj-$(CONFIG_OMAP) += omap_tap.o
>  obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> +obj-$(CONFIG_ZYNQ) += zynq_xadc.o
>  obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
>
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> diff --git a/hw/misc/zynq_xadc.c b/hw/misc/zynq_xadc.c
> new file mode 100644
> index 000..480e2bf
> --- /dev/null
> +++ b/hw/misc/zynq_xadc.c
> @@ -0,0 +1,305 @@
> +/*
> + * ADC registers for Xilinx Zynq Platform
> + *
> + * Copyright (c) 2015 Guenter Roeck
> + * Based on hw/misc/zynq_slcr.c, written by Michal Simek
> + *
> + * 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 "hw/hw.h"
> +#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +
> +enum {
> +CFG= 0x000 / 4,
> +INTSTS,
> +INTMSK,
> +STATUS,
> +CFIFO,
> +DFIFO,
> +CTL,
> +};
> +
> +#define XADC_ZYNQ_CFG_ENABLEBIT(31)
> +#define XADC_ZYNQ_CFG_CFIFOTH_RD(x) (((x) >> 20) & 0x0f)
> +#define XADC_ZYNQ_CFG_DFIFOTH_RD(x) (((x) >> 16) & 0x0f)
> +#define XADC_ZYNQ_CFG_WEDGE BIT(13)
> +#define XADC_ZYNQ_CFG_REDGE BIT(12)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV2  (0x0 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV4  (0x1 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV8  (0x2 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV16 (0x3 << 8)
> +#define XADC_ZYNQ_CFG_IGAP_MASK 0x1f
> +#define XADC_ZYNQ_CFG_IGAP(x)   ((x) & XADC_ZYNQ_CFG_IGAP_MASK)
> +
> +#define XADC_ZYNQ_INT_CFIFO_LTH BIT(9)
> +#define XADC_ZYNQ_INT_DFIFO_GTH BIT(8)
> +#define XADC_ZYNQ_INT_ALARM_MASK0xff
> +#define XADC_ZYNQ_INT_ALARM_OFFSET  0
> +
> +#define XADC_ZYNQ_STATUS_DFIFO_LVL(x)   (((x) & 0x0f) << 12)
> +#define XADC_ZYNQ_STATUS_CFIFOF BIT(11)
> +#define XADC_ZYNQ_STATUS_CFIFOE BIT(10)
> +#define XADC_ZYNQ_STATUS_DFIFOF BIT(9)
> +#define XADC_ZYNQ_STATUS_DFIFOE BIT(8)
> +#define XADC_ZYNQ_STATUS_OT BIT(7)
> +#define XADC_ZYNQ_STATUS_ALM(x) BIT(x)
> +
> +#define XADC_ZYNQ_CTL_RESET BIT(4)
> +
> +#define XADC_ZYNQ_CMD_NOP   0x00
> +#define XADC_ZYNQ_CMD_READ  0x01
> +#define XADC_ZYNQ_CMD_WRITE 0x02
> +
> +#define ZYNQ_XADC_MMIO_SIZE 0x0020
> +#define ZYNQ_XADC_NUM_IO_REGS   (ZYNQ_XADC_MMIO_SIZE / 4)
> +#define ZYNQ_XADC_NUM_ADC_REGS  128
> +#define ZYNQ_XADC_FIFO_DEPTH15
> +
> +#define TYPE_ZYNQ_XADC  "xilinx,zynq_xadc"

Should this be "xlnx'? I was trying to standardise on xlnx for new
code, as that is the vendor name for device tree compatible strings.
This should match the compatible string of the device tree.

> +#define ZYNQ_XADC(obj) \
> +OBJECT_CHECK(ZynqXADCState, (obj), TYPE_ZYNQ_XADC)
> +
> +typedef struct ZynqXADCState {

/*< 

Re: [Qemu-devel] [PATCH] hw/misc: Add support for ADC controller in Xilinx Zynq 7000

2015-09-08 Thread Edgar E. Iglesias
On Wed, Aug 12, 2015 at 02:33:47PM -0700, Guenter Roeck wrote:
> Add support for the Xilinx XADC core used in Zynq 7000.
> 
> References:
> - Zynq-7000 All Programmable SoC Technical Reference Manual
> - 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC
>   Dual 12-Bit 1 MSPS Analog-to-Digital Converter
> 
> Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree
> files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration
> multi_v7_defconfig.


Adding Alistair to CC.
A few comments inline.


> 
> Signed-off-by: Guenter Roeck 
> ---
>  hw/arm/xilinx_zynq.c  |   5 +
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/zynq_xadc.c   | 305 
> ++
>  3 files changed, 311 insertions(+)
>  create mode 100644 hw/misc/zynq_xadc.c
> 
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index a4e7b5c..195e9f1 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -225,6 +225,11 @@ static void zynq_init(MachineState *machine)
>  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
>  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
>  
> +dev = qdev_create(NULL, "xilinx,zynq_xadc");
> +qdev_init_nofail(dev);
> +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
> +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
> +
>  dev = qdev_create(NULL, "pl330");
>  qdev_prop_set_uint8(dev, "num_chnls",  8);
>  qdev_prop_set_uint8(dev, "num_periph_req",  4);
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 4aa76ff..5f76f05 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
>  obj-$(CONFIG_OMAP) += omap_tap.o
>  obj-$(CONFIG_SLAVIO) += slavio_misc.o
>  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> +obj-$(CONFIG_ZYNQ) += zynq_xadc.o
>  obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
> diff --git a/hw/misc/zynq_xadc.c b/hw/misc/zynq_xadc.c
> new file mode 100644
> index 000..480e2bf
> --- /dev/null
> +++ b/hw/misc/zynq_xadc.c
> @@ -0,0 +1,305 @@
> +/*
> + * ADC registers for Xilinx Zynq Platform
> + *
> + * Copyright (c) 2015 Guenter Roeck
> + * Based on hw/misc/zynq_slcr.c, written by Michal Simek
> + *
> + * 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 "hw/hw.h"
> +#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +
> +enum {
> +CFG= 0x000 / 4,
> +INTSTS,
> +INTMSK,
> +STATUS,
> +CFIFO,
> +DFIFO,
> +CTL,
> +};
> +
> +#define XADC_ZYNQ_CFG_ENABLEBIT(31)
> +#define XADC_ZYNQ_CFG_CFIFOTH_RD(x) (((x) >> 20) & 0x0f)
> +#define XADC_ZYNQ_CFG_DFIFOTH_RD(x) (((x) >> 16) & 0x0f)
> +#define XADC_ZYNQ_CFG_WEDGE BIT(13)
> +#define XADC_ZYNQ_CFG_REDGE BIT(12)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV2  (0x0 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV4  (0x1 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV8  (0x2 << 8)
> +#define XADC_ZYNQ_CFG_TCKRATE_DIV16 (0x3 << 8)
> +#define XADC_ZYNQ_CFG_IGAP_MASK 0x1f
> +#define XADC_ZYNQ_CFG_IGAP(x)   ((x) & XADC_ZYNQ_CFG_IGAP_MASK)
> +
> +#define XADC_ZYNQ_INT_CFIFO_LTH BIT(9)
> +#define XADC_ZYNQ_INT_DFIFO_GTH BIT(8)
> +#define XADC_ZYNQ_INT_ALARM_MASK0xff
> +#define XADC_ZYNQ_INT_ALARM_OFFSET  0
> +
> +#define XADC_ZYNQ_STATUS_DFIFO_LVL(x)   (((x) & 0x0f) << 12)
> +#define XADC_ZYNQ_STATUS_CFIFOF BIT(11)
> +#define XADC_ZYNQ_STATUS_CFIFOE BIT(10)
> +#define XADC_ZYNQ_STATUS_DFIFOF BIT(9)
> +#define XADC_ZYNQ_STATUS_DFIFOE BIT(8)
> +#define XADC_ZYNQ_STATUS_OT BIT(7)
> +#define XADC_ZYNQ_STATUS_ALM(x) BIT(x)
> +
> +#define XADC_ZYNQ_CTL_RESET BIT(4)
> +
> +#define XADC_ZYNQ_CMD_NOP   0x00
> +#define XADC_ZYNQ_CMD_READ  0x01
> +#define XADC_ZYNQ_CMD_WRITE 0x02
> +
> +#define ZYNQ_XADC_MMIO_SIZE 0x0020
> +#define ZYNQ_XADC_NUM_IO_REGS   (ZYNQ_XADC_MMIO_SIZE / 4)
> +#define ZYNQ_XADC_NUM_ADC_REGS  128
> +#define ZYNQ_XADC_FIFO_DEPTH15
> +
> +#define TYPE_ZYNQ_XADC  "xilinx,zynq_xadc"
> +#define ZYNQ_XADC(obj) \
> +OBJECT_CHECK(ZynqXADCState, (obj), TYPE_ZYNQ_XADC)
> +
> +typedef struct ZynqXADCState {
> +SysBusDevice parent_obj;
> +
> +MemoryRegion iomem;
> +
> +uint32_t regs[ZYNQ_XADC_NUM_IO_REGS];   /* I/O registers */
> +uint16_t xadc_regs[ZYNQ_XADC_NUM_ADC_REGS]; /* XADC registers */
> +uint16_t 

[Qemu-devel] [PATCH] hw/misc: Add support for ADC controller in Xilinx Zynq 7000

2015-08-12 Thread Guenter Roeck
Add support for the Xilinx XADC core used in Zynq 7000.

References:
- Zynq-7000 All Programmable SoC Technical Reference Manual
- 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC
  Dual 12-Bit 1 MSPS Analog-to-Digital Converter

Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree
files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration
multi_v7_defconfig.

Signed-off-by: Guenter Roeck li...@roeck-us.net
---
 hw/arm/xilinx_zynq.c  |   5 +
 hw/misc/Makefile.objs |   1 +
 hw/misc/zynq_xadc.c   | 305 ++
 3 files changed, 311 insertions(+)
 create mode 100644 hw/misc/zynq_xadc.c

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index a4e7b5c..195e9f1 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -225,6 +225,11 @@ static void zynq_init(MachineState *machine)
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
 
+dev = qdev_create(NULL, xilinx,zynq_xadc);
+qdev_init_nofail(dev);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
+
 dev = qdev_create(NULL, pl330);
 qdev_prop_set_uint8(dev, num_chnls,  8);
 qdev_prop_set_uint8(dev, num_periph_req,  4);
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4aa76ff..5f76f05 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
 obj-$(CONFIG_OMAP) += omap_tap.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
+obj-$(CONFIG_ZYNQ) += zynq_xadc.o
 obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
diff --git a/hw/misc/zynq_xadc.c b/hw/misc/zynq_xadc.c
new file mode 100644
index 000..480e2bf
--- /dev/null
+++ b/hw/misc/zynq_xadc.c
@@ -0,0 +1,305 @@
+/*
+ * ADC registers for Xilinx Zynq Platform
+ *
+ * Copyright (c) 2015 Guenter Roeck
+ * Based on hw/misc/zynq_slcr.c, written by Michal Simek
+ *
+ * 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 hw/hw.h
+#include qemu/timer.h
+#include hw/sysbus.h
+#include sysemu/sysemu.h
+
+enum {
+CFG= 0x000 / 4,
+INTSTS,
+INTMSK,
+STATUS,
+CFIFO,
+DFIFO,
+CTL,
+};
+
+#define XADC_ZYNQ_CFG_ENABLEBIT(31)
+#define XADC_ZYNQ_CFG_CFIFOTH_RD(x) (((x)  20)  0x0f)
+#define XADC_ZYNQ_CFG_DFIFOTH_RD(x) (((x)  16)  0x0f)
+#define XADC_ZYNQ_CFG_WEDGE BIT(13)
+#define XADC_ZYNQ_CFG_REDGE BIT(12)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV2  (0x0  8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV4  (0x1  8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV8  (0x2  8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV16 (0x3  8)
+#define XADC_ZYNQ_CFG_IGAP_MASK 0x1f
+#define XADC_ZYNQ_CFG_IGAP(x)   ((x)  XADC_ZYNQ_CFG_IGAP_MASK)
+
+#define XADC_ZYNQ_INT_CFIFO_LTH BIT(9)
+#define XADC_ZYNQ_INT_DFIFO_GTH BIT(8)
+#define XADC_ZYNQ_INT_ALARM_MASK0xff
+#define XADC_ZYNQ_INT_ALARM_OFFSET  0
+
+#define XADC_ZYNQ_STATUS_DFIFO_LVL(x)   (((x)  0x0f)  12)
+#define XADC_ZYNQ_STATUS_CFIFOF BIT(11)
+#define XADC_ZYNQ_STATUS_CFIFOE BIT(10)
+#define XADC_ZYNQ_STATUS_DFIFOF BIT(9)
+#define XADC_ZYNQ_STATUS_DFIFOE BIT(8)
+#define XADC_ZYNQ_STATUS_OT BIT(7)
+#define XADC_ZYNQ_STATUS_ALM(x) BIT(x)
+
+#define XADC_ZYNQ_CTL_RESET BIT(4)
+
+#define XADC_ZYNQ_CMD_NOP   0x00
+#define XADC_ZYNQ_CMD_READ  0x01
+#define XADC_ZYNQ_CMD_WRITE 0x02
+
+#define ZYNQ_XADC_MMIO_SIZE 0x0020
+#define ZYNQ_XADC_NUM_IO_REGS   (ZYNQ_XADC_MMIO_SIZE / 4)
+#define ZYNQ_XADC_NUM_ADC_REGS  128
+#define ZYNQ_XADC_FIFO_DEPTH15
+
+#define TYPE_ZYNQ_XADC  xilinx,zynq_xadc
+#define ZYNQ_XADC(obj) \
+OBJECT_CHECK(ZynqXADCState, (obj), TYPE_ZYNQ_XADC)
+
+typedef struct ZynqXADCState {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+
+uint32_t regs[ZYNQ_XADC_NUM_IO_REGS];   /* I/O registers */
+uint16_t xadc_regs[ZYNQ_XADC_NUM_ADC_REGS]; /* XADC registers */
+uint16_t xadc_read_reg_previous;/* result of most recent read command 
*/
+uint16_t xadc_dfifo[ZYNQ_XADC_FIFO_DEPTH];  /* data fifo */
+uint16_t xadc_dfifo_depth;  /* entries in data fifo */
+
+struct IRQState *irq;
+
+} ZynqXADCState;
+
+static void _zynq_xadc_reset(ZynqXADCState *s)
+{
+int i;
+
+s-regs[CFG] = XADC_ZYNQ_CFG_IGAP(0x14) | XADC_ZYNQ_CFG_TCKRATE_DIV4 |
+XADC_ZYNQ_CFG_REDGE;
+