From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46475) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bV6VJ-0007ck-S4 for qemu-devel@nongnu.org; Wed, 03 Aug 2016 20:27:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bV6VH-00046K-H4 for qemu-devel@nongnu.org; Wed, 03 Aug 2016 20:27:16 -0400 Received: from mail-oi0-x241.google.com ([2607:f8b0:4003:c06::241]:33856) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bV6VH-00046D-AC for qemu-devel@nongnu.org; Wed, 03 Aug 2016 20:27:15 -0400 Received: by mail-oi0-x241.google.com with SMTP id a135so3983874oii.1 for ; Wed, 03 Aug 2016 17:27:14 -0700 (PDT) MIME-Version: 1.0 Sender: alistair23@gmail.com In-Reply-To: <536b021f-b95d-3b97-1d47-493a1bb2f30e@greensocs.com> References: <1465835259-21449-1-git-send-email-fred.konrad@greensocs.com> <1465835259-21449-3-git-send-email-fred.konrad@greensocs.com> <536b021f-b95d-3b97-1d47-493a1bb2f30e@greensocs.com> From: Alistair Francis Date: Wed, 3 Aug 2016 17:26:43 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH 02/11] qemu-clk: allow to attach a clock to a device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: KONRAD Frederic Cc: Alistair Francis , Edgar Iglesias , Peter Maydell , Mark Burton , "qemu-devel@nongnu.org Developers" On Tue, Aug 2, 2016 at 12:47 AM, KONRAD Frederic wrote: > > > Le 29/06/2016 =C3=A0 02:15, Alistair Francis a =C3=A9crit : >> >> On Mon, Jun 13, 2016 at 9:27 AM, wrote: >>> >>> From: KONRAD Frederic >>> >>> 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 >>> --- >>> 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 cha= r >>> *name) >>> +{ >>> + assert(name); >>> + assert(!clk->name); >>> + object_property_add_child(OBJECT(d), name, OBJECT(clk), >>> &error_abort); >>> + clk->name =3D g_strdup(name); >>> +} >>> + >>> +qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name) >>> +{ >>> + gchar *path =3D NULL; >>> + Object *clk; >>> + bool ambiguous; >>> + >>> + path =3D g_strdup_printf("%s/%s", >>> object_get_canonical_path(OBJECT(d)), >>> + name); >>> + clk =3D 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 t= he > 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 wil= l > 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 =3D 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) { > if (obj->class->object_cast_cache[i] =3D=3D typename) { > goto out; > } > } > > inst =3D 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 =3D=3D inst); > > if (obj && obj =3D=3D inst) { > for (i =3D 1; i < OBJECT_CLASS_CAST_CACHE; i++) { > obj->class->object_cast_cache[i - 1] =3D > obj->class->object_cast_cache[i]; > } > obj->class->object_cast_cache[i - 1] =3D 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 =3D { >>> .name =3D TYPE_CLOCK, >>> .parent =3D TYPE_OBJECT, >>> -- >>> 2.5.5 >>> >>> > >