From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS1pE-0007k5-J7 for qemu-devel@nongnu.org; Mon, 24 Mar 2014 06:09:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WS1p5-0001YU-OP for qemu-devel@nongnu.org; Mon, 24 Mar 2014 06:09:48 -0400 Received: from mail-ee0-x22a.google.com ([2a00:1450:4013:c00::22a]:39397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS1p5-0001YM-Fi for qemu-devel@nongnu.org; Mon, 24 Mar 2014 06:09:39 -0400 Received: by mail-ee0-f42.google.com with SMTP id d17so4221096eek.15 for ; Mon, 24 Mar 2014 03:09:38 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5330045F.2040502@redhat.com> Date: Mon, 24 Mar 2014 11:09:35 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1392634650-24282-1-git-send-email-pbonzini@redhat.com> <5301FBAB.6060705@suse.de> In-Reply-To: <5301FBAB.6060705@suse.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] tmp105: read temperature in milli-celsius List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, Alex Horn Il 17/02/2014 13:08, Andreas Färber ha scritto: > 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 >> --- >> 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. This is quite hard to do because the tmp105 is under /machine/unattached and thus does not have a stable QOM path. The other way to test it would be to add a libqos driver for the i386 smbus device, add tmp105 to qemu-system-x86_64, and write another tmp105 test. Given this, is this patch okay for 2.0? Paolo