[Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Qemu-devel] [PATCH v3 00/10] Clock framework API.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Hi,

This is the third version of the clock framework API it contains:

  * The first 6 patches which introduce the framework.
  * The 7th patch which introduces a fixed-clock model.
  * The rest which gives an example how to model a PLL from the existing
    zynqmp-crf extracted from the qemu xilinx tree.

No specific behavior is expected yet when the CRF register set is accessed but
the user can see for example the dp_video_ref and vpll_to_lpd rate changing in
the monitor with the "info qtree" command when the vpll_ctrl register is
modified.

bus: main-system-bus
  type System
  dev: xlnx.zynqmp_crf, id ""
    gpio-out "sysbus-irq" 1
    qemu-clk "dbg_trace" 0
    qemu-clk "dp_stc_ref" 0
    qemu-clk "dpll_to_lpd" 12500000
    qemu-clk "acpu_clk" 0
    qemu-clk "pcie_ref" 0
    qemu-clk "topsw_main" 0
    qemu-clk "topsw_lsbus" 0
    qemu-clk "dp_audio_ref" 0
    qemu-clk "sata_ref" 0
    qemu-clk "dp_video_ref" 1428571
    qemu-clk "vpll_clk" 50000000
    qemu-clk "apll_to_lpd" 12500000
    qemu-clk "dpll_clk" 50000000
    qemu-clk "gpu_ref" 0
    qemu-clk "aux_refclk" 0
    qemu-clk "video_clk" 27000000
    qemu-clk "gdma_ref" 0
    qemu-clk "gt_crx_ref_clk" 0
    qemu-clk "dbg_fdp" 0
    qemu-clk "apll_clk" 50000000
    qemu-clk "pss_alt_ref_clk" 0
    qemu-clk "ddr" 0
    qemu-clk "dbg_tstmp" 0
    qemu-clk "pss_ref_clk" 50000000
    qemu-clk "dpdma_ref" 0
    qemu-clk "vpll_to_lpd" 12500000
    mmio 00000000fd1a0000/000000000000010c
    
This series is based on the current master
(d992f2f1368ceb92e6bfd8efece174110f4236ff).

Thanks,
Fred

V2 -> V3:
  * Rebased on current master.
  * Renamed qemu_clk / QEMUClock as suggested by Cédric.
  * Renamed in_rate to ref_rate and out_rate to rate.
  * Renamed qemu_clk_bind_clock to qemu_clk_bind.
  * Example added to the documentation as suggested by Peter.

V1 -> V2:
  * Rebased on current master.
  * Some function renamed and documentation fixed.

RFC -> V1:
  * Rebased on current master.
  * The docs has been fixed.
  * qemu_clk_init_device helper has been provided to ease the initialization
    of the devices.

KONRAD Frederic (10):
  qemu-clk: introduce qemu-clk qom object
  qemu-clk: allow to add a clock to a device
  qemu-clk: allow to bind two clocks together
  qemu-clk: introduce an init array to help the device construction
  qdev-monitor: print the device's clock with info qtree
  docs: add qemu-clock documentation
  introduce fixed-clock
  introduce zynqmp_crf
  zynqmp: add the zynqmp_crf to the platform
  zynqmp: add reference clock

 Makefile.objs                 |   1 +
 docs/clock.txt                | 278 ++++++++++++
 hw/arm/xlnx-zynqmp.c          |  56 +++
 hw/misc/Makefile.objs         |   2 +
 hw/misc/fixed-clock.c         |  88 ++++
 hw/misc/xilinx_zynqmp_crf.c   | 968 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h  |   8 +
 include/hw/misc/fixed-clock.h |  30 ++
 include/qemu/qemu-clock.h     | 161 +++++++
 qdev-monitor.c                |   2 +
 qemu-clock.c                  | 176 ++++++++
 11 files changed, 1770 insertions(+)
 create mode 100644 docs/clock.txt
 create mode 100644 hw/misc/fixed-clock.c
 create mode 100644 hw/misc/xilinx_zynqmp_crf.c
 create mode 100644 include/hw/misc/fixed-clock.h
 create mode 100644 include/qemu/qemu-clock.h
 create mode 100644 qemu-clock.c

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/10] qemu-clk: introduce qemu-clk qom object

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces qemu-clk qom object.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V2 -> V3:
  * s/qemu_clk/QEMUClock/g
---
 Makefile.objs             |  1 +
 include/qemu/qemu-clock.h | 40 +++++++++++++++++++++++++++++++++++++
 qemu-clock.c              | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 include/qemu/qemu-clock.h
 create mode 100644 qemu-clock.c

diff --git a/Makefile.objs b/Makefile.objs
index e740500..0b8b8ff 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -75,6 +75,7 @@ common-obj-y += backends/
 common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
 
 common-obj-$(CONFIG_FDT) += device_tree.o
+common-obj-y += qemu-clock.o
 
 ######################################################################
 # qapi
diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
new file mode 100644
index 0000000..446a490
--- /dev/null
+++ b/include/qemu/qemu-clock.h
@@ -0,0 +1,40 @@
+/*
+ * QEMU Clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QEMU_CLOCK_H
+#define QEMU_CLOCK_H
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+
+#define TYPE_CLOCK "qemu-clk"
+#define QEMU_CLOCK(obj) OBJECT_CHECK(QEMUClock, (obj), TYPE_CLOCK)
+
+typedef struct QEMUClock {
+    /*< private >*/
+    Object parent_obj;
+} QEMUClock;
+
+#endif /* QEMU_CLOCK_H */
+
+
diff --git a/qemu-clock.c b/qemu-clock.c
new file mode 100644
index 0000000..3bef144
--- /dev/null
+++ b/qemu-clock.c
@@ -0,0 +1,50 @@
+/*
+ * QEMU Clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/qemu-clock.h"
+#include "hw/hw.h"
+#include "qemu/log.h"
+
+#ifndef DEBUG_QEMU_CLOCK
+#define DEBUG_QEMU_CLOCK 0
+#endif
+
+#define DPRINTF(fmt, args...) do {                                           \
+    if (DEBUG_QEMU_CLOCK) {                                                  \
+        qemu_log("%s: " fmt, __func__, ## args);                             \
+    }                                                                        \
+} while (0);
+
+static const TypeInfo qemu_clk_info = {
+    .name          = TYPE_CLOCK,
+    .parent        = TYPE_OBJECT,
+    .instance_size = sizeof(QEMUClock),
+};
+
+static void qemu_clk_register_types(void)
+{
+    type_register_static(&qemu_clk_info);
+}
+
+type_init(qemu_clk_register_types);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 02/10] qemu-clk: allow to add a clock to a device

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This allows to add a clock to a DeviceState.
Contrary to gpios, the clock pins are not contained in the DeviceState but
with the child property so they can appears in the qom-tree.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V2 -> V3:
  * s/qemu_clk/QEMUClock/g
V1 -> V2:
  * Rename the function use 'add' instead of 'attach'
---
 include/qemu/qemu-clock.h | 24 +++++++++++++++++++++++-
 qemu-clock.c              | 23 +++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index 446a490..ffe743d 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -33,8 +33,30 @@
 typedef struct QEMUClock {
     /*< private >*/
     Object parent_obj;
+    char *name;            /* name of this clock in the device. */
 } QEMUClock;
 
-#endif /* QEMU_CLOCK_H */
+/**
+ * qemu_clk_device_add_clock:
+ * @dev: the device on which the clock needs to be added.
+ * @clk: the clock which needs to be added.
+ * @name: the name of the clock can't be NULL.
+ *
+ * Add @clk to device @dev as a clock named @name.
+ *
+ */
+void qemu_clk_device_add_clock(DeviceState *dev, QEMUClock *clk,
+                               const char *name);
 
+/**
+ * qemu_clk_device_get_clock:
+ * @dev: the device which contains the clock.
+ * @name: the name of the clock.
+ *
+ * Get the clock named @name contained in the device @dev, or NULL if not found.
+ *
+ * Returns the clock named @name contained in @dev.
+ */
+QEMUClock *qemu_clk_device_get_clock(DeviceState *dev, const char *name);
 
+#endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index 3bef144..6eeecf3 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -25,6 +25,7 @@
 #include "qemu/qemu-clock.h"
 #include "hw/hw.h"
 #include "qemu/log.h"
+#include "qapi/error.h"
 
 #ifndef DEBUG_QEMU_CLOCK
 #define DEBUG_QEMU_CLOCK 0
@@ -36,6 +37,28 @@
     }                                                                        \
 } while (0);
 
+void qemu_clk_device_add_clock(DeviceState *dev, QEMUClock *clk,
+                               const char *name)
+{
+    assert(name);
+    assert(!clk->name);
+    object_property_add_child(OBJECT(dev), name, OBJECT(clk), &error_abort);
+    clk->name = g_strdup(name);
+}
+
+QEMUClock *qemu_clk_device_get_clock(DeviceState *dev, const char *name)
+{
+    gchar *path = NULL;
+    Object *clk;
+    bool ambiguous;
+
+    path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(dev)),
+                           name);
+    clk = object_resolve_path(path, &ambiguous);
+    g_free(path);
+    return QEMU_CLOCK(clk);
+}
+
 static const TypeInfo qemu_clk_info = {
     .name          = TYPE_CLOCK,
     .parent        = TYPE_OBJECT,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/10] qemu-clk: allow to bind two clocks together

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces the clock binding and the update part.
When the qemu_clk_rate_update(qemu_clk, int) function is called:
  * The clock callback is called on the qemu_clk so it can change the rate.
  * The qemu_clk_rate_update function is called on all the driven clock.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V2 -> V3:
  * s/qemu_clk/QEMUClock/g
  * Rename in_rate to ref_rate
  * Rename out_rate to rate
  * Rename qemu_clk_bind_clock to qemu_clk_bind
V1 -> V2:
  * Rename qemu_clk_on_rate_update_cb to QEMUClkRateUpdateCallback and
    move the pointer to the structure instead of having a pointer-to-function
    type.
---
 include/qemu/qemu-clock.h | 67 +++++++++++++++++++++++++++++++++++++++++++++++
 qemu-clock.c              | 58 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index ffe743d..f80de56 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -30,12 +30,25 @@
 #define TYPE_CLOCK "qemu-clk"
 #define QEMU_CLOCK(obj) OBJECT_CHECK(QEMUClock, (obj), TYPE_CLOCK)
 
+typedef struct ClkList ClkList;
+typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
+
 typedef struct QEMUClock {
     /*< private >*/
     Object parent_obj;
     char *name;            /* name of this clock in the device. */
+    uint64_t ref_rate;     /* rate of the clock which drive this pin. */
+    uint64_t rate;         /* rate of this clock pin. */
+    void *opaque;
+    QEMUClkRateUpdateCallback *cb;
+    QLIST_HEAD(, ClkList) bound;
 } QEMUClock;
 
+struct ClkList {
+    QEMUClock *clk;
+    QLIST_ENTRY(ClkList) node;
+};
+
 /**
  * qemu_clk_device_add_clock:
  * @dev: the device on which the clock needs to be added.
@@ -59,4 +72,58 @@ void qemu_clk_device_add_clock(DeviceState *dev, QEMUClock *clk,
  */
 QEMUClock *qemu_clk_device_get_clock(DeviceState *dev, const char *name);
 
+/**
+ * qemu_clk_bind:
+ * @out: the clock output.
+ * @in: the clock input.
+ *
+ * Connect the clock together. This is unidirectional so a
+ * qemu_clk_update_rate will go from @out to @in.
+ *
+ */
+void qemu_clk_bind(QEMUClock *out, QEMUClock *in);
+
+/**
+ * qemu_clk_unbind:
+ * @out: the clock output.
+ * @in: the clock input.
+ *
+ * Disconnect the clocks if they were bound together.
+ *
+ */
+void qemu_clk_unbind(QEMUClock *out, QEMUClock *in);
+
+/**
+ * qemu_clk_update_rate:
+ * @clk: the clock to update.
+ * @rate: the new rate in Hz.
+ *
+ * Update the @clk to the new @rate.
+ *
+ */
+void qemu_clk_update_rate(QEMUClock *clk, uint64_t rate);
+
+/**
+ * qemu_clk_refresh:
+ * @clk: the clock to be refreshed.
+ *
+ * If a model alters the topology of a clock tree, it must call this function on
+ * the clock source to refresh the clock tree.
+ *
+ */
+void qemu_clk_refresh(QEMUClock *clk);
+
+/**
+ * qemu_clk_set_callback:
+ * @clk: the clock associated to the callback.
+ * @cb: the function which is called when a refresh happen on the clock @clk.
+ * @opaque: the opaque data passed to the callback.
+ *
+ * Set the callback @cb which will be called when the clock @clk is updated.
+ *
+ */
+void qemu_clk_set_callback(QEMUClock *clk,
+                           QEMUClkRateUpdateCallback *cb,
+                           void *opaque);
+
 #endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index 6eeecf3..24719e2 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -37,6 +37,64 @@
     }                                                                        \
 } while (0);
 
+void qemu_clk_refresh(QEMUClock *clk)
+{
+    qemu_clk_update_rate(clk, clk->ref_rate);
+}
+
+void qemu_clk_update_rate(QEMUClock *clk, uint64_t rate)
+{
+    ClkList *child;
+
+    clk->ref_rate = rate;
+    clk->rate = rate;
+
+    if (clk->cb) {
+        clk->rate = clk->cb(clk->opaque, rate);
+    }
+
+    DPRINTF("%s output rate updated to %" PRIu64 "\n",
+            object_get_canonical_path(OBJECT(clk)),
+            clk->rate);
+
+    QLIST_FOREACH(child, &clk->bound, node) {
+        qemu_clk_update_rate(child->clk, clk->rate);
+    }
+}
+
+void qemu_clk_bind(QEMUClock *out, QEMUClock *in)
+{
+    ClkList *child;
+
+    child = g_malloc(sizeof(child));
+    assert(child);
+    child->clk = in;
+    object_ref(OBJECT(in));
+    QLIST_INSERT_HEAD(&out->bound, child, node);
+    qemu_clk_update_rate(in, out->rate);
+}
+
+void qemu_clk_unbind(QEMUClock *out, QEMUClock *in)
+{
+    ClkList *child, *next;
+
+    QLIST_FOREACH_SAFE(child, &out->bound, node, next) {
+        if (child->clk == in) {
+            QLIST_REMOVE(child, node);
+            g_free(child);
+            object_unref(OBJECT(in));
+        }
+    }
+}
+
+void qemu_clk_set_callback(QEMUClock *clk,
+                           QEMUClkRateUpdateCallback *cb,
+                           void *opaque)
+{
+    clk->cb = cb;
+    clk->opaque = opaque;
+}
+
 void qemu_clk_device_add_clock(DeviceState *dev, QEMUClock *clk,
                                const char *name)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/10] qemu-clk: introduce an init array to help the device construction

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduces a clock init array to ease the clock tree construction.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V2 -> V3:
  * s/QEMUClock/qemu_clk/g
---
 include/qemu/qemu-clock.h | 23 +++++++++++++++++++++++
 qemu-clock.c              | 17 +++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index f80de56..1c57df4 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -49,6 +49,29 @@ struct ClkList {
     QLIST_ENTRY(ClkList) node;
 };
 
