| Message ID | 1485424060-12217-3-git-send-email-fred.konrad@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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, >
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, >> > >
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.
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. > >
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,