From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46276) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abvbt-0000Ar-Lp for qemu-devel@nongnu.org; Fri, 04 Mar 2016 14:42:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abvbr-00084X-2j for qemu-devel@nongnu.org; Fri, 04 Mar 2016 14:42:01 -0500 Received: from mail-ob0-x243.google.com ([2607:f8b0:4003:c01::243]:34394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abvbq-00084T-Qv for qemu-devel@nongnu.org; Fri, 04 Mar 2016 14:41:59 -0500 Received: by mail-ob0-x243.google.com with SMTP id wz1so4491303obc.1 for ; Fri, 04 Mar 2016 11:41:58 -0800 (PST) MIME-Version: 1.0 Sender: alistair23@gmail.com In-Reply-To: <87fuwbk8rw.fsf@linaro.org> References: <16e4188eb659d127c3ddb0cfd7f2d4515d642296.1455055858.git.alistair.francis@xilinx.com> <87fuwbk8rw.fsf@linaro.org> From: Alistair Francis Date: Fri, 4 Mar 2016 11:41:28 -0800 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 09/16] dma: Add Xilinx Zynq devcfg device model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: Edgar Iglesias , Peter Maydell , "qemu-devel@nongnu.org Developers" , Alistair Francis , Peter Crosthwaite , Edgar Iglesias , =?UTF-8?Q?Andreas_F=C3=A4rber?= , =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= On Mon, Feb 29, 2016 at 4:20 AM, Alex Benn=C3=A9e = wrote: > > Alistair Francis writes: > >> From: Peter Crosthwaite >> >> Minimal device model for devcfg module of Zynq. DMA capabilities and >> interrupt generation supported. >> >> Signed-off-by: Peter Crosthwaite >> Signed-off-by: Alistair Francis >> --- >> >> default-configs/arm-softmmu.mak | 1 + >> hw/dma/Makefile.objs | 1 + >> hw/dma/xlnx-zynq-devcfg.c | 397 +++++++++++++++++++++++++++++++= +++++++ >> include/hw/dma/xlnx-zynq-devcfg.h | 62 ++++++ >> 4 files changed, 461 insertions(+) >> create mode 100644 hw/dma/xlnx-zynq-devcfg.c >> create mode 100644 include/hw/dma/xlnx-zynq-devcfg.h >> >> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softm= mu.mak >> index a9f82a1..d524584 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -66,6 +66,7 @@ CONFIG_PXA2XX=3Dy >> CONFIG_BITBANG_I2C=3Dy >> CONFIG_FRAMEBUFFER=3Dy >> CONFIG_XILINX_SPIPS=3Dy >> +CONFIG_ZYNQ_DEVCFG=3Dy >> >> CONFIG_ARM11SCU=3Dy >> CONFIG_A9SCU=3Dy >> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs >> index 0e65ed0..eaf0a81 100644 >> --- a/hw/dma/Makefile.objs >> +++ b/hw/dma/Makefile.objs >> @@ -5,6 +5,7 @@ common-obj-$(CONFIG_PL330) +=3D pl330.o >> common-obj-$(CONFIG_I82374) +=3D i82374.o >> common-obj-$(CONFIG_I8257) +=3D i8257.o >> common-obj-$(CONFIG_XILINX_AXI) +=3D xilinx_axidma.o >> +common-obj-$(CONFIG_ZYNQ_DEVCFG) +=3D xlnx-zynq-devcfg.o >> common-obj-$(CONFIG_ETRAXFS) +=3D etraxfs_dma.o >> common-obj-$(CONFIG_STP2000) +=3D sparc32_dma.o >> common-obj-$(CONFIG_SUN4M) +=3D sun4m_iommu.o >> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c >> new file mode 100644 >> index 0000000..0420123 >> --- /dev/null >> +++ b/hw/dma/xlnx-zynq-devcfg.c >> @@ -0,0 +1,397 @@ >> +/* >> + * QEMU model of the Xilinx Zynq Devcfg Interface >> + * >> + * (C) 2011 PetaLogix Pty Ltd >> + * (C) 2014 Xilinx Inc. >> + * Written by Peter Crosthwaite >> + * >> + * Permission is hereby granted, free of charge, to any person obtainin= g a copy >> + * of this software and associated documentation files (the "Software")= , to deal >> + * in the Software without restriction, including without limitation th= e rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or= sell >> + * copies of the Software, and to permit persons to whom the Software i= s >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be inclu= ded in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPR= ESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABIL= ITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR= OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARIS= ING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALIN= GS IN >> + * THE SOFTWARE. >> + */ >> + >> +#include "hw/dma/xlnx-zynq-devcfg.h" >> +#include "qemu/bitops.h" >> +#include "sysemu/sysemu.h" >> +#include "sysemu/dma.h" >> + >> +#define FREQ_HZ 900000000 >> + >> +#define BTT_MAX 0x400 >> + >> +#ifndef XLNX_ZYNQ_DEVCFG_ERR_DEBUG >> +#define XLNX_ZYNQ_DEVCFG_ERR_DEBUG 0 >> +#endif >> + >> +#define DB_PRINT(...) do { \ >> + if (XLNX_ZYNQ_DEVCFG_ERR_DEBUG) { \ >> + qemu_log("%s: ", __func__); \ >> + qemu_log(__VA_ARGS__); \ >> + } \ >> +} while (0); > > This can be done in one qemu_log invocation as per other patches. Fixed. > >> + >> +REG32(CTRL, 0x00) >> + FIELD(CTRL, FORCE_RST, 31, 1) /* Not supported, wr ig= nored */ >> + FIELD(CTRL, PCAP_PR, 27, 1) /* Forced to 0 on bad u= nlock */ >> + FIELD(CTRL, PCAP_MODE, 26, 1) >> + FIELD(CTRL, MULTIBOOT_EN, 24, 1) >> + FIELD(CTRL, USER_MODE, 15, 1) >> + FIELD(CTRL, PCFG_AES_FUSE, 12, 1) >> + FIELD(CTRL, PCFG_AES_EN, 9, 3) >> + FIELD(CTRL, SEU_EN, 8, 1) >> + FIELD(CTRL, SEC_EN, 7, 1) >> + FIELD(CTRL, SPNIDEN, 6, 1) >> + FIELD(CTRL, SPIDEN, 5, 1) >> + FIELD(CTRL, NIDEN, 4, 1) >> + FIELD(CTRL, DBGEN, 3, 1) >> + FIELD(CTRL, DAP_EN, 0, 3) >> + >> +REG32(LOCK, 0x04) >> + #define AES_FUSE_LOCK 4 >> + #define AES_EN_LOCK 3 >> + #define SEU_LOCK 2 >> + #define SEC_LOCK 1 >> + #define DBG_LOCK 0 >> + >> +/* mapping bits in R_LOCK to what they lock in R_CTRL */ >> +static const uint32_t lock_ctrl_map[] =3D { >> + [AES_FUSE_LOCK] =3D R_CTRL_PCFG_AES_FUSE_MASK, >> + [AES_EN_LOCK] =3D R_CTRL_PCFG_AES_EN_MASK, >> + [SEU_LOCK] =3D R_CTRL_SEU_EN_MASK, >> + [SEC_LOCK] =3D R_CTRL_SEC_EN_MASK, >> + [DBG_LOCK] =3D R_CTRL_SPNIDEN_MASK | R_CTRL_SPIDEN_MASK | >> + R_CTRL_NIDEN_MASK | R_CTRL_DBGEN_MASK | >> + R_CTRL_DAP_EN_MASK, >> +}; >> + >> +REG32(CFG, 0x08) >> + FIELD(CFG, RFIFO_TH, 10, 2) >> + FIELD(CFG, WFIFO_TH, 8, 2) >> + FIELD(CFG, RCLK_EDGE, 7, 1) >> + FIELD(CFG, WCLK_EDGE, 6, 1) >> + FIELD(CFG, DISABLE_SRC_INC, 5, 1) >> + FIELD(CFG, DISABLE_DST_INC, 4, 1) >> +#define R_CFG_RO 0xFFFFF000 > > If this was: > > FIELD(CFG, RO, 12, 20) > > Wouldn't you get: > > R_CFG_RO_MASK for free? That's a good point, although the point of the field macros is for the individual bits in the register, which I don't think this is, which is why it is seperate. The macro isn't used, so I'm just going to remove it. > >> +#define R_CFG_RESET 0x50B >> + >> +REG32(INT_STS, 0x0C) >> + FIELD(INT_STS, PSS_GTS_USR_B, 31, 1) >> + FIELD(INT_STS, PSS_FST_CFG_B, 30, 1) >> + FIELD(INT_STS, PSS_CFG_RESET_B, 27, 1) >> + FIELD(INT_STS, RX_FIFO_OV, 18, 1) >> + FIELD(INT_STS, WR_FIFO_LVL, 17, 1) >> + FIELD(INT_STS, RD_FIFO_LVL, 16, 1) >> + FIELD(INT_STS, DMA_CMD_ERR, 15, 1) >> + FIELD(INT_STS, DMA_Q_OV, 14, 1) >> + FIELD(INT_STS, DMA_DONE, 13, 1) >> + FIELD(INT_STS, DMA_P_DONE, 12, 1) >> + FIELD(INT_STS, P2D_LEN_ERR, 11, 1) >> + FIELD(INT_STS, PCFG_DONE, 2, 1) >> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7)= ) >> + >> +REG32(INT_MASK, 0x10) >> + >> +REG32(STATUS, 0x14) >> + FIELD(STATUS, DMA_CMD_Q_F, 31, 1) >> + FIELD(STATUS, DMA_CMD_Q_E, 30, 1) >> + FIELD(STATUS, DMA_DONE_CNT, 28, 2) >> + FIELD(STATUS, RX_FIFO_LVL, 20, 5) >> + FIELD(STATUS, TX_FIFO_LVL, 12, 7) >> + FIELD(STATUS, PSS_GTS_USR_B, 11, 1) >> + FIELD(STATUS, PSS_FST_CFG_B, 10, 1) >> + FIELD(STATUS, PSS_CFG_RESET_B, 5, 1) >> + >> +REG32(DMA_SRC_ADDR, 0x18) >> +REG32(DMA_DST_ADDR, 0x1C) >> +REG32(DMA_SRC_LEN, 0x20) >> +REG32(DMA_DST_LEN, 0x24) >> +REG32(ROM_SHADOW, 0x28) >> +REG32(SW_ID, 0x30) >> +REG32(UNLOCK, 0x34) >> + >> +#define R_UNLOCK_MAGIC 0x757BDF0D >> + >> +REG32(MCTRL, 0x80) >> + FIELD(MCTRL, PS_VERSION, 28, 4) >> + FIELD(MCTRL, PCFG_POR_B, 8, 1) >> + FIELD(MCTRL, INT_PCAP_LPBK, 4, 1) >> + FIELD(MCTRL, QEMU, 3, 1) >> + >> +static void xlnx_zynq_devcfg_update_ixr(XlnxZynqDevcfg *s) >> +{ >> + qemu_set_irq(s->irq, ~s->regs[R_INT_MASK] & s->regs[R_INT_STS]); > > What defines R_INT_MASK? It looks as though only FIELD() definitions > give R_reg_field_MASK defines? This is defined by the REG32() macro. So the line with: 'REG32(INT_MASK, 0x10)' defines this > > This is the problem with the "auto-magic" derived names. If I grep the > source code for R_INT_MASK I only find the use. Actually now I realise > that INT_MASK is a register name not a field. I think we need access > helpers to guide the user. ~s->regs[GET_REG_NUM(INT_MASK)] would prompt > the user to search for INT_MASK to find the original definitions. Hmm, that is a good idea, the problem I see is that the lines of code will end up very long. Is that something that we want? > >> +} >> + >> +static void xlnx_zynq_devcfg_reset(DeviceState *dev) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(dev); >> + int i; >> + >> + for (i =3D 0; i < XLNX_ZYNQ_DEVCFG_R_MAX; ++i) { >> + register_reset(&s->regs_info[i]); >> + } >> +} >> + >> +static void xlnx_zynq_devcfg_dma_go(XlnxZynqDevcfg *s) >> +{ >> + for (;;) { >> + uint8_t buf[BTT_MAX]; >> + XlnxZynqDevcfgDMACmd *dmah =3D s->dma_cmd_fifo; >> + uint32_t btt =3D BTT_MAX; >> + bool loopback =3D s->regs[R_MCTRL] & R_MCTRL_INT_PCAP_LPBK_MASK= ; >> + >> + btt =3D MIN(btt, dmah->src_len); >> + if (loopback) { >> + btt =3D MIN(btt, dmah->dest_len); >> + } >> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr); >> + dma_memory_read(&address_space_memory, dmah->src_addr, buf, btt= ); >> + dmah->src_len -=3D btt; >> + dmah->src_addr +=3D btt; >> + if (loopback) { >> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr= ); >> + dma_memory_write(&address_space_memory, dmah->dest_addr, bu= f, btt); >> + dmah->dest_len -=3D btt; >> + dmah->dest_addr +=3D btt; >> + } >> + if (!dmah->src_len && !dmah->dest_len) { >> + DB_PRINT("dma operation finished\n"); >> + s->regs[R_INT_STS] |=3D R_INT_STS_DMA_DONE_MASK | >> + R_INT_STS_DMA_P_DONE_MASK; >> + s->dma_cmd_fifo_num--; >> + memmove(s->dma_cmd_fifo, &s->dma_cmd_fifo[1], >> + sizeof(*s->dma_cmd_fifo) * XLNX_ZYNQ_DEVCFG_DMA_CMD= _FIFO_LEN >> + = - 1); >> + } >> + xlnx_zynq_devcfg_update_ixr(s); >> + if (!s->dma_cmd_fifo_num) { /* All done */ >> + return; >> + } >> + } > > I noticed this before in other patches I think. I guess it is a > stylistic choice but certainly for a single well defined end condition > do () while {} reads more naturally. I prefer do whiles as well, I have converted this one to a do while. > >> +} >> + >> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + xlnx_zynq_devcfg_update_ixr(s); >> +} >> + >> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(reg->opaque); >> + int i; >> + >> + for (i =3D 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) { >> + if (s->regs[R_LOCK] & 1 << i) { >> + val &=3D ~lock_ctrl_map[i]; >> + val |=3D lock_ctrl_map[i] & s->regs[R_CTRL]; >> + } >> + } >> + return val; >> +} >> + >> +static void r_ctrl_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + uint32_t aes_en =3D F_EX32(val, CTRL, PCFG_AES_EN); >> + >> + if (aes_en !=3D 0 && aes_en !=3D 7) { >> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent= ," >> + "unimplemented security reset should happen!\n", >> + reg->prefix); >> + } >> +} >> + >> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + if (val =3D=3D R_UNLOCK_MAGIC) { >> + DB_PRINT("successful unlock\n"); >> + /* BootROM will have already done the actual unlock so no need = to do >> + * anything in successful subsequent unlock >> + */ >> + } else { /* bad unlock attempt */ >> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->pref= ix); >> + s->regs[R_CTRL] &=3D ~R_CTRL_PCAP_PR_MASK; >> + s->regs[R_CTRL] &=3D ~R_CTRL_PCFG_AES_EN_MASK; >> + /* core becomes inaccessible */ >> + memory_region_set_enabled(&s->iomem, false); >> + } >> +} >> + >> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + /* once bits are locked they stay locked */ >> + return s->regs[R_LOCK] | val; >> +} >> + >> +static void r_dma_dst_len_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + s->dma_cmd_fifo[s->dma_cmd_fifo_num] =3D (XlnxZynqDevcfgDMACmd) { >> + .src_addr =3D s->regs[R_DMA_SRC_ADDR] & ~0x3UL, >> + .dest_addr =3D s->regs[R_DMA_DST_ADDR] & ~0x3UL, >> + .src_len =3D s->regs[R_DMA_SRC_LEN] << 2, >> + .dest_len =3D s->regs[R_DMA_DST_LEN] << 2, >> + }; >> + s->dma_cmd_fifo_num++; >> + DB_PRINT("dma transfer started; %d total transfers pending\n", >> + s->dma_cmd_fifo_num); >> + xlnx_zynq_devcfg_dma_go(s); >> +} >> + >> +static const RegisterAccessInfo xlnx_zynq_devcfg_regs_info[] =3D { >> + { .name =3D "CTRL", .decode.addr =3D A_CTRL, >> + .reset =3D R_CTRL_PCAP_PR_MASK | R_CTRL_PCAP_MODE_MASK | 0x3 <<= 13, >> + .rsvd =3D 0x1 << 28 | 0x3ff << 13 | 0x3 << 13, >> + .pre_write =3D r_ctrl_pre_write, >> + .post_write =3D r_ctrl_post_write, >> + }, >> + { .name =3D "LOCK", .decode.addr =3D A_LOCK, > > The same comment can be mode for the automagic addresses. > GET_REG_ADDR(LOCK) would be clearer. > >> + .rsvd =3D ~ONES(5), >> + .pre_write =3D r_lock_pre_write, >> + }, >> + { .name =3D "CFG", .decode.addr =3D A_CFG, >> + .reset =3D 1 << R_CFG_RFIFO_TH_SHIFT | 1 << R_CFG_WFIFO_TH_SHIF= T | 0x8, >> + .rsvd =3D 0xfffff00f, >> + }, >> + { .name =3D "INT_STS", .decode.addr =3D A_INT_STS, >> + .w1c =3D ~R_INT_STS_RSVD, >> + .reset =3D R_INT_STS_PSS_GTS_USR_B_MASK | >> + R_INT_STS_PSS_CFG_RESET_B_MASK | >> + R_INT_STS_WR_FIFO_LVL_MASK, >> + .rsvd =3D R_INT_STS_RSVD, >> + .post_write =3D r_ixr_post_write, >> + }, >> + { .name =3D "INT_MASK", .decode.addr =3D A_INT_MASK, >> + .reset =3D ~0, >> + .rsvd =3D R_INT_STS_RSVD, >> + .post_write =3D r_ixr_post_write, >> + }, >> + { .name =3D "STATUS", .decode.addr =3D A_STATUS, >> + .reset =3D R_STATUS_DMA_CMD_Q_E_MASK | >> + R_STATUS_PSS_GTS_USR_B_MASK | >> + R_STATUS_PSS_CFG_RESET_B_MASK, >> + .ro =3D ~0, >> + }, >> + { .name =3D "DMA_SRC_ADDR", .decode.addr =3D A_DMA_SRC_AD= DR, }, >> + { .name =3D "DMA_DST_ADDR", .decode.addr =3D A_DMA_DST_AD= DR, }, >> + { .name =3D "DMA_SRC_LEN", .decode.addr =3D A_DMA_SRC_LE= N, >> + .ro =3D ~ONES(27) }, >> + { .name =3D "DMA_DST_LEN", .decode.addr =3D A_DMA_DST_LE= N, >> + .ro =3D ~ONES(27), >> + .post_write =3D r_dma_dst_len_post_write, >> + }, >> + { .name =3D "ROM_SHADOW", .decode.addr =3D A_ROM_SHADOW= , >> + .rsvd =3D ~0ull, >> + }, >> + { .name =3D "SW_ID", .decode.addr =3D A_SW_ID, }, >> + { .name =3D "UNLOCK", .decode.addr =3D A_UNLOCK, >> + .post_write =3D r_unlock_post_write, >> + }, >> + { .name =3D "MCTRL", .decode.addr =3D R_MCTRL * 4, >> + /* Silicon 3.0 for version field, the mysterious reserved bit 23 >> + * and QEMU platform identifier. >> + */ >> + .reset =3D 0x2 << R_MCTRL_PS_VERSION_SHIFT | 1 << 23 | R_MCTRL_Q= EMU_MASK, >> + .ro =3D ~R_MCTRL_INT_PCAP_LPBK_MASK, >> + .rsvd =3D 0x00f00303, >> + }, >> +}; >> + >> +static const MemoryRegionOps xlnx_zynq_devcfg_reg_ops =3D { >> + .read =3D register_read_memory_le, >> + .write =3D register_write_memory_le, >> + .endianness =3D DEVICE_LITTLE_ENDIAN, >> + .valid =3D { >> + .min_access_size =3D 4, >> + .max_access_size =3D 4, >> + } >> +}; >> + >> +static const VMStateDescription vmstate_xlnx_zynq_devcfg_dma_cmd =3D { >> + .name =3D "xlnx_zynq_devcfg_dma_cmd", >> + .version_id =3D 1, >> + .minimum_version_id =3D 1, >> + .minimum_version_id_old =3D 1, >> + .fields =3D (VMStateField[]) { >> + VMSTATE_UINT32(src_addr, XlnxZynqDevcfgDMACmd), >> + VMSTATE_UINT32(dest_addr, XlnxZynqDevcfgDMACmd), >> + VMSTATE_UINT32(src_len, XlnxZynqDevcfgDMACmd), >> + VMSTATE_UINT32(dest_len, XlnxZynqDevcfgDMACmd), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static const VMStateDescription vmstate_xlnx_zynq_devcfg =3D { >> + .name =3D "xlnx_zynq_devcfg", >> + .version_id =3D 1, >> + .minimum_version_id =3D 1, >> + .minimum_version_id_old =3D 1, >> + .fields =3D (VMStateField[]) { >> + VMSTATE_STRUCT_ARRAY(dma_cmd_fifo, XlnxZynqDevcfg, >> + XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN, 0, >> + vmstate_xlnx_zynq_devcfg_dma_cmd, >> + XlnxZynqDevcfgDMACmd), >> + VMSTATE_UINT8(dma_cmd_fifo_num, XlnxZynqDevcfg), >> + VMSTATE_UINT32_ARRAY(regs, XlnxZynqDevcfg, XLNX_ZYNQ_DEVCFG_R_M= AX), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void xlnx_zynq_devcfg_init(Object *obj) >> +{ >> + SysBusDevice *sbd =3D SYS_BUS_DEVICE(obj); >> + XlnxZynqDevcfg *s =3D XLNX_ZYNQ_DEVCFG(obj); >> + >> + sysbus_init_irq(sbd, &s->irq); >> + >> + memory_region_init(&s->iomem, obj, "devcfg", XLNX_ZYNQ_DEVCFG_R_MAX= * 4); >> + register_init_block32(DEVICE(obj), xlnx_zynq_devcfg_regs_info, >> + ARRAY_SIZE(xlnx_zynq_devcfg_regs_info), >> + s->regs_info, s->regs, &s->iomem, >> + &xlnx_zynq_devcfg_reg_ops, >> + XLNX_ZYNQ_DEVCFG_ERR_DEBUG); >> + sysbus_init_mmio(sbd, &s->iomem); >> +} >> + >> +static void xlnx_zynq_devcfg_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc =3D DEVICE_CLASS(klass); >> + >> + dc->reset =3D xlnx_zynq_devcfg_reset; >> + dc->vmsd =3D &vmstate_xlnx_zynq_devcfg; >> +} >> + >> +static const TypeInfo xlnx_zynq_devcfg_info =3D { >> + .name =3D TYPE_XLNX_ZYNQ_DEVCFG, >> + .parent =3D TYPE_SYS_BUS_DEVICE, >> + .instance_size =3D sizeof(XlnxZynqDevcfg), >> + .instance_init =3D xlnx_zynq_devcfg_init, >> + .class_init =3D xlnx_zynq_devcfg_class_init, >> +}; >> + >> +static void xlnx_zynq_devcfg_register_types(void) >> +{ >> + type_register_static(&xlnx_zynq_devcfg_info); >> +} >> + >> +type_init(xlnx_zynq_devcfg_register_types) >> diff --git a/include/hw/dma/xlnx-zynq-devcfg.h b/include/hw/dma/xlnx-zyn= q-devcfg.h >> new file mode 100644 >> index 0000000..d40e5c8 >> --- /dev/null >> +++ b/include/hw/dma/xlnx-zynq-devcfg.h >> @@ -0,0 +1,62 @@ >> +/* >> + * QEMU model of the Xilinx Devcfg Interface >> + * >> + * (C) 2011 PetaLogix Pty Ltd >> + * (C) 2014 Xilinx Inc. >> + * Written by Peter Crosthwaite >> + * >> + * Permission is hereby granted, free of charge, to any person obtainin= g a copy >> + * of this software and associated documentation files (the "Software")= , to deal >> + * in the Software without restriction, including without limitation th= e rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or= sell >> + * copies of the Software, and to permit persons to whom the Software i= s >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be inclu= ded in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPR= ESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABIL= ITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR= OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARIS= ING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALIN= GS IN >> + * THE SOFTWARE. >> + */ >> + >> +#ifndef XLNX_ZYNQ_DEVCFG_H >> + >> +#include "hw/register.h" >> +#include "hw/sysbus.h" >> + >> +#define TYPE_XLNX_ZYNQ_DEVCFG "xlnx.ps7-dev-cfg" >> + >> +#define XLNX_ZYNQ_DEVCFG(obj) \ >> + OBJECT_CHECK(XlnxZynqDevcfg, (obj), TYPE_XLNX_ZYNQ_DEVCFG) >> + >> +#define XLNX_ZYNQ_DEVCFG_R_MAX 0x118 >> + >> +#define XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN 10 >> + >> +typedef struct XlnxZynqDevcfgDMACmd { >> + uint32_t src_addr; >> + uint32_t dest_addr; >> + uint32_t src_len; >> + uint32_t dest_len; >> +} XlnxZynqDevcfgDMACmd; >> + >> +typedef struct XlnxZynqDevcfg { >> + SysBusDevice parent_obj; >> + >> + MemoryRegion iomem; >> + qemu_irq irq; >> + >> + XlnxZynqDevcfgDMACmd dma_cmd_fifo[XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN= ]; >> + uint8_t dma_cmd_fifo_num; >> + >> + uint32_t regs[XLNX_ZYNQ_DEVCFG_R_MAX]; >> + RegisterInfo regs_info[XLNX_ZYNQ_DEVCFG_R_MAX]; >> +} XlnxZynqDevcfg; >> + >> +#define XLNX_ZYNQ_DEVCFG_H >> +#endif > > > Sp having looked through the macro magic I'm happier with what it is > doing. Using accessors will help with the navigation though. The common > question someone looking over the code will want to answer is where is X > defined. I see what you mean, I am just a little worreid that it will result in very long lines and it will be difficult to read. What about just replacing A_ with A() and R_ with R()? Would that work for = you? Thanks, Alistair > > -- > Alex Benn=C3=A9e >