+typedef struct ClockInitElement {
+    const char *name;      /* Name to give to the clock. */
+    size_t offset;         /* Offset of the qemu_clk field in the object. */
+    QEMUClkRateUpdateCallback *cb;
+} ClockInitElement;
+
+#define DEVICE_CLOCK(_state, _field, _cb) {                                  \
+    .name = #_field,                                                         \
+    .offset = offsetof(_state, _field),                                      \
+    .cb = _cb                                                                \
+}
+
+#define DEVICE_CLOCK_END() {                                                 \
+    .name = NULL                                                             \
+}
+
+/**
+ * qemu_clk_init_device:
+ * @obj: the Object which need to be initialized.
+ * @array: the array of ClockInitElement to be used.
+ */
+void qemu_clk_init_device(Object *obj, ClockInitElement *array);
+
 /**
  * qemu_clk_device_add_clock:
  * @dev: the device on which the clock needs to be added.
diff --git a/qemu-clock.c b/qemu-clock.c
index 24719e2..8f342f6 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -26,6 +26,7 @@
 #include "hw/hw.h"
 #include "qemu/log.h"
 #include "qapi/error.h"
+#include "hw/qdev-core.h"
 
 #ifndef DEBUG_QEMU_CLOCK
 #define DEBUG_QEMU_CLOCK 0
@@ -37,6 +38,22 @@
     }                                                                        \
 } while (0);
 
+void qemu_clk_init_device(Object *obj, ClockInitElement *array)
+{
+    QEMUClock **cur = NULL;
+
+    while (array->name != NULL) {
+        DPRINTF("init clock named %s\n", array->name);
+        cur = (((void *)obj) + array->offset);
+        *cur = QEMU_CLOCK(object_new(TYPE_CLOCK));
+        qemu_clk_device_add_clock(DEVICE(obj), *cur, array->name);
+        if (array->cb) {
+            qemu_clk_set_callback(*cur, array->cb, obj);
+        }
+        array++;
+    }
+}
+
 void qemu_clk_refresh(QEMUClock *clk)
 {
     qemu_clk_update_rate(clk, clk->ref_rate);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/10] qdev-monitor: print the device's clock with info qtree

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This prints the clock attached to a DeviceState when using "info qtree" monitor
command.

For example:

bus: main-system-bus
  type System
  dev: xlnx.zynqmp_crf, id ""
    gpio-out "sysbus-irq" 1
    gpio-out "RST_A9" 4
    qemu-clk "dbg_trace" 0.0
    qemu-clk "vpll_to_lpd" 12500000.0
    qemu-clk "dp_stc_ref" 0.0
    qemu-clk "dpll_to_lpd" 12500000.0
    qemu-clk "acpu_clk" 0.0
    qemu-clk "pcie_ref" 0.0
    qemu-clk "topsw_main" 0.0
    qemu-clk "topsw_lsbus" 0.0
    qemu-clk "dp_audio_ref" 0.0
    qemu-clk "sata_ref" 0.0
    qemu-clk "dp_video_ref" 1428571.4
    qemu-clk "vpll_clk" 50000000.0
    qemu-clk "apll_to_lpd" 12500000.0
    qemu-clk "dpll_clk" 50000000.0
    qemu-clk "gpu_ref" 0.0
    qemu-clk "aux_refclk" 0.0
    qemu-clk "video_clk" 27000000.0
    qemu-clk "gdma_ref" 0.0
    qemu-clk "gt_crx_ref_clk" 0.0
    qemu-clk "dbg_fdp" 0.0
    qemu-clk "apll_clk" 50000000.0
    qemu-clk "pss_alt_ref_clk" 0.0
    qemu-clk "ddr" 0.0
    qemu-clk "pss_ref_clk" 50000000.0
    qemu-clk "dpdma_ref" 0.0
    qemu-clk "dbg_tstmp" 0.0
    mmio 00000000fd1a0000/000000000000010c

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 include/qemu/qemu-clock.h |  9 +++++++++
 qdev-monitor.c            |  2 ++
 qemu-clock.c              | 28 ++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index 1c57df4..8f0daa8 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -149,4 +149,13 @@ void qemu_clk_set_callback(QEMUClock *clk,
                            QEMUClkRateUpdateCallback *cb,
                            void *opaque);
 
+/**
+ * qemu_clk_print:
+ * @dev: the device for which the clock need to be printed.
+ *
+ * Print the clock information for a given device.
+ *
+ */
+void qemu_clk_print(Monitor *mon, DeviceState *dev, int indent);
+
 #endif /* QEMU_CLOCK_H */
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 549f45f..02f8255 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -30,6 +30,7 @@
 #include "qemu/help_option.h"
 #include "sysemu/block-backend.h"
 #include "migration/migration.h"
+#include "qemu/qemu-clock.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -699,6 +700,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
                         ngl->num_out);
         }
     }
+    qemu_clk_print(mon, dev, indent);
     class = object_get_class(OBJECT(dev));
     do {
         qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
diff --git a/qemu-clock.c b/qemu-clock.c
index 8f342f6..d5dd928 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -27,6 +27,7 @@
 #include "qemu/log.h"
 #include "qapi/error.h"
 #include "hw/qdev-core.h"
+#include "monitor/monitor.h"
 
 #ifndef DEBUG_QEMU_CLOCK
 #define DEBUG_QEMU_CLOCK 0
@@ -134,6 +135,33 @@ QEMUClock *qemu_clk_device_get_clock(DeviceState *dev, const char *name)
     return QEMU_CLOCK(clk);
 }
 
+struct print_opaque {
+    Monitor *mon;
+    int indent;
+};
+
+static int qemu_clk_print_rec(Object *obj, void *opaque)
+{
+    QEMUClock *clk = (QEMUClock *)(object_dynamic_cast(obj, TYPE_CLOCK));
+    struct print_opaque *po = opaque;
+
+    if (clk) {
+        monitor_printf(po->mon, "%*s" "qemu-clk \"%s\" %" PRIu64 "\n",
+                       po->indent, " ", clk->name, clk->rate);
+    }
+
+    return 0;
+}
+
+void qemu_clk_print(Monitor *mon, DeviceState *dev, int indent)
+{
+    struct print_opaque po;
+
+    po.indent = indent;
+    po.mon = mon;
+    object_child_foreach(OBJECT(dev), qemu_clk_print_rec, &po);
+}
+
 static const TypeInfo qemu_clk_info = {
     .name          = TYPE_CLOCK,
     .parent        = TYPE_OBJECT,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 06/10] docs: add qemu-clock documentation

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This adds the qemu-clock documentation.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>

V1 -> V2:
  * Fixed in accordance with the changes in the previous patches.
---
 docs/clock.txt | 278 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 278 insertions(+)
 create mode 100644 docs/clock.txt

diff --git a/docs/clock.txt b/docs/clock.txt
new file mode 100644
index 0000000..010ae50
--- /dev/null
+++ b/docs/clock.txt
@@ -0,0 +1,278 @@
+
+What is a QEMU_CLOCK
+====================
+
+A QEMU_CLOCK is a QOM Object developed for the purpose of modeling a clock tree
+with QEMU.
+
+It only simulates the clock by keeping a copy of the current frequency and
+doesn't model the signal itself such as pin toggle or duty cycle.
+
+It allows to model the impact of badly configured PLL, clock source selection
+or disabled clock on the models.
+
+Binding the clock together to create a tree
+===========================================
+
+In order to create a clock tree with QEMU_CLOCK two or more clock must be bound
+together. Let's say there are two clocks clk_a and clk_b:
+Using qemu_clk_bind(clk_a, clk_b) will bind clk_a and clk_b.
+
+Binding two qemu-clk together creates a unidirectional link which means that
+changing the rate of clk_a will propagate to clk_b and not the opposite.
+The binding process automatically refreshes clk_b rate.
+
+Clock can be bound and unbound during execution for modeling eg: a clock
+selector.
+
+A clock can drive more than one other clock. eg with this code:
+qemu_clk_bind(clk_a, clk_b);
+qemu_clk_bind(clk_a, clk_c);
+
+A clock rate change one clk_a will propagate to clk_b and clk_c.
+
+Implementing a callback on a rate change
+========================================
+
+The function prototype is the following:
+typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
+
+It's main goal is to modify the rate before it's passed to the next clocks in
+the tree.
+
+eg: for a 4x PLL the function will be:
+uint64_t qemu_clk_rate_change_cb(void *opaque, uint64_t rate)
+{
+    return 4 * rate;
+}
+
+To set the callback for the clock:
+void qemu_clk_set_callback(qemu_clk clk, QEMUClkRateUpdateCallback *cb,
+                           void *opaque);
+can be called.
+
+The rate update process
+=======================
+
+The rate update happen in this way:
+When a model wants to update a clock frequency (eg: based on a register change
+or something similar) it will call qemu_clk_update_rate(..) on the clock:
+  * The callback associated to the clock is called with the new rate.
+  * qemu_clk_update_rate(..) is then called on all bound clocks with the value
+    returned by the callback.
+
+NOTE: When no callback is attached, the clock qemu_clk_update_rate(..) is called
+on the next clock in the tree with the rate unmodified.
+
+Adding a QEMU_CLOCK to a DeviceState
+====================================
+
+Adding a qemu-clk to a DeviceState is required to be able to get the clock
+outside the model through qemu_clk_device_get_clock(..).
+
+It is also required to be able to print the clock and its rate with info qtree.
+For example:
+
+  type System
+  dev: xlnx.zynqmp_crf, id ""
+    gpio-out "sysbus-irq" 1
+    gpio-out "RST_A9" 4
+    qemu-clk "dbg_trace" 0
+    qemu-clk "vpll_to_lpd" 625000000
+    qemu-clk "dp_stc_ref" 0
+    qemu-clk "dpll_to_lpd" 12500000
+    qemu-clk "acpu_clk" 0
+    qemu-clk "pcie_ref" 0
+    qemu-clk "topsw_main" 0
+    qemu-clk "topsw_lsbus" 0
+    qemu-clk "dp_audio_ref" 0
+    qemu-clk "sata_ref" 0
+    qemu-clk "dp_video_ref" 71428568
+    qemu-clk "vpll_clk" 2500000000
+    qemu-clk "apll_to_lpd" 12500000
+    qemu-clk "dpll_clk" 50000000
+    qemu-clk "gpu_ref" 0
+    qemu-clk "aux_refclk" 0
+    qemu-clk "video_clk" 27000000
+    qemu-clk "gdma_ref" 0
+    qemu-clk "gt_crx_ref_clk" 0
+    qemu-clk "dbg_fdp" 0
+    qemu-clk "apll_clk" 50000000
+    qemu-clk "pss_alt_ref_clk" 0
+    qemu-clk "ddr" 0
+    qemu-clk "pss_ref_clk" 50000000
+    qemu-clk "dpdma_ref" 0
+    qemu-clk "dbg_tstmp" 0
+    mmio 00000000fd1a0000/000000000000010c
+
+This way a DeviceState can have multiple clock input or output.
+
+Examples
+========
+
+Those are the different way of using the QEMUClock object.
+
+Modelling a fixed clock generator
+=================================
+
+Here is a brief example of a device acting as a clock source:
+
+typedef struct {
+    DeviceState parent_obj;
+
+    uint32_t rate;
+    QEMUClock out;
+} FixedClock;
+
+During the initialization the device must initialize its clock object:
+
+static void fixed_clock_instance_init(Object *obj)
+{
+    FixedClock *s = FIXED_CLOCK(obj);
+
+    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
+    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
+}
+
+As the device acts as a clock source it must refresh the clock tree during the
+realize phase:
+
+static void fixed_clock_realizefn(DeviceState *dev, Error **errp)
+{
+    FixedClock *s = FIXED_CLOCK(dev);
+
+    qemu_clk_update_rate(&s->out, s->rate);
+}
+
+This means that the clock tree must be finished before realize is called on the
+fixed clock.
+
+Modelling a clock user device
+=============================
+
+Here is a brief example of a clock user:
+
+typedef struct {
+    DeviceState parent_obj;
+
+    QEMUClock in;
+} ClockUser;
+
+As before the clock must be initialized through the device initialize function:
+
+static void clock_user_instance_init(Object *obj)
+{
+    ClockUser *s = CLOCK_USER(obj);
+
+    object_initialize(&s->in, sizeof(s->in), TYPE_CLOCK);
+    qemu_clk_device_add_clock(DEVICE(obj), &s->in, "clk_in");
+    /*
+     * Call on_rate_change_cb when something change on clk_in.
+     */
+    qemu_clk_set_callback(s->in, on_rate_change_cb, obj);
+}
+
+The callback is in this case used as a notifier when the clock tree which
+sources the device change:
+
+static uint64_t on_rate_change_cb(void *opaque, uint64_t input_rate)
+{
+    printf("the new rate is %ld\n", input_rate);
+
+    /* The return is ignored if nothing is bound to clk_in. */
+    return input_rate;
+}
+
+Modelling a clock multiplier
+============================
+
+Here is a brief example of a device acting as a clock modifier:
+
+typedef struct {
+    DeviceState parent_obj;
+
+    uint32_t rate;
+    QEMUClock out;
+    QEMUClock in;
+} ClockMultiplier;
+
+As before the clocks must be initialized through the device initialize function
+but they must be bound together so a change on the input will propagate on the
+output:
+
+static void clock_multiplier_instance_init(Object *obj)
+{
+    ClockMultiplier *s = CLOCK_MULTIPLIER(obj);
+
+    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
+    object_initialize(&s->in, sizeof(s->in), TYPE_CLOCK);
+
+    qemu_clk_device_add_clock(DEVICE(obj), &s->in, "clk_in");
+    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
+    /*
+     * Propagate the change from in to out, this can be done dynamically during
+     * the simulation but we need to do the initial binding here to get the
+     * initial refresh happening when the realize function is called on the
+     * fixed clock.
+     */
+    qemu_clk_bind(s->in, s->out);
+    /*
+     * But before propagating the rate modify it with multiplier_cb.
+     */
+    qemu_clk_set_callback(s->out, multiplier_cb, obj);
+}
+
+In this example when clk_in changes it will trigger a change on clk_out and this
+callback can modify the rate of clk_out (the return value of this callback)
+accordingly to clk_in rate (input_rate).
+
+static uint64_t multiplier_cb(void *opaque, uint64_t input_rate)
+{
+    return input_rate * 4;
+}
+
+This device doesn't refresh the clock tree as it will be done by the clock tree
+source.
+
+Modelling a clock selector
+==========================
+
+Here is a brief example of a device acting as a device which alter the clock
+topology such as a clock selector:
+
+typedef struct {
+    DeviceState parent_obj;
+
+    QEMUClock out;
+    QEMUClock in_a;
+    QEMUClock in_b;
+} ClockSelector;
+
+The clocks must be initialized through the device initialize function but they
+must be bound together like they will be when the device is reset so a change on
+the input during the realize of the fixed clock will propagate to the output:
+
+static void clock_selector_instance_init(Object *obj)
+{
+    ClockSelector *s = CLOCK_SELECTOR(obj);
+
+    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
+    object_initialize(&s->in_a, sizeof(s->in_a), TYPE_CLOCK);
+    object_initialize(&s->in_b, sizeof(s->in_b), TYPE_CLOCK);
+
+    qemu_clk_device_add_clock(DEVICE(obj), &s->in_a, "clk_in_a");
+    qemu_clk_device_add_clock(DEVICE(obj), &s->in_b, "clk_in_b");
+    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
+
+    /* Assuming at the reset that the input_a is connected to output. */
+    qemu_clk_bind(s->in_a, s->out);
+}
+
+/* This is called for example by a register change or something like that */
+void update_topology(ClockSelector *s)
+{
+    /* Unbind the old clock */
+    qemu_clk_unbind(s->in_a, s->out);
+    /* Bind the new one, the rate is automatically refreshed. */
+    qemu_clk_bind(s->in_b, s->out);
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 07/10] introduce fixed-clock

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This is a fixed clock device.
It justs behave as an empty device with a parametrable output rate.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/misc/Makefile.objs         |  1 +
 hw/misc/fixed-clock.c         | 88 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/fixed-clock.h | 30 +++++++++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 hw/misc/fixed-clock.c
 create mode 100644 include/hw/misc/fixed-clock.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 898e4cc..e9c4dd3 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -55,3 +55,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-y += fixed-clock.o
