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

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

[fred.konrad]

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

Hi,

This is the second 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
(a9e404600a9bd1e6a26431fc89e5069092e67f14).

Thanks,
Fred

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                | 108 +++++
 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                  | 174 ++++++++
 11 files changed, 1598 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 V2 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>
---
 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 01cef86..de0219d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -78,6 +78,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..e7acd68
--- /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(struct qemu_clk, (obj), TYPE_CLOCK)
+
+typedef struct qemu_clk {
+    /*< private >*/
+    Object parent_obj;
+} *qemu_clk;
+
+#endif /* QEMU_CLOCK_H */
+
+
diff --git a/qemu-clock.c b/qemu-clock.c
new file mode 100644
index 0000000..ceea98d
--- /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(struct qemu_clk),
+};
+
+static void qemu_clk_register_types(void)
+{
+    type_register_static(&qemu_clk_info);
+}
+
+type_init(qemu_clk_register_types);
-- 
1.8.3.1

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

[Cédric Le Goater]

ello Frederic,

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This introduces qemu-clk qom object.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  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 01cef86..de0219d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -78,6 +78,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..e7acd68
> --- /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/>.

May be we could shorten the GPL v2 statement to something like :
 
 * This code is licensed under the GPL version 2 or later. See the
 * COPYING file in the top-level directory.

I haven't been very good at that my self, but we could save 
quite a few lines in qemu if files were not repeating the 
License statement.


> + *
> + */
> +
> +#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(struct qemu_clk, (obj), TYPE_CLOCK)
> +
> +typedef struct qemu_clk {
> +    /*< private >*/
> +    Object parent_obj;
> +} *qemu_clk;
>

CODING_STYLE says that we should use CamelCase for typedef. Also, I don't 
think the '*' is required. What's the idea ? 

> +#endif /* QEMU_CLOCK_H */
> +
> +
> diff --git a/qemu-clock.c b/qemu-clock.c
> new file mode 100644
> index 0000000..ceea98d
> --- /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);

I think the trace framework should be used instead.

Thanks

C.

> +static const TypeInfo qemu_clk_info = {
> +    .name          = TYPE_CLOCK,
> +    .parent        = TYPE_OBJECT,
> +    .instance_size = sizeof(struct qemu_clk),
> +};
> +
> +static void qemu_clk_register_types(void)
> +{
> +    type_register_static(&qemu_clk_info);
> +}
> +
> +type_init(qemu_clk_register_types);
> 

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

[Frederic Konrad]

On 02/05/2017 04:25 PM, Cédric Le Goater wrote:
> ello Frederic,

Hi Cédric,

Thanks for taking a look at this!

> 
> On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This introduces qemu-clk qom object.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>  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 01cef86..de0219d 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -78,6 +78,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..e7acd68
>> --- /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/>.
> 
> May be we could shorten the GPL v2 statement to something like :
>  
>  * This code is licensed under the GPL version 2 or later. See the
>  * COPYING file in the top-level directory.
> 
> I haven't been very good at that my self, but we could save 
> quite a few lines in qemu if files were not repeating the 
> License statement.

Well if it is really needed I can get rid of this few lines :).

> 
> 
>> + *
>> + */
>> +
>> +#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(struct qemu_clk, (obj), TYPE_CLOCK)
>> +
>> +typedef struct qemu_clk {
>> +    /*< private >*/
>> +    Object parent_obj;
>> +} *qemu_clk;
>>
> 
> CODING_STYLE says that we should use CamelCase for typedef. Also, I don't 
> think the '*' is required. What's the idea ? 

Your right, I tried to follow the gpio "style".. But maybe it's old and
we better go for the camelcase and without the pointer.

> 
>> +#endif /* QEMU_CLOCK_H */
>> +
>> +
>> diff --git a/qemu-clock.c b/qemu-clock.c
>> new file mode 100644
>> index 0000000..ceea98d
>> --- /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);
> 
> I think the trace framework should be used instead.

Seems not really standard AFAIK.
Some files use printf, some other qemu_log etc..

