Nitpick: remove trailing dot in patch subject
On 9/4/19 2:55 PM, Damien Hedde wrote:
Add functions to easily add input or output clocks to a device.
A clock objects is added as a child of the device.
<newline>?
The api is very similar the gpio's one.
Maybe "This API is very similar to the QDEV GPIO API."
This is based on the original work of Frederic Konrad.
Signed-off-by: Damien Hedde <damien.he...@greensocs.com>
---
I've removed the reviewed-by/tested-by of Philippe because I did a small
modification.
qdev_connect_clock() which allowed to connect an input to an output is
now split in 2:
+ qdev_get_clock_in() which gets a given input from a device
+ qdev_connect_clock_out() which connect a given output to a clock
(previously fetched by qdev_get_clock_in())
This part is located in (qdev-clock.[c|h]).
It better matches gpios api and also add the possibility to connect a
device's input clock to a random output clock (used in patch 9).
Also add missing qdev-clock in the test-qdev-global-props so that tests
pass.
---
hw/core/Makefile.objs | 2 +-
hw/core/qdev-clock.c | 155 ++++++++++++++++++++++++++++++++++++++++
hw/core/qdev.c | 32 +++++++++
include/hw/qdev-clock.h | 67 +++++++++++++++++
include/hw/qdev-core.h | 14 ++++
tests/Makefile.include | 1 +
Please setup the scripts/git.orderfile to ease reviews.
6 files changed, 270 insertions(+), 1 deletion(-)
create mode 100644 hw/core/qdev-clock.c
create mode 100644 include/hw/qdev-clock.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 8fcebf2e67..4523d3b5c7 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,5 +1,5 @@
# core qdev-related obj files, also used by *-user:
-common-obj-y += qdev.o qdev-properties.o
+common-obj-y += qdev.o qdev-properties.o qdev-clock.o
common-obj-y += bus.o reset.o
common-obj-y += resettable.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
new file mode 100644
index 0000000000..bebdd8fa15
--- /dev/null
+++ b/hw/core/qdev-clock.c
@@ -0,0 +1,155 @@
+/*
+ * Device's clock
+ *
+ * Copyright GreenSocs 2016-2018
2019
+ *
+ * Authors:
+ * Frederic Konrad <fred.kon...@greensocs.com>
+ * Damien Hedde <damien.he...@greensocs.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-core.h"
+#include "qapi/error.h"
+
+static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
+ bool forward)
Indentation off.
+{
+ NamedClockList *ncl;
+
+ /*
+ * The clock path will be computed by the device's realize function call.
+ * This is required to ensure the clock's canonical path is right and log
+ * messages are meaningfull.
+ */
+ assert(name);
+ assert(!dev->realized);
+
+ /* The ncl structure will be freed in device's finalize function call */
+ ncl = g_malloc0(sizeof(*ncl));
Similar but easier to review:
ncl = g_new0(NamedClockList, 1)
+ ncl->name = g_strdup(name);
+ ncl->forward = forward;
+
+ QLIST_INSERT_HEAD(&dev->clocks, ncl, node);
+ return ncl;
+}
+
+ClockOut *qdev_init_clock_out(DeviceState *dev, const char *name)
+{
+ NamedClockList *ncl;
+ Object *clk;
+
+ ncl = qdev_init_clocklist(dev, name, false);
+
+ clk = object_new(TYPE_CLOCK_OUT);
+
+ /* will fail if name already exists */
+ object_property_add_child(OBJECT(dev), name, clk, &error_abort);
+ object_unref(clk); /* remove the initial ref made by object_new */
+
+ ncl->out = CLOCK_OUT(clk);
+ return ncl->out;
+}
+
+ClockIn *qdev_init_clock_in(DeviceState *dev, const char *name,
+ ClockCallback *callback, void *opaque)
Indentation off.
+{
+ NamedClockList *ncl;
+ Object *clk;
+
+ ncl = qdev_init_clocklist(dev, name, false);
+
+ clk = object_new(TYPE_CLOCK_IN);
+ /*
+ * the ref initialized by object_new will be cleared during dev finalize.
+ * It allows us to safely remove the callback.
+ */
+
+ /* will fail if name already exists */
+ object_property_add_child(OBJECT(dev), name, clk, &error_abort);
+
+ ncl->in = CLOCK_IN(clk);
+ if (callback) {
+ clock_set_callback(ncl->in, callback, opaque);
+ }
+ return ncl->in;
+}
+
+static NamedClockList *qdev_get_clocklist(DeviceState *dev, const char *name)
+{
+ NamedClockList *ncl;
+
+ QLIST_FOREACH(ncl, &dev->clocks, node) {
+ if (strcmp(name, ncl->name) == 0) {
+ return ncl;
+ }
+ }
+
+ return NULL;
+}
+
+void qdev_pass_clock(DeviceState *dev, const char *name,
+ DeviceState *container, const char *cont_name)
+{
+ NamedClockList *original_ncl, *ncl;
+ Object **clk;
Is it really a Object** or a Object*?
+
+ assert(container && cont_name);
+
+ original_ncl = qdev_get_clocklist(container, cont_name);
+ assert(original_ncl); /* clock must exist in origin */
+
+ ncl = qdev_init_clocklist(dev, name, true);
+
+ if (ncl->out) {
+ clk = (Object **)&ncl->out;
+ } else {
+ clk = (Object **)&ncl->in;
+ }
+
+ /* will fail if name already exists */
+ object_property_add_link(OBJECT(dev), name, object_get_typename(*clk),
+ clk, NULL, OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
+ClockIn *qdev_get_clock_in(DeviceState *dev, const char *name)
+{
+ NamedClockList *ncl;
+
+ assert(dev && name);
+
+ ncl = qdev_get_clocklist(dev, name);
+ return ncl ? ncl->in : NULL;
+}
+
+static ClockOut *qdev_get_clock_out(DeviceState *dev, const char *name)
+{
+ NamedClockList *ncl;
+
+ assert(dev && name);
+
+ ncl = qdev_get_clocklist(dev, name);
+ return ncl ? ncl->out : NULL;
+}
+
+void qdev_connect_clock_out(DeviceState *dev, const char *name, ClockIn *clk,
+ Error **errp)
+{
+ ClockOut *clkout = qdev_get_clock_out(dev, name);
+
+ if (!clk) {
+ error_setg(errp, "NULL input clock");
+ return;
+ }
+
+ if (!clkout) {
+ error_setg(errp, "no output clock '%s' in device", name);
+ return;
+ }
+
+ clock_connect(clk, clkout);
+}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9095f2b9c1..eae6cd3e09 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -37,6 +37,7 @@
#include "hw/qdev-properties.h"
#include "hw/boards.h"
#include "hw/sysbus.h"
+#include "hw/clock.h"
#include "migration/vmstate.h"
bool qdev_hotplug = false;
@@ -821,6 +822,7 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
DeviceClass *dc = DEVICE_GET_CLASS(dev);
HotplugHandler *hotplug_ctrl;
BusState *bus;
+ NamedClockList *clk;
Error *local_err = NULL;
bool unattached_parent = false;
static int unattached_count;
@@ -869,6 +871,15 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
*/
g_free(dev->canonical_path);
dev->canonical_path = object_get_canonical_path(OBJECT(dev));
+ QLIST_FOREACH(clk, &dev->clocks, node) {
+ if (clk->forward) {
+ continue;
+ } else if (clk->in != NULL) {
+ clock_in_setup_canonical_path(clk->in);
+ } else {
+ clock_out_setup_canonical_path(clk->out);
+ }
+ }
if (qdev_get_vmsd(dev)) {
if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev),
dev,
@@ -999,6 +1010,7 @@ static void device_initfn(Object *obj)
(Object **)&dev->parent_bus, NULL, 0,
&error_abort);
QLIST_INIT(&dev->gpios);
+ QLIST_INIT(&dev->clocks);
}
static void device_post_init(Object *obj)
@@ -1015,6 +1027,7 @@ static void device_post_init(Object *obj)
static void device_finalize(Object *obj)
{
NamedGPIOList *ngl, *next;
+ NamedClockList *clk, *clk_next;
DeviceState *dev = DEVICE(obj);
@@ -1028,6 +1041,25 @@ static void device_finalize(Object *obj)
*/
}
+ QLIST_FOREACH_SAFE(clk, &dev->clocks, node, clk_next) {
+ QLIST_REMOVE(clk, node);
+ if (!clk->forward && clk->in) {
+ /*
+ * if this clock is not forwarded, clk->in is a child of dev.
+ * At this point the child property and associated reference is
+ * already deleted but we kept a ref on clk->in to ensure it lives
+ * up to this point and we can safely remove the callback.
+ * It avoids having a lost callback to a deleted device if the
+ * clk->in is still referenced somewhere else (eg: by a clock
+ * output).
+ */
+ clock_clear_callback(clk->in);
+ object_unref(OBJECT(clk->in));
+ }
+ g_free(clk->name);
+ g_free(clk);
+ }
+
/* Only send event if the device had been completely realized */
if (dev->pending_deleted_event) {
g_assert(dev->canonical_path);
diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
new file mode 100644
index 0000000000..c4ea912fdc
--- /dev/null
+++ b/include/hw/qdev-clock.h
@@ -0,0 +1,67 @@
+#ifndef QDEV_CLOCK_H
+#define QDEV_CLOCK_H
+
+#include "hw/clock.h"
+
+/**
+ * qdev_init_clock_in:
+ * @dev: the device in which to add a clock
+ * @name: the name of the clock (can't be NULL).
I'd drop the trailing dot in property descriptions.
+ * @callback: optional callback to be called on update or NULL.
+ * @opaque: argument for the callback
+ * @returns: a pointer to the newly added clock
+ *
+ * Add a input clock to device @dev as a clock named @name.
+ * This adds a child<> property.
+ * The callback will be called with @dev as opaque parameter.
+ */
+ClockIn *qdev_init_clock_in(DeviceState *dev, const char *name,
+ ClockCallback *callback, void *opaque);
+
+/**
+ * qdev_init_clock_out:
+ * @dev: the device to add a clock to
+ * @name: the name of the clock (can't be NULL).
+ * @callback: optional callback to be called on update or NULL.
+ * @returns: a pointer to the newly added clock
+ *
+ * Add a output clock to device @dev as a clock named @name.
+ * This adds a child<> property.
+ */
+ClockOut *qdev_init_clock_out(DeviceState *dev, const char *name);
+
+/**
+ * qdev_get_clock_in:
+ * @dev: the device which has the clock
+ * @name: the name of the clock (can't be NULL).
+ * @returns: a pointer to the clock
+ *
+ * Get the clock @name from @dev or NULL if does not exists.
+ */
+ClockIn *qdev_get_clock_in(DeviceState *dev, const char *name);
+
+/**
+ * qdev_connect_clock_out:
+ * @dev: the device which has the clock
+ * @name: the name of the clock (can't be NULL).
+ * @errp: error report
+ *
+ * Connect @clk to the output clock @name of @dev.
+ * Reports an error if clk is NULL or @name does not exists in @dev.
+ */
+void qdev_connect_clock_out(DeviceState *dev, const char *name, ClockIn *clk,
+ Error **errp);
+
+/**
+ * qdev_pass_clock:
+ * @dev: the device to forward the clock to
+ * @name: the name of the clock to be added (can't be NULL)
+ * @container: the device which already has the clock
+ * @cont_name: the name of the clock in the container device
+ *
+ * Add a clock @name to @dev which forward to the clock @cont_name in
@container
+ */
+void qdev_pass_clock(DeviceState *dev, const char *name,
+ DeviceState *container, const char *cont_name);
+
+#endif /* QDEV_CLOCK_H */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index eb11f0f801..60a65f6142 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -131,6 +131,19 @@ struct NamedGPIOList {
QLIST_ENTRY(NamedGPIOList) node;
};
+typedef struct NamedClockList NamedClockList;
+
+typedef struct ClockIn ClockIn;
+typedef struct ClockOut ClockOut;
+
+struct NamedClockList {
+ char *name;
+ bool forward;
+ ClockIn *in;
+ ClockOut *out;
+ QLIST_ENTRY(NamedClockList) node;
+};
+
/**
* DeviceState:
* @realized: Indicates whether the device has been fully constructed.
@@ -152,6 +165,7 @@ struct DeviceState {
int hotplugged;
BusState *parent_bus;
QLIST_HEAD(, NamedGPIOList) gpios;
+ QLIST_HEAD(, NamedClockList) clocks;
QLIST_HEAD(, BusState) child_bus;
int num_child_bus;
int instance_id_alias;
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f0b4628cc6..5c54beb29e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -566,6 +566,7 @@ tests/test-qdev-global-props$(EXESUF):
tests/test-qdev-global-props.o \
hw/core/irq.o \
hw/core/fw-path-provider.o \
hw/core/reset.o \
+ hw/core/clock.o \
$(test-qapi-obj-y)
tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
migration/vmstate.o migration/vmstate-types.o migration/qemu-file.o \
Except the cosmetic comments:
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
(Note, this series needs a rebase now).
