/On Mon, Feb 24, 2020 at 9:12 AM Damien Hedde <damien.he...@greensocs.com> wrote
>
> Add functions to easily handle clocks with devices.
> Clock inputs and outputs should be used to handle clock propagation
> between devices.
> The API is very similar the GPIO API.
>
> This is based on the original work of Frederic Konrad.
>
> Signed-off-by: Damien Hedde <damien.he...@greensocs.com>
> ---
>
> I did not changed the constness of @name pointer field in
> NamedClockList structure.
> There is no obstacle to do it but the fact that we need to free the
> allocated data it points to. It is possible to make it const and
> hack/remove the const to call g_free but I don't know if its
> allowed in qemu.
>
> v7:
> + update ClockIn/Out types
> + qdev_connect_clock_out function removed / qdev_connect_clock_in added
> instead
> + qdev_pass_clock renamed to qdev_alias_clock
> + various small fixes (typos, comment, asserts) (Peter)
> + move device's instance_finalize code related to clock in qdev-clock.c
> ---
> include/hw/qdev-clock.h | 105 +++++++++++++++++++++++++
> include/hw/qdev-core.h | 12 +++
> hw/core/qdev-clock.c | 169 ++++++++++++++++++++++++++++++++++++++++
> hw/core/qdev.c | 12 +++
> hw/core/Makefile.objs | 2 +-
> tests/Makefile.include | 1 +
> 6 files changed, 300 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..899a95ca6a
> --- /dev/null
> +++ b/include/hw/qdev-clock.h
> @@ -0,0 +1,105 @@
> +/*
> + * Device's clock input and output
> + *
> + * Copyright GreenSocs 2016-2020
> + *
> + * Authors:
> + * Frederic Konrad
> + * Damien Hedde
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QDEV_CLOCK_H
> +#define QDEV_CLOCK_H
> +
> +#include "hw/clock.h"
> +
> +/**
> + * qdev_init_clock_in:
> + * @dev: the device to add an input clock to
> + * @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 an input clock to device @dev as a clock named @name.
> + * This adds a child<> property.
> + * The callback will be called with @opaque as opaque parameter.
> + */
> +Clock *qdev_init_clock_in(DeviceState *dev, const char *name,
> + ClockCallback *callback, void *opaque);
> +
> +/**
> + * qdev_init_clock_out:
> + * @dev: the device to add an output clock to
> + * @name: the name of the clock (can't be NULL).
> + * @callback: optional callback to be called on update or NULL.
qdev_init_clock_out() doesn't have a callback.
> + * @returns: a pointer to the newly added clock
> + *
> + * Add an output clock to device @dev as a clock named @name.
> + * This adds a child<> property.
> + */
> +Clock *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 input clock @name from @dev or NULL if does not exist.
> + */
> +Clock *qdev_get_clock_in(DeviceState *dev, const char *name);
> +
> +/**
> + * qdev_get_clock_out:
> + * @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 output clock @name from @dev or NULL if does not exist.
> + */
> +Clock *qdev_get_clock_out(DeviceState *dev, const char *name);
> +
> +/**
> + * qdev_connect_clock_in:
> + * @dev: a device
> + * @name: the name of an input clock in @dev
> + * @source: the source clock (an output clock of another device for example)
> + *
> + * Set the source clock of input clock @name of device @dev to @source.
> + * @source period update will be propagated to @name clock.
> + */
> +static inline void qdev_connect_clock_in(DeviceState *dev, const char *name,
> + Clock *source)
> +{
> + clock_set_source(qdev_get_clock_in(dev, name), source);
> +}
> +
> +/**
> + * qdev_alias_clock:
> + * @dev: the device which has the clock
> + * @name: the name of the clock in @dev (can't be NULL)
> + * @alias_dev: the device to add the clock
> + * @alias_name: the name of the clock in @container
> + * @returns: a pointer to the clock
> + *
> + * Add a clock @alias_name in @alias_dev which is an alias of the clock @name
> + * in @dev. The direction _in_ or _out_ will the same as the original.
> + * An alias clock must not be modified or used by @alias_dev and should
> + * typically be only only for device composition purpose.
> + */
> +Clock *qdev_alias_clock(DeviceState *dev, const char *name,
> + DeviceState *alias_dev, const char *alias_name);
> +
> +/**
> + * qdev_finalize_clocklist:
> + * @dev: the device being finalize
It probably should be:
@dev: the device being finalized
> + *
> + * Clear the clocklist from @dev. Only used internally in qdev.
> + */
> +void qdev_finalize_clocklist(DeviceState *dev);
> +
> +#endif /* QDEV_CLOCK_H */
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 1405b8a990..d87d989e72 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -149,6 +149,17 @@ struct NamedGPIOList {
> QLIST_ENTRY(NamedGPIOList) node;
> };
>
> +typedef struct Clock Clock;
> +typedef struct NamedClockList NamedClockList;
> +
> +struct NamedClockList {
> + char *name;
> + Clock *clock;
> + bool output;
> + bool alias;
> + QLIST_ENTRY(NamedClockList) node;
> +};
> +
> /**
> * DeviceState:
> * @realized: Indicates whether the device has been fully constructed.
> @@ -171,6 +182,7 @@ struct DeviceState {
> bool allow_unplug_during_migration;
> 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/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> new file mode 100644
> index 0000000000..9af0159517
> --- /dev/null
> +++ b/hw/core/qdev-clock.c
> @@ -0,0 +1,169 @@
> +/*
> + * Device's clock input and output
> + *
> + * Copyright GreenSocs 2016-2020
> + *
> + * Authors:
> + * Frederic Konrad
> + * Damien Hedde
> + *
> + * 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"
> +
> +/*
> + * qdev_init_clocklist:
> + * Add a new clock in a device
> + */
> +static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char
> *name,
> + bool output, Clock *clk)
> +{
> + NamedClockList *ncl;
> +
> + /*
> + * Clock must be added before realize() so that we can compute the
> + * clock's canonical path durint device_realize().
s/durint/during/g
> + */
> + assert(!dev->realized);
> +
> + /*
> + * The ncl structure is freed by qdev_finalize_clocklist() which will
> + * be called during @dev's device_finalize().
> + */
> + ncl = g_new0(NamedClockList, 1);
> + ncl->name = g_strdup(name);
> + ncl->output = output;
> + ncl->alias = (clk != NULL);
> +
> + /*
> + * Trying to create a clock whose name clashes with some other
> + * clock or property is a bug in the caller and we will abort().
> + */
> + if (clk == NULL) {
> + clk = CLOCK(object_new(TYPE_CLOCK));
> + object_property_add_child(OBJECT(dev), name, OBJECT(clk),
> &error_abort);
> + if (output) {
> + /*
> + * Remove object_new()'s initial reference.
> + * Note that for inputs, the reference created by object_new()
> + * will be deleted in qdev_finalize_clocklist().
> + */
> + object_unref(OBJECT(clk));
> + }
> + } else {
> + object_property_add_link(OBJECT(dev), name,
> + object_get_typename(OBJECT(clk)),
> + (Object **) &ncl->clock,
> + NULL, OBJ_PROP_LINK_STRONG, &error_abort);
> + }
> +
> + ncl->clock = clk;
> +
> + QLIST_INSERT_HEAD(&dev->clocks, ncl, node);
> + return ncl;
> +}
> +
> +void qdev_finalize_clocklist(DeviceState *dev)
> +{
> + /* called by @dev's device_finalize() */
> + NamedClockList *ncl, *ncl_next;
> +
> + QLIST_FOREACH_SAFE(ncl, &dev->clocks, node, ncl_next) {
> + QLIST_REMOVE(ncl, node);
> + if (!ncl->output && !ncl->alias) {
> + /*
> + * We kept a reference on the input clock to ensure it lives up
> to
> + * this point so we can safely remove the callback.
> + * It avoids having a callback to a deleted object if ncl->clock
> + * is still referenced somewhere else (eg: by a clock output).
> + */
> + clock_clear_callback(ncl->clock);
> + object_unref(OBJECT(ncl->clock));
> + }
> + g_free(ncl->name);
> + g_free(ncl);
> + }
> +}
> +
> +Clock *qdev_init_clock_out(DeviceState *dev, const char *name)
> +{
> + NamedClockList *ncl;
> +
> + assert(name);
> +
> + ncl = qdev_init_clocklist(dev, name, true, NULL);
> +
> + return ncl->clock;
> +}
> +
> +Clock *qdev_init_clock_in(DeviceState *dev, const char *name,
> + ClockCallback *callback, void *opaque)
> +{
> + NamedClockList *ncl;
> +
> + assert(name);
> +
> + ncl = qdev_init_clocklist(dev, name, false, NULL);
> +
> + if (callback) {
> + clock_set_callback(ncl->clock, callback, opaque);
> + }
> + return ncl->clock;
> +}
> +
> +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;
> + }
> + }
> +
> + assert(false);
Remove this.
Otherwise it looks good.
Alistair
> + return NULL;
> +}
> +
> +Clock *qdev_get_clock_in(DeviceState *dev, const char *name)
> +{
> + NamedClockList *ncl;
> +
> + assert(name);
> +
> + ncl = qdev_get_clocklist(dev, name);
> + assert(!ncl->output);
> +
> + return ncl->clock;
> +}
> +
> +Clock *qdev_get_clock_out(DeviceState *dev, const char *name)
> +{
> + NamedClockList *ncl;
> +
> + assert(name);
> +
> + ncl = qdev_get_clocklist(dev, name);
> + assert(ncl->output);
> +
> + return ncl->clock;
> +}
> +
> +Clock *qdev_alias_clock(DeviceState *dev, const char *name,
> + DeviceState *alias_dev, const char *alias_name)
> +{
> + NamedClockList *ncl;
> +
> + assert(name && alias_name);
> +
> + ncl = qdev_get_clocklist(dev, name);
> +
> + qdev_init_clocklist(alias_dev, alias_name, ncl->output, ncl->clock);
> +
> + return ncl->clock;
> +}
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 3937d1eb1a..f390697348 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/qdev-clock.h"
> #include "migration/vmstate.h"
> #include "trace.h"
>
> @@ -855,6 +856,7 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> DeviceClass *dc = DEVICE_GET_CLASS(dev);
> HotplugHandler *hotplug_ctrl;
> BusState *bus;
> + NamedClockList *ncl;
> Error *local_err = NULL;
> bool unattached_parent = false;
> static int unattached_count;
> @@ -902,6 +904,13 @@ 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(ncl, &dev->clocks, node) {
> + if (ncl->alias) {
> + continue;
> + } else {
> + clock_setup_canonical_path(ncl->clock);
> + }
> + }
>
> if (qdev_get_vmsd(dev)) {
> if (vmstate_register_with_alias_id(VMSTATE_IF(dev),
> @@ -1025,6 +1034,7 @@ static void device_initfn(Object *obj)
> dev->allow_unplug_during_migration = false;
>
> QLIST_INIT(&dev->gpios);
> + QLIST_INIT(&dev->clocks);
> }
>
> static void device_post_init(Object *obj)
> @@ -1054,6 +1064,8 @@ static void device_finalize(Object *obj)
> */
> }
>
> + qdev_finalize_clocklist(dev);
> +
> /* 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 e3d796fdd4..2fdcb7dd00 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -7,7 +7,7 @@ common-obj-y += hotplug.o
> common-obj-y += vmstate-if.o
> # irq.o needed for qdev GPIO handling:
> common-obj-y += irq.o
> -common-obj-y += clock.o
> +common-obj-y += clock.o qdev-clock.o
>
> common-obj-$(CONFIG_SOFTMMU) += reset.o
> common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index edcbd475aa..5a4511a86f 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -436,6 +436,7 @@ tests/test-qdev-global-props$(EXESUF):
> tests/test-qdev-global-props.o \
> hw/core/fw-path-provider.o \
> hw/core/reset.o \
> hw/core/vmstate-if.o \
> + hw/core/clock.o hw/core/qdev-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 \
> --
> 2.24.1
>
>
