Re: [Qemu-devel] [PATCH v2 7/7] tmp105: Add temperature QOM property

[Andreas Färber]

Am 15.12.2012 17:52, schrieb Alex Horn:
>> +static void tmp105_initfn(Object *obj)
>> +{
>> +    object_property_add(obj, "temperature", "int",
>> +                        tmp105_get_temperature,
>> +                        tmp105_set_temperature, NULL, NULL, NULL);
>> +}
>> +
>>  static void tmp105_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -234,6 +258,7 @@ static const TypeInfo tmp105_info = {
>>      .name          = TYPE_TMP105,
>>      .parent        = TYPE_I2C_SLAVE,
>>      .instance_size = sizeof(TMP105State),
>> +    .instance_init = tmp105_initfn,
>>      .class_init    = tmp105_class_init,
>>  };
>>
>> diff --git a/hw/tmp105.h b/hw/tmp105.h
>> index c21396f..d218919 100644
>> --- a/hw/tmp105.h
>> +++ b/hw/tmp105.h
>> @@ -44,19 +44,4 @@ typedef struct TMP105State {
>>      uint8_t alarm;
>>  } TMP105State;
>>
>> -/**
>> - * tmp105_set:
>> - * @i2c: dispatcher to TMP105 hardware model
>> - * @temp: temperature with 0.001 centigrades units in the range -40 C to 
>> +125 C
>> - *
>> - * Sets the temperature of the TMP105 hardware model.
>> - *
>> - * Bits 5 and 6 (value 32 and 64) in the register indexed by 
>> TMP105_REG_CONFIG
>> - * determine the precision of the temperature. See Table 8 in the data 
>> sheet.
>> - *
>> - * @see_also: I2C_SLAVE macro
>> - * @see_also: http://www.ti.com/lit/gpn/tmp105
>> - */
>> -void tmp105_set(I2CSlave *i2c, int temp);
> 
> Would it be possible to keep the C API? The traditional C API is
> useful when a project cannot support QOM. These C APIs also simplify
> unit testing and allow QEMU hardware models to be more easily reused
> as standalone modules.

QEMU devices are meant to be instantiated through QOM, and the use of
QOM is gradually increasing (it was introduced around last Christmas).
So while it would be possible to leave tmp105_set() behind as wrapper for

Error *err = NULL;
object_property_set_int(OBJECT(i2c), "temperature", temp, err);
if (err != NULL) {
    fprintf(stderr, "%s", error_get_pretty(err));
    error_free(err);
    exit(-1);
}

it uses a very fatal way of error handling and, as you said yourself,
has zero users in qemu.git.

By comparison the QOM property in this patch allows to set the value via
C API or QMP commands, including scripts such as QMP/qom-set or
QMP/qom-fuse.

So I don't see why the effort? What project are you trying to reuse QEMU
devices in without using QEMU's device infrastructure? Not even Xen did
that AFAIK. If you're trying to integrate with SystemC or so there are
some (possibly not quite up-to-date) projects (e.g., linked from the
Wiki) to bridge worlds.

Regards,
Andreas