Hi Damien,
On 10/2/18 4:24 PM, Damien Hedde wrote:
> Add functions to easily add input or output clocks to a device.
> The clock port objects are added as children of the device.
>
> A function allows to connect two clocks together.
> It should be called by some toplevel to make a connection between
> 2 (sub-)devices.
>
> Also add a function which forwards a port to another device. This
> function allows, in the case of device composition, to expose a
> sub-device clock port as its own clock port.
> This is really an alias: when forwarding an input, only one callback can
> be registered on it since there is only one Clockin object behind all
> aliases.
>
> This is based on the original work of Frederic Konrad.
>
> Signed-off-by: Damien Hedde <damien.he...@greensocs.com>
> ---
> include/hw/qdev-clock.h | 62 ++++++++++++++++++
> include/hw/qdev-core.h | 14 ++++
> include/hw/qdev.h | 1 +
> hw/core/qdev-clock.c | 140 ++++++++++++++++++++++++++++++++++++++++
> hw/core/qdev.c | 29 +++++++++
> hw/core/Makefile.objs | 2 +-
> 6 files changed, 247 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/qdev-clock.h
> create mode 100644 hw/core/qdev-clock.c
>
> diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
> new file mode 100644
> index 0000000000..d76aa9f479
> --- /dev/null
> +++ b/include/hw/qdev-clock.h
> @@ -0,0 +1,62 @@
> +#ifndef QDEV_CLOCK_H
> +#define QDEV_CLOCK_H
> +
> +#include "hw/clock-port.h"
> +#include "hw/qdev-core.h"
> +#include "qapi/error.h"
> +
> +/**
> + * qdev_init_clock_in:
> + * @dev: the device in which to add a clock
> + * @name: the name of the clock (can't be NULL).
> + * @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_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);
Indent ;)
> +
> +/**
> + * qdev_connect_clock:
> + * @dev: the drived clock device.
> + * @name: the drived clock name.
> + * @driver: the driving clock device.
> + * @driver_name: the driving clock name.
> + * @errp: error report
> + *
> + * Setup @driver_name output clock of @driver to drive @name input clock of
> + * @dev. Errors are trigerred if clock does not exists
> + */
> +void qdev_connect_clock(DeviceState *dev, const char *name,
> + DeviceState *driver, const char *driver_name,
> + Error **errp);
> +
> +#endif /* QDEV_CLOCK_H */
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index f1fd0f8736..e6014d3a41 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -127,6 +127,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.
> @@ -147,6 +160,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/include/hw/qdev.h b/include/hw/qdev.h
> index 5cb8b080a6..b031da7b41 100644
> --- a/include/hw/qdev.h
> +++ b/include/hw/qdev.h
> @@ -4,5 +4,6 @@
> #include "hw/hw.h"
> #include "hw/qdev-core.h"
> #include "hw/qdev-properties.h"
> +#include "hw/qdev-clock.h"
>
> #endif
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> new file mode 100644
> index 0000000000..f0e4839aed
> --- /dev/null
> +++ b/hw/core/qdev-clock.c
> @@ -0,0 +1,140 @@
> +/*
> + * Device's clock
> + *
> + * Copyright GreenSocs 2016-2018
> + *
> + * 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 "qom/object.h"
> +#include "hw/qdev-clock.h"
> +#include "qapi/error.h"
> +
> +static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char
> *name,
> + bool forward)
> +{
> + 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);
> +
Maybe add a comment "This will be free'd in device_finalize()".
> + ncl = g_malloc0(sizeof(*ncl));
> + 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)
> +{
> + 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;
> +
> + 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);
Indent.
> +}
> +
> +void qdev_connect_clock(DeviceState *dev, const char *name,
> + DeviceState *driver, const char *driver_name,
> + Error **errp)
> +{
> + NamedClockList *ncl, *drv_ncl;
> +
> + assert(dev && name);
> + assert(driver && driver_name);
> +
> + ncl = qdev_get_clocklist(dev, name);
> + if (!ncl || !ncl->in) {
> + error_setg(errp, "no input clock '%s' in device", name);
> + return;
> + }
> +
> + drv_ncl = qdev_get_clocklist(driver, driver_name);
> + if (!drv_ncl || !drv_ncl->out) {
> + error_setg(errp, "no output clock '%s' in driver", driver_name);
> + return;
> + }
> +
> + clock_connect(ncl->in , drv_ncl->out);
> +}
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 529b82de18..c48edf180f 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -790,6 +790,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;
> @@ -846,6 +847,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,
> @@ -972,6 +982,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)
> @@ -983,6 +994,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);
>
> @@ -996,6 +1008,23 @@ 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 & clk->out are child
> of
> + * dev.
> + * At this point the properties and associated reference are
> + * already deleted but we kept a ref on clk->in to ensure we
> + * don't have a lost callback to a deleted device somewhere.
> + */
> + clock_clear_callback(clk->in);
> + object_unref(OBJECT(clk->in));
> + }
> + g_free(clk->name);
> + g_free(clk);
OK.
> + }
> +
> /* Only send event if the device had been completely realized */
> if (dev->pending_deleted_event) {
> g_assert(dev->canonical_path);
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index f7102121f4..fc0505e716 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-$(CONFIG_SOFTMMU) += qdev-fw.o
> common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
