| Message ID | 1465835259-21449-3-git-send-email-fred.konrad@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 13, 2016 at 9:27 AM, <fred.konrad@greensocs.com> wrote: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > This allows to attach 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> > --- > include/qemu/qemu-clock.h | 24 +++++++++++++++++++++++- > qemu-clock.c | 22 ++++++++++++++++++++++ > 2 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h > index e7acd68..a2ba105 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_attach_to_device: > + * @d: the device on which the clock need to be attached. > + * @clk: the clock which need to be attached. > + * @name: the name of the clock can't be NULL. > + * > + * Attach @clk named @name to the device @d. > + * > + */ > +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, dev instead of just d > + const char *name); > > +/** > + * qemu_clk_get_pin: > + * @d: the device which contain the clock. > + * @name: the name of the clock. > + * > + * Get the clock named @name located in the device @d, or NULL if not found. > + * > + * Returns the clock named @name contained in @d. > + */ > +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name); > > +#endif /* QEMU_CLOCK_H */ > diff --git a/qemu-clock.c b/qemu-clock.c > index 4a47fb4..81f2852 100644 > --- a/qemu-clock.c > +++ b/qemu-clock.c > @@ -23,6 +23,7 @@ > > #include "qemu/qemu-clock.h" > #include "hw/hw.h" > +#include "qapi/error.h" > > /* #define DEBUG_QEMU_CLOCK */ > > @@ -33,6 +34,27 @@ do { printf("qemu-clock: " fmt , ## __VA_ARGS__); } while (0) > #define DPRINTF(fmt, ...) do { } while (0) > #endif > > +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, const char *name) > +{ > + assert(name); > + assert(!clk->name); > + object_property_add_child(OBJECT(d), name, OBJECT(clk), &error_abort); > + clk->name = g_strdup(name); > +} > + > +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name) > +{ > + gchar *path = NULL; > + Object *clk; > + bool ambiguous; > + > + path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(d)), > + name); > + clk = object_resolve_path(path, &ambiguous); Should ambiguous be passed back to the caller? > + g_free(path); > + return QEMU_CLOCK(clk); Shouldn't you check to see if you got something valid before casting? Thanks, Alistair > +} > + > static const TypeInfo qemu_clk_info = { > .name = TYPE_CLOCK, > .parent = TYPE_OBJECT, > -- > 2.5.5 > >
Le 29/06/2016 à 02:15, Alistair Francis a écrit : > On Mon, Jun 13, 2016 at 9:27 AM, <fred.konrad@greensocs.com> wrote: >> From: KONRAD Frederic <fred.konrad@greensocs.com> >> >> This allows to attach 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> >> --- >> include/qemu/qemu-clock.h | 24 +++++++++++++++++++++++- >> qemu-clock.c | 22 ++++++++++++++++++++++ >> 2 files changed, 45 insertions(+), 1 deletion(-) >> >> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h >> index e7acd68..a2ba105 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_attach_to_device: >> + * @d: the device on which the clock need to be attached. >> + * @clk: the clock which need to be attached. >> + * @name: the name of the clock can't be NULL. >> + * >> + * Attach @clk named @name to the device @d. >> + * >> + */ >> +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, > dev instead of just d > >> + const char *name); >> >> +/** >> + * qemu_clk_get_pin: >> + * @d: the device which contain the clock. >> + * @name: the name of the clock. >> + * >> + * Get the clock named @name located in the device @d, or NULL if not found. >> + * >> + * Returns the clock named @name contained in @d. >> + */ >> +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name); >> >> +#endif /* QEMU_CLOCK_H */ >> diff --git a/qemu-clock.c b/qemu-clock.c >> index 4a47fb4..81f2852 100644 >> --- a/qemu-clock.c >> +++ b/qemu-clock.c >> @@ -23,6 +23,7 @@ >> >> #include "qemu/qemu-clock.h" >> #include "hw/hw.h" >> +#include "qapi/error.h" >> >> /* #define DEBUG_QEMU_CLOCK */ >> >> @@ -33,6 +34,27 @@ do { printf("qemu-clock: " fmt , ## __VA_ARGS__); } while (0) >> #define DPRINTF(fmt, ...) do { } while (0) >> #endif >> >> +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, const char *name) >> +{ >> + assert(name); >> + assert(!clk->name); >> + object_property_add_child(OBJECT(d), name, OBJECT(clk), &error_abort); >> + clk->name = g_strdup(name); >> +} >> + >> +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name) >> +{ >> + gchar *path = NULL; >> + Object *clk; >> + bool ambiguous; >> + >> + path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(d)), >> + name); >> + clk = object_resolve_path(path, &ambiguous); > Should ambiguous be passed back to the caller? Up to you, I don't see the use case in the machine where we want to get the clock? > >> + g_free(path); >> + return QEMU_CLOCK(clk); > Shouldn't you check to see if you got something valid before casting? Yes true, I was relying on the fact that QEMU_CLOCK is in the end: object_dynamic_cast_assert(..) which according to the doc: * If an invalid object is passed to this function, a run time assert will be * generated. but it seems not to be the case in reality if CONFIG_QOM_CAST_DEBUG is disabled: Object *object_dynamic_cast_assert(Object *obj, const char *typename, const char *file, int line, const char *func) { trace_object_dynamic_cast_assert(obj ? obj->class->type->name : "(null)", typename, file, line, func); #ifdef CONFIG_QOM_CAST_DEBUG int i; Object *inst; for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) { if (obj->class->object_cast_cache[i] == typename) { goto out; } } inst = object_dynamic_cast(obj, typename); if (!inst && obj) { fprintf(stderr, "%s:%d:%s: Object %p is not an instance of type %s\n", file, line, func, obj, typename); abort(); } assert(obj == inst); if (obj && obj == inst) { for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) { obj->class->object_cast_cache[i - 1] = obj->class->object_cast_cache[i]; } obj->class->object_cast_cache[i - 1] = typename; } out: #endif return obj; } Is that normal? Thanks, Fred > > Thanks, > > Alistair > >> +} >> + >> static const TypeInfo qemu_clk_info = { >> .name = TYPE_CLOCK, >> .parent = TYPE_OBJECT, >> -- >> 2.5.5 >> >>
On Tue, Aug 2, 2016 at 12:47 AM, KONRAD Frederic <fred.konrad@greensocs.com> wrote: > > > Le 29/06/2016 à 02:15, Alistair Francis a écrit : >> >> On Mon, Jun 13, 2016 at 9:27 AM, <fred.konrad@greensocs.com> wrote: >>> >>> From: KONRAD Frederic <fred.konrad@greensocs.com> >>> >>> This allows to attach 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> >>> --- >>> include/qemu/qemu-clock.h | 24 +++++++++++++++++++++++- >>> qemu-clock.c | 22 ++++++++++++++++++++++ >>> 2 files changed, 45 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h >>> index e7acd68..a2ba105 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_attach_to_device: >>> + * @d: the device on which the clock need to be attached. >>> + * @clk: the clock which need to be attached. >>> + * @name: the name of the clock can't be NULL. >>> + * >>> + * Attach @clk named @name to the device @d. >>> + * >>> + */ >>> +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, >> >> dev instead of just d >> >>> + const char *name); >>> >>> +/** >>> + * qemu_clk_get_pin: >>> + * @d: the device which contain the clock. >>> + * @name: the name of the clock. >>> + * >>> + * Get the clock named @name located in the device @d, or NULL if not >>> found. >>> + * >>> + * Returns the clock named @name contained in @d. >>> + */ >>> +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name); >>> >>> +#endif /* QEMU_CLOCK_H */ >>> diff --git a/qemu-clock.c b/qemu-clock.c >>> index 4a47fb4..81f2852 100644 >>> --- a/qemu-clock.c >>> +++ b/qemu-clock.c >>> @@ -23,6 +23,7 @@ >>> >>> #include "qemu/qemu-clock.h" >>> #include "hw/hw.h" >>> +#include "qapi/error.h" >>> >>> /* #define DEBUG_QEMU_CLOCK */ >>> >>> @@ -33,6 +34,27 @@ do { printf("qemu-clock: " fmt , ## __VA_ARGS__); } >>> while (0) >>> #define DPRINTF(fmt, ...) do { } while (0) >>> #endif >>> >>> +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, const char >>> *name) >>> +{ >>> + assert(name); >>> + assert(!clk->name); >>> + object_property_add_child(OBJECT(d), name, OBJECT(clk), >>> &error_abort); >>> + clk->name = g_strdup(name); >>> +} >>> + >>> +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name) >>> +{ >>> + gchar *path = NULL; >>> + Object *clk; >>> + bool ambiguous; >>> + >>> + path = g_strdup_printf("%s/%s", >>> object_get_canonical_path(OBJECT(d)), >>> + name); >>> + clk = object_resolve_path(path, &ambiguous); >> >> Should ambiguous be passed back to the caller? > > > Up to you, I don't see the use case in the machine where we want to get the > clock? Can't you just set it as NULL then. > >> >>> + g_free(path); >>> + return QEMU_CLOCK(clk); >> >> Shouldn't you check to see if you got something valid before casting? > > > Yes true, I was relying on the fact that QEMU_CLOCK is in the end: > object_dynamic_cast_assert(..) which according to the doc: > > * If an invalid object is passed to this function, a run time assert will > be > * generated. > > but it seems not to be the case in reality if CONFIG_QOM_CAST_DEBUG is > disabled: > > Object *object_dynamic_cast_assert(Object *obj, const char *typename, > const char *file, int line, const char > *func) > { > trace_object_dynamic_cast_assert(obj ? obj->class->type->name : > "(null)", > typename, file, line, func); > > #ifdef CONFIG_QOM_CAST_DEBUG > int i; > Object *inst; > > for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) { > if (obj->class->object_cast_cache[i] == typename) { > goto out; > } > } > > inst = object_dynamic_cast(obj, typename); > > if (!inst && obj) { > fprintf(stderr, "%s:%d:%s: Object %p is not an instance of type > %s\n", > file, line, func, obj, typename); > abort(); > } > > assert(obj == inst); > > if (obj && obj == inst) { > for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) { > obj->class->object_cast_cache[i - 1] = > obj->class->object_cast_cache[i]; > } > obj->class->object_cast_cache[i - 1] = typename; > } > > out: > #endif > return obj; > } > > Is that normal? I'm not sure to be honest. I never expected the cast to do any checking. Someone else probably knows the history here. Thanks, Alistair > > Thanks, > Fred > > >> >> Thanks, >> >> Alistair >> >>> +} >>> + >>> static const TypeInfo qemu_clk_info = { >>> .name = TYPE_CLOCK, >>> .parent = TYPE_OBJECT, >>> -- >>> 2.5.5 >>> >>> > >
diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h index e7acd68..a2ba105 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_attach_to_device: + * @d: the device on which the clock need to be attached. + * @clk: the clock which need to be attached. + * @name: the name of the clock can't be NULL. + * + * Attach @clk named @name to the device @d. + * + */ +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, + const char *name); +/** + * qemu_clk_get_pin: + * @d: the device which contain the clock. + * @name: the name of the clock. + * + * Get the clock named @name located in the device @d, or NULL if not found. + * + * Returns the clock named @name contained in @d. + */ +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name); +#endif /* QEMU_CLOCK_H */ diff --git a/qemu-clock.c b/qemu-clock.c index 4a47fb4..81f2852 100644 --- a/qemu-clock.c +++ b/qemu-clock.c @@ -23,6 +23,7 @@ #include "qemu/qemu-clock.h" #include "hw/hw.h" +#include "qapi/error.h" /* #define DEBUG_QEMU_CLOCK */ @@ -33,6 +34,27 @@ do { printf("qemu-clock: " fmt , ## __VA_ARGS__); } while (0) #define DPRINTF(fmt, ...) do { } while (0) #endif +void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, const char *name) +{ + assert(name); + assert(!clk->name); + object_property_add_child(OBJECT(d), name, OBJECT(clk), &error_abort); + clk->name = g_strdup(name); +} + +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name) +{ + gchar *path = NULL; + Object *clk; + bool ambiguous; + + path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(d)), + 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,