Thanks,
Fred
> 
> Thanks
> 
> C.
> 
>> +static const TypeInfo qemu_clk_info = {
>> +    .name          = TYPE_CLOCK,
>> +    .parent        = TYPE_OBJECT,
>> +    .instance_size = sizeof(struct qemu_clk),
>> +};
>> +
>> +static void qemu_clk_register_types(void)
>> +{
>> +    type_register_static(&qemu_clk_info);
>> +}
>> +
>> +type_init(qemu_clk_register_types);
>>
> 
> 

[Qemu-devel] [PATCH V2 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>

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 e7acd68..3e692d3 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -33,8 +33,30 @@
 typedef struct qemu_clk {
     /*< private >*/
     Object parent_obj;
+    char *name;            /* name of this clock in the device. */
 } *qemu_clk;
 
-#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, qemu_clk 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.
+ */
+qemu_clk 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 ceea98d..803deb3 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, qemu_clk 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);
+}
+
+qemu_clk 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

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

[Cédric Le Goater]

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> 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>
> 
> 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 e7acd68..3e692d3 100644
> --- a/include/qemu/qemu-clock.h
> +++ b/include/qemu/qemu-clock.h
> @@ -33,8 +33,30 @@
>  typedef struct qemu_clk {
>      /*< private >*/
>      Object parent_obj;
> +    char *name;            /* name of this clock in the device. */
>  } *qemu_clk;
>  
> -#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, qemu_clk 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.
> + */
> +qemu_clk 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 ceea98d..803deb3 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, qemu_clk 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);
> +}
> +
> +qemu_clk 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);
> +}


I see how these routines are used in patch 10/10. But if we were
open coding device CRF_APB, I don't think we would need them at
all and it would make the code a little simple IMHO.

Thanks,

C.  

>  static const TypeInfo qemu_clk_info = {
>      .name          = TYPE_CLOCK,
>      .parent        = TYPE_OBJECT,
> 

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

[Frederic Konrad]

On 02/06/2017 03:20 PM, Cédric Le Goater wrote:
> On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
>> 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>
>>
>> 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 e7acd68..3e692d3 100644
>> --- a/include/qemu/qemu-clock.h
>> +++ b/include/qemu/qemu-clock.h
>> @@ -33,8 +33,30 @@
>>  typedef struct qemu_clk {
>>      /*< private >*/
>>      Object parent_obj;
>> +    char *name;            /* name of this clock in the device. */
>>  } *qemu_clk;
>>  
>> -#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, qemu_clk 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.
>> + */
>> +qemu_clk 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 ceea98d..803deb3 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, qemu_clk 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);
>> +}
>> +
>> +qemu_clk 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);
>> +}
> 
> 
> I see how these routines are used in patch 10/10. But if we were
> open coding device CRF_APB, I don't think we would need them at
> all and it would make the code a little simple IMHO.

What do you mean by open coding?

