Re: [Qemu-devel] [PATCH] tmp105: read temperature in milli-celsius

[Andreas Färber]

Am 17.02.2014 11:57, schrieb Paolo Bonzini:
> Right now, the temperature property must be written in milli-celsius, but it
> reads back the value in 8.8 fixed point.  Fix this by letting the property
> read back the original value (possibly rounded).  Also simplify the code that
> does the conversion.
> 
> Before:
> 
>     (QEMU) qom-set path=/machine/peripheral/sensor property=temperature 
> value=20000
>     {u'return': {}}
>     (QEMU) qom-get path=sensor property=temperature
>     {u'return': 5120}
> 
> After:
> 
>     (QEMU) qom-set path=/machine/peripheral/sensor property=temperature 
> value=20000
>     {u'return': {}}
>     (QEMU) qom-get path=sensor property=temperature
>     {u'return': 20000}
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/misc/tmp105.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c
> index 155e03d..63aa3d6 100644
> --- a/hw/misc/tmp105.c
> +++ b/hw/misc/tmp105.c
> @@ -56,12 +56,14 @@ 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;
> +    int64_t value = s->temperature * 1000 / 256;

Hmm, I'll have to check history, but I guess the setter was there and I
wrongly added the getter. That would be easier to ack of course if I
didn't have to think about a complete new formula in both places... ;)

>  
>      visit_type_int(v, &value, name, errp);
>  }
>  
> -/* Units are 0.001 centigrades relative to 0 C.  */
> +/* Units are 0.001 centigrades relative to 0 C.  s->temperature is 8.8
> + * fixed point, so units are 1/256 centigrades.  A simple ratio will do.
> + */
>  static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque,
>                                     const char *name, Error **errp)
>  {
> @@ -78,7 +80,7 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, 
> void *opaque,
>          return;
>      }
>  
> -    s->temperature = ((int16_t) (temp * 0x800 / 128000)) << 4;
> +    s->temperature = (int16_t) (temp * 256 / 1000);

Did you check whether those magic 4 bits shift were for some other
purpose such as flags possibly? CC'ing Alex Horn.

Since we do have a tmp105-test, we should also add a regression test for
the getter bug.

Regards,
Andreas

>  
>      tmp105_alarm_update(s);
>  }
> 

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg