[Qemu-devel] [PATCH v4 0/3] Add and connect the ZynqMP RTC

[Qemu-devel] [PATCH v4 0/3] Add and connect the ZynqMP RTC

[Alistair Francis]

V4:
 - Use the RegisterAccessInfo .unimp functionality

V3:
 - Store an offset value
 - Use mktimegm()
 - Log unimplemented writes

V2:
 - Delete unused realise function
 - Add cover letter
 - Convert DB_PRINT() macro to trace


Alistair Francis (3):
  xlnx-zynqmp-rtc: Initial commit
  xlnx-zynqmp-rtc: Add basic time support
  xlnx-zynqmp: Connect the RTC device

 include/hw/arm/xlnx-zynqmp.h       |   2 +
 include/hw/timer/xlnx-zynqmp-rtc.h |  88 ++++++++++++
 hw/arm/xlnx-zynqmp.c               |  14 ++
 hw/timer/xlnx-zynqmp-rtc.c         | 276 +++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs             |   1 +
 hw/timer/trace-events              |   3 +
 6 files changed, 384 insertions(+)
 create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
 create mode 100644 hw/timer/xlnx-zynqmp-rtc.c

-- 
2.14.1

[Qemu-devel] [PATCH v4 1/3] xlnx-zynqmp-rtc: Initial commit

[Alistair Francis]

Initial commit of the ZynqMP RTC device.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V2:
 - Delete unused realise function
 - Remove DB_PRINT()

 include/hw/timer/xlnx-zynqmp-rtc.h |  84 ++++++++++++++
 hw/timer/xlnx-zynqmp-rtc.c         | 218 +++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs             |   1 +
 3 files changed, 303 insertions(+)
 create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
 create mode 100644 hw/timer/xlnx-zynqmp-rtc.c

diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/timer/xlnx-zynqmp-rtc.h
new file mode 100644
index 0000000000..87649836cc
--- /dev/null
+++ b/include/hw/timer/xlnx-zynqmp-rtc.h
@@ -0,0 +1,84 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
+ *
+ * Copyright (c) 2017 Xilinx Inc.
+ *
+ * Written-by: Alistair Francis <alistair.francis@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/register.h"
+
+#define TYPE_XLNX_ZYNQMP_RTC "xlnx-zynmp.rtc"
+
+#define XLNX_ZYNQMP_RTC(obj) \
+     OBJECT_CHECK(XlnxZynqMPRTC, (obj), TYPE_XLNX_ZYNQMP_RTC)
+
+REG32(SET_TIME_WRITE, 0x0)
+REG32(SET_TIME_READ, 0x4)
+REG32(CALIB_WRITE, 0x8)
+    FIELD(CALIB_WRITE, FRACTION_EN, 20, 1)
+    FIELD(CALIB_WRITE, FRACTION_DATA, 16, 4)
+    FIELD(CALIB_WRITE, MAX_TICK, 0, 16)
+REG32(CALIB_READ, 0xc)
+    FIELD(CALIB_READ, FRACTION_EN, 20, 1)
+    FIELD(CALIB_READ, FRACTION_DATA, 16, 4)
+    FIELD(CALIB_READ, MAX_TICK, 0, 16)
+REG32(CURRENT_TIME, 0x10)
+REG32(CURRENT_TICK, 0x14)
+    FIELD(CURRENT_TICK, VALUE, 0, 16)
+REG32(ALARM, 0x18)
+REG32(RTC_INT_STATUS, 0x20)
+    FIELD(RTC_INT_STATUS, ALARM, 1, 1)
+    FIELD(RTC_INT_STATUS, SECONDS, 0, 1)
+REG32(RTC_INT_MASK, 0x24)
+    FIELD(RTC_INT_MASK, ALARM, 1, 1)
+    FIELD(RTC_INT_MASK, SECONDS, 0, 1)
+REG32(RTC_INT_EN, 0x28)
+    FIELD(RTC_INT_EN, ALARM, 1, 1)
+    FIELD(RTC_INT_EN, SECONDS, 0, 1)
+REG32(RTC_INT_DIS, 0x2c)
+    FIELD(RTC_INT_DIS, ALARM, 1, 1)
+    FIELD(RTC_INT_DIS, SECONDS, 0, 1)
+REG32(ADDR_ERROR, 0x30)
+    FIELD(ADDR_ERROR, STATUS, 0, 1)
+REG32(ADDR_ERROR_INT_MASK, 0x34)
+    FIELD(ADDR_ERROR_INT_MASK, MASK, 0, 1)
+REG32(ADDR_ERROR_INT_EN, 0x38)
+    FIELD(ADDR_ERROR_INT_EN, MASK, 0, 1)
+REG32(ADDR_ERROR_INT_DIS, 0x3c)
+    FIELD(ADDR_ERROR_INT_DIS, MASK, 0, 1)
+REG32(CONTROL, 0x40)
+    FIELD(CONTROL, BATTERY_DISABLE, 31, 1)
+    FIELD(CONTROL, OSC_CNTRL, 24, 4)
+    FIELD(CONTROL, SLVERR_ENABLE, 0, 1)
+REG32(SAFETY_CHK, 0x50)
+
+#define XLNX_ZYNQMP_RTC_R_MAX (R_SAFETY_CHK + 1)
+
+typedef struct XlnxZynqMPRTC {
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+    qemu_irq irq_rtc_int;
+    qemu_irq irq_addr_error_int;
+
+    uint32_t regs[XLNX_ZYNQMP_RTC_R_MAX];
+    RegisterInfo regs_info[XLNX_ZYNQMP_RTC_R_MAX];
+} XlnxZynqMPRTC;
diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/timer/xlnx-zynqmp-rtc.c
new file mode 100644
index 0000000000..ead40fc42d
--- /dev/null
+++ b/hw/timer/xlnx-zynqmp-rtc.c
@@ -0,0 +1,218 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
+ *
+ * Copyright (c) 2017 Xilinx Inc.
+ *
+ * Written-by: Alistair Francis <alistair.francis@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 "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/register.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "hw/timer/xlnx-zynqmp-rtc.h"
+
+#ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
+#define XLNX_ZYNQMP_RTC_ERR_DEBUG 0
+#endif
+
+static void rtc_int_update_irq(XlnxZynqMPRTC *s)
+{
+    bool pending = s->regs[R_RTC_INT_STATUS] & ~s->regs[R_RTC_INT_MASK];
+    qemu_set_irq(s->irq_rtc_int, pending);
+}
+
+static void addr_error_int_update_irq(XlnxZynqMPRTC *s)
+{
+    bool pending = s->regs[R_ADDR_ERROR] & ~s->regs[R_ADDR_ERROR_INT_MASK];
+    qemu_set_irq(s->irq_addr_error_int, pending);
+}
+
+static void rtc_int_status_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    rtc_int_update_irq(s);
+}
+
+static uint64_t rtc_int_en_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_RTC_INT_MASK] &= ~val;
+    rtc_int_update_irq(s);
+    return 0;
+}
+
+static uint64_t rtc_int_dis_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_RTC_INT_MASK] |= val;
+    rtc_int_update_irq(s);
+    return 0;
+}
+
+static void addr_error_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    addr_error_int_update_irq(s);
+}
+
+static uint64_t addr_error_int_en_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_ADDR_ERROR_INT_MASK] &= ~val;
+    addr_error_int_update_irq(s);
+    return 0;
+}
+
+static uint64_t addr_error_int_dis_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_ADDR_ERROR_INT_MASK] |= val;
+    addr_error_int_update_irq(s);
+    return 0;
+}
+
+static const RegisterAccessInfo rtc_regs_info[] = {
+    {   .name = "SET_TIME_WRITE",  .addr = A_SET_TIME_WRITE,
+    },{ .name = "SET_TIME_READ",  .addr = A_SET_TIME_READ,
+        .ro = 0xffffffff,
+    },{ .name = "CALIB_WRITE",  .addr = A_CALIB_WRITE,
+    },{ .name = "CALIB_READ",  .addr = A_CALIB_READ,
+        .ro = 0x1fffff,
+    },{ .name = "CURRENT_TIME",  .addr = A_CURRENT_TIME,
+        .ro = 0xffffffff,
+    },{ .name = "CURRENT_TICK",  .addr = A_CURRENT_TICK,
+        .ro = 0xffff,
+    },{ .name = "ALARM",  .addr = A_ALARM,
+    },{ .name = "RTC_INT_STATUS",  .addr = A_RTC_INT_STATUS,
+        .w1c = 0x3,
+        .post_write = rtc_int_status_postw,
+    },{ .name = "RTC_INT_MASK",  .addr = A_RTC_INT_MASK,
+        .reset = 0x3,
+        .ro = 0x3,
+    },{ .name = "RTC_INT_EN",  .addr = A_RTC_INT_EN,
+        .pre_write = rtc_int_en_prew,
+    },{ .name = "RTC_INT_DIS",  .addr = A_RTC_INT_DIS,
+        .pre_write = rtc_int_dis_prew,
+    },{ .name = "ADDR_ERROR",  .addr = A_ADDR_ERROR,
+        .w1c = 0x1,
+        .post_write = addr_error_postw,
+    },{ .name = "ADDR_ERROR_INT_MASK",  .addr = A_ADDR_ERROR_INT_MASK,
+        .reset = 0x1,
+        .ro = 0x1,
+    },{ .name = "ADDR_ERROR_INT_EN",  .addr = A_ADDR_ERROR_INT_EN,
+        .pre_write = addr_error_int_en_prew,
+    },{ .name = "ADDR_ERROR_INT_DIS",  .addr = A_ADDR_ERROR_INT_DIS,
+        .pre_write = addr_error_int_dis_prew,
+    },{ .name = "CONTROL",  .addr = A_CONTROL,
+        .reset = 0x1000000,
+        .rsvd = 0x70fffffe,
+    },{ .name = "SAFETY_CHK",  .addr = A_SAFETY_CHK,
+    }
+};
+
+static void rtc_reset(DeviceState *dev)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(dev);
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+
+    rtc_int_update_irq(s);
+    addr_error_int_update_irq(s);
+}
+
+static const MemoryRegionOps rtc_ops = {
+    .read = register_read_memory,
+    .write = register_write_memory,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void rtc_init(Object *obj)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    RegisterInfoArray *reg_array;
+
+    memory_region_init(&s->iomem, obj, TYPE_XLNX_ZYNQMP_RTC,
+                       XLNX_ZYNQMP_RTC_R_MAX * 4);
+    reg_array =
+        register_init_block32(DEVICE(obj), rtc_regs_info,
+                              ARRAY_SIZE(rtc_regs_info),
+                              s->regs_info, s->regs,
+                              &rtc_ops,
+                              XLNX_ZYNQMP_RTC_ERR_DEBUG,
+                              XLNX_ZYNQMP_RTC_R_MAX * 4);
+    memory_region_add_subregion(&s->iomem,
+                                0x0,
+                                &reg_array->mem);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq_rtc_int);
+    sysbus_init_irq(sbd, &s->irq_addr_error_int);
+}
+
+static const VMStateDescription vmstate_rtc = {
+    .name = TYPE_XLNX_ZYNQMP_RTC,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void rtc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = rtc_reset;
+    dc->vmsd = &vmstate_rtc;
+}
+
+static const TypeInfo rtc_info = {
+    .name          = TYPE_XLNX_ZYNQMP_RTC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPRTC),
+    .class_init    = rtc_class_init,
+    .instance_init = rtc_init,
+};
+
+static void rtc_register_types(void)
+{
+    type_register_static(&rtc_info);
+}
+
+type_init(rtc_register_types)
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8c19eac3b6..8b27a4b7ef 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -21,6 +21,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
 common-obj-$(CONFIG_IMX) += imx_gpt.o
 common-obj-$(CONFIG_LM32) += lm32_timer.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
 
 obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
-- 
2.14.1

[Qemu-devel] [PATCH v4 3/3] xlnx-zynqmp: Connect the RTC device

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

 include/hw/arm/xlnx-zynqmp.h |  2 ++
 hw/arm/xlnx-zynqmp.c         | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 3e6fb9b7bd..9e8c9e18dd 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -28,6 +28,7 @@
 #include "hw/ssi/xilinx_spips.h"
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/display/xlnx_dp.h"
+#include "hw/timer/xlnx-zynqmp-rtc.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -90,6 +91,7 @@ typedef struct XlnxZynqMPState {
     XlnxZynqMPQSPIPS qspi;
     XlnxDPState dp;
     XlnxDPDMAState dpdma;
+    XlnxZynqMPRTC rtc;
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 325642058b..deef583c2a 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -50,6 +50,9 @@
 #define DPDMA_ADDR          0xfd4c0000
 #define DPDMA_IRQ           116
 
+#define RTC_ADDR            0xffa60000
+#define RTC_IRQ             26
+
 static const uint64_t gem_addr[XLNX_ZYNQMP_NUM_GEMS] = {
     0xFF0B0000, 0xFF0C0000, 0xFF0D0000, 0xFF0E0000,
 };
@@ -183,6 +186,9 @@ static void xlnx_zynqmp_init(Object *obj)
 
     object_initialize(&s->dpdma, sizeof(s->dpdma), TYPE_XLNX_DPDMA);
     qdev_set_parent_bus(DEVICE(&s->dpdma), sysbus_get_default());
+
+    object_initialize(&s->rtc, sizeof(s->rtc), TYPE_XLNX_ZYNQMP_RTC);
+    qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default());
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -454,6 +460,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                              &error_abort);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->dpdma), 0, DPDMA_ADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->dpdma), 0, gic_spi[DPDMA_IRQ]);
+
+    object_property_set_bool(OBJECT(&s->rtc), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, RTC_ADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0, gic_spi[RTC_IRQ]);
 }
 
 static Property xlnx_zynqmp_props[] = {
-- 
2.14.1

Re: [Qemu-devel] [PATCH v4 1/3] xlnx-zynqmp-rtc: Initial commit

[Philippe Mathieu-Daudé]

Hi Alistair,

On 01/19/2018 03:35 PM, Alistair Francis wrote:
> Initial commit of the ZynqMP RTC device.
> 
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> V2:
>  - Delete unused realise function
>  - Remove DB_PRINT()
> 
>  include/hw/timer/xlnx-zynqmp-rtc.h |  84 ++++++++++++++
>  hw/timer/xlnx-zynqmp-rtc.c         | 218 +++++++++++++++++++++++++++++++++++++
>  hw/timer/Makefile.objs             |   1 +
>  3 files changed, 303 insertions(+)
>  create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
>  create mode 100644 hw/timer/xlnx-zynqmp-rtc.c
> 
> diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/timer/xlnx-zynqmp-rtc.h
> new file mode 100644
> index 0000000000..87649836cc
> --- /dev/null
> +++ b/include/hw/timer/xlnx-zynqmp-rtc.h
> @@ -0,0 +1,84 @@
> +/*
> + * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
> + *
> + * Copyright (c) 2017 Xilinx Inc.
> + *
> + * Written-by: Alistair Francis <alistair.francis@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/register.h"
> +
> +#define TYPE_XLNX_ZYNQMP_RTC "xlnx-zynmp.rtc"
> +
> +#define XLNX_ZYNQMP_RTC(obj) \
> +     OBJECT_CHECK(XlnxZynqMPRTC, (obj), TYPE_XLNX_ZYNQMP_RTC)
> +
> +REG32(SET_TIME_WRITE, 0x0)
> +REG32(SET_TIME_READ, 0x4)
> +REG32(CALIB_WRITE, 0x8)
> +    FIELD(CALIB_WRITE, FRACTION_EN, 20, 1)
> +    FIELD(CALIB_WRITE, FRACTION_DATA, 16, 4)
> +    FIELD(CALIB_WRITE, MAX_TICK, 0, 16)
> +REG32(CALIB_READ, 0xc)
> +    FIELD(CALIB_READ, FRACTION_EN, 20, 1)
> +    FIELD(CALIB_READ, FRACTION_DATA, 16, 4)
> +    FIELD(CALIB_READ, MAX_TICK, 0, 16)
> +REG32(CURRENT_TIME, 0x10)
> +REG32(CURRENT_TICK, 0x14)
> +    FIELD(CURRENT_TICK, VALUE, 0, 16)
> +REG32(ALARM, 0x18)
> +REG32(RTC_INT_STATUS, 0x20)
> +    FIELD(RTC_INT_STATUS, ALARM, 1, 1)
> +    FIELD(RTC_INT_STATUS, SECONDS, 0, 1)
> +REG32(RTC_INT_MASK, 0x24)
> +    FIELD(RTC_INT_MASK, ALARM, 1, 1)
> +    FIELD(RTC_INT_MASK, SECONDS, 0, 1)
> +REG32(RTC_INT_EN, 0x28)
> +    FIELD(RTC_INT_EN, ALARM, 1, 1)
> +    FIELD(RTC_INT_EN, SECONDS, 0, 1)
> +REG32(RTC_INT_DIS, 0x2c)
> +    FIELD(RTC_INT_DIS, ALARM, 1, 1)
> +    FIELD(RTC_INT_DIS, SECONDS, 0, 1)
> +REG32(ADDR_ERROR, 0x30)
> +    FIELD(ADDR_ERROR, STATUS, 0, 1)
> +REG32(ADDR_ERROR_INT_MASK, 0x34)
> +    FIELD(ADDR_ERROR_INT_MASK, MASK, 0, 1)
> +REG32(ADDR_ERROR_INT_EN, 0x38)
> +    FIELD(ADDR_ERROR_INT_EN, MASK, 0, 1)
> +REG32(ADDR_ERROR_INT_DIS, 0x3c)
> +    FIELD(ADDR_ERROR_INT_DIS, MASK, 0, 1)
> +REG32(CONTROL, 0x40)
> +    FIELD(CONTROL, BATTERY_DISABLE, 31, 1)
> +    FIELD(CONTROL, OSC_CNTRL, 24, 4)
> +    FIELD(CONTROL, SLVERR_ENABLE, 0, 1)
> +REG32(SAFETY_CHK, 0x50)
> +
> +#define XLNX_ZYNQMP_RTC_R_MAX (R_SAFETY_CHK + 1)
> +
> +typedef struct XlnxZynqMPRTC {
> +    SysBusDevice parent_obj;
> +    MemoryRegion iomem;
> +    qemu_irq irq_rtc_int;
> +    qemu_irq irq_addr_error_int;
> +
> +    uint32_t regs[XLNX_ZYNQMP_RTC_R_MAX];
> +    RegisterInfo regs_info[XLNX_ZYNQMP_RTC_R_MAX];
> +} XlnxZynqMPRTC;
> diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/timer/xlnx-zynqmp-rtc.c
> new file mode 100644
> index 0000000000..ead40fc42d
> --- /dev/null
> +++ b/hw/timer/xlnx-zynqmp-rtc.c
> @@ -0,0 +1,218 @@
> +/*
> + * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
> + *
> + * Copyright (c) 2017 Xilinx Inc.
> + *
> + * Written-by: Alistair Francis <alistair.francis@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 "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/register.h"
> +#include "qemu/bitops.h"
> +#include "qemu/log.h"
> +#include "hw/timer/xlnx-zynqmp-rtc.h"
> +
> +#ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
> +#define XLNX_ZYNQMP_RTC_ERR_DEBUG 0
> +#endif
> +
> +static void rtc_int_update_irq(XlnxZynqMPRTC *s)
> +{
> +    bool pending = s->regs[R_RTC_INT_STATUS] & ~s->regs[R_RTC_INT_MASK];
> +    qemu_set_irq(s->irq_rtc_int, pending);
> +}
> +
> +static void addr_error_int_update_irq(XlnxZynqMPRTC *s)
> +{
> +    bool pending = s->regs[R_ADDR_ERROR] & ~s->regs[R_ADDR_ERROR_INT_MASK];
> +    qemu_set_irq(s->irq_addr_error_int, pending);
> +}
> +
> +static void rtc_int_status_postw(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    rtc_int_update_irq(s);
> +}
> +
> +static uint64_t rtc_int_en_prew(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    uint32_t val = val64;

Why is this intermediate variable necessary?

> +
> +    s->regs[R_RTC_INT_MASK] &= ~val;
> +    rtc_int_update_irq(s);
> +    return 0;
> +}
> +
> +static uint64_t rtc_int_dis_prew(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    uint32_t val = val64;

ditto,

> +
> +    s->regs[R_RTC_INT_MASK] |= val;
> +    rtc_int_update_irq(s);
> +    return 0;
> +}
> +
> +static void addr_error_postw(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    addr_error_int_update_irq(s);
> +}
> +
> +static uint64_t addr_error_int_en_prew(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    uint32_t val = val64;

ditto,

> +
> +    s->regs[R_ADDR_ERROR_INT_MASK] &= ~val;
> +    addr_error_int_update_irq(s);
> +    return 0;
> +}
> +
> +static uint64_t addr_error_int_dis_prew(RegisterInfo *reg, uint64_t val64)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
> +    uint32_t val = val64;

ditto.

I find this clearer to review:

       s->regs[R_ADDR_ERROR_INT_MASK] |= (uint32_t)val64;

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
> +    s->regs[R_ADDR_ERROR_INT_MASK] |= val;
> +    addr_error_int_update_irq(s);
> +    return 0;
> +}
> +
> +static const RegisterAccessInfo rtc_regs_info[] = {
> +    {   .name = "SET_TIME_WRITE",  .addr = A_SET_TIME_WRITE,
> +    },{ .name = "SET_TIME_READ",  .addr = A_SET_TIME_READ,
> +        .ro = 0xffffffff,
> +    },{ .name = "CALIB_WRITE",  .addr = A_CALIB_WRITE,
> +    },{ .name = "CALIB_READ",  .addr = A_CALIB_READ,
> +        .ro = 0x1fffff,
> +    },{ .name = "CURRENT_TIME",  .addr = A_CURRENT_TIME,
> +        .ro = 0xffffffff,
> +    },{ .name = "CURRENT_TICK",  .addr = A_CURRENT_TICK,
> +        .ro = 0xffff,
> +    },{ .name = "ALARM",  .addr = A_ALARM,
> +    },{ .name = "RTC_INT_STATUS",  .addr = A_RTC_INT_STATUS,
> +        .w1c = 0x3,
> +        .post_write = rtc_int_status_postw,
> +    },{ .name = "RTC_INT_MASK",  .addr = A_RTC_INT_MASK,
> +        .reset = 0x3,
> +        .ro = 0x3,
> +    },{ .name = "RTC_INT_EN",  .addr = A_RTC_INT_EN,
> +        .pre_write = rtc_int_en_prew,
> +    },{ .name = "RTC_INT_DIS",  .addr = A_RTC_INT_DIS,
> +        .pre_write = rtc_int_dis_prew,
> +    },{ .name = "ADDR_ERROR",  .addr = A_ADDR_ERROR,
> +        .w1c = 0x1,
> +        .post_write = addr_error_postw,
> +    },{ .name = "ADDR_ERROR_INT_MASK",  .addr = A_ADDR_ERROR_INT_MASK,
> +        .reset = 0x1,
> +        .ro = 0x1,
> +    },{ .name = "ADDR_ERROR_INT_EN",  .addr = A_ADDR_ERROR_INT_EN,
> +        .pre_write = addr_error_int_en_prew,
> +    },{ .name = "ADDR_ERROR_INT_DIS",  .addr = A_ADDR_ERROR_INT_DIS,
> +        .pre_write = addr_error_int_dis_prew,
> +    },{ .name = "CONTROL",  .addr = A_CONTROL,
> +        .reset = 0x1000000,
> +        .rsvd = 0x70fffffe,
> +    },{ .name = "SAFETY_CHK",  .addr = A_SAFETY_CHK,
> +    }
> +};
> +
> +static void rtc_reset(DeviceState *dev)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(dev);
> +    unsigned int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
> +        register_reset(&s->regs_info[i]);
> +    }
> +
> +    rtc_int_update_irq(s);
> +    addr_error_int_update_irq(s);
> +}
> +
> +static const MemoryRegionOps rtc_ops = {
> +    .read = register_read_memory,
> +    .write = register_write_memory,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +static void rtc_init(Object *obj)
> +{
> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    RegisterInfoArray *reg_array;
> +
> +    memory_region_init(&s->iomem, obj, TYPE_XLNX_ZYNQMP_RTC,
> +                       XLNX_ZYNQMP_RTC_R_MAX * 4);
> +    reg_array =
> +        register_init_block32(DEVICE(obj), rtc_regs_info,
> +                              ARRAY_SIZE(rtc_regs_info),
> +                              s->regs_info, s->regs,
> +                              &rtc_ops,
> +                              XLNX_ZYNQMP_RTC_ERR_DEBUG,
> +                              XLNX_ZYNQMP_RTC_R_MAX * 4);
> +    memory_region_add_subregion(&s->iomem,
> +                                0x0,
> +                                &reg_array->mem);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq_rtc_int);
> +    sysbus_init_irq(sbd, &s->irq_addr_error_int);
> +}
> +
> +static const VMStateDescription vmstate_rtc = {
> +    .name = TYPE_XLNX_ZYNQMP_RTC,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +static void rtc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->reset = rtc_reset;
> +    dc->vmsd = &vmstate_rtc;
> +}
> +
> +static const TypeInfo rtc_info = {
> +    .name          = TYPE_XLNX_ZYNQMP_RTC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(XlnxZynqMPRTC),
> +    .class_init    = rtc_class_init,
> +    .instance_init = rtc_init,
> +};
> +
> +static void rtc_register_types(void)
> +{
> +    type_register_static(&rtc_info);
> +}
> +
> +type_init(rtc_register_types)
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 8c19eac3b6..8b27a4b7ef 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -21,6 +21,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
>  common-obj-$(CONFIG_IMX) += imx_gpt.o
>  common-obj-$(CONFIG_LM32) += lm32_timer.o
>  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
> +common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
>  
>  obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
> 

Re: [Qemu-devel] [PATCH v4 1/3] xlnx-zynqmp-rtc: Initial commit

[Alistair Francis]

On Fri, Jan 19, 2018 at 11:22 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Hi Alistair,
>
> On 01/19/2018 03:35 PM, Alistair Francis wrote:
>> Initial commit of the ZynqMP RTC device.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>> V2:
>>  - Delete unused realise function
>>  - Remove DB_PRINT()
>>
>>  include/hw/timer/xlnx-zynqmp-rtc.h |  84 ++++++++++++++
>>  hw/timer/xlnx-zynqmp-rtc.c         | 218 +++++++++++++++++++++++++++++++++++++
>>  hw/timer/Makefile.objs             |   1 +
>>  3 files changed, 303 insertions(+)
>>  create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
>>  create mode 100644 hw/timer/xlnx-zynqmp-rtc.c
>>
>> diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/timer/xlnx-zynqmp-rtc.h
>> new file mode 100644
>> index 0000000000..87649836cc
>> --- /dev/null
>> +++ b/include/hw/timer/xlnx-zynqmp-rtc.h
>> @@ -0,0 +1,84 @@
>> +/*
>> + * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
>> + *
>> + * Copyright (c) 2017 Xilinx Inc.
>> + *
>> + * Written-by: Alistair Francis <alistair.francis@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/register.h"
>> +
>> +#define TYPE_XLNX_ZYNQMP_RTC "xlnx-zynmp.rtc"
>> +
>> +#define XLNX_ZYNQMP_RTC(obj) \
>> +     OBJECT_CHECK(XlnxZynqMPRTC, (obj), TYPE_XLNX_ZYNQMP_RTC)
>> +
>> +REG32(SET_TIME_WRITE, 0x0)
>> +REG32(SET_TIME_READ, 0x4)
>> +REG32(CALIB_WRITE, 0x8)
>> +    FIELD(CALIB_WRITE, FRACTION_EN, 20, 1)
>> +    FIELD(CALIB_WRITE, FRACTION_DATA, 16, 4)
>> +    FIELD(CALIB_WRITE, MAX_TICK, 0, 16)
>> +REG32(CALIB_READ, 0xc)
>> +    FIELD(CALIB_READ, FRACTION_EN, 20, 1)
>> +    FIELD(CALIB_READ, FRACTION_DATA, 16, 4)
>> +    FIELD(CALIB_READ, MAX_TICK, 0, 16)
>> +REG32(CURRENT_TIME, 0x10)
>> +REG32(CURRENT_TICK, 0x14)
>> +    FIELD(CURRENT_TICK, VALUE, 0, 16)
>> +REG32(ALARM, 0x18)
>> +REG32(RTC_INT_STATUS, 0x20)
>> +    FIELD(RTC_INT_STATUS, ALARM, 1, 1)
>> +    FIELD(RTC_INT_STATUS, SECONDS, 0, 1)
>> +REG32(RTC_INT_MASK, 0x24)
>> +    FIELD(RTC_INT_MASK, ALARM, 1, 1)
>> +    FIELD(RTC_INT_MASK, SECONDS, 0, 1)
>> +REG32(RTC_INT_EN, 0x28)
>> +    FIELD(RTC_INT_EN, ALARM, 1, 1)
>> +    FIELD(RTC_INT_EN, SECONDS, 0, 1)
>> +REG32(RTC_INT_DIS, 0x2c)
>> +    FIELD(RTC_INT_DIS, ALARM, 1, 1)
>> +    FIELD(RTC_INT_DIS, SECONDS, 0, 1)
>> +REG32(ADDR_ERROR, 0x30)
>> +    FIELD(ADDR_ERROR, STATUS, 0, 1)
>> +REG32(ADDR_ERROR_INT_MASK, 0x34)
>> +    FIELD(ADDR_ERROR_INT_MASK, MASK, 0, 1)
>> +REG32(ADDR_ERROR_INT_EN, 0x38)
>> +    FIELD(ADDR_ERROR_INT_EN, MASK, 0, 1)
>> +REG32(ADDR_ERROR_INT_DIS, 0x3c)
>> +    FIELD(ADDR_ERROR_INT_DIS, MASK, 0, 1)
>> +REG32(CONTROL, 0x40)
>> +    FIELD(CONTROL, BATTERY_DISABLE, 31, 1)
>> +    FIELD(CONTROL, OSC_CNTRL, 24, 4)
>> +    FIELD(CONTROL, SLVERR_ENABLE, 0, 1)
>> +REG32(SAFETY_CHK, 0x50)
>> +
>> +#define XLNX_ZYNQMP_RTC_R_MAX (R_SAFETY_CHK + 1)
>> +
>> +typedef struct XlnxZynqMPRTC {
>> +    SysBusDevice parent_obj;
>> +    MemoryRegion iomem;
>> +    qemu_irq irq_rtc_int;
>> +    qemu_irq irq_addr_error_int;
>> +
>> +    uint32_t regs[XLNX_ZYNQMP_RTC_R_MAX];
>> +    RegisterInfo regs_info[XLNX_ZYNQMP_RTC_R_MAX];
>> +} XlnxZynqMPRTC;
>> diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/timer/xlnx-zynqmp-rtc.c
>> new file mode 100644
>> index 0000000000..ead40fc42d
>> --- /dev/null
>> +++ b/hw/timer/xlnx-zynqmp-rtc.c
>> @@ -0,0 +1,218 @@
>> +/*
>> + * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
>> + *
>> + * Copyright (c) 2017 Xilinx Inc.
>> + *
>> + * Written-by: Alistair Francis <alistair.francis@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 "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/register.h"
>> +#include "qemu/bitops.h"
>> +#include "qemu/log.h"
>> +#include "hw/timer/xlnx-zynqmp-rtc.h"
>> +
>> +#ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
>> +#define XLNX_ZYNQMP_RTC_ERR_DEBUG 0
>> +#endif
>> +
>> +static void rtc_int_update_irq(XlnxZynqMPRTC *s)
>> +{
>> +    bool pending = s->regs[R_RTC_INT_STATUS] & ~s->regs[R_RTC_INT_MASK];
>> +    qemu_set_irq(s->irq_rtc_int, pending);
>> +}
>> +
>> +static void addr_error_int_update_irq(XlnxZynqMPRTC *s)
>> +{
>> +    bool pending = s->regs[R_ADDR_ERROR] & ~s->regs[R_ADDR_ERROR_INT_MASK];
>> +    qemu_set_irq(s->irq_addr_error_int, pending);
>> +}
>> +
>> +static void rtc_int_status_postw(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    rtc_int_update_irq(s);
>> +}
>> +
>> +static uint64_t rtc_int_en_prew(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    uint32_t val = val64;
>
> Why is this intermediate variable necessary?
>
>> +
>> +    s->regs[R_RTC_INT_MASK] &= ~val;
>> +    rtc_int_update_irq(s);
>> +    return 0;
>> +}
>> +
>> +static uint64_t rtc_int_dis_prew(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    uint32_t val = val64;
>
> ditto,
>
>> +
>> +    s->regs[R_RTC_INT_MASK] |= val;
>> +    rtc_int_update_irq(s);
>> +    return 0;
>> +}
>> +
>> +static void addr_error_postw(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    addr_error_int_update_irq(s);
>> +}
>> +
>> +static uint64_t addr_error_int_en_prew(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    uint32_t val = val64;
>
> ditto,
>
>> +
>> +    s->regs[R_ADDR_ERROR_INT_MASK] &= ~val;
>> +    addr_error_int_update_irq(s);
>> +    return 0;
>> +}
>> +
>> +static uint64_t addr_error_int_dis_prew(RegisterInfo *reg, uint64_t val64)
>> +{
>> +    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
>> +    uint32_t val = val64;
>
> ditto.
>
> I find this clearer to review:
>
>        s->regs[R_ADDR_ERROR_INT_MASK] |= (uint32_t)val64;

Fixed!

Alistair

>
> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>

Re: [Qemu-devel] [PATCH v4 2/3] xlnx-zynqmp-rtc: Add basic time support

[Peter Maydell]

On 19 January 2018 at 18:35, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Allow the guest to determine the time set from the QEMU command line.
>
> This includes adding a trace event to debug the new time.
>
> The migration logic was copied from the PL031 migration logic.

Unfortunately, that logic won't work for your case. The PL031
presave/postload functions were added in commit b0f26631bc5
and are there to handle a change in the semantics of a pre-existing
tick offset field in the vmstate. In your case there is no tick
offset field at all, and just adding one like this will cause
migration from the previous QEMU version to break.

>  static const VMStateDescription vmstate_rtc = {
>      .name = TYPE_XLNX_ZYNQMP_RTC,
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_save = rtc_pre_save,
> +    .post_load = rtc_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
> +        VMSTATE_UINT32(tick_offset_vmstate, XlnxZynqMPRTC),
>          VMSTATE_END_OF_LIST(),
>      }
>  };

For migration compatibility here you'll need to put the new field
into its own subsection, and then in pre-load and/or post-load you
can handle the "inbound data is from an old version".

thanks
-- PMM