Fred
> 
> Thanks,
> 
> C.  
> 
>>  static const TypeInfo qemu_clk_info = {
>>      .name          = TYPE_CLOCK,
>>      .parent        = TYPE_OBJECT,
>>
> 
> 

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

[Cédric Le Goater]

On 02/07/2017 10:22 AM, Frederic Konrad wrote:
>> I see how these routines are used in patch 10/10. But if we were
>> open coding device CRF_APB, I don't think we would need them at
>> all and it would make the code a little simple IMHO.
> What do you mean by open coding?

I mean to externalize the definition of the objects in a 
header file. 

Then you can replace the type 'Object *' of the attribute 
holding a reference to an instance by a non pointer type.

Anyhow, I think we need to replace object_new() by 
object_initialize() for QOM, and so we need to expose a 
little more the object internals. 

Thanks,

C.

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

[Frederic Konrad]

On 02/07/2017 10:31 AM, Cédric Le Goater wrote:
> On 02/07/2017 10:22 AM, Frederic Konrad wrote:
>>> I see how these routines are used in patch 10/10. But if we were
>>> open coding device CRF_APB, I don't think we would need them at
>>> all and it would make the code a little simple IMHO.
>> What do you mean by open coding?
> 
> I mean to externalize the definition of the objects in a 
> header file. 
> 
> Then you can replace the type 'Object *' of the attribute 
> holding a reference to an instance by a non pointer type.
> 
> Anyhow, I think we need to replace object_new() by 
> object_initialize() for QOM, and so we need to expose a 
> little more the object internals. 

Ok got you.

I'll take a look.

Thanks,
Fred

> 
> Thanks,
> 
> C.
> 
> 

[Qemu-devel] [PATCH V2 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>

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              | 56 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index 3e692d3..6d30299 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(struct qemu_clk, (obj), TYPE_CLOCK)
 
+typedef struct ClkList ClkList;
+typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
+
 typedef struct qemu_clk {
     /*< private >*/
     Object parent_obj;
     char *name;            /* name of this clock in the device. */
+    uint64_t in_rate;      /* rate of the clock which drive this pin. */
+    uint64_t out_rate;     /* rate of this clock pin. */
+    void *opaque;
+    QEMUClkRateUpdateCallback *cb;
+    QLIST_HEAD(, ClkList) bound;
 } *qemu_clk;
 
+struct ClkList {
+    qemu_clk 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, qemu_clk clk,
  */
 qemu_clk qemu_clk_device_get_clock(DeviceState *dev, const char *name);
 
+/**
+ * qemu_clk_bind_clock:
+ * @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_clock(qemu_clk out, qemu_clk in);
+
+/**
+ * qemu_clk_unbind:
+ * @out: the clock output.
+ * @in: the clock input.
+ *
+ * Disconnect the clocks if they were bound together.
+ *
+ */
+void qemu_clk_unbind(qemu_clk out, qemu_clk 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(qemu_clk 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(qemu_clk 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(qemu_clk clk,
+                           QEMUClkRateUpdateCallback *cb,
+                           void *opaque);
+
 #endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index 803deb3..8c12368 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -37,6 +37,62 @@
     }                                                                        \
 } while (0);
 
+void qemu_clk_refresh(qemu_clk clk)
+{
+    qemu_clk_update_rate(clk, clk->in_rate);
+}
+
+void qemu_clk_update_rate(qemu_clk clk, uint64_t rate)
+{
+    ClkList *child;
+
+    clk->in_rate = rate;
+    clk->out_rate = rate;
+
+    if (clk->cb) {
+        clk->out_rate = clk->cb(clk->opaque, rate);
+    }
+
+    DPRINTF("%s output rate updated to %" PRIu64 "\n",
+            object_get_canonical_path(OBJECT(clk)),
+            clk->out_rate);
+
+    QLIST_FOREACH(child, &clk->bound, node) {
+        qemu_clk_update_rate(child->clk, clk->out_rate);
+    }
+}
+
+void qemu_clk_bind_clock(qemu_clk out, qemu_clk in)
+{
+    ClkList *child;
+
+    child = g_malloc(sizeof(child));
+    assert(child);
+    child->clk = in;
+    QLIST_INSERT_HEAD(&out->bound, child, node);
+    qemu_clk_update_rate(in, out->out_rate);
+}
+
+void qemu_clk_unbind(qemu_clk out, qemu_clk in)
+{
+    ClkList *child, *next;
+
+    QLIST_FOREACH_SAFE(child, &out->bound, node, next) {
+        if (child->clk == in) {
+            QLIST_REMOVE(child, node);
+            g_free(child);
+        }
+    }
+}
+
+void qemu_clk_set_callback(qemu_clk clk,
+                           QEMUClkRateUpdateCallback *cb,
+                           void *opaque)
+{
+    clk->cb = cb;
+    clk->opaque = opaque;
+}
+
 void qemu_clk_device_add_clock(DeviceState *dev, qemu_clk clk,
                                const char *name)
 {
-- 
1.8.3.1

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

[Cédric Le Goater]

Hello,

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> 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>
> 
> 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              | 56 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 123 insertions(+)
> 
> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
> index 3e692d3..6d30299 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(struct qemu_clk, (obj), TYPE_CLOCK)
>  
> +typedef struct ClkList ClkList;
> +typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
> +
>  typedef struct qemu_clk {
>      /*< private >*/
>      Object parent_obj;
>      char *name;            /* name of this clock in the device. */
> +    uint64_t in_rate;      /* rate of the clock which drive this pin. */


So if this is the reference clock rate, which can be divided by 
settings in control registers, I would call the attribute 
'ref_rate' or 'rate_reference' 

> +    uint64_t out_rate;     /* rate of this clock pin. */

and this attribute could just be 'rate' and may be if we make 
a property out of it, we could get rid of FixedClock in patch
7/10.
   

> +    void *opaque;
> +    QEMUClkRateUpdateCallback *cb;

If I understand correctly, the need is to be able to refresh
the rate of a clock object depending on some settings in a 
controller. I think we should be using a derived class and 
an operation for it. it would look better from a QOM point 
of view. 

Here is a possibility,

We could make 'rate' a property and use a set property handler 
to call the derived class specific operation. This is very 
similar to the callback but we would remove the need of  
qemu_clk_update_rate() and use the std mechanisms already in
place  :

	object_property_set_int() 

qemu_clk_refresh() would still be needed to propagate the 
changes in the settings of a controller to the target clocks. 

The 'opaque' attribute, which holds the pointer to a controller 
object, would have to be passed to the derived clock object
with 
	object_property_add_const_link() 

and then grabbed with 

	object_property_get_link()

in the realize function of the derived clock object, or whenever 
it's needed, in the operation for instance.

This clearly means more code than the actual solution, but 
that is QOM way I think.  

> +    QLIST_HEAD(, ClkList) bound;
>  } *qemu_clk;
>  
> +struct ClkList {
> +    qemu_clk clk;
> +    QLIST_ENTRY(ClkList) node;
> +};
> +

Do we really need this intermediate object ? Can't we just
put the list_entry attribute under qemu_clk ? I haven't
tried, may be I am wrong but that would simplify the code.

>  /**
>   * 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, qemu_clk clk,
>   */
>  qemu_clk qemu_clk_device_get_clock(DeviceState *dev, const char *name);
>  
> +/**
> + * qemu_clk_bind_clock:
> + * @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_clock(qemu_clk out, qemu_clk in);

maybe remove the  _clock suffix. It feels redundant.

Thanks,

C. 

> +
> +/**
> + * qemu_clk_unbind:
> + * @out: the clock output.
> + * @in: the clock input.
> + *
> + * Disconnect the clocks if they were bound together.
> + *
> + */
> +void qemu_clk_unbind(qemu_clk out, qemu_clk 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(qemu_clk 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(qemu_clk 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(qemu_clk clk,
> +                           QEMUClkRateUpdateCallback *cb,
> +                           void *opaque);
> +
>  #endif /* QEMU_CLOCK_H */
> diff --git a/qemu-clock.c b/qemu-clock.c
> index 803deb3..8c12368 100644
> --- a/qemu-clock.c
> +++ b/qemu-clock.c
> @@ -37,6 +37,62 @@
>      }                                                                        \
>  } while (0);
>  
> +void qemu_clk_refresh(qemu_clk clk)
> +{
> +    qemu_clk_update_rate(clk, clk->in_rate);
> +}
> +
> +void qemu_clk_update_rate(qemu_clk clk, uint64_t rate)
> +{
> +    ClkList *child;
> +
> +    clk->in_rate = rate;
> +    clk->out_rate = rate;
> +
> +    if (clk->cb) {
> +        clk->out_rate = clk->cb(clk->opaque, rate);
> +    }
> +
> +    DPRINTF("%s output rate updated to %" PRIu64 "\n",
> +            object_get_canonical_path(OBJECT(clk)),
> +            clk->out_rate);
> +
> +    QLIST_FOREACH(child, &clk->bound, node) {
> +        qemu_clk_update_rate(child->clk, clk->out_rate);
> +    }
> +}
> +
> +void qemu_clk_bind_clock(qemu_clk out, qemu_clk in)
> +{
> +    ClkList *child;
> +
> +    child = g_malloc(sizeof(child));
> +    assert(child);
> +    child->clk = in;
> +    QLIST_INSERT_HEAD(&out->bound, child, node);
> +    qemu_clk_update_rate(in, out->out_rate);
> +}
> +
> +void qemu_clk_unbind(qemu_clk out, qemu_clk in)
> +{
> +    ClkList *child, *next;
> +
> +    QLIST_FOREACH_SAFE(child, &out->bound, node, next) {
> +        if (child->clk == in) {
> +            QLIST_REMOVE(child, node);
> +            g_free(child);
> +        }
> +    }
> +}
> +
> +void qemu_clk_set_callback(qemu_clk clk,
> +                           QEMUClkRateUpdateCallback *cb,
> +                           void *opaque)
> +{
> +    clk->cb = cb;
> +    clk->opaque = opaque;
> +}
> +
>  void qemu_clk_device_add_clock(DeviceState *dev, qemu_clk clk,
>                                 const char *name)
>  {
> 

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

[Frederic Konrad]

On 02/06/2017 04:58 PM, Cédric Le Goater wrote:
> Hello,
> 
> On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
>> 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>
>>
>> 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              | 56 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 123 insertions(+)
>>
>> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
>> index 3e692d3..6d30299 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(struct qemu_clk, (obj), TYPE_CLOCK)
>>  
>> +typedef struct ClkList ClkList;
>> +typedef uint64_t QEMUClkRateUpdateCallback(void *opaque, uint64_t rate);
>> +
>>  typedef struct qemu_clk {
>>      /*< private >*/
>>      Object parent_obj;
>>      char *name;            /* name of this clock in the device. */
>> +    uint64_t in_rate;      /* rate of the clock which drive this pin. */
> 
> 
> So if this is the reference clock rate, which can be divided by 
> settings in control registers, I would call the attribute 
> 'ref_rate' or 'rate_reference' 
> 
>> +    uint64_t out_rate;     /* rate of this clock pin. */
> 
> and this attribute could just be 'rate' and may be if we make 
> a property out of it, we could get rid of FixedClock in patch
> 7/10.
>    
> 
>> +    void *opaque;
>> +    QEMUClkRateUpdateCallback *cb;
> 
> If I understand correctly, the need is to be able to refresh
> the rate of a clock object depending on some settings in a 
> controller. I think we should be using a derived class and 
> an operation for it. it would look better from a QOM point 
> of view. 
> 
> Here is a possibility,
> 
> We could make 'rate' a property and use a set property handler 
> to call the derived class specific operation. This is very 
> similar to the callback but we would remove the need of  
> qemu_clk_update_rate() and use the std mechanisms already in
> place  :
> 
> 	object_property_set_int() 
> 
> qemu_clk_refresh() would still be needed to propagate the 
> changes in the settings of a controller to the target clocks. 
> 
> The 'opaque' attribute, which holds the pointer to a controller 
> object, would have to be passed to the derived clock object
> with 
> 	object_property_add_const_link() 
> 
> and then grabbed with 
> 
> 	object_property_get_link()
> 
> in the realize function of the derived clock object, or whenever 
> it's needed, in the operation for instance.
> 
> This clearly means more code than the actual solution, but 
> that is QOM way I think.  

Yes, the big issue here is that there are several clock inputs and clock
outputs. We need to be able to bind / unbind them if there is a selector
or some such.

So a device will have more than one clock object internally in it which
are "chained" to form a clock tree.

It seems overkill to me to implement one derived object per clock
"block" in the device to get the callback.

> 
>> +    QLIST_HEAD(, ClkList) bound;
>>  } *qemu_clk;
>>  
>> +struct ClkList {
>> +    qemu_clk clk;
>> +    QLIST_ENTRY(ClkList) node;
>> +};
>> +
> 
> Do we really need this intermediate object ? Can't we just
> put the list_entry attribute under qemu_clk ? I haven't
> tried, may be I am wrong but that would simplify the code.

Yes I think it is needed. Actually I didn't find anything which
does this differently.

> 
>>  /**
>>   * 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, qemu_clk clk,
>>   */
>>  qemu_clk qemu_clk_device_get_clock(DeviceState *dev, const char *name);
>>  
>> +/**
>> + * qemu_clk_bind_clock:
>> + * @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_clock(qemu_clk out, qemu_clk in);
> 
> maybe remove the  _clock suffix. It feels redundant.

Ok.

Thanks,
Fred

> 
> Thanks,
> 
> C. 
> 
>> +
>> +/**
>> + * qemu_clk_unbind:
>> + * @out: the clock output.
>> + * @in: the clock input.
>> + *
>> + * Disconnect the clocks if they were bound together.
>> + *
>> + */
>> +void qemu_clk_unbind(qemu_clk out, qemu_clk 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(qemu_clk 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(qemu_clk 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(qemu_clk clk,
>> +                           QEMUClkRateUpdateCallback *cb,
>> +                           void *opaque);
>> +
>>  #endif /* QEMU_CLOCK_H */
>> diff --git a/qemu-clock.c b/qemu-clock.c
>> index 803deb3..8c12368 100644
>> --- a/qemu-clock.c
>> +++ b/qemu-clock.c
>> @@ -37,6 +37,62 @@
>>      }                                                                        \
>>  } while (0);
>>  
>> +void qemu_clk_refresh(qemu_clk clk)
>> +{
>> +    qemu_clk_update_rate(clk, clk->in_rate);
>> +}
>> +
>> +void qemu_clk_update_rate(qemu_clk clk, uint64_t rate)
>> +{
>> +    ClkList *child;
>> +
>> +    clk->in_rate = rate;
>> +    clk->out_rate = rate;
>> +
>> +    if (clk->cb) {
>> +        clk->out_rate = clk->cb(clk->opaque, rate);
>> +    }
>> +
>> +    DPRINTF("%s output rate updated to %" PRIu64 "\n",
>> +            object_get_canonical_path(OBJECT(clk)),
>> +            clk->out_rate);
>> +
>> +    QLIST_FOREACH(child, &clk->bound, node) {
>> +        qemu_clk_update_rate(child->clk, clk->out_rate);
>> +    }
>> +}
>> +
>> +void qemu_clk_bind_clock(qemu_clk out, qemu_clk in)
>> +{
>> +    ClkList *child;
>> +
>> +    child = g_malloc(sizeof(child));
>> +    assert(child);
>> +    child->clk = in;
>> +    QLIST_INSERT_HEAD(&out->bound, child, node);
>> +    qemu_clk_update_rate(in, out->out_rate);
>> +}
>> +
>> +void qemu_clk_unbind(qemu_clk out, qemu_clk in)
>> +{
>> +    ClkList *child, *next;
>> +
>> +    QLIST_FOREACH_SAFE(child, &out->bound, node, next) {
>> +        if (child->clk == in) {
>> +            QLIST_REMOVE(child, node);
>> +            g_free(child);
>> +        }
>> +    }
>> +}
>> +
>> +void qemu_clk_set_callback(qemu_clk clk,
>> +                           QEMUClkRateUpdateCallback *cb,
>> +                           void *opaque)
>> +{
>> +    clk->cb = cb;
>> +    clk->opaque = opaque;
>> +}
>> +
>>  void qemu_clk_device_add_clock(DeviceState *dev, qemu_clk clk,
>>                                 const char *name)
>>  {
>>
> 
> 

[Qemu-devel] [PATCH V2 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>
---
 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 6d30299..45f8766 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 8c12368..300e38f 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)
+{
+    qemu_clk *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(qemu_clk clk)
 {
     qemu_clk_update_rate(clk, clk->in_rate);
-- 
1.8.3.1

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

[Cédric Le Goater]

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This introduces a clock init array to ease the clock tree construction.

I think this belongs to the zynqmp-crf model.

Thanks,

C. 

 
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  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 6d30299..45f8766 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 8c12368..300e38f 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)
> +{
> +    qemu_clk *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(qemu_clk clk)
>  {
>      qemu_clk_update_rate(clk, clk->in_rate);
> 

[Qemu-devel] [PATCH V2 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 45f8766..ccc381c 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -149,4 +149,13 @@ void qemu_clk_set_callback(qemu_clk 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 c73410c..8f6bbdf 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -29,6 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
 #include "sysemu/block-backend.h"
+#include "qemu/qemu-clock.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -689,6 +690,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 300e38f..b34021c 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
@@ -132,6 +133,33 @@ qemu_clk 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)
+{
+    qemu_clk clk = (qemu_clk)(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->out_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

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

[Cédric Le Goater]

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This prints the clock attached to a DeviceState when using "info qtree" monitor
> command.

It would be nice to print the bound clocks also.  

And so, may be, we could handle this binding between clocks
simply by using object_property_add_child() on the objects 
and build a QOM clock hierarchy. 

I would certainly make sense from a physical POV but would 
that be possible ? 

Thanks,

C.   


> 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 45f8766..ccc381c 100644
> --- a/include/qemu/qemu-clock.h
> +++ b/include/qemu/qemu-clock.h
> @@ -149,4 +149,13 @@ void qemu_clk_set_callback(qemu_clk 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 c73410c..8f6bbdf 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -29,6 +29,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/help_option.h"
>  #include "sysemu/block-backend.h"
> +#include "qemu/qemu-clock.h"
>  
>  /*
>   * Aliases were a bad idea from the start.  Let's keep them
> @@ -689,6 +690,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 300e38f..b34021c 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
> @@ -132,6 +133,33 @@ qemu_clk 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)
> +{
> +    qemu_clk clk = (qemu_clk)(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->out_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,
> 

[Qemu-devel] [PATCH V2 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 | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 docs/clock.txt

diff --git a/docs/clock.txt b/docs/clock.txt
new file mode 100644
index 0000000..aa23910
--- /dev/null
+++ b/docs/clock.txt
@@ -0,0 +1,108 @@
+
+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
+with the unmodified rate.
+
+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.
-- 
1.8.3.1

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

[Peter Maydell]

On 26 January 2017 at 09:47,  <fred.konrad@greensocs.com> wrote:
> 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 | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>  create mode 100644 docs/clock.txt

Could we have a simple example of what you need to do if you are:
 * a device providing a clock source of some kind
 * a device that takes a clock input and has one or more
   clock outputs which are configured by that device (for
   frequency, on/off, etc)
 * a device that consumes a clock

please? I think that would help make it more concrete how you
need to use these things.

thanks
-- PMM

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

[Frederic Konrad]

On 02/07/2017 05:13 PM, Peter Maydell wrote:
> On 26 January 2017 at 09:47,  <fred.konrad@greensocs.com> wrote:
>> 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 | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 108 insertions(+)
>>  create mode 100644 docs/clock.txt
> 
> Could we have a simple example of what you need to do if you are:
>  * a device providing a clock source of some kind

The fixed clock in patch 7 might be helpful for that.
Maybe I can just point to it in the documentation?

>  * a device that takes a clock input and has one or more
>    clock outputs which are configured by that device (for
>    frequency, on/off, etc)
>  * a device that consumes a clock

The CRF is doing that. Maybe I can add a simpler testcase device to
build a make check for that?

Fred

> 
> please? I think that would help make it more concrete how you
> need to use these things.
> 
> thanks
> -- PMM
> 

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

[Peter Maydell]

On 7 February 2017 at 17:15, Frederic Konrad <fred.konrad@greensocs.com> wrote:
> On 02/07/2017 05:13 PM, Peter Maydell wrote:
>> On 26 January 2017 at 09:47,  <fred.konrad@greensocs.com> wrote:
>>> 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 | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 108 insertions(+)
>>>  create mode 100644 docs/clock.txt
>>
>> Could we have a simple example of what you need to do if you are:
>>  * a device providing a clock source of some kind
>
> The fixed clock in patch 7 might be helpful for that.
> Maybe I can just point to it in the documentation?
>
>>  * a device that takes a clock input and has one or more
>>    clock outputs which are configured by that device (for
>>    frequency, on/off, etc)
>>  * a device that consumes a clock
>
> The CRF is doing that. Maybe I can add a simpler testcase device to
> build a make check for that?

I don't really want a whole test case device (which is a lot
of extra boilerplate and detail obscuring the necessities),
I'd just like a sketch of what these devices need to do in
the documentation.

thanks
-- PMM

[Qemu-devel] [PATCH V2 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 1a89615..2670c2d 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -53,3 +53,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..aa124d8
--- /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;
+    struct qemu_clk 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

Re: [Qemu-devel] [PATCH V2 07/10] introduce fixed-clock

[Cédric Le Goater]

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> 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 1a89615..2670c2d 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -53,3 +53,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..aa124d8
> --- /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;
> +    struct qemu_clk out;

This really feels redundant. See my previous suggestion.

Thanks,

C. 

> +} 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 */
> 

[Qemu-devel] [PATCH V2 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 2670c2d..60f9f4d 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -43,6 +43,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..e4b9225
--- /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 */
+    qemu_clk pss_ref_clk;
+    qemu_clk video_clk;
+    qemu_clk pss_alt_ref_clk;
+    qemu_clk aux_refclk;
+    qemu_clk gt_crx_ref_clk;
+
+    /* internal clocks */
+    qemu_clk apll_clk;
+    qemu_clk dpll_clk;
+    qemu_clk vpll_clk;
+
+    /* output clocks */
+    qemu_clk acpu_clk;
+    qemu_clk dbg_trace;
+    qemu_clk dbg_fdp;
+    qemu_clk dp_video_ref;
+    qemu_clk dp_audio_ref;
+    qemu_clk dp_stc_ref;
+    qemu_clk ddr;
+    qemu_clk gpu_ref;
+    qemu_clk sata_ref;
+    qemu_clk pcie_ref;
+    qemu_clk gdma_ref;
+    qemu_clk dpdma_ref;
+    qemu_clk topsw_main;
+    qemu_clk topsw_lsbus;
+    qemu_clk dbg_tstmp;
+    qemu_clk apll_to_lpd;
+    qemu_clk dpll_to_lpd;
+    qemu_clk 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_clock(s->video_clk, s->apll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind_clock(s->pss_alt_ref_clk, s->apll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind_clock(s->aux_refclk, s->apll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind_clock(s->gt_crx_ref_clk, s->apll_clk);
+        break;
+    default:
+        qemu_clk_bind_clock(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_clock(s->video_clk, s->dpll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind_clock(s->pss_alt_ref_clk, s->dpll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind_clock(s->aux_refclk, s->dpll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind_clock(s->gt_crx_ref_clk, s->dpll_clk);
+        break;
+    default:
+        qemu_clk_bind_clock(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_clock(s->video_clk, s->vpll_clk);
+        break;
+    case PSS_ALT_REF_CLK:
+        qemu_clk_bind_clock(s->pss_alt_ref_clk, s->vpll_clk);
+        break;
+    case AUX_REF_CLK:
+        qemu_clk_bind_clock(s->aux_refclk, s->vpll_clk);
+        break;
+    case GT_CRX_REF_CLK:
+        qemu_clk_bind_clock(s->gt_crx_ref_clk, s->vpll_clk);
+        break;
+    default:
+        qemu_clk_bind_clock(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_clock(s->vpll_clk, s->dp_video_ref);
+        break;
+    case 0x02:
+        qemu_clk_bind_clock(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_clock(s->pss_ref_clk, s->apll_clk);
+    qemu_clk_bind_clock(s->pss_ref_clk, s->dpll_clk);
+    qemu_clk_bind_clock(s->pss_ref_clk, s->vpll_clk);
+    qemu_clk_bind_clock(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_clock(s->apll_clk, s->apll_to_lpd);
+    qemu_clk_bind_clock(s->dpll_clk, s->dpll_to_lpd);
+    qemu_clk_bind_clock(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 V2 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 V2 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..1bef77d 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_clock(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_clock(qemu_clk_device_get_clock(DEVICE(s->video_clk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf), "video_clk"));
+
+    qemu_clk_bind_clock(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_clock(qemu_clk_device_get_clock(DEVICE(s->aux_refclk),
+                                                  "clk_out"),
+                        qemu_clk_device_get_clock(DEVICE(s->crf),
+                                                  "aux_refclk"));
+
+    qemu_clk_bind_clock(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 V2 10/10] zynqmp: add reference clock

[Cédric Le Goater]

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> 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..1bef77d 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);

object_initialize() should be preferred to object_new(). 

The exact reason is not very clear to me but a maintainer 
answered to some code of mine saying that object_new() can 
break QOM lifetime rules it seems. May be others can chime 
in on that topic

Thanks,

C.  


> +    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_clock(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_clock(qemu_clk_device_get_clock(DEVICE(s->video_clk),
> +                                                  "clk_out"),
> +                        qemu_clk_device_get_clock(DEVICE(s->crf), "video_clk"));
> +
> +    qemu_clk_bind_clock(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_clock(qemu_clk_device_get_clock(DEVICE(s->aux_refclk),
> +                                                  "clk_out"),
> +                        qemu_clk_device_get_clock(DEVICE(s->crf),
> +                                                  "aux_refclk"));
> +
> +    qemu_clk_bind_clock(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;
> 

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

[Frederic Konrad]

Ping!

Thanks,
Fred

On 01/26/2017 10:47 AM, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> Hi,
> 
> This is the second 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
> (a9e404600a9bd1e6a26431fc89e5069092e67f14).
> 
> Thanks,
> Fred
> 
> 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                | 108 +++++
>  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                  | 174 ++++++++
>  11 files changed, 1598 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
>