[v2,7/7] tmp105: Add temperature QOM property

This obsoletes tmp105_set() and allows for better error handling.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/tmp105.c |   39 ++++++++++++++++++++++++++++++++-------
 hw/tmp105.h |   15 ---------------
 2 Dateien geändert, 32 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)

> +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.

With kind regards,
Alex

On 14 December 2012 11:34, Andreas Färber <andreas.faerber@web.de> wrote:
> This obsoletes tmp105_set() and allows for better error handling.
>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/tmp105.c |   39 ++++++++++++++++++++++++++++++++-------
>  hw/tmp105.h |   15 ---------------
>  2 Dateien geändert, 32 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)
>
> diff --git a/hw/tmp105.c b/hw/tmp105.c
> index a16f538..34c7d24 100644
> --- a/hw/tmp105.c
> +++ b/hw/tmp105.c
> @@ -21,6 +21,7 @@
>  #include "hw.h"
>  #include "i2c.h"
>  #include "tmp105.h"
> +#include "qapi/qapi-visit-core.h"
>
>  static void tmp105_interrupt_update(TMP105State *s)
>  {
> @@ -51,18 +52,34 @@ static void tmp105_alarm_update(TMP105State *s)
>      tmp105_interrupt_update(s);
>  }
>
> +static void tmp105_get_temperature(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
> +{
> +    TMP105State *s = TMP105(obj);
> +    int64_t value = s->temperature;
> +
> +    visit_type_int(v, &value, name, errp);
> +}
> +
>  /* Units are 0.001 centigrades relative to 0 C.  */
> -void tmp105_set(I2CSlave *i2c, int temp)
> +static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)
>  {
> -    TMP105State *s = TMP105(i2c);
> +    TMP105State *s = TMP105(obj);
>
> -    if (temp >= 128000 || temp < -128000) {
> -        fprintf(stderr, "%s: values is out of range (%i.%03i C)\n",
> -                        __FUNCTION__, temp / 1000, temp % 1000);
> -        exit(-1);
> +    int64_t value;
> +
> +    visit_type_int(v, &value, name, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +    if (value >= 128000 || value < -128000) {
> +        error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range",
> +                   value / 1000, value % 1000);
> +        return;
>      }
>
> -    s->temperature = ((int16_t) (temp * 0x800 / 128000)) << 4;
> +    s->temperature = ((int16_t) (value * 0x800 / 128000)) << 4;
>
>      tmp105_alarm_update(s);
>  }
> @@ -218,6 +235,13 @@ static int tmp105_init(I2CSlave *i2c)
>      return 0;
>  }
>
> +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);
> -
>  #endif
> --
> 1.7.10.4
>

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