On Mon, Feb 29, 2016 at 4:20 AM, Alex Bennée <alex.ben...@linaro.org> wrote: > > Alistair Francis <alistair.fran...@xilinx.com> writes: > >> From: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >> >> Minimal device model for devcfg module of Zynq. DMA capabilities and >> interrupt generation supported. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> >> --- >> >> 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-softmmu.mak >> index a9f82a1..d524584 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -66,6 +66,7 @@ CONFIG_PXA2XX=y >> CONFIG_BITBANG_I2C=y >> CONFIG_FRAMEBUFFER=y >> CONFIG_XILINX_SPIPS=y >> +CONFIG_ZYNQ_DEVCFG=y >> >> CONFIG_ARM11SCU=y >> CONFIG_A9SCU=y >> 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) += pl330.o >> common-obj-$(CONFIG_I82374) += i82374.o >> common-obj-$(CONFIG_I8257) += i8257.o >> common-obj-$(CONFIG_XILINX_AXI) += xilinx_axidma.o >> +common-obj-$(CONFIG_ZYNQ_DEVCFG) += xlnx-zynq-devcfg.o >> common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o >> common-obj-$(CONFIG_STP2000) += sparc32_dma.o >> common-obj-$(CONFIG_SUN4M) += 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 <peter.crosthwa...@xilinx.com> >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> copy >> + * of this software and associated documentation files (the "Software"), to >> deal >> + * in the Software without restriction, including without limitation the >> rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included >> in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >> FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 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 >> ignored */ >> + FIELD(CTRL, PCAP_PR, 27, 1) /* Forced to 0 on bad >> unlock */ >> + 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[] = { >> + [AES_FUSE_LOCK] = R_CTRL_PCFG_AES_FUSE_MASK, >> + [AES_EN_LOCK] = R_CTRL_PCFG_AES_EN_MASK, >> + [SEU_LOCK] = R_CTRL_SEU_EN_MASK, >> + [SEC_LOCK] = R_CTRL_SEC_EN_MASK, >> + [DBG_LOCK] = 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 = XLNX_ZYNQ_DEVCFG(dev); >> + int i; >> + >> + for (i = 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 = s->dma_cmd_fifo; >> + uint32_t btt = BTT_MAX; >> + bool loopback = s->regs[R_MCTRL] & R_MCTRL_INT_PCAP_LPBK_MASK; >> + >> + btt = MIN(btt, dmah->src_len); >> + if (loopback) { >> + btt = 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 -= btt; >> + dmah->src_addr += btt; >> + if (loopback) { >> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr); >> + dma_memory_write(&address_space_memory, dmah->dest_addr, buf, >> btt); >> + dmah->dest_len -= btt; >> + dmah->dest_addr += btt; >> + } >> + if (!dmah->src_len && !dmah->dest_len) { >> + DB_PRINT("dma operation finished\n"); >> + s->regs[R_INT_STS] |= 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 = 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 = XLNX_ZYNQ_DEVCFG(reg->opaque); >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) { >> + if (s->regs[R_LOCK] & 1 << i) { >> + val &= ~lock_ctrl_map[i]; >> + val |= 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 = F_EX32(val, CTRL, PCFG_AES_EN); >> + >> + if (aes_en != 0 && aes_en != 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 = XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + if (val == 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->prefix); >> + s->regs[R_CTRL] &= ~R_CTRL_PCAP_PR_MASK; >> + s->regs[R_CTRL] &= ~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 = 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 = XLNX_ZYNQ_DEVCFG(reg->opaque); >> + >> + s->dma_cmd_fifo[s->dma_cmd_fifo_num] = (XlnxZynqDevcfgDMACmd) { >> + .src_addr = s->regs[R_DMA_SRC_ADDR] & ~0x3UL, >> + .dest_addr = s->regs[R_DMA_DST_ADDR] & ~0x3UL, >> + .src_len = s->regs[R_DMA_SRC_LEN] << 2, >> + .dest_len = 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[] = { >> + { .name = "CTRL", .decode.addr = A_CTRL, >> + .reset = R_CTRL_PCAP_PR_MASK | R_CTRL_PCAP_MODE_MASK | 0x3 << 13, >> + .rsvd = 0x1 << 28 | 0x3ff << 13 | 0x3 << 13, >> + .pre_write = r_ctrl_pre_write, >> + .post_write = r_ctrl_post_write, >> + }, >> + { .name = "LOCK", .decode.addr = A_LOCK, > > The same comment can be mode for the automagic addresses. > GET_REG_ADDR(LOCK) would be clearer. > >> + .rsvd = ~ONES(5), >> + .pre_write = r_lock_pre_write, >> + }, >> + { .name = "CFG", .decode.addr = A_CFG, >> + .reset = 1 << R_CFG_RFIFO_TH_SHIFT | 1 << R_CFG_WFIFO_TH_SHIFT | >> 0x8, >> + .rsvd = 0xfffff00f, >> + }, >> + { .name = "INT_STS", .decode.addr = A_INT_STS, >> + .w1c = ~R_INT_STS_RSVD, >> + .reset = R_INT_STS_PSS_GTS_USR_B_MASK | >> + R_INT_STS_PSS_CFG_RESET_B_MASK | >> + R_INT_STS_WR_FIFO_LVL_MASK, >> + .rsvd = R_INT_STS_RSVD, >> + .post_write = r_ixr_post_write, >> + }, >> + { .name = "INT_MASK", .decode.addr = A_INT_MASK, >> + .reset = ~0, >> + .rsvd = R_INT_STS_RSVD, >> + .post_write = r_ixr_post_write, >> + }, >> + { .name = "STATUS", .decode.addr = A_STATUS, >> + .reset = R_STATUS_DMA_CMD_Q_E_MASK | >> + R_STATUS_PSS_GTS_USR_B_MASK | >> + R_STATUS_PSS_CFG_RESET_B_MASK, >> + .ro = ~0, >> + }, >> + { .name = "DMA_SRC_ADDR", .decode.addr = A_DMA_SRC_ADDR, }, >> + { .name = "DMA_DST_ADDR", .decode.addr = A_DMA_DST_ADDR, }, >> + { .name = "DMA_SRC_LEN", .decode.addr = A_DMA_SRC_LEN, >> + .ro = ~ONES(27) }, >> + { .name = "DMA_DST_LEN", .decode.addr = A_DMA_DST_LEN, >> + .ro = ~ONES(27), >> + .post_write = r_dma_dst_len_post_write, >> + }, >> + { .name = "ROM_SHADOW", .decode.addr = A_ROM_SHADOW, >> + .rsvd = ~0ull, >> + }, >> + { .name = "SW_ID", .decode.addr = A_SW_ID, }, >> + { .name = "UNLOCK", .decode.addr = A_UNLOCK, >> + .post_write = r_unlock_post_write, >> + }, >> + { .name = "MCTRL", .decode.addr = R_MCTRL * 4, >> + /* Silicon 3.0 for version field, the mysterious reserved bit 23 >> + * and QEMU platform identifier. >> + */ >> + .reset = 0x2 << R_MCTRL_PS_VERSION_SHIFT | 1 << 23 | >> R_MCTRL_QEMU_MASK, >> + .ro = ~R_MCTRL_INT_PCAP_LPBK_MASK, >> + .rsvd = 0x00f00303, >> + }, >> +}; >> + >> +static const MemoryRegionOps xlnx_zynq_devcfg_reg_ops = { >> + .read = register_read_memory_le, >> + .write = register_write_memory_le, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> + .valid = { >> + .min_access_size = 4, >> + .max_access_size = 4, >> + } >> +}; >> + >> +static const VMStateDescription vmstate_xlnx_zynq_devcfg_dma_cmd = { >> + .name = "xlnx_zynq_devcfg_dma_cmd", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> + .fields = (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 = { >> + .name = "xlnx_zynq_devcfg", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> + .fields = (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_MAX), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void xlnx_zynq_devcfg_init(Object *obj) >> +{ >> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); >> + XlnxZynqDevcfg *s = 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 = DEVICE_CLASS(klass); >> + >> + dc->reset = xlnx_zynq_devcfg_reset; >> + dc->vmsd = &vmstate_xlnx_zynq_devcfg; >> +} >> + >> +static const TypeInfo xlnx_zynq_devcfg_info = { >> + .name = TYPE_XLNX_ZYNQ_DEVCFG, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(XlnxZynqDevcfg), >> + .instance_init = xlnx_zynq_devcfg_init, >> + .class_init = 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-zynq-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 <peter.crosthwa...@xilinx.com> >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> copy >> + * of this software and associated documentation files (the "Software"), to >> deal >> + * in the Software without restriction, including without limitation the >> rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included >> in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >> FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 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ée >