Re: [Qemu-devel] [PATCH V1 03/10] qemu-clk: allow to bind two clocks together
Le 17/10/2016 à 20:19, Peter Maydell a écrit : On 5 October 2016 at 23:10, wrote: From: KONRAD Frederic 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 --- include/qemu/qemu-clock.h | 66 +++ qemu-clock.c | 56 2 files changed, 122 insertions(+) diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h index 1d56a2e..d575566 100644 --- a/include/qemu/qemu-clock.h +++ b/include/qemu/qemu-clock.h @@ -27,15 +27,29 @@ #include "qemu/osdep.h" #include "qom/object.h" +typedef uint64_t (*qemu_clk_on_rate_update_cb)(void *opaque, uint64_t rate); I think it's more readable to have the typedef define the function type, not the pointer-to-function type. (See for instance CPReadFn, CPWriteFn in target-arm/cpu.h.) That way your function pointers in structs and so on have type "MyFnType *fn;" and are more obviously pointers. (Also QEMU style says camelcase for types. QEMUClkRateUpdateCallback?) Ok I'll fix that! + #define TYPE_CLOCK "qemu-clk" #define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK) +typedef struct ClkList ClkList; + 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; +qemu_clk_on_rate_update_cb cb; +QLIST_HEAD(, ClkList) bound; } *qemu_clk; +struct ClkList { +qemu_clk clk; +QLIST_ENTRY(ClkList) node; +}; + /** * qemu_clk_attach_to_device: * @dev: the device on which the clock need to be attached. @@ -59,4 +73,56 @@ void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk clk, */ qemu_clk qemu_clk_get_pin(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); Hang on, I thought that passing a clock to qemu_clk_attach_to_device() was going to be the thing that connected the clock up... qemu_clk_attach_to_device() adds the clock to the device so it can be found later to allow eg: qtree to show the clock tree. qemu_clk_bind_clock do the actual bind between the clock. + +/** + * qemu_clk_unbound: + * @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); Function prototype and comment don't match... + +/** + * qemu_clk_update_rate: + * @clk: the clock to update. + * @rate: the new rate. + * + * Update the @clk to the new @rate. + * + */ +void qemu_clk_update_rate(qemu_clk clk, uint64_t rate); What units is this 'rate' in ? It's Hz, will add that to the comment. + +/** + * qemu_clk_refresh: + * @clk: the clock to be refreshed. + * + * If a model alters the topology of a clock tree, it must call this function + * to refresh the clock tree. + * + */ +void qemu_clk_refresh(qemu_clk clk); ...for which clock in the tree does it have to call the function? All of them? Any one of them at random? Ok I will precise that. Thanks, Fred + +/** + * qemu_clk_set_callback: + * @clk: the clock where to set the callback. + * @cb: the callback to associate to the callback. + * @opaque: the opaque data passed to the calback. + * + */ +void qemu_clk_set_callback(qemu_clk clk, + qemu_clk_on_rate_update_cb cb, + void *opaque); + #endif /* QEMU_CLOCK_H */ diff --git a/qemu-clock.c b/qemu-clock.c index 0ba6caf..541f615 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
Re: [Qemu-devel] [PATCH V1 03/10] qemu-clk: allow to bind two clocks together
On 5 October 2016 at 23:10, wrote: > From: KONRAD Frederic > > 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 > --- > include/qemu/qemu-clock.h | 66 > +++ > qemu-clock.c | 56 > 2 files changed, 122 insertions(+) > > diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h > index 1d56a2e..d575566 100644 > --- a/include/qemu/qemu-clock.h > +++ b/include/qemu/qemu-clock.h > @@ -27,15 +27,29 @@ > #include "qemu/osdep.h" > #include "qom/object.h" > > +typedef uint64_t (*qemu_clk_on_rate_update_cb)(void *opaque, uint64_t rate); I think it's more readable to have the typedef define the function type, not the pointer-to-function type. (See for instance CPReadFn, CPWriteFn in target-arm/cpu.h.) That way your function pointers in structs and so on have type "MyFnType *fn;" and are more obviously pointers. (Also QEMU style says camelcase for types. QEMUClkRateUpdateCallback?) > + > #define TYPE_CLOCK "qemu-clk" > #define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK) > > +typedef struct ClkList ClkList; > + > 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; > +qemu_clk_on_rate_update_cb cb; > +QLIST_HEAD(, ClkList) bound; > } *qemu_clk; > > +struct ClkList { > +qemu_clk clk; > +QLIST_ENTRY(ClkList) node; > +}; > + > /** > * qemu_clk_attach_to_device: > * @dev: the device on which the clock need to be attached. > @@ -59,4 +73,56 @@ void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk > clk, > */ > qemu_clk qemu_clk_get_pin(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); Hang on, I thought that passing a clock to qemu_clk_attach_to_device() was going to be the thing that connected the clock up... > + > +/** > + * qemu_clk_unbound: > + * @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); Function prototype and comment don't match... > + > +/** > + * qemu_clk_update_rate: > + * @clk: the clock to update. > + * @rate: the new rate. > + * > + * Update the @clk to the new @rate. > + * > + */ > +void qemu_clk_update_rate(qemu_clk clk, uint64_t rate); What units is this 'rate' in ? > + > +/** > + * qemu_clk_refresh: > + * @clk: the clock to be refreshed. > + * > + * If a model alters the topology of a clock tree, it must call this function > + * to refresh the clock tree. > + * > + */ > +void qemu_clk_refresh(qemu_clk clk); ...for which clock in the tree does it have to call the function? All of them? Any one of them at random? > + > +/** > + * qemu_clk_set_callback: > + * @clk: the clock where to set the callback. > + * @cb: the callback to associate to the callback. > + * @opaque: the opaque data passed to the calback. > + * > + */ > +void qemu_clk_set_callback(qemu_clk clk, > + qemu_clk_on_rate_update_cb cb, > + void *opaque); > + > #endif /* QEMU_CLOCK_H */ > diff --git a/qemu-clock.c b/qemu-clock.c > index 0ba6caf..541f615 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
[Qemu-devel] [PATCH V1 03/10] qemu-clk: allow to bind two clocks together
From: KONRAD Frederic 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 --- include/qemu/qemu-clock.h | 66 +++ qemu-clock.c | 56 2 files changed, 122 insertions(+) diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h index 1d56a2e..d575566 100644 --- a/include/qemu/qemu-clock.h +++ b/include/qemu/qemu-clock.h @@ -27,15 +27,29 @@ #include "qemu/osdep.h" #include "qom/object.h" +typedef uint64_t (*qemu_clk_on_rate_update_cb)(void *opaque, uint64_t rate); + #define TYPE_CLOCK "qemu-clk" #define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK) +typedef struct ClkList ClkList; + 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; +qemu_clk_on_rate_update_cb cb; +QLIST_HEAD(, ClkList) bound; } *qemu_clk; +struct ClkList { +qemu_clk clk; +QLIST_ENTRY(ClkList) node; +}; + /** * qemu_clk_attach_to_device: * @dev: the device on which the clock need to be attached. @@ -59,4 +73,56 @@ void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk clk, */ qemu_clk qemu_clk_get_pin(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_unbound: + * @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. + * + * 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 + * to refresh the clock tree. + * + */ +void qemu_clk_refresh(qemu_clk clk); + +/** + * qemu_clk_set_callback: + * @clk: the clock where to set the callback. + * @cb: the callback to associate to the callback. + * @opaque: the opaque data passed to the calback. + * + */ +void qemu_clk_set_callback(qemu_clk clk, + qemu_clk_on_rate_update_cb cb, + void *opaque); + #endif /* QEMU_CLOCK_H */ diff --git a/qemu-clock.c b/qemu-clock.c index 0ba6caf..541f615 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, + qemu_clk_on_rate_update_cb cb, + void *opaque) +{ +clk->cb = cb; +clk->opaque = opaque; +} + void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk clk, const char *name) { -- 2.5.5