Re: [Qemu-devel] [RFC PATCH 02/11] qemu-clk: allow to attach a clock to a device

[KONRAD Frederic]

Le 29/06/2016 à 02:15, Alistair Francis a écrit :
> On Mon, Jun 13, 2016 at 9:27 AM,  <address@hidden> wrote:
> > From: KONRAD Frederic <address@hidden>
> >
> > 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 <address@hidden>
> > ---
> >   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