[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 02/11] qemu-clk: allow to attach a clock to
From: |
Alistair Francis |
Subject: |
Re: [Qemu-devel] [RFC PATCH 02/11] qemu-clk: allow to attach a clock to a device |
Date: |
Wed, 3 Aug 2016 17:26:43 -0700 |
On Tue, Aug 2, 2016 at 12:47 AM, KONRAD Frederic
<address@hidden> wrote:
>
>
> 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?
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
>>>
>>>
>
>