[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 09/16] dma: Add Xilinx Zynq devcfg device mod
|
From: |
Alistair Francis |
|
Subject: |
Re: [Qemu-devel] [PATCH v4 09/16] dma: Add Xilinx Zynq devcfg device model |
|
Date: |
Fri, 4 Mar 2016 11:41:28 -0800 |
On Mon, Feb 29, 2016 at 4:20 AM, Alex Bennée <address@hidden> wrote:
>
> Alistair Francis <address@hidden> writes:
>
>> From: Peter Crosthwaite <address@hidden>
>>
>> Minimal device model for devcfg module of Zynq. DMA capabilities and
>> interrupt generation supported.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> Signed-off-by: Alistair Francis <address@hidden>
>> ---
>>
>> 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 <address@hidden>
>> + *
>> + * 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 <address@hidden>
>> + *
>> + * 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
>
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v4 09/16] dma: Add Xilinx Zynq devcfg device model,
Alistair Francis <=