diff --git a/hw/misc/fixed-clock.c b/hw/misc/fixed-clock.c
new file mode 100644
index 0000000..b5af24d
--- /dev/null
+++ b/hw/misc/fixed-clock.c
@@ -0,0 +1,88 @@
+/*
+ * Fixed clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "hw/misc/fixed-clock.h"
+#include "qemu/qemu-clock.h"
+#include "qapi/error.h"
+
+#ifndef DEBUG_FIXED_CLOCK
+#define DEBUG_FIXED_CLOCK 0
+#endif
+
+#define DPRINTF(fmt, ...) do {                                               \
+    if (DEBUG_FIXED_CLOCK) {                                                 \
+        qemu_log(__FILE__": " fmt , ## __VA_ARGS__);                         \
+    }                                                                        \
+} while (0);
+
+typedef struct {
+    DeviceState parent_obj;
+
+    uint32_t rate;
+    QEMUClock out;
+} FixedClock;
+
+static Property fixed_clock_properties[] = {
+    DEFINE_PROP_UINT32("rate", FixedClock, rate, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void fixed_clock_realizefn(DeviceState *dev, Error **errp)
+{
+    FixedClock *s = FIXED_CLOCK(dev);
+
+    qemu_clk_update_rate(&s->out, s->rate);
+}
+
+static void fixed_clock_instance_init(Object *obj)
+{
+    FixedClock *s = FIXED_CLOCK(obj);
+
+    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
+    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
+}
+
+static void fixed_clock_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = fixed_clock_realizefn;
+    dc->props = fixed_clock_properties;
+}
+
+static const TypeInfo fixed_clock_info = {
+    .name          = TYPE_FIXED_CLOCK,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(FixedClock),
+    .instance_init = fixed_clock_instance_init,
+    .class_init    = fixed_clock_class_init,
+};
+
+static void fixed_clock_register_types(void)
+{
+    type_register_static(&fixed_clock_info);
+}
+
+type_init(fixed_clock_register_types);
diff --git a/include/hw/misc/fixed-clock.h b/include/hw/misc/fixed-clock.h
new file mode 100644
index 0000000..1376444
--- /dev/null
+++ b/include/hw/misc/fixed-clock.h
@@ -0,0 +1,30 @@
+/*
+ * Fixed clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *      http://www.greensocs.com/ , email: info@greensocs.com
+ *
+ *  Frederic Konrad   <fred.konrad@greensocs.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef FIXED_CLOCK_H
+#define FIXED_CLOCK_H
+
+#define TYPE_FIXED_CLOCK "fixed-clock"
+#define FIXED_CLOCK(obj) OBJECT_CHECK(FixedClock, (obj), TYPE_FIXED_CLOCK)
+
+#endif /* FIXED_CLOCK_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/10] introduce zynqmp_crf

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This introduce Xilinx zynqmp-crf.
It is extracted from the qemu xilinx tree (02d2f0203dd489ed30d9c8d90c14a52c57332b25) and is used as
an example for the clock framework.
---
 hw/misc/Makefile.objs       |   1 +
 hw/misc/xilinx_zynqmp_crf.c | 968 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 969 insertions(+)
 create mode 100644 hw/misc/xilinx_zynqmp_crf.c

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index e9c4dd3..92cdb40 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -45,6 +45,7 @@ obj-$(CONFIG_RASPI) += bcm2835_property.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
 obj-$(CONFIG_ZYNQ) += zynq-xadc.o
+obj-$(CONFIG_ZYNQ) += xilinx_zynqmp_crf.o
 obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
 obj-$(CONFIG_MIPS_CPS) += mips_cmgcr.o
 obj-$(CONFIG_MIPS_CPS) += mips_cpc.o
diff --git a/hw/misc/xilinx_zynqmp_crf.c b/hw/misc/xilinx_zynqmp_crf.c
new file mode 100644
index 0000000..fa3a9cb
--- /dev/null
+++ b/hw/misc/xilinx_zynqmp_crf.c
@@ -0,0 +1,968 @@
+/*
+ * QEMU model of the CRF_APB APB control registers for clock controller. The
+ * RST_ctrl_fpd will be added to this as well
+ *
+ * Copyright (c) 2014 Xilinx Inc.
+ *
+ * Autogenerated by xregqemu.py 2014-01-22.
+ *
+ * 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 "qemu/qemu-clock.h"
+
+#ifndef XILINX_CRF_APB_ERR_DEBUG
+#define XILINX_CRF_APB_ERR_DEBUG 0
+#endif
+
+#define TYPE_XILINX_CRF_APB "xlnx.zynqmp_crf"
+
+#define XILINX_CRF_APB(obj) \
+     OBJECT_CHECK(CRF_APB, (obj), TYPE_XILINX_CRF_APB)
+
+REG32(ERR_CTRL, 0x0)
+    FIELD(ERR_CTRL, SLVERR_ENABLE, 0, 1)
+REG32(IR_STATUS, 0x4)
+    FIELD(IR_STATUS, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_MASK, 0x8)
+    FIELD(IR_MASK, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_ENABLE, 0xc)
+    FIELD(IR_ENABLE, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_DISABLE, 0x10)
+    FIELD(IR_DISABLE, ADDR_DECODE_ERR, 0, 1)
+REG32(CRF_ECO, 0x18)
+REG32(APLL_CTRL, 0x20)
+    FIELD(APLL_CTRL, POST_SRC, 24, 3)
+    FIELD(APLL_CTRL, PRE_SRC, 20, 3)
+    FIELD(APLL_CTRL, CLKOUTDIV, 17, 1)
+    FIELD(APLL_CTRL, DIV2, 16, 1)
+    FIELD(APLL_CTRL, FBDIV, 8, 7)
+    FIELD(APLL_CTRL, BYPASS, 3, 1)
+    FIELD(APLL_CTRL, RESET, 0, 1)
+REG32(APLL_CFG, 0x24)
+    FIELD(APLL_CFG, LOCK_DLY, 25, 7)
+    FIELD(APLL_CFG, LOCK_CNT, 13, 10)
+    FIELD(APLL_CFG, LFHF, 10, 2)
+    FIELD(APLL_CFG, CP, 5, 4)
+    FIELD(APLL_CFG, RES, 0, 4)
+REG32(APLL_FRAC_CFG, 0x28)
+    FIELD(APLL_FRAC_CFG, ENABLED, 31, 1)
+    FIELD(APLL_FRAC_CFG, SEED, 22, 3)
+    FIELD(APLL_FRAC_CFG, ALGRTHM, 19, 1)
+    FIELD(APLL_FRAC_CFG, ORDER, 18, 1)
+    FIELD(APLL_FRAC_CFG, DATA, 0, 16)
+REG32(DPLL_CTRL, 0x2c)
+    FIELD(DPLL_CTRL, POST_SRC, 24, 3)
+    FIELD(DPLL_CTRL, PRE_SRC, 20, 3)
+    FIELD(DPLL_CTRL, CLKOUTDIV, 17, 1)
+    FIELD(DPLL_CTRL, DIV2, 16, 1)
+    FIELD(DPLL_CTRL, FBDIV, 8, 7)
+    FIELD(DPLL_CTRL, BYPASS, 3, 1)
+    FIELD(DPLL_CTRL, RESET, 0, 1)
+REG32(DPLL_CFG, 0x30)
+    FIELD(DPLL_CFG, LOCK_DLY, 25, 7)
+    FIELD(DPLL_CFG, LOCK_CNT, 13, 10)
+    FIELD(DPLL_CFG, LFHF, 10, 2)
+    FIELD(DPLL_CFG, CP, 5, 4)
+    FIELD(DPLL_CFG, RES, 0, 4)
+REG32(DPLL_FRAC_CFG, 0x34)
+    FIELD(DPLL_FRAC_CFG, ENABLED, 31, 1)
+    FIELD(DPLL_FRAC_CFG, SEED, 22, 3)
+    FIELD(DPLL_FRAC_CFG, ALGRTHM, 19, 1)
+    FIELD(DPLL_FRAC_CFG, ORDER, 18, 1)
+    FIELD(DPLL_FRAC_CFG, DATA, 0, 16)
+REG32(VPLL_CTRL, 0x38)
+    FIELD(VPLL_CTRL, POST_SRC, 24, 3)
+    FIELD(VPLL_CTRL, PRE_SRC, 20, 3)
+    FIELD(VPLL_CTRL, CLKOUTDIV, 17, 1)
+    FIELD(VPLL_CTRL, DIV2, 16, 1)
+    FIELD(VPLL_CTRL, FBDIV, 8, 7)
+    FIELD(VPLL_CTRL, BYPASS, 3, 1)
+    FIELD(VPLL_CTRL, RESET, 0, 1)
+REG32(VPLL_CFG, 0x3c)
+    FIELD(VPLL_CFG, LOCK_DLY, 25, 7)
+    FIELD(VPLL_CFG, LOCK_CNT, 13, 10)
+    FIELD(VPLL_CFG, LFHF, 10, 2)
+    FIELD(VPLL_CFG, CP, 5, 4)
+    FIELD(VPLL_CFG, RES, 0, 4)
+REG32(VPLL_FRAC_CFG, 0x40)
+    FIELD(VPLL_FRAC_CFG, ENABLED, 31, 1)
+    FIELD(VPLL_FRAC_CFG, SEED, 22, 3)
+    FIELD(VPLL_FRAC_CFG, ALGRTHM, 19, 1)
+    FIELD(VPLL_FRAC_CFG, ORDER, 18, 1)
+    FIELD(VPLL_FRAC_CFG, DATA, 0, 16)
+REG32(PLL_STATUS, 0x44)
+    FIELD(PLL_STATUS, VPLL_STABLE, 5, 1)
+    FIELD(PLL_STATUS, DPLL_STABLE, 4, 1)
+    FIELD(PLL_STATUS, APLL_STABLE, 3, 1)
+    FIELD(PLL_STATUS, VPLL_LOCK, 2, 1)
+    FIELD(PLL_STATUS, DPLL_LOCK, 1, 1)
+    FIELD(PLL_STATUS, APLL_LOCK, 0, 1)
+REG32(APLL_TO_LPD_CTRL, 0x48)
+    FIELD(APLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(DPLL_TO_LPD_CTRL, 0x4c)
+    FIELD(DPLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(VPLL_TO_LPD_CTRL, 0x50)
+    FIELD(VPLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(CPU_A9_CTRL, 0x60)
+    FIELD(CPU_A9_CTRL, A9CLKSTOP, 26, 2)
+    FIELD(CPU_A9_CTRL, CLKACT_HALF, 25, 1)
+    FIELD(CPU_A9_CTRL, CLKACT_FULL, 24, 1)
+    FIELD(CPU_A9_CTRL, DIVISOR0, 8, 6)
+    FIELD(CPU_A9_CTRL, SRCSEL, 0, 3)
+REG32(DBG_TRACE_CTRL, 0x64)
+    FIELD(DBG_TRACE_CTRL, CLKACT, 24, 1)
+    FIELD(DBG_TRACE_CTRL, DIVISOR0, 8, 6)
+    FIELD(DBG_TRACE_CTRL, SRCSEL, 0, 3)
+REG32(DBG_FPD_CTRL, 0x68)
+    FIELD(DBG_FPD_CTRL, CLKACT, 24, 1)
+    FIELD(DBG_FPD_CTRL, DIVISOR0, 8, 6)
+    FIELD(DBG_FPD_CTRL, SRCSEL, 0, 3)
+REG32(DP_VIDEO_REF_CTRL, 0x70)
+    FIELD(DP_VIDEO_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DP_VIDEO_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DP_VIDEO_REF_CTRL, SRCSEL, 0, 3)
+REG32(DP_AUDIO_REF_CTRL, 0x74)
+    FIELD(DP_AUDIO_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DP_AUDIO_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DP_AUDIO_REF_CTRL, SRCSEL, 0, 3)
+REG32(DP_LINK_REF_CTRL, 0x78)
+    FIELD(DP_LINK_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DP_LINK_REF_CTRL, DIVISOR1, 16, 6)
+    FIELD(DP_LINK_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DP_LINK_REF_CTRL, SRCSEL, 0, 3)
+REG32(DP_STC_REF_CTRL, 0x7c)
+    FIELD(DP_STC_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DP_STC_REF_CTRL, DIVISOR1, 16, 6)
+    FIELD(DP_STC_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DP_STC_REF_CTRL, SRCSEL, 0, 3)
+REG32(DDR_CTRL, 0x80)
+    FIELD(DDR_CTRL, CLKACT, 24, 1)
+    FIELD(DDR_CTRL, DIVISOR0, 8, 6)
+    FIELD(DDR_CTRL, SRCSEL, 0, 3)
+REG32(GPU_REF_CTRL, 0x84)
+    FIELD(GPU_REF_CTRL, PP1_CLKACT, 26, 1)
+    FIELD(GPU_REF_CTRL, PP0_CLKACT, 25, 1)
+    FIELD(GPU_REF_CTRL, CLKACT, 24, 1)
+    FIELD(GPU_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(GPU_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI0_REF_CTRL, 0x88)
+    FIELD(AFI0_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI0_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI0_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI1_REF_CTRL, 0x8c)
+    FIELD(AFI1_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI1_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI1_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI2_REF_CTRL, 0x90)
+    FIELD(AFI2_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI2_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI2_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI3_REF_CTRL, 0x94)
+    FIELD(AFI3_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI3_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI3_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI4_REF_CTRL, 0x98)
+    FIELD(AFI4_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI4_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI4_REF_CTRL, SRCSEL, 0, 3)
+REG32(AFI5_REF_CTRL, 0x9c)
+    FIELD(AFI5_REF_CTRL, CLKACT, 24, 1)
+    FIELD(AFI5_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(AFI5_REF_CTRL, SRCSEL, 0, 3)
+REG32(SATA_REF_CTRL, 0xa0)
+    FIELD(SATA_REF_CTRL, CLKACT, 24, 1)
+    FIELD(SATA_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(SATA_REF_CTRL, SRCSEL, 0, 3)
+REG32(PCIE_REF_CTRL, 0xb4)
+    FIELD(PCIE_REF_CTRL, CLKACT, 24, 1)
+    FIELD(PCIE_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(PCIE_REF_CTRL, SRCSEL, 0, 3)
+REG32(GDMA_REF_CTRL, 0xb8)
+    FIELD(GDMA_REF_CTRL, CLKACT, 24, 1)
+    FIELD(GDMA_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(GDMA_REF_CTRL, SRCSEL, 0, 3)
+REG32(DPDMA_REF_CTRL, 0xbc)
+    FIELD(DPDMA_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DPDMA_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DPDMA_REF_CTRL, SRCSEL, 0, 3)
+REG32(TOPSW_MAIN_CTRL, 0xc0)
+    FIELD(TOPSW_MAIN_CTRL, CLKACT, 24, 1)
+    FIELD(TOPSW_MAIN_CTRL, DIVISOR0, 8, 6)
+    FIELD(TOPSW_MAIN_CTRL, SRCSEL, 0, 3)
+REG32(TOPSW_LSBUS_CTRL, 0xc4)
+    FIELD(TOPSW_LSBUS_CTRL, CLKACT, 24, 1)
+    FIELD(TOPSW_LSBUS_CTRL, DIVISOR0, 8, 6)
+    FIELD(TOPSW_LSBUS_CTRL, SRCSEL, 0, 3)
+REG32(GTGREF0_REF_CTRL, 0xc8)
+    FIELD(GTGREF0_REF_CTRL, CLKACT, 24, 1)
+    FIELD(GTGREF0_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(GTGREF0_REF_CTRL, SRCSEL, 0, 3)
+REG32(GTGREF1_REF_CTRL, 0xcc)
+    FIELD(GTGREF1_REF_CTRL, CLKACT, 24, 1)
+    FIELD(GTGREF1_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(GTGREF1_REF_CTRL, SRCSEL, 0, 3)
+REG32(DFT300_REF_CTRL, 0xd0)
+    FIELD(DFT300_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DFT300_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DFT300_REF_CTRL, SRCSEL, 0, 3)
+REG32(DFT270_REF_CTRL, 0xd4)
+    FIELD(DFT270_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DFT270_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DFT270_REF_CTRL, SRCSEL, 0, 3)
+REG32(DFT250_REF_CTRL, 0xd8)
+    FIELD(DFT250_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DFT250_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DFT250_REF_CTRL, SRCSEL, 0, 3)
+REG32(DFT125_REF_CTRL, 0xdc)
+    FIELD(DFT125_REF_CTRL, CLKACT, 24, 1)
+    FIELD(DFT125_REF_CTRL, DIVISOR0, 8, 6)
+    FIELD(DFT125_REF_CTRL, SRCSEL, 0, 3)
+
+REG32(RST_FPD_TOP, 0x100)
+    FIELD(RST_FPD_TOP, PCIE_BRDG_RESET, 18, 1)
+    FIELD(RST_FPD_TOP, PCIE_CTRL_RESET, 17, 1)
+    FIELD(RST_FPD_TOP, DP_RESET, 16, 1)
+    FIELD(RST_FPD_TOP, AFI_FM5_RESET, 12, 1)
+    FIELD(RST_FPD_TOP, AFI_FM4_RESET, 11, 1)
+    FIELD(RST_FPD_TOP, AFI_FM3_RESET, 10, 1)
+    FIELD(RST_FPD_TOP, AFI_FM2_RESET, 9, 1)
+    FIELD(RST_FPD_TOP, AFI_FM1_RESET, 8, 1)
+    FIELD(RST_FPD_TOP, AFI_FM0_RESET, 7, 1)
+    FIELD(RST_FPD_TOP, GDMA_RESET, 6, 1)
+    FIELD(RST_FPD_TOP, GPU_PP1_RESET, 5, 1)
+    FIELD(RST_FPD_TOP, GPU_PP0_RESET, 4, 1)
+    FIELD(RST_FPD_TOP, GPU_RESET, 3, 1)
+    FIELD(RST_FPD_TOP, GT_RESET, 2, 1)
+    FIELD(RST_FPD_TOP, SATA_RESET, 1, 1)
+REG32(RST_FPD_APU, 0x104)
+    FIELD(RST_FPD_APU, PERI_RESET, 13, 1)
+    FIELD(RST_FPD_APU, SCU_RESET, 12, 1)
+    FIELD(RST_FPD_APU, CPU1_AWDT_RESET, 9, 1)
+    FIELD(RST_FPD_APU, CPU0_AWDT_RESET, 8, 1)
+    FIELD(RST_FPD_APU, CPU1_CP14_RESET, 5, 1)
+    FIELD(RST_FPD_APU, CPU0_CP14_RESET, 4, 1)
+    FIELD(RST_FPD_APU, CPU3_A9_RESET, 3, 1)
+    FIELD(RST_FPD_APU, CPU2_A9_RESET, 2, 1)
+    FIELD(RST_FPD_APU, CPU1_A9_RESET, 1, 1)
+    FIELD(RST_FPD_APU, CPU0_A9_RESET, 0, 1)
+REG32(RST_DDR_SS, 0x108)
+    FIELD(RST_DDR_SS, DDR_RESET, 3, 1)
+    FIELD(RST_DDR_SS, APM_RESET, 2, 1)
+    FIELD(RST_DDR_SS, QOS_RESET, 1, 1)
+    FIELD(RST_DDR_SS, XMPU_RESET, 0, 1)
+
+#define R_MAX (R_RST_DDR_SS + 1)
+
+typedef struct CRF_APB {
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+    qemu_irq irq_ir;
+
+    uint32_t regs[R_MAX];
+    RegisterInfo regs_info[R_MAX];
+
+    /* input clocks */
+    QEMUClock *pss_ref_clk;
+    QEMUClock *video_clk;
+    QEMUClock *pss_alt_ref_clk;
+    QEMUClock *aux_refclk;
+    QEMUClock *gt_crx_ref_clk;
+
+    /* internal clocks */
+    QEMUClock *apll_clk;
+    QEMUClock *dpll_clk;
+    QEMUClock *vpll_clk;
+
+    /* output clocks */
+    QEMUClock *acpu_clk;
+    QEMUClock *dbg_trace;
+    QEMUClock *dbg_fdp;
+    QEMUClock *dp_video_ref;
+    QEMUClock *dp_audio_ref;
+    QEMUClock *dp_stc_ref;
+    QEMUClock *ddr;
+    QEMUClock *gpu_ref;
+    QEMUClock *sata_ref;
+    QEMUClock *pcie_ref;
+    QEMUClock *gdma_ref;
+    QEMUClock *dpdma_ref;
+    QEMUClock *topsw_main;
+    QEMUClock *topsw_lsbus;
+    QEMUClock *dbg_tstmp;
+    QEMUClock *apll_to_lpd;
+    QEMUClock *dpll_to_lpd;
+    QEMUClock *vpll_to_lpd;
+} CRF_APB;
+
+static uint64_t apll_to_lpd_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t dpll_to_lpd_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t vpll_to_lpd_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t apll_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t dpll_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t vpll_update_rate(void *opaque, uint64_t input_rate);
+static uint64_t dp_video_update_rate(void *opaque, uint64_t input_rate);
+
+/* Clock array */
+ClockInitElement crf_clock[] = {
+    /* input clocks */
+    DEVICE_CLOCK(CRF_APB, pss_ref_clk, NULL),
+    DEVICE_CLOCK(CRF_APB, video_clk, NULL),
+    DEVICE_CLOCK(CRF_APB, pss_alt_ref_clk, NULL),
+    DEVICE_CLOCK(CRF_APB, aux_refclk, NULL),
+    DEVICE_CLOCK(CRF_APB, gt_crx_ref_clk, NULL),
+    /* internal clocks */
+    DEVICE_CLOCK(CRF_APB, apll_clk, apll_update_rate),
+    DEVICE_CLOCK(CRF_APB, dpll_clk, dpll_update_rate),
+    DEVICE_CLOCK(CRF_APB, vpll_clk, vpll_update_rate),
+    /* output clocks */
+    DEVICE_CLOCK(CRF_APB, acpu_clk, NULL),
+    DEVICE_CLOCK(CRF_APB, dbg_trace,  NULL),
+    DEVICE_CLOCK(CRF_APB, dbg_fdp, NULL),
+    DEVICE_CLOCK(CRF_APB, dp_video_ref, dp_video_update_rate),
+    DEVICE_CLOCK(CRF_APB, dp_audio_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, dp_stc_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, ddr, NULL),
+    DEVICE_CLOCK(CRF_APB, gpu_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, sata_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, pcie_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, gdma_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, dpdma_ref, NULL),
+    DEVICE_CLOCK(CRF_APB, topsw_main, NULL),
+    DEVICE_CLOCK(CRF_APB, topsw_lsbus, NULL),
+    DEVICE_CLOCK(CRF_APB, dbg_tstmp, NULL),
+    DEVICE_CLOCK(CRF_APB, apll_to_lpd, apll_to_lpd_update_rate),
+    DEVICE_CLOCK(CRF_APB, dpll_to_lpd, dpll_to_lpd_update_rate),
+    DEVICE_CLOCK(CRF_APB, vpll_to_lpd, vpll_to_lpd_update_rate),
+    DEVICE_CLOCK_END()
+};
+
+static const MemoryRegionOps crf_apb_ops = {
+    .read = register_read_memory,
+    .write = register_write_memory,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void ir_update_irq(CRF_APB *s)
+{
+    bool pending = s->regs[R_IR_STATUS] & ~s->regs[R_IR_MASK];
+    qemu_set_irq(s->irq_ir, pending);
+}
+
+static void ir_status_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    ir_update_irq(s);
+}
+
+static uint64_t ir_enable_prew(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_IR_MASK] &= ~val;
+    ir_update_irq(s);
+    return 0;
+}
+
+static uint64_t ir_disable_prew(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_IR_MASK] |= val;
+    ir_update_irq(s);
+    return 0;
+}
+
+enum clk_src {
+    VIDEO_CLK = 4,
+    PSS_ALT_REF_CLK = 5,
+    AUX_REF_CLK = 6,
+    GT_CRX_REF_CLK = 7,
+    PSS_REF_CLK = 0
+};
+
+static void apll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+    qemu_clk_refresh(s->apll_to_lpd);
+}
+
+static uint64_t apll_to_lpd_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    uint32_t divisor = FIELD_EX32(s->regs[R_APLL_TO_LPD_CTRL],
+                                  APLL_TO_LPD_CTRL,
+                                  DIVISOR0);
+
+    if (!divisor) {
+        return 0.0f;
+    } else {
+        return input_rate / (float)divisor;
+    }
+}
+
+static void dpll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+    qemu_clk_refresh(s->dpll_to_lpd);
+}
+
+static uint64_t dpll_to_lpd_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    uint32_t divisor = FIELD_EX32(s->regs[R_DPLL_TO_LPD_CTRL],
+                                  DPLL_TO_LPD_CTRL,
+                                  DIVISOR0);
+
+    if (!divisor) {
+        return 0.0f;
+    } else {
+        return input_rate / (float)divisor;
+    }
+}
+
+static void vpll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+    qemu_clk_refresh(s->vpll_to_lpd);
+}
+
+static uint64_t vpll_to_lpd_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    uint32_t divisor = FIELD_EX32(s->regs[R_VPLL_TO_LPD_CTRL],
+                                  VPLL_TO_LPD_CTRL, DIVISOR0);
+
+    if (!divisor) {
+        return 0;
+    } else {
+        return input_rate / (float)divisor;
+    }
+}
+
+static void apll_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t source = FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, BYPASS)
+                    ? FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, POST_SRC)
+                    : FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, PRE_SRC);
+
+    /*
+     * We must ensure that only one clock is bound to the apll internal clock.
+     */
+    qemu_clk_unbind(s->pss_ref_clk, s->apll_clk);
+    qemu_clk_unbind(s->video_clk, s->apll_clk);
+    qemu_clk_unbind(s->pss_alt_ref_clk, s->apll_clk);
+    qemu_clk_unbind(s->aux_refclk, s->apll_clk);
+    qemu_clk_unbind(s->gt_crx_ref_clk, s->apll_clk);
+
+    switch (source) {
+    case VIDEO_CLK:
+        qemu_clk_bind(s->video_clk, s->apll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind(s->pss_alt_ref_clk, s->apll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind(s->aux_refclk, s->apll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind(s->gt_crx_ref_clk, s->apll_clk);
+        break;
+    default:
+        qemu_clk_bind(s->pss_ref_clk, s->apll_clk);
+        break;
+    }
+}
+
+static void dpll_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t source = FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, BYPASS)
+                    ? FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, POST_SRC)
+                    : FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, PRE_SRC);
+
+    /*
+     * We must ensure that only one clock is bound to the dpll internal clock.
+     */
+    qemu_clk_unbind(s->pss_ref_clk, s->dpll_clk);
+    qemu_clk_unbind(s->video_clk, s->dpll_clk);
+    qemu_clk_unbind(s->pss_alt_ref_clk, s->dpll_clk);
+    qemu_clk_unbind(s->aux_refclk, s->dpll_clk);
+    qemu_clk_unbind(s->gt_crx_ref_clk, s->dpll_clk);
+
+    switch (source) {
+    case VIDEO_CLK:
+        qemu_clk_bind(s->video_clk, s->dpll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind(s->pss_alt_ref_clk, s->dpll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind(s->aux_refclk, s->dpll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind(s->gt_crx_ref_clk, s->dpll_clk);
+        break;
+    default:
+        qemu_clk_bind(s->pss_ref_clk, s->dpll_clk);
+        break;
+    }
+}
+
+static void vpll_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t source = FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, BYPASS)
+                    ? FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, POST_SRC)
+                    : FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, PRE_SRC);
+
+    /*
+     * We must ensure that only one clock is bound to the vpll internal clock.
+     */
+    qemu_clk_unbind(s->pss_ref_clk, s->vpll_clk);
+    qemu_clk_unbind(s->video_clk, s->vpll_clk);
+    qemu_clk_unbind(s->pss_alt_ref_clk, s->vpll_clk);
+    qemu_clk_unbind(s->aux_refclk, s->vpll_clk);
+    qemu_clk_unbind(s->gt_crx_ref_clk, s->vpll_clk);
+
+    switch (source) {
+    case VIDEO_CLK:
+        qemu_clk_bind(s->video_clk, s->vpll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind(s->pss_alt_ref_clk, s->vpll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind(s->aux_refclk, s->vpll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind(s->gt_crx_ref_clk, s->vpll_clk);
+        break;
+    default:
+        qemu_clk_bind(s->pss_ref_clk, s->vpll_clk);
+        break;
+    }
+}
+
+/*
+ * This happen when apll get updated.
+ * As we ensure that only one clk_pin can drive apll we can just do the
+ * computation from input_rate.
+ */
+static uint64_t apll_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    bool bypass = FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, BYPASS);
+    bool reset = FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, RESET);
+    float div2 = FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, DIV2) ? 0.5f
+                                                                   : 1.0f;
+    float integer = (float)(FIELD_EX32(s->regs[R_APLL_CTRL],
+                                       APLL_CTRL, FBDIV));
+    float frac = FIELD_EX32(s->regs[R_APLL_FRAC_CFG], APLL_FRAC_CFG, ENABLED)
+                  ? (float)(FIELD_EX32(s->regs[R_APLL_FRAC_CFG],
+                                       APLL_FRAC_CFG, DATA))
+                  / 65536.0f
+                  : 0.0f;
+
+    if (bypass) {
+        return input_rate;
+    } else {
+        if (reset) {
+            /*
+             * This is not supposed to happen user must ensure that BYPASS is
+             * set before the PLL are reset.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "APLL is reseted but not bypassed.");
+            return 0;
+        } else {
+            return input_rate * div2 * (integer + frac);
+        }
+    }
+}
+
+/*
+ * This happen when dpll get updated.
+ * As we ensure that only one clk_pin can drive dpll we can just do the
+ * computation from input_rate.
+ */
+static uint64_t dpll_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    bool bypass = FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, BYPASS);
+    bool reset = FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, RESET);
+    float div2 = FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL, DIV2) ? 0.5f
+                                                                 : 1.0f;
+    float integer = (float)(FIELD_EX32(s->regs[R_DPLL_CTRL], DPLL_CTRL,
+                                       FBDIV));
+    float frac = FIELD_EX32(s->regs[R_DPLL_FRAC_CFG], DPLL_FRAC_CFG, ENABLED)
+                  ? (float)(FIELD_EX32(s->regs[R_DPLL_FRAC_CFG],
+                                       DPLL_FRAC_CFG, DATA))
+                  / 65536.0f
+                  : 0.0f;
+
+    if (bypass) {
+        return input_rate;
+    } else {
+        if (reset) {
+            /*
+             * This is not supposed to happen user must ensure that BYPASS is
+             * set before the PLL are reset.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "DPLL is reseted but not bypassed.");
+            return 0;
+        } else {
+            return input_rate * div2 * (integer + frac);
+        }
+    }
+}
+
+/*
+ * This happen when vpll get updated.
+ * As we ensure that only one clk_pin can drive vpll we can just do the
+ * computation from input_rate.
+ */
+static uint64_t vpll_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    bool bypass = FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, BYPASS);
+    bool reset = FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, RESET);
+    float div2 = FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL, DIV2) ? 0.5f
+                                                                   : 1.0f;
+    float integer = (float)(FIELD_EX32(s->regs[R_VPLL_CTRL], VPLL_CTRL,
+                                       FBDIV));
+    float frac = FIELD_EX32(s->regs[R_VPLL_FRAC_CFG], VPLL_FRAC_CFG, ENABLED)
+                  ? (float)(FIELD_EX32(s->regs[R_VPLL_FRAC_CFG],
+                                       VPLL_FRAC_CFG, DATA))
+                  / 65536.0f
+                  : 0.0f;
+
+    if (bypass) {
+        return input_rate;
+    } else {
+        if (reset) {
+            /*
+             * This is not supposed to happen user must ensure that BYPASS is
+             * set before the PLL are reset.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "VPLL is reseted but not bypassed.");
+            return 0;
+        } else {
+            return input_rate * div2 * (integer + frac);
+        }
+    }
+}
+
+/*
+ * FIXME: Only DP video reference clock is modeled here, others are missing.
+ */
+static uint64_t dp_video_update_rate(void *opaque, uint64_t input_rate)
+{
+    CRF_APB *s = XILINX_CRF_APB(opaque);
+    bool clock_act = FIELD_EX32(s->regs[R_DP_VIDEO_REF_CTRL],
+                                DP_VIDEO_REF_CTRL, CLKACT);
+    uint32_t divisor0 = FIELD_EX32(s->regs[R_DP_VIDEO_REF_CTRL],
+                                   DP_VIDEO_REF_CTRL, DIVISOR0);
+
+    if ((!divisor0) || (!clock_act)) {
+        return 0.0f;
+    } else {
+        return input_rate / (float)(divisor0);
+    }
+}
+
+static void dp_video_ref_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+    CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+    uint32_t source = FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, BYPASS)
+                    ? FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, POST_SRC)
+                    : FIELD_EX32(s->regs[R_APLL_CTRL], APLL_CTRL, PRE_SRC);
+
+    /*
+     * We must ensure that only one clock is bound to the dp_video_ref
+     * internal clock, so the callback have always the right rate in it.
+     */
+    qemu_clk_unbind(s->vpll_clk, s->dp_video_ref);
+    qemu_clk_unbind(s->dpll_clk, s->dp_video_ref);
+
+    switch (source) {
+    case 0x00:
+        qemu_clk_bind(s->vpll_clk, s->dp_video_ref);
+        break;
+    case 0x02:
+        qemu_clk_bind(s->dpll_clk, s->dp_video_ref);
+        break;
+    default:
+        abort();
+        break;
+    }
+}
+
+static RegisterAccessInfo crf_apb_regs_info[] = {
+    {   .name = "ERR_CTRL",  .addr = A_ERR_CTRL,
+    },{ .name = "IR_STATUS",  .addr = A_IR_STATUS,
+        .w1c = 0x1,
+        .post_write = ir_status_postw,
+    },{ .name = "IR_MASK",  .addr = A_IR_MASK,
+        .reset = 0x1,
+        .ro = 0x1,
+    },{ .name = "IR_ENABLE",  .addr = A_IR_ENABLE,
+        .pre_write = ir_enable_prew,
+    },{ .name = "IR_DISABLE",  .addr = A_IR_DISABLE,
+        .pre_write = ir_disable_prew,
+    },{ .name = "CRF_ECO",  .addr = A_CRF_ECO,
+    },{ .name = "APLL_CTRL",  .addr = A_APLL_CTRL,
+        .reset = 0x2809,
+        .rsvd = 0xf88c80f6L,
+        .post_write = apll_ctrl_postw,
+    },{ .name = "APLL_CFG",  .addr = A_APLL_CFG,
+        .rsvd = 0x1801210,
+    },{ .name = "APLL_FRAC_CFG",  .addr = A_APLL_FRAC_CFG,
+        .rsvd = 0x7e330000,
+    },{ .name = "DPLL_CTRL",  .addr = A_DPLL_CTRL,
+        .reset = 0x2809,
+        .rsvd = 0xf88c80f6L,
+        .post_write = dpll_ctrl_postw,
+    },{ .name = "DPLL_CFG",  .addr = A_DPLL_CFG,
+        .rsvd = 0x1801210,
+    },{ .name = "DPLL_FRAC_CFG",  .addr = A_DPLL_FRAC_CFG,
+        .rsvd = 0x7e330000,
+    },{ .name = "VPLL_CTRL",  .addr = A_VPLL_CTRL,
+        .reset = 0x2809,
+        .rsvd = 0xf88c80f6L,
+        .post_write = vpll_ctrl_postw,
+    },{ .name = "VPLL_CFG",  .addr = A_VPLL_CFG,
+        .rsvd = 0x1801210,
+    },{ .name = "VPLL_FRAC_CFG",  .addr = A_VPLL_FRAC_CFG,
+        .rsvd = 0x7e330000,
+    },{ .name = "PLL_STATUS",  .addr = A_PLL_STATUS,
+        .reset = 0x3f,
+        .rsvd = 0xc0,
+        .ro = 0x3f,
+    },{ .name = "APLL_TO_LPD_CTRL", .addr = A_APLL_TO_LPD_CTRL,
+        .reset = 0x400,
+        .rsvd = 0xc0ff,
+        .post_write = apll_to_lpd_postw,
+    },{ .name = "DPLL_TO_LPD_CTRL", .addr = A_DPLL_TO_LPD_CTRL,
+        .reset = 0x400,
+        .rsvd = 0xc0ff,
+        .post_write = dpll_to_lpd_postw,
+    },{ .name = "VPLL_TO_LPD_CTRL", .addr = A_VPLL_TO_LPD_CTRL,
+        .reset = 0x400,
+        .rsvd = 0xc0ff,
+        .post_write = vpll_to_lpd_postw,
+    },{ .name = "CPU_A9_CTRL", .addr = A_CPU_A9_CTRL,
+        .reset = 0xf000400,
+        .rsvd = 0xf0ffc0f8L,
+    },{ .name = "DBG_TRACE_CTRL", .addr = A_DBG_TRACE_CTRL,
+        .reset = 0x2500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DBG_FPD_CTRL", .addr = A_DBG_FPD_CTRL,
+        .reset = 0x1002500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DP_VIDEO_REF_CTRL", .addr = A_DP_VIDEO_REF_CTRL,
+        .reset = 0x1002300,
+        .rsvd = 0xfeffc0f8L,
+        .post_write = dp_video_ref_ctrl_postw,
+    },{ .name = "DP_AUDIO_REF_CTRL", .addr = A_DP_AUDIO_REF_CTRL,
+        .reset = 0x1002300,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DP_LINK_REF_CTRL", .addr = A_DP_LINK_REF_CTRL,
+        .reset = 0x1203200,
+        .rsvd = 0xfec0c0f8L,
+    },{ .name = "DP_STC_REF_CTRL", .addr = A_DP_STC_REF_CTRL,
+        .reset = 0x1203200,
+        .rsvd = 0xfec0c0f8L,
+    },{ .name = "DDR_CTRL", .addr = A_DDR_CTRL,
+        .reset = 0x1000500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "GPU_REF_CTRL", .addr = A_GPU_REF_CTRL,
+        .reset = 0x1500,
+        .rsvd = 0xf8ffc0f8L,
+    },{ .name = "AFI0_REF_CTRL", .addr = A_AFI0_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "AFI1_REF_CTRL", .addr = A_AFI1_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "AFI2_REF_CTRL", .addr = A_AFI2_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "AFI3_REF_CTRL", .addr = A_AFI3_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "AFI4_REF_CTRL", .addr = A_AFI4_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "AFI5_REF_CTRL", .addr = A_AFI5_REF_CTRL,
+        .reset = 0x600,
+        .rsvd = 0x7effc0f8,
+    },{ .name = "SATA_REF_CTRL", .addr = A_SATA_REF_CTRL,
+        .reset = 0x1001600,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "PCIE_REF_CTRL", .addr = A_PCIE_REF_CTRL,
+        .reset = 0x1500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "GDMA_REF_CTRL", .addr = A_GDMA_REF_CTRL,
+        .reset = 0x1000500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DPDMA_REF_CTRL", .addr = A_DPDMA_REF_CTRL,
+        .reset = 0x1000500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "TOPSW_MAIN_CTRL", .addr = A_TOPSW_MAIN_CTRL,
+        .reset = 0x1000500,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "TOPSW_LSBUS_CTRL", .addr = A_TOPSW_LSBUS_CTRL,
+        .reset = 0x1000800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "GTGREF0_REF_CTRL", .addr = A_GTGREF0_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "GTGREF1_REF_CTRL", .addr = A_GTGREF1_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DFT300_REF_CTRL", .addr = A_DFT300_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DFT270_REF_CTRL", .addr = A_DFT270_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DFT250_REF_CTRL", .addr = A_DFT250_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "DFT125_REF_CTRL", .addr = A_DFT125_REF_CTRL,
+        .reset = 0x800,
+        .rsvd = 0xfeffc0f8L,
+    },{ .name = "RST_FPD_TOP", .addr = A_RST_FPD_TOP,
+        .reset = 0x71ffe,
+        .rsvd = 0xf8e001,
+    },{ .name = "RST_FPD_APU", .addr = A_RST_FPD_APU,
+        .reset = 0x334f,
+        .rsvd = 0xffcccc,
+    },{ .name = "RST_DDR_SS", .addr = A_RST_DDR_SS,
+        .reset = 0xf,
+        .rsvd = 0xf0,
+    }
+};
+
+static void crf_apb_reset(DeviceState *dev)
+{
+    CRF_APB *s = XILINX_CRF_APB(dev);
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+
+    ir_update_irq(s);
+
+    /*
+     * During reset, the clock selection registers bound the clock like this.
+     */
+    qemu_clk_bind(s->pss_ref_clk, s->apll_clk);
+    qemu_clk_bind(s->pss_ref_clk, s->dpll_clk);
+    qemu_clk_bind(s->pss_ref_clk, s->vpll_clk);
+    qemu_clk_bind(s->vpll_clk, s->dp_video_ref);
+}
+
+static void crf_apb_realize(DeviceState *d, Error **errp)
+{
+    CRF_APB *s = XILINX_CRF_APB(d);
+
+    qemu_clk_bind(s->apll_clk, s->apll_to_lpd);
+    qemu_clk_bind(s->dpll_clk, s->dpll_to_lpd);
+    qemu_clk_bind(s->vpll_clk, s->vpll_to_lpd);
+
+    crf_apb_reset(d);
+}
+
+static void crf_apb_init(Object *obj)
+{
+    CRF_APB *s = XILINX_CRF_APB(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    RegisterInfoArray *reg_array;
+
+    memory_region_init(&s->iomem, obj, TYPE_XILINX_CRF_APB, R_MAX * 4);
+    reg_array = register_init_block32(DEVICE(obj), crf_apb_regs_info,
+                                      ARRAY_SIZE(crf_apb_regs_info),
+                                      s->regs_info, s->regs,
+                                      &crf_apb_ops,
+                                      XILINX_CRF_APB_ERR_DEBUG,
+                                      R_RST_DDR_SS);
+    memory_region_add_subregion(&s->iomem,
+                                A_ERR_CTRL,
+                                &reg_array->mem);
+
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq_ir);
+
+    qemu_clk_init_device(obj, crf_clock);
+}
+
+static const VMStateDescription vmstate_crf_apb = {
+    .name = TYPE_XILINX_CRF_APB,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, CRF_APB, R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void crf_apb_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = crf_apb_reset;
+    dc->vmsd = &vmstate_crf_apb;
+    dc->realize = crf_apb_realize;
+}
+
+static const TypeInfo crf_apb_info = {
+    .name          = TYPE_XILINX_CRF_APB,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(CRF_APB),
+    .class_init    = crf_apb_class_init,
+    .instance_init = crf_apb_init,
+};
+
+static void crf_apb_register_types(void)
+{
+    type_register_static(&crf_apb_info);
+}
+
+type_init(crf_apb_register_types)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 09/10] zynqmp: add the zynqmp_crf to the platform

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This adds the zynqmp_crf to the zynqmp platform.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/arm/xlnx-zynqmp.c         | 7 +++++++
 include/hw/arm/xlnx-zynqmp.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index bc4e66b..27dccdb 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -177,6 +177,11 @@ 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());
+
+    s->crf = object_new("xlnx.zynqmp_crf");
+    qdev_set_parent_bus(DEVICE(s->crf), sysbus_get_default());
+    object_property_add_child(obj, "xlnx.zynqmp_crf", OBJECT(s->crf),
+                              &error_abort);
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -424,6 +429,8 @@ 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]);
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(s->crf), 0, 0xFD1A0000);
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index c2931bf..379a17a 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -86,6 +86,8 @@ typedef struct XlnxZynqMPState {
     XlnxDPState dp;
     XlnxDPDMAState dpdma;
 
+    Object *crf;
+
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 10/10] zynqmp: add reference clock

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This adds some fixed reference clock to the zynqmp platform.
They will feed the zynqmp_crf block.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/arm/xlnx-zynqmp.c         | 49 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h |  6 ++++++
 2 files changed, 55 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 27dccdb..b9cd856 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -24,6 +24,7 @@
 #include "exec/address-spaces.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
+#include "qemu/qemu-clock.h"
 
 #define GIC_NUM_SPI_INTR 160
 
@@ -182,6 +183,22 @@ static void xlnx_zynqmp_init(Object *obj)
     qdev_set_parent_bus(DEVICE(s->crf), sysbus_get_default());
     object_property_add_child(obj, "xlnx.zynqmp_crf", OBJECT(s->crf),
                               &error_abort);
+
+    s->pss_ref_clk = object_new(TYPE_FIXED_CLOCK);
+    object_property_add_child(obj, "pss_ref_clk", s->pss_ref_clk,
+                              &error_abort);
+    object_property_set_int(s->pss_ref_clk, 50000000, "rate", &error_abort);
+    s->video_clk = object_new(TYPE_FIXED_CLOCK);
+    object_property_add_child(obj, "video_clk", s->video_clk, &error_abort);
+    object_property_set_int(s->video_clk, 27000000, "rate", &error_abort);
+    s->pss_alt_ref_clk = object_new(TYPE_FIXED_CLOCK);
+    object_property_add_child(obj, "pss_alt_ref_clk", s->pss_alt_ref_clk,
+                              &error_abort);
+    s->aux_refclk = object_new(TYPE_FIXED_CLOCK);
+    object_property_add_child(obj, "aux_refclk", s->aux_refclk, &error_abort);
+    s->gt_crx_ref_clk = object_new(TYPE_FIXED_CLOCK);
+    object_property_add_child(obj, "gt_crx_ref_clk", s->gt_crx_ref_clk,
+                              &error_abort);
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -431,6 +448,38 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->dpdma), 0, gic_spi[DPDMA_IRQ]);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(s->crf), 0, 0xFD1A0000);
+
+    /* Bind the clock */
+    qemu_clk_bind(qemu_clk_device_get_clock(DEVICE(s->pss_ref_clk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf),
+                                                  "pss_ref_clk"));
+
+    qemu_clk_bind(qemu_clk_device_get_clock(DEVICE(s->video_clk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf), "video_clk"));
+
+    qemu_clk_bind(qemu_clk_device_get_clock(DEVICE(s->pss_alt_ref_clk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf),
+                                                  "pss_alt_ref_clk"));
+
+    qemu_clk_bind(qemu_clk_device_get_clock(DEVICE(s->aux_refclk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf),
+                                                  "aux_refclk"));
+
+    qemu_clk_bind(qemu_clk_device_get_clock(DEVICE(s->gt_crx_ref_clk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf),
+                                                  "gt_crx_ref_clk"));
+
+    object_property_set_bool(s->crf, true, "realized", &err);
+    object_property_set_bool(s->pss_ref_clk, true, "realized", &err);
+    object_property_set_bool(s->video_clk, true, "realized", &err);
+    object_property_set_bool(s->pss_alt_ref_clk, true, "realized", &err);
+    object_property_set_bool(s->aux_refclk, true, "realized", &err);
+    object_property_set_bool(s->gt_crx_ref_clk, true, "realized", &err);
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 379a17a..d0cc57f 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/misc/fixed-clock.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -86,6 +87,11 @@ typedef struct XlnxZynqMPState {
     XlnxDPState dp;
     XlnxDPDMAState dpdma;
 
+    Object *pss_ref_clk;
+    Object *video_clk;
+    Object *pss_alt_ref_clk;
+    Object *aux_refclk;
+    Object *gt_crx_ref_clk;
     Object *crf;
 
     char *boot_cpu;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Ping.

Thanks,
Fred

Le 28/02/2017 à 11:02, fred.konrad@greensocs.com a écrit :
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Hi,
>
> This is the third version of the clock framework API it contains:
>
>   * The first 6 patches which introduce the framework.
>   * The 7th patch which introduces a fixed-clock model.
>   * The rest which gives an example how to model a PLL from the existing
>     zynqmp-crf extracted from the qemu xilinx tree.
>
> No specific behavior is expected yet when the CRF register set is accessed but
> the user can see for example the dp_video_ref and vpll_to_lpd rate changing in
> the monitor with the "info qtree" command when the vpll_ctrl register is
> modified.
>
> bus: main-system-bus
>   type System
>   dev: xlnx.zynqmp_crf, id ""
>     gpio-out "sysbus-irq" 1
>     qemu-clk "dbg_trace" 0
>     qemu-clk "dp_stc_ref" 0
>     qemu-clk "dpll_to_lpd" 12500000
>     qemu-clk "acpu_clk" 0
>     qemu-clk "pcie_ref" 0
>     qemu-clk "topsw_main" 0
>     qemu-clk "topsw_lsbus" 0
>     qemu-clk "dp_audio_ref" 0
>     qemu-clk "sata_ref" 0
>     qemu-clk "dp_video_ref" 1428571
>     qemu-clk "vpll_clk" 50000000
>     qemu-clk "apll_to_lpd" 12500000
>     qemu-clk "dpll_clk" 50000000
>     qemu-clk "gpu_ref" 0
>     qemu-clk "aux_refclk" 0
>     qemu-clk "video_clk" 27000000
>     qemu-clk "gdma_ref" 0
>     qemu-clk "gt_crx_ref_clk" 0
>     qemu-clk "dbg_fdp" 0
>     qemu-clk "apll_clk" 50000000
>     qemu-clk "pss_alt_ref_clk" 0
>     qemu-clk "ddr" 0
>     qemu-clk "dbg_tstmp" 0
>     qemu-clk "pss_ref_clk" 50000000
>     qemu-clk "dpdma_ref" 0
>     qemu-clk "vpll_to_lpd" 12500000
>     mmio 00000000fd1a0000/000000000000010c
>
> This series is based on the current master
> (d992f2f1368ceb92e6bfd8efece174110f4236ff).
>
> Thanks,
> Fred
>
> V2 -> V3:
>   * Rebased on current master.
>   * Renamed qemu_clk / QEMUClock as suggested by Cédric.
>   * Renamed in_rate to ref_rate and out_rate to rate.
>   * Renamed qemu_clk_bind_clock to qemu_clk_bind.
>   * Example added to the documentation as suggested by Peter.
>
> V1 -> V2:
>   * Rebased on current master.
>   * Some function renamed and documentation fixed.
>
> RFC -> V1:
>   * Rebased on current master.
>   * The docs has been fixed.
>   * qemu_clk_init_device helper has been provided to ease the initialization
>     of the devices.
>
> KONRAD Frederic (10):
>   qemu-clk: introduce qemu-clk qom object
>   qemu-clk: allow to add a clock to a device
>   qemu-clk: allow to bind two clocks together
>   qemu-clk: introduce an init array to help the device construction
>   qdev-monitor: print the device's clock with info qtree
>   docs: add qemu-clock documentation
>   introduce fixed-clock
>   introduce zynqmp_crf
>   zynqmp: add the zynqmp_crf to the platform
>   zynqmp: add reference clock
>
>  Makefile.objs                 |   1 +
>  docs/clock.txt                | 278 ++++++++++++
>  hw/arm/xlnx-zynqmp.c          |  56 +++
>  hw/misc/Makefile.objs         |   2 +
>  hw/misc/fixed-clock.c         |  88 ++++
>  hw/misc/xilinx_zynqmp_crf.c   | 968 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/xlnx-zynqmp.h  |   8 +
>  include/hw/misc/fixed-clock.h |  30 ++
>  include/qemu/qemu-clock.h     | 161 +++++++
>  qdev-monitor.c                |   2 +
>  qemu-clock.c                  | 176 ++++++++
>  11 files changed, 1770 insertions(+)
>  create mode 100644 docs/clock.txt
>  create mode 100644 hw/misc/fixed-clock.c
>  create mode 100644 hw/misc/xilinx_zynqmp_crf.c
>  create mode 100644 include/hw/misc/fixed-clock.h
>  create mode 100644 include/qemu/qemu-clock.h
>  create mode 100644 qemu-clock.c
>

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 24 May 2017 at 08:35, KONRAD Frederic <fred.konrad@greensocs.com> wrote:
> Le 28/02/2017 à 11:02, fred.konrad@greensocs.com a écrit :
>> This is the third version of the clock framework API it contains:
>>
>>   * The first 6 patches which introduce the framework.
>>   * The 7th patch which introduces a fixed-clock model.
>>   * The rest which gives an example how to model a PLL from the existing
>>     zynqmp-crf extracted from the qemu xilinx tree.
>>
>> No specific behavior is expected yet when the CRF register set is accessed
>> but
>> the user can see for example the dp_video_ref and vpll_to_lpd rate
>> changing in
>> the monitor with the "info qtree" command when the vpll_ctrl register is
>> modified.

Some top-level review:

* I like the documentation, this is very helpful
* consider tracepoints rather than DPRINTF macro
* qemu_clk_device_add_clock() does a g_strdup, but when is this freed?
  (consider devices which are hot-unpluggable)
* similarly, what if a device with a clock with a lot of child nodes
  is destroyed? how are the ClkList structs freed?
* interaction with migration -- how is the "this clock is at this rate"
  state intended to be migrated?
* I'll leave the review of the xilinx patches to the xilinx folk
* the 'introduce zynqmp_crf' patch is missing any signoffs
  (in particular if it's from the xilinx tree it will need
  signoff from them)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/06/2017 à 17:18, Peter Maydell a écrit :
> On 24 May 2017 at 08:35, KONRAD Frederic <fred.konrad@greensocs.com> wrote:
>> Le 28/02/2017 à 11:02, fred.konrad@greensocs.com a écrit :
>>> This is the third version of the clock framework API it contains:
>>>
>>>   * The first 6 patches which introduce the framework.
>>>   * The 7th patch which introduces a fixed-clock model.
>>>   * The rest which gives an example how to model a PLL from the existing
>>>     zynqmp-crf extracted from the qemu xilinx tree.
>>>
>>> No specific behavior is expected yet when the CRF register set is accessed
>>> but
>>> the user can see for example the dp_video_ref and vpll_to_lpd rate
>>> changing in
>>> the monitor with the "info qtree" command when the vpll_ctrl register is
>>> modified.
>
> Some top-level review:
>
> * I like the documentation, this is very helpful
> * consider tracepoints rather than DPRINTF macro
> * qemu_clk_device_add_clock() does a g_strdup, but when is this freed?
>   (consider devices which are hot-unpluggable)
> * similarly, what if a device with a clock with a lot of child nodes
>   is destroyed? how are the ClkList structs freed?
> * interaction with migration -- how is the "this clock is at this rate"
>   state intended to be migrated?
> * I'll leave the review of the xilinx patches to the xilinx folk
> * the 'introduce zynqmp_crf' patch is missing any signoffs
>   (in particular if it's from the xilinx tree it will need
>   signoff from them)
>
> thanks
> -- PMM
>

Nice thanks for the feedback!

Actually the hot unpluggable is a good question what can we do for that?
Is that reasonable for a device which produce a clock to be hot
unpluggable?

For the migration maybe we can refresh the whole clock tree at the end
of the migration. Is that a good idea?

Fred

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 8 June 2017 at 08:54, KONRAD Frederic <fred.konrad@greensocs.com> wrote:
> Le 06/06/2017 à 17:18, Peter Maydell a écrit :
>> Some top-level review:

>> * qemu_clk_device_add_clock() does a g_strdup, but when is this freed?
>>   (consider devices which are hot-unpluggable)
>> * similarly, what if a device with a clock with a lot of child nodes
>>   is destroyed? how are the ClkList structs freed?
>> * interaction with migration -- how is the "this clock is at this rate"
>>   state intended to be migrated?

> Nice thanks for the feedback!
>
> Actually the hot unpluggable is a good question what can we do for that?
> Is that reasonable for a device which produce a clock to be hot
> unpluggable?

I don't know if it's very likely (but you might have a PCI device
for instance which uses clocks internally). But I think for a bit
of low-level infrastructure like clocktree it's worth getting the
details of the lifecycle right from the start.

> For the migration maybe we can refresh the whole clock tree at the end
> of the migration. Is that a good idea?

That seems kind of awkward -- where would this code that did a
clock tree refresh be? Also you're then reliant on all the
callback functions registered to not actually do anything that
affects device state when the refresh happens. It would be
cleaner if the qemu-clk objects managed their own internal
state migration (but can we do this without having to make
them be Device objects rather than just objects ?).

Cc'd Paolo who might have an opinion on these -- my opinion currently
is mostly "this doesn't really look right" rather than knowing what
the right approach is.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Paolo Bonzini]

On 13/06/2017 12:33, Peter Maydell wrote:
>> For the migration maybe we can refresh the whole clock tree at the end
>> of the migration. Is that a good idea?
> That seems kind of awkward -- where would this code that did a
> clock tree refresh be? Also you're then reliant on all the
> callback functions registered to not actually do anything that
> affects device state when the refresh happens. It would be
> cleaner if the qemu-clk objects managed their own internal
> state migration (but can we do this without having to make
> them be Device objects rather than just objects ?).
> 
> Cc'd Paolo who might have an opinion on these -- my opinion currently
> is mostly "this doesn't really look right" rather than knowing what
> the right approach is.

Same here. :)

I think the various bindings and rates could be refreshed as devices are
migrated.  This assumes that the device migration order is okay
according to the clock tree, that is if you have three devices X/Y/Z and
five clocks a/b/c/d/e/f:

  fixed-clock
    |     |
   X:a   X:b
    |     |    \
   Y:c   Y:d      Z:e
          |
         Z:f

you could do this:

- migrate X
  - retrieve the PLL ratios for a and b's bound clocks (if the ratio
    is variable, otherwise no need for this)
  - in the post_load callback, bind a and b to the fixed-clock
    (if the binding is variable, otherwise no need for this)
- migrate Y
  - retrieve the PLL ratio for d's bound clocks (if the ratio
    is variable, otherwise no need for this)
  - in the post_load callback, bind c and d to a and b respectively
    (if the binding is variable, otherwise no need for this)
- migrate Z
  - in the post_load callback, bind e and f to b and d respectively
    (if the binding is variable, otherwise no need for this)

Paolo

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 14/06/2017 à 13:54, Paolo Bonzini a écrit :
> On 13/06/2017 12:33, Peter Maydell wrote:
>>> For the migration maybe we can refresh the whole clock tree at the end
>>> of the migration. Is that a good idea?
>> That seems kind of awkward -- where would this code that did a
>> clock tree refresh be? Also you're then reliant on all the
>> callback functions registered to not actually do anything that
>> affects device state when the refresh happens. It would be
>> cleaner if the qemu-clk objects managed their own internal
>> state migration (but can we do this without having to make
>> them be Device objects rather than just objects ?).
>>
>> Cc'd Paolo who might have an opinion on these -- my opinion currently
>> is mostly "this doesn't really look right" rather than knowing what
>> the right approach is.
>
> Same here. :)
>
> I think the various bindings and rates could be refreshed as devices are
> migrated.
 > "This assumes that the device migration order is okay"

I think it's the real problem here?

> according to the clock tree, that is if you have three devices X/Y/Z and
> five clocks a/b/c/d/e/f:
>
>   fixed-clock
>     |     |
>    X:a   X:b
>     |     |    \
>    Y:c   Y:d      Z:e
>           |
>          Z:f
>
> you could do this:
>
> - migrate X
>   - retrieve the PLL ratios for a and b's bound clocks (if the ratio
>     is variable, otherwise no need for this)
>   - in the post_load callback, bind a and b to the fixed-clock
>     (if the binding is variable, otherwise no need for this)
> - migrate Y
>   - retrieve the PLL ratio for d's bound clocks (if the ratio
>     is variable, otherwise no need for this)
>   - in the post_load callback, bind c and d to a and b respectively
>     (if the binding is variable, otherwise no need for this)
> - migrate Z
>   - in the post_load callback, bind e and f to b and d respectively
>     (if the binding is variable, otherwise no need for this)
>

So for you the migration of the clock tree will be done by the device?
I think this is OK :).

Fred

> Paolo
>

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 14 June 2017 at 12:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I think the various bindings and rates could be refreshed as devices are
> migrated.  This assumes that the device migration order is okay
> according to the clock tree, that is if you have three devices X/Y/Z and
> five clocks a/b/c/d/e/f:
>
>   fixed-clock
>     |     |
>    X:a   X:b
>     |     |    \
>    Y:c   Y:d      Z:e
>           |
>          Z:f
>
> you could do this:
>
> - migrate X
>   - retrieve the PLL ratios for a and b's bound clocks (if the ratio
>     is variable, otherwise no need for this)
>   - in the post_load callback, bind a and b to the fixed-clock
>     (if the binding is variable, otherwise no need for this)
> - migrate Y
>   - retrieve the PLL ratio for d's bound clocks (if the ratio
>     is variable, otherwise no need for this)
>   - in the post_load callback, bind c and d to a and b respectively
>     (if the binding is variable, otherwise no need for this)
> - migrate Z
>   - in the post_load callback, bind e and f to b and d respectively
>     (if the binding is variable, otherwise no need for this)

Unfortunately we make no guarantees at all about migration order
for devices as far as I'm aware, so devices have to cope regardless.

(There's also the possibility of an oddball bit of hardware which
has a clocktree with loops in it.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Paolo Bonzini]

On 15/06/2017 16:40, Peter Maydell wrote:
> On 14 June 2017 at 12:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I think the various bindings and rates could be refreshed as devices are
>> migrated.  This assumes that the device migration order is okay
>> according to the clock tree
> 
> Unfortunately we make no guarantees at all about migration order
> for devices as far as I'm aware, so devices have to cope regardless.

It's device realization order, so there are some guarantees.  A board
can realize devices in its preferred order (and realization will also
bind clocks and set rates IIUC, just like migration), and a qdev bus's
children will be visited after the bus owner.

> (There's also the possibility of an oddball bit of hardware which
> has a clocktree with loops in it.)

For these cases there is post_load, I guess.

Paolo

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Edgar E. Iglesias]

On Thu, Jun 15, 2017 at 03:40:40PM +0100, Peter Maydell wrote:
> On 14 June 2017 at 12:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > I think the various bindings and rates could be refreshed as devices are
> > migrated.  This assumes that the device migration order is okay
> > according to the clock tree, that is if you have three devices X/Y/Z and
> > five clocks a/b/c/d/e/f:
> >
> >   fixed-clock
> >     |     |
> >    X:a   X:b
> >     |     |    \
> >    Y:c   Y:d      Z:e
> >           |
> >          Z:f
> >
> > you could do this:
> >
> > - migrate X
> >   - retrieve the PLL ratios for a and b's bound clocks (if the ratio
> >     is variable, otherwise no need for this)
> >   - in the post_load callback, bind a and b to the fixed-clock
> >     (if the binding is variable, otherwise no need for this)
> > - migrate Y
> >   - retrieve the PLL ratio for d's bound clocks (if the ratio
> >     is variable, otherwise no need for this)
> >   - in the post_load callback, bind c and d to a and b respectively
> >     (if the binding is variable, otherwise no need for this)
> > - migrate Z
> >   - in the post_load callback, bind e and f to b and d respectively
> >     (if the binding is variable, otherwise no need for this)
> 
> Unfortunately we make no guarantees at all about migration order
> for devices as far as I'm aware, so devices have to cope regardless.


How does this work for interrupts/gpios?

Perhaps we would need a way to force all clock sources that don't
depend on input clocks to re-notify their clocks after migration
is complete but before we run the new guest?

That would propagate updates throughout the chain...

Cheers,
Edgar

> 
> (There's also the possibility of an oddball bit of hardware which
> has a clocktree with loops in it.)

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 15 June 2017 at 15:57, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Jun 15, 2017 at 03:40:40PM +0100, Peter Maydell wrote:
>> Unfortunately we make no guarantees at all about migration order
>> for devices as far as I'm aware, so devices have to cope regardless.
>
>
> How does this work for interrupts/gpios?

Interrupts/gpios (qemu_irqs) don't have any internal state,
so all that is needed is for both devices to correctly migrate
their idea of their internal state, and it doesn't matter which
order that happens in. (Typically in QEMU devices track the state
of their inbound interrupt lines even if in real hardware there's
no flop doing that.)

The difference here is that the clock objects themselves have
internal state. That's not necessarily a bad idea, but it does
mean that something's got to migrate that state or otherwise
regenerate it. (Anthony once proposed that we should change qemu_irq
objects to have internal state, because that's effectively what real
hardware is and it would save the need for each device to track its
input line state and be notified if the line didn't actually change
state. It would just have been an enormous upheaval and migration
compat break...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Edgar E. Iglesias]

On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:
> On 15 June 2017 at 15:57, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Jun 15, 2017 at 03:40:40PM +0100, Peter Maydell wrote:
> >> Unfortunately we make no guarantees at all about migration order
> >> for devices as far as I'm aware, so devices have to cope regardless.
> >
> >
> > How does this work for interrupts/gpios?
> 
> Interrupts/gpios (qemu_irqs) don't have any internal state,
> so all that is needed is for both devices to correctly migrate
> their idea of their internal state, and it doesn't matter which
> order that happens in. (Typically in QEMU devices track the state
> of their inbound interrupt lines even if in real hardware there's
> no flop doing that.)
> 
> The difference here is that the clock objects themselves have
> internal state. That's not necessarily a bad idea, but it does
> mean that something's got to migrate that state or otherwise
> regenerate it. (Anthony once proposed that we should change qemu_irq
> objects to have internal state, because that's effectively what real
> hardware is and it would save the need for each device to track its
> input line state and be notified if the line didn't actually change
> state. It would just have been an enormous upheaval and migration
> compat break...)

Thanks. I didn't realize that the internal clock state would be used in a
way that is not deriveable from other device state & input clocks.
I'm not sure this is a use-case we need to support, thoughts?

Maybe there's some value in keeping interrupt and clock handling alike,
e.g removing the internal state from clocks.

I need to have another look at the series before I comment too much
since I don't remember the details...

Best regards,
Edgar

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 15/06/2017 à 17:15, Edgar E. Iglesias a écrit :
> On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:
>> On 15 June 2017 at 15:57, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>>> On Thu, Jun 15, 2017 at 03:40:40PM +0100, Peter Maydell wrote:
>>>> Unfortunately we make no guarantees at all about migration order
>>>> for devices as far as I'm aware, so devices have to cope regardless.
>>>
>>>
>>> How does this work for interrupts/gpios?
>>
>> Interrupts/gpios (qemu_irqs) don't have any internal state,
>> so all that is needed is for both devices to correctly migrate
>> their idea of their internal state, and it doesn't matter which
>> order that happens in. (Typically in QEMU devices track the state
>> of their inbound interrupt lines even if in real hardware there's
>> no flop doing that.)
>>
>> The difference here is that the clock objects themselves have
>> internal state. That's not necessarily a bad idea, but it does
>> mean that something's got to migrate that state or otherwise
>> regenerate it. (Anthony once proposed that we should change qemu_irq
>> objects to have internal state, because that's effectively what real
>> hardware is and it would save the need for each device to track its
>> input line state and be notified if the line didn't actually change
>> state. It would just have been an enormous upheaval and migration
>> compat break...)
>
> Thanks. I didn't realize that the internal clock state would be used in a
> way that is not deriveable from other device state & input clocks.
> I'm not sure this is a use-case we need to support, thoughts?
>
> Maybe there's some value in keeping interrupt and clock handling alike,
> e.g removing the internal state from clocks.
>
> I need to have another look at the series before I comment too much
> since I don't remember the details...
>

The use case for that is essentially knowing the rate value when
needed:
   * for example in the monitor.
   * if don't want to refresh the whole tree.

For the rest.. yes we can just keep the value in the device state..

Fred

> Best regards,
> Edgar
>

Re: [Qemu-devel] [PATCH v3 06/10] docs: add qemu-clock documentation

[Edgar E. Iglesias]

On Tue, Feb 28, 2017 at 11:03:01AM +0100, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This adds the qemu-clock documentation.

Hi Fred



> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> V1 -> V2:
>   * Fixed in accordance with the changes in the previous patches.
> ---
>  docs/clock.txt | 278 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 278 insertions(+)
>  create mode 100644 docs/clock.txt
> 
> diff --git a/docs/clock.txt b/docs/clock.txt
> new file mode 100644
> index 0000000..010ae50
> --- /dev/null
> +++ b/docs/clock.txt
> @@ -0,0 +1,278 @@
> +
> +What is a QEMU_CLOCK
> +====================
> +
> +A QEMU_CLOCK is a QOM Object developed for the purpose of modeling a clock tree
> +with QEMU.
> +
> +It only simulates the clock by keeping a copy of the current frequency and
> +doesn't model the signal itself such as pin toggle or duty cycle.
> +
> +It allows to model the impact of badly configured PLL, clock source selection
> +or disabled clock on the models.
> +
> +Binding the clock together to create a tree
> +===========================================
> +
> +In order to create a clock tree with QEMU_CLOCK two or more clock must be bound
> +together. Let's say there are two clocks clk_a and clk_b:
> +Using qemu_clk_bind(clk_a, clk_b) will bind clk_a and clk_b.
> +
> +Binding two qemu-clk together creates a unidirectional link which means that
> +changing the rate of clk_a will propagate to clk_b and not the opposite.
> +The binding process automatically refreshes clk_b rate.
> +
> +Clock can be bound and unbound during execution for modeling eg: a clock
> +selector.
> +
> +A clock can drive more than one other clock. eg with this code:
> +qemu_clk_bind(clk_a, clk_b);
> +qemu_clk_bind(clk_a, clk_c);
> +
> +A clock rate change one clk_a will propagate to clk_b and clk_c.
> +
> +Implementing a callback on a rate change
> +========================================
> +
> +The function prototype is the following:
> +typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
> +
> +It's main goal is to modify the rate before it's passed to the next clocks in
> +the tree.
> +
> +eg: for a 4x PLL the function will be:
> +uint64_t qemu_clk_rate_change_cb(void *opaque, uint64_t rate)
> +{
> +    return 4 * rate;
> +}


I was thinking that this would work a little different.

Clocks would notify other objects when rates change by somehow calling
registered callbacks. Devices that derive and propagates new clocks
would:

1. Bind to a clock source by registering a callback.
2. When the callback gets called, they would compute a new
   rate and actively set that new rate to another clock object.
   I.e, a clock_in object does not exist, input clocks are just
   registered callbacks.
   As the new rate gets set, more callbacks are potentially
   triggered across the clock tree.


That would get rid of the need for storing ref_rate in the clock
objects.
rate would perhaps still be useful making it easy to print stuff in
qom-tree, but it would really only be a cache of state that at any
time can be derived by a devices internal state + rate change of
any of the clocks the device has registered to.

Does this make sense?
I think this brings us closer to how gpios/interrupts work and makes
migration simpler. I personally also find it more natural and easier
to understand but that may be subjective.

What do you and others think?

Cheers,
Edgar


> +
> +To set the callback for the clock:
> +void qemu_clk_set_callback(qemu_clk clk, QEMUClkRateUpdateCallback *cb,
> +                           void *opaque);
> +can be called.
> +
> +The rate update process
> +=======================
> +
> +The rate update happen in this way:
> +When a model wants to update a clock frequency (eg: based on a register change
> +or something similar) it will call qemu_clk_update_rate(..) on the clock:
> +  * The callback associated to the clock is called with the new rate.
> +  * qemu_clk_update_rate(..) is then called on all bound clocks with the value
> +    returned by the callback.
> +
> +NOTE: When no callback is attached, the clock qemu_clk_update_rate(..) is called
> +on the next clock in the tree with the rate unmodified.
> +
> +Adding a QEMU_CLOCK to a DeviceState
> +====================================
> +
> +Adding a qemu-clk to a DeviceState is required to be able to get the clock
> +outside the model through qemu_clk_device_get_clock(..).
> +
> +It is also required to be able to print the clock and its rate with info qtree.
> +For example:
> +
> +  type System
> +  dev: xlnx.zynqmp_crf, id ""
> +    gpio-out "sysbus-irq" 1
> +    gpio-out "RST_A9" 4
> +    qemu-clk "dbg_trace" 0
> +    qemu-clk "vpll_to_lpd" 625000000
> +    qemu-clk "dp_stc_ref" 0
> +    qemu-clk "dpll_to_lpd" 12500000
> +    qemu-clk "acpu_clk" 0
> +    qemu-clk "pcie_ref" 0
> +    qemu-clk "topsw_main" 0
> +    qemu-clk "topsw_lsbus" 0
> +    qemu-clk "dp_audio_ref" 0
> +    qemu-clk "sata_ref" 0
> +    qemu-clk "dp_video_ref" 71428568
> +    qemu-clk "vpll_clk" 2500000000
> +    qemu-clk "apll_to_lpd" 12500000
> +    qemu-clk "dpll_clk" 50000000
> +    qemu-clk "gpu_ref" 0
> +    qemu-clk "aux_refclk" 0
> +    qemu-clk "video_clk" 27000000
> +    qemu-clk "gdma_ref" 0
> +    qemu-clk "gt_crx_ref_clk" 0
> +    qemu-clk "dbg_fdp" 0
> +    qemu-clk "apll_clk" 50000000
> +    qemu-clk "pss_alt_ref_clk" 0
> +    qemu-clk "ddr" 0
> +    qemu-clk "pss_ref_clk" 50000000
> +    qemu-clk "dpdma_ref" 0
> +    qemu-clk "dbg_tstmp" 0
> +    mmio 00000000fd1a0000/000000000000010c
> +
> +This way a DeviceState can have multiple clock input or output.
> +
> +Examples
> +========
> +
> +Those are the different way of using the QEMUClock object.
> +
> +Modelling a fixed clock generator
> +=================================
> +
> +Here is a brief example of a device acting as a clock source:
> +
> +typedef struct {
> +    DeviceState parent_obj;
> +
> +    uint32_t rate;
> +    QEMUClock out;
> +} FixedClock;
> +
> +During the initialization the device must initialize its clock object:
> +
> +static void fixed_clock_instance_init(Object *obj)
> +{
> +    FixedClock *s = FIXED_CLOCK(obj);
> +
> +    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
> +}
> +
> +As the device acts as a clock source it must refresh the clock tree during the
> +realize phase:
> +
> +static void fixed_clock_realizefn(DeviceState *dev, Error **errp)
> +{
> +    FixedClock *s = FIXED_CLOCK(dev);
> +
> +    qemu_clk_update_rate(&s->out, s->rate);
> +}
> +
> +This means that the clock tree must be finished before realize is called on the
> +fixed clock.
> +
> +Modelling a clock user device
> +=============================
> +
> +Here is a brief example of a clock user:
> +
> +typedef struct {
> +    DeviceState parent_obj;
> +
> +    QEMUClock in;
> +} ClockUser;
> +
> +As before the clock must be initialized through the device initialize function:
> +
> +static void clock_user_instance_init(Object *obj)
> +{
> +    ClockUser *s = CLOCK_USER(obj);
> +
> +    object_initialize(&s->in, sizeof(s->in), TYPE_CLOCK);
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->in, "clk_in");
> +    /*
> +     * Call on_rate_change_cb when something change on clk_in.
> +     */
> +    qemu_clk_set_callback(s->in, on_rate_change_cb, obj);
> +}
> +
> +The callback is in this case used as a notifier when the clock tree which
> +sources the device change:
> +
> +static uint64_t on_rate_change_cb(void *opaque, uint64_t input_rate)
> +{
> +    printf("the new rate is %ld\n", input_rate);
> +
> +    /* The return is ignored if nothing is bound to clk_in. */
> +    return input_rate;
> +}
> +
> +Modelling a clock multiplier
> +============================
> +
> +Here is a brief example of a device acting as a clock modifier:
> +
> +typedef struct {
> +    DeviceState parent_obj;
> +
> +    uint32_t rate;
> +    QEMUClock out;
> +    QEMUClock in;
> +} ClockMultiplier;
> +
> +As before the clocks must be initialized through the device initialize function
> +but they must be bound together so a change on the input will propagate on the
> +output:
> +
> +static void clock_multiplier_instance_init(Object *obj)
> +{
> +    ClockMultiplier *s = CLOCK_MULTIPLIER(obj);
> +
> +    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
> +    object_initialize(&s->in, sizeof(s->in), TYPE_CLOCK);
> +
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->in, "clk_in");
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
> +    /*
> +     * Propagate the change from in to out, this can be done dynamically during
> +     * the simulation but we need to do the initial binding here to get the
> +     * initial refresh happening when the realize function is called on the
> +     * fixed clock.
> +     */
> +    qemu_clk_bind(s->in, s->out);
> +    /*
> +     * But before propagating the rate modify it with multiplier_cb.
> +     */
> +    qemu_clk_set_callback(s->out, multiplier_cb, obj);
> +}
> +
> +In this example when clk_in changes it will trigger a change on clk_out and this
> +callback can modify the rate of clk_out (the return value of this callback)
> +accordingly to clk_in rate (input_rate).
> +
> +static uint64_t multiplier_cb(void *opaque, uint64_t input_rate)
> +{
> +    return input_rate * 4;
> +}
> +
> +This device doesn't refresh the clock tree as it will be done by the clock tree
> +source.
> +
> +Modelling a clock selector
> +==========================
> +
> +Here is a brief example of a device acting as a device which alter the clock
> +topology such as a clock selector:
> +
> +typedef struct {
> +    DeviceState parent_obj;
> +
> +    QEMUClock out;
> +    QEMUClock in_a;
> +    QEMUClock in_b;
> +} ClockSelector;
> +
> +The clocks must be initialized through the device initialize function but they
> +must be bound together like they will be when the device is reset so a change on
> +the input during the realize of the fixed clock will propagate to the output:
> +
> +static void clock_selector_instance_init(Object *obj)
> +{
> +    ClockSelector *s = CLOCK_SELECTOR(obj);
> +
> +    object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
> +    object_initialize(&s->in_a, sizeof(s->in_a), TYPE_CLOCK);
> +    object_initialize(&s->in_b, sizeof(s->in_b), TYPE_CLOCK);
> +
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->in_a, "clk_in_a");
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->in_b, "clk_in_b");
> +    qemu_clk_device_add_clock(DEVICE(obj), &s->out, "clk_out");
> +
> +    /* Assuming at the reset that the input_a is connected to output. */
> +    qemu_clk_bind(s->in_a, s->out);
> +}
> +
> +/* This is called for example by a register change or something like that */
> +void update_topology(ClockSelector *s)
> +{
> +    /* Unbind the old clock */
> +    qemu_clk_unbind(s->in_a, s->out);
> +    /* Bind the new one, the rate is automatically refreshed. */
> +    qemu_clk_bind(s->in_b, s->out);
> +}
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 15 June 2017 at 16:15, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:
>> The difference here is that the clock objects themselves have
>> internal state. That's not necessarily a bad idea, but it does
>> mean that something's got to migrate that state or otherwise
>> regenerate it. (Anthony once proposed that we should change qemu_irq
>> objects to have internal state, because that's effectively what real
>> hardware is and it would save the need for each device to track its
>> input line state and be notified if the line didn't actually change
>> state. It would just have been an enormous upheaval and migration
>> compat break...)
>
> Thanks. I didn't realize that the internal clock state would be used in a
> way that is not deriveable from other device state & input clocks.
> I'm not sure this is a use-case we need to support, thoughts?
>
> Maybe there's some value in keeping interrupt and clock handling alike,
> e.g removing the internal state from clocks.

It occurs to me that it might be possible to split the
difference, ie the clock object still has internal state,
but the device that owns it is responsible for that state's
migration, so you would have
  VMSTATE_CLOCK(myclock, MyDeviceState),

in the vmstate for every clock you owned. (Not sure whether
that should be "every input clock" or "every output clock"
or even both?)

As an aside, I still find it very odd that you get a clock
object for both an input clock and an output clock. I feel
like we should have one end owns the clock object and the
other just has a reference to it of some kind.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 11:51 AM, Peter Maydell a écrit :
> On 15 June 2017 at 16:15, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
>> On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:
>>> The difference here is that the clock objects themselves have
>>> internal state. That's not necessarily a bad idea, but it does
>>> mean that something's got to migrate that state or otherwise
>>> regenerate it. (Anthony once proposed that we should change qemu_irq
>>> objects to have internal state, because that's effectively what real
>>> hardware is and it would save the need for each device to track its
>>> input line state and be notified if the line didn't actually change
>>> state. It would just have been an enormous upheaval and migration
>>> compat break...)
>>
>> Thanks. I didn't realize that the internal clock state would be used in a
>> way that is not deriveable from other device state & input clocks.
>> I'm not sure this is a use-case we need to support, thoughts?
>>
>> Maybe there's some value in keeping interrupt and clock handling alike,
>> e.g removing the internal state from clocks.
> 
> It occurs to me that it might be possible to split the
> difference, ie the clock object still has internal state,
> but the device that owns it is responsible for that state's
> migration, so you would have
>    VMSTATE_CLOCK(myclock, MyDeviceState),

Yes I think that makes sense.

> 
> in the vmstate for every clock you owned. (Not sure whether
> that should be "every input clock" or "every output clock"
> or even both?)
> 
> As an aside, I still find it very odd that you get a clock
> object for both an input clock and an output clock. I feel
> like we should have one end owns the clock object and the
> other just has a reference to it of some kind.

The point is beeing able to bind/unbind clocks inside the device
to model a clock selector for example. So you just bind/unbind
the input and output clocks.

Secondly you can print the clock input rate in the mtree command
so you can eg: debug it.

Thanks,
Fred

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 23 June 2017 at 13:38, KONRAD Frederic <frederic.konrad@adacore.com> wrote:
> Le 06/23/2017 à 11:51 AM, Peter Maydell a écrit :
>> As an aside, I still find it very odd that you get a clock
>> object for both an input clock and an output clock. I feel
>> like we should have one end owns the clock object and the
>> other just has a reference to it of some kind.
>
>
> The point is beeing able to bind/unbind clocks inside the device
> to model a clock selector for example. So you just bind/unbind
> the input and output clocks.

I don't see why this needs both 'input clock' and 'output
clock' to be separate objects. What I have in mind is:

Device A:
  has-a clock C1

Device B:
  has-a clock C2

Device C: (a clock selector)
  clock inputs CI1, CI2
  has-a clock C3

Each device "owns" its output clock objects, but input
clocks are just pointers to the clock object owned by the
device at the other end. In the board you wire up CI1 to C1,
and CI2 to C2 (using link properties I guess).
Then in device C you can implement the clock switching by
some kind of bind(s->CI1, &s->C3) call because you have
pointers to all the relevant clock objects.

As I understand it your current implementation makes not
just the output clocks C1 C2 C2 be clock objects, but also
the inputs CI1 CI2, so effectively each link from a clock
source to a clock sink has two objects involved.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 02:47 PM, Peter Maydell a écrit :
> On 23 June 2017 at 13:38, KONRAD Frederic <frederic.konrad@adacore.com> wrote:
>> Le 06/23/2017 à 11:51 AM, Peter Maydell a écrit :
>>> As an aside, I still find it very odd that you get a clock
>>> object for both an input clock and an output clock. I feel
>>> like we should have one end owns the clock object and the
>>> other just has a reference to it of some kind.
>>
>>
>> The point is beeing able to bind/unbind clocks inside the device
>> to model a clock selector for example. So you just bind/unbind
>> the input and output clocks.
> 
> I don't see why this needs both 'input clock' and 'output
> clock' to be separate objects. What I have in mind is:
> 
> Device A:
>    has-a clock C1
> 
> Device B:
>    has-a clock C2
> 
> Device C: (a clock selector)
>    clock inputs CI1, CI2
>    has-a clock C3
> 
> Each device "owns" its output clock objects, but input
> clocks are just pointers to the clock object owned by the
> device at the other end. In the board you wire up CI1 to C1,
> and CI2 to C2 (using link properties I guess).
> Then in device C you can implement the clock switching by
> some kind of bind(s->CI1, &s->C3) call because you have
> pointers to all the relevant clock objects.
> 
> As I understand it your current implementation makes not
> just the output clocks C1 C2 C2 be clock objects, but also
> the inputs CI1 CI2, so effectively each link from a clock
> source to a clock sink has two objects involved.

Yes that makes sense but you won't have the name.

And one other thing I wanted to have is beeing able to refresh
the clock tree without refreshing everything. If you start a
refresh from the "pointer" and it is bind to other things you
will refresh the other devices as well. Maybe we don't care or we
can workaround that but..

Fred

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 23 June 2017 at 14:07, KONRAD Frederic <frederic.konrad@adacore.com> wrote:
> Le 06/23/2017 à 02:47 PM, Peter Maydell a écrit :
>> Each device "owns" its output clock objects, but input
>> clocks are just pointers to the clock object owned by the
>> device at the other end. In the board you wire up CI1 to C1,
>> and CI2 to C2 (using link properties I guess).
>> Then in device C you can implement the clock switching by
>> some kind of bind(s->CI1, &s->C3) call because you have
>> pointers to all the relevant clock objects.
>>
>> As I understand it your current implementation makes not
>> just the output clocks C1 C2 C2 be clock objects, but also
>> the inputs CI1 CI2, so effectively each link from a clock
>> source to a clock sink has two objects involved.

> Yes that makes sense but you won't have the name.

I don't think that's a big deal.

> And one other thing I wanted to have is beeing able to refresh
> the clock tree without refreshing everything. If you start a
> refresh from the "pointer" and it is bind to other things you
> will refresh the other devices as well. Maybe we don't care or we
> can workaround that but..

The pointer is for your clock inputs -- when would you
want to start a refresh from that? I would expect
refreshes to only ever go downstream -- you update
the config of your clock outputs and things downstream
of them will update in turn.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 03:58 PM, Peter Maydell a écrit :
> On 23 June 2017 at 14:07, KONRAD Frederic <frederic.konrad@adacore.com> wrote:
>> Le 06/23/2017 à 02:47 PM, Peter Maydell a écrit :
>>> Each device "owns" its output clock objects, but input
>>> clocks are just pointers to the clock object owned by the
>>> device at the other end. In the board you wire up CI1 to C1,
>>> and CI2 to C2 (using link properties I guess).
>>> Then in device C you can implement the clock switching by
>>> some kind of bind(s->CI1, &s->C3) call because you have
>>> pointers to all the relevant clock objects.
>>>
>>> As I understand it your current implementation makes not
>>> just the output clocks C1 C2 C2 be clock objects, but also
>>> the inputs CI1 CI2, so effectively each link from a clock
>>> source to a clock sink has two objects involved.
> 
>> Yes that makes sense but you won't have the name.
> 
> I don't think that's a big deal.
> 
>> And one other thing I wanted to have is beeing able to refresh
>> the clock tree without refreshing everything. If you start a
>> refresh from the "pointer" and it is bind to other things you
>> will refresh the other devices as well. Maybe we don't care or we
>> can workaround that but..
> 
> The pointer is for your clock inputs -- when would you
> want to start a refresh from that? I would expect
> refreshes to only ever go downstream -- you update
> the config of your clock outputs and things downstream
> of them will update in turn.

I started with the goal in mind that the binding + the callback
can refresh themself without user intervention.

So actually the user only have to change the binding in case of a
clock selector.. Everything else is done by the framework.

For example if we want to change a multiplier through a register:

void register_write(..)
{
   /* Refresh related clock input. */
}

void clock_cb(..)
{
   return register_value * rate_in;
}

If we drop the clock first possibility:

void register_write(..)
{
   /* refresh all depending clock with
    * rate_in * register value.
    * Either this can be tricky as we need to know exactly which
    * clock need to be refreshed or we need to refresh everybody.
    * On the device example in the patch-set this can become a
    * mess.
    */
}

void clock_cb(..)
{
   return register_value * rate_in;
}

Second possibility:

void register_write(..)
{
   /* refresh the clock which is referenced as input.
    * This is easy BUT it will refresh all other devices bound to
    * this clock.
    */
}

void clock_cb(..)
{
   return register_value * rate_in;
}

Thanks,
Fred

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[Peter Maydell]

On 27 June 2017 at 08:04, KONRAD Frederic <frederic.konrad@adacore.com> wrote:
> Le 06/23/2017 à 03:58 PM, Peter Maydell a écrit :
>> The pointer is for your clock inputs -- when would you
>> want to start a refresh from that? I would expect
>> refreshes to only ever go downstream -- you update
>> the config of your clock outputs and things downstream
>> of them will update in turn.
>
>
> I started with the goal in mind that the binding + the callback
> can refresh themself without user intervention.
>
> So actually the user only have to change the binding in case of a
> clock selector.. Everything else is done by the framework.

I agree you only need to change the bindings for clock selectors,
that should be true whatever approach we take.

> For example if we want to change a multiplier through a register:
>
> void register_write(..)
> {
>   /* Refresh related clock input. */
> }
>
> void clock_cb(..)
> {
>   return register_value * rate_in;
> }
>
> If we drop the clock first possibility:
>
> void register_write(..)
> {
>   /* refresh all depending clock with
>    * rate_in * register value.
>    * Either this can be tricky as we need to know exactly which
>    * clock need to be refreshed or we need to refresh everybody.
>    * On the device example in the patch-set this can become a
>    * mess.
>    */
> }
>
> void clock_cb(..)
> {
>   return register_value * rate_in;
> }
>
> Second possibility:
>
> void register_write(..)
> {
>   /* refresh the clock which is referenced as input.
>    * This is easy BUT it will refresh all other devices bound to
>    * this clock.
>    */

If this device can change the multiplier for a clock
then it must be changing the multiplier for one of
its *output* clocks. The input clock is owned by
whatever device controls that clock. (In hardware,
a device can't change the frequency of a clock signal
that is an input to it.)

Eg you might have a fixed clock (output) representing
the system clock, and a PLL device which takes that
clock as an input, and has one or more output clocks
at various rates whose multipliers can be changed.
Changing the multiplier value in a register in the
PLL device should make it change and refresh the
relevant output clock. (If a device has a builtin
clock divider then you'd model that as it having an
input clock, plus an "output" clock which it doesn't
actually expose to the rest of the world but just
consumes internally. The builtin divider would
change the settings on the 'output' clock.)

thanks
-- PMM