[Qemu-devel] [PATCH v2 0/7] I2C libqos and tmp105 qtest support

[Qemu-devel] [PATCH v2 0/7] I2C libqos and tmp105 qtest support

[Andreas Färber]

Hello,

Here's I2C support for the libqos framework and an OMAP driver.
The design was inspired by QEMU's i2c_bus and Linux' i2c_adapter.
Seems like low hanging fruit (while we're still waiting for PCI support :)),
hopefully motivating more people to contribute qtests for new devices or
bug fixes.

I found a bug in omap_i2c related to the SBD bit not getting cleared after
a single-byte read; the patch has been dropped as we were not able to verify
it against a TRM or hardware. The driver was therefore changed to ignore the
SBD bit, like the Linux driver does.

I've split up the proposed tmp105.h header further to facilitate reuse of
the register enum in qtest.

The test case itself initially fails; in lack of a v2 patch by Alex I'm
appending my own fix proposal that makes the test pass.

I'm also appending a QOM'ish cleanup of the file, replacing the current API
with a QOM property.
To do a qom-set for testing a non-zero temperature reading, we'll need either
a canonical path for the TMP105 or Jason's qtest_qmp_resp() to iterate over
qom-list /machine/unassigned output looking for type "tmp105" with qom-get.

Alex, if you could add a Tested-by (using object_property_set_int()) that
would be appreciated.

Regards,
Andreas

v1 -> v2:
* Factor out libqos API for I2C and an OMAP driver
* Avoid casts by using uint8_t* in API, suggested by Blue
* Ignore SBD bit in omap_i2c driver
* Drop omap_i2c patch clearing SBD
* Incorporate Alex' tmp105.h patch
* Split out tmp105_regs.h for qtest, inspired by rtc-test
* Append replacement for Alex' fix
* Append QOM cleanup and turn setter function into QOM property

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Alex Horn <alex.horn@cs.ox.ac.uk>
Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Blue Swirl <blauwirbel@gmail.com>

Cc: Jason Baron <jbaron@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>

Alex Horn (1):
  tmp105: Create API for TMP105 temperature sensor

Andreas Färber (6):
  libqtest: Prepare I2C libqos
  tmp105: Split out I2C message constants from header
  tests: Add tmp105 qtest test case
  tmp105: Fix I2C protocol bug
  tmp105: QOM'ify
  tmp105: Add temperature QOM property

 hw/i2c.h            |    3 -
 hw/tmp105.c         |  101 ++++++++++++++++++-------------
 hw/tmp105.h         |   47 +++++++++++++++
 hw/tmp105_regs.h    |   50 ++++++++++++++++
 tests/Makefile      |    3 +
 tests/libi2c-omap.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libi2c.c      |   22 +++++++
 tests/libi2c.h      |   30 ++++++++++
 tests/tmp105-test.c |   76 +++++++++++++++++++++++
 9 Dateien geändert, 453 Zeilen hinzugefügt(+), 45 Zeilen entfernt(-)
 create mode 100644 hw/tmp105.h
 create mode 100644 hw/tmp105_regs.h
 create mode 100644 tests/libi2c-omap.c
 create mode 100644 tests/libi2c.c
 create mode 100644 tests/libi2c.h
 create mode 100644 tests/tmp105-test.c

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 1/7] tmp105: Create API for TMP105 temperature sensor

[Andreas Färber]

From: Alex Horn <alex.horn@cs.ox.ac.uk>

* Define enum for TMP105 registers
* Move tmp105_set() from I2C to TMP105 header
* Document units and range of temperature as preconditions

Signed-off-by: Alex Horn <alex.horn@cs.ox.ac.uk>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/i2c.h    |    3 ---
 hw/tmp105.c |   17 ++++++++-------
 hw/tmp105.h |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 Dateien geändert, 76 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)
 create mode 100644 hw/tmp105.h

diff --git a/hw/i2c.h b/hw/i2c.h
index 0f5682b..883b5c5 100644
--- a/hw/i2c.h
+++ b/hw/i2c.h
@@ -73,9 +73,6 @@ void *wm8750_dac_buffer(void *opaque, int samples);
 void wm8750_dac_commit(void *opaque);
 void wm8750_set_bclk_in(void *opaque, int new_hz);
 
-/* tmp105.c */
-void tmp105_set(I2CSlave *i2c, int temp);
-
 /* lm832x.c */
 void lm832x_key_event(DeviceState *dev, int key, int state);
 
diff --git a/hw/tmp105.c b/hw/tmp105.c
index 8e8dbd9..9c67e64 100644
--- a/hw/tmp105.c
+++ b/hw/tmp105.c
@@ -20,6 +20,7 @@
 
 #include "hw.h"
 #include "i2c.h"
+#include "tmp105.h"
 
 typedef struct {
     I2CSlave i2c;
@@ -92,22 +93,22 @@ static void tmp105_read(TMP105State *s)
     }
 
     switch (s->pointer & 3) {
-    case 0:	/* Temperature */
+    case TMP105_REG_TEMPERATURE:
         s->buf[s->len ++] = (((uint16_t) s->temperature) >> 8);
         s->buf[s->len ++] = (((uint16_t) s->temperature) >> 0) &
                 (0xf0 << ((~s->config >> 5) & 3));		/* R */
         break;
 
-    case 1:	/* Configuration */
+    case TMP105_REG_CONFIG:
         s->buf[s->len ++] = s->config;
         break;
 
-    case 2:	/* T_LOW */
+    case TMP105_REG_T_LOW:
         s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 8;
         s->buf[s->len ++] = ((uint16_t) s->limit[0]) >> 0;
         break;
 
-    case 3:	/* T_HIGH */
+    case TMP105_REG_T_HIGH:
         s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 8;
         s->buf[s->len ++] = ((uint16_t) s->limit[1]) >> 0;
         break;
@@ -117,10 +118,10 @@ static void tmp105_read(TMP105State *s)
 static void tmp105_write(TMP105State *s)
 {
     switch (s->pointer & 3) {
-    case 0:	/* Temperature */
+    case TMP105_REG_TEMPERATURE:
         break;
 
-    case 1:	/* Configuration */
+    case TMP105_REG_CONFIG:
         if (s->buf[0] & ~s->config & (1 << 0))			/* SD */
             printf("%s: TMP105 shutdown\n", __FUNCTION__);
         s->config = s->buf[0];
@@ -128,8 +129,8 @@ static void tmp105_write(TMP105State *s)
         tmp105_alarm_update(s);
         break;
 
-    case 2:	/* T_LOW */
-    case 3:	/* T_HIGH */
+    case TMP105_REG_T_LOW:
+    case TMP105_REG_T_HIGH:
         if (s->len >= 3)
             s->limit[s->pointer & 1] = (int16_t)
                     ((((uint16_t) s->buf[0]) << 8) | s->buf[1]);
diff --git a/hw/tmp105.h b/hw/tmp105.h
new file mode 100644
index 0000000..51eff4b
--- /dev/null
+++ b/hw/tmp105.h
@@ -0,0 +1,67 @@
+/*
+ * Texas Instruments TMP105 Temperature Sensor
+ *
+ * Browse the data sheet:
+ *
+ *    http://www.ti.com/lit/gpn/tmp105
+ *
+ * Copyright (C) 2012 Alex Horn <alex.horn@cs.ox.ac.uk>
+ * Copyright (C) 2008-2012 Andrzej Zaborowski <balrogg@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_TMP105_H
+#define QEMU_TMP105_H
+
+#include "i2c.h"
+
+/**
+ * TMP105Reg:
+ * @TMP105_REG_TEMPERATURE: Temperature register
+ * @TMP105_REG_CONFIG: Configuration register
+ * @TMP105_REG_T_LOW: Low temperature register (also known as T_hyst)
+ * @TMP105_REG_T_HIGH: High temperature register (also known as T_OS)
+ *
+ * The following temperature sensors are
+ * compatible with the TMP105 registers:
+ * - adt75
+ * - ds1775
+ * - ds75
+ * - lm75
+ * - lm75a
+ * - max6625
+ * - max6626
+ * - mcp980x
+ * - stds75
+ * - tcn75
+ * - tmp100
+ * - tmp101
+ * - tmp105
+ * - tmp175
+ * - tmp275
+ * - tmp75
+ **/
+typedef enum TMP105Reg {
+    TMP105_REG_TEMPERATURE = 0,
+    TMP105_REG_CONFIG,
+    TMP105_REG_T_LOW,
+    TMP105_REG_T_HIGH,
+} TMP105Reg;
+
+/**
+ * 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

[Qemu-devel] [PATCH v2 2/7] libqtest: Prepare I2C libqos

[Andreas Färber]

This adds a simple I2C API and a driver implementation for omap_i2c.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 tests/Makefile      |    1 +
 tests/libi2c-omap.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libi2c.c      |   22 +++++++
 tests/libi2c.h      |   30 ++++++++++
 4 Dateien geändert, 219 Zeilen hinzugefügt(+)
 create mode 100644 tests/libi2c-omap.c
 create mode 100644 tests/libi2c.c
 create mode 100644 tests/libi2c.h

diff --git a/tests/Makefile b/tests/Makefile
index b60f0fb..1ec41cb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -86,6 +86,7 @@ QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TA
 check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y))
 
 qtest-obj-y = tests/libqtest.o $(oslib-obj-y) libqemustub.a
+qtest-obj-y += tests/libi2c.o tests/libi2c-omap.o
 $(check-qtest-y): $(qtest-obj-y)
 
 .PHONY: check-help
diff --git a/tests/libi2c-omap.c b/tests/libi2c-omap.c
new file mode 100644
index 0000000..65a9b66
--- /dev/null
+++ b/tests/libi2c-omap.c
@@ -0,0 +1,166 @@
+/*
+ * QTest I2C driver
+ *
+ * Copyright (c) 2012 Andreas Färber
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "libi2c.h"
+
+#include <glib.h>
+#include <string.h>
+
+#include "osdep.h"
+#include "libqtest.h"
+
+enum OMAPI2CRegisters {
+    OMAP_I2C_REV  = 0x00,
+    OMAP_I2C_STAT = 0x08,
+    OMAP_I2C_CNT  = 0x18,
+    OMAP_I2C_DATA = 0x1c,
+    OMAP_I2C_CON  = 0x24,
+    OMAP_I2C_SA   = 0x2c,
+};
+
+enum OMAPI2CSTATBits {
+    OMAP_I2C_STAT_NACK = 1 << 1,
+    OMAP_I2C_STAT_ARDY = 1 << 2,
+    OMAP_I2C_STAT_RRDY = 1 << 3,
+    OMAP_I2C_STAT_XRDY = 1 << 4,
+    OMAP_I2C_STAT_ROVR = 1 << 11,
+    OMAP_I2C_STAT_SBD  = 1 << 15,
+};
+
+enum OMAPI2CCONBits {
+    OMAP_I2C_CON_STT    = 1 << 0,
+    OMAP_I2C_CON_STP    = 1 << 1,
+    OMAP_I2C_CON_TRX    = 1 << 9,
+    OMAP_I2C_CON_MST    = 1 << 10,
+    OMAP_I2C_CON_BE     = 1 << 14,
+    OMAP_I2C_CON_I2C_EN = 1 << 15,
+};
+
+typedef struct OMAPI2C {
+    I2CAdapter parent;
+
+    uint64_t addr;
+} OMAPI2C;
+
+
+static void omap_i2c_set_slave_addr(OMAPI2C *s, uint8_t addr)
+{
+    uint16_t data = addr;
+
+    memwrite(s->addr + OMAP_I2C_SA, &data, 2);
+    memread(s->addr + OMAP_I2C_SA, &data, 2);
+    g_assert_cmphex(data, ==, addr);
+}
+
+static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr,
+                          const uint8_t *buf, uint16_t len)
+{
+    OMAPI2C *s = (OMAPI2C *)i2c;
+    uint16_t data;
+
+    omap_i2c_set_slave_addr(s, addr);
+
+    data = len;
+    memwrite(s->addr + OMAP_I2C_CNT, &data, 2);
+
+    data = OMAP_I2C_CON_I2C_EN |
+           OMAP_I2C_CON_TRX |
+           OMAP_I2C_CON_MST |
+           OMAP_I2C_CON_STT |
+           OMAP_I2C_CON_STP;
+    memwrite(s->addr + OMAP_I2C_CON, &data, 2);
+    memread(s->addr + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) != 0);
+
+    memread(s->addr + OMAP_I2C_STAT, &data, 2);
+    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
+
+    while (len > 1) {
+        memread(s->addr + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
+
+        memwrite(s->addr + OMAP_I2C_DATA, buf, 2);
+        buf = (uint8_t *)buf + 2;
+        len -= 2;
+    }
+    if (len == 1) {
+        memread(s->addr + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
+
+        memwrite(s->addr + OMAP_I2C_DATA, buf, 1);
+    }
+
+    memread(s->addr + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+}
+
+static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr,
+                          uint8_t *buf, uint16_t len)
+{
+    OMAPI2C *s = (OMAPI2C *)i2c;
+    uint16_t data, stat;
+
+    omap_i2c_set_slave_addr(s, addr);
+
+    data = len;
+    memwrite(s->addr + OMAP_I2C_CNT, &data, 2);
+
+    data = OMAP_I2C_CON_I2C_EN |
+           OMAP_I2C_CON_MST |
+           OMAP_I2C_CON_STT |
+           OMAP_I2C_CON_STP;
+    memwrite(s->addr + OMAP_I2C_CON, &data, 2);
+    memread(s->addr + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+
+    memread(s->addr + OMAP_I2C_STAT, &data, 2);
+    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
+
+    memread(s->addr + OMAP_I2C_CNT, &data, 2);
+    g_assert_cmpuint(data, ==, len);
+
+    while (len > 0) {
+        memread(s->addr + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_RRDY) != 0);
+        g_assert((data & OMAP_I2C_STAT_ROVR) == 0);
+
+        memread(s->addr + OMAP_I2C_DATA, &data, 2);
+
+        memread(s->addr + OMAP_I2C_STAT, &stat, 2);
+        if (unlikely(len == 1)) {
+            *buf = data & 0xf;
+            buf++;
+            len--;
+        } else {
+            memcpy(buf, &data, 2);
+            buf += 2;
+            len -= 2;
+        }
+    }
+
+    memread(s->addr + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+}
+
+I2CAdapter *omap_i2c_create(uint64_t addr)
+{
+    OMAPI2C *s = g_malloc0(sizeof(*s));
+    I2CAdapter *i2c = (I2CAdapter *)s;
+    uint16_t data;
+
+    s->addr = addr;
+
+    i2c->send = omap_i2c_send;
+    i2c->recv = omap_i2c_recv;
+
+    /* verify the mmio address by looking for a known signature */
+    memread(addr + OMAP_I2C_REV, &data, 2);
+    g_assert_cmphex(data, ==, 0x34);
+
+    return i2c;
+}
diff --git a/tests/libi2c.c b/tests/libi2c.c
new file mode 100644
index 0000000..13ec85c
--- /dev/null
+++ b/tests/libi2c.c
@@ -0,0 +1,22 @@
+/*
+ * QTest I2C driver
+ *
+ * Copyright (c) 2012 Andreas Färber
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "libi2c.h"
+#include "libqtest.h"
+
+void i2c_send(I2CAdapter *i2c, uint8_t addr,
+              const uint8_t *buf, uint16_t len)
+{
+    i2c->send(i2c, addr, buf, len);
+}
+
+void i2c_recv(I2CAdapter *i2c, uint8_t addr,
+              uint8_t *buf, uint16_t len)
+{
+    i2c->recv(i2c, addr, buf, len);
+}
diff --git a/tests/libi2c.h b/tests/libi2c.h
new file mode 100644
index 0000000..1ce9af4
--- /dev/null
+++ b/tests/libi2c.h
@@ -0,0 +1,30 @@
+/*
+ * I2C libqos
+ *
+ * Copyright (c) 2012 Andreas Färber
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef LIBQOS_I2C_H
+#define LIBQOS_I2C_H
+
+#include <stdint.h>
+
+typedef struct I2CAdapter I2CAdapter;
+struct I2CAdapter {
+    void (*send)(I2CAdapter *adapter, uint8_t addr,
+                 const uint8_t *buf, uint16_t len);
+    void (*recv)(I2CAdapter *adapter, uint8_t addr,
+                 uint8_t *buf, uint16_t len);
+};
+
+void i2c_send(I2CAdapter *i2c, uint8_t addr,
+              const uint8_t *buf, uint16_t len);
+void i2c_recv(I2CAdapter *i2c, uint8_t addr,
+              uint8_t *buf, uint16_t len);
+
+/* libi2c-omap.c */
+I2CAdapter *omap_i2c_create(uint64_t addr);
+
+#endif
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/7] tmp105: Split out I2C message constants from header

[Andreas Färber]

Allows value sharing with qtest.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/tmp105.h      |   34 +---------------------------------
 hw/tmp105_regs.h |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 Dateien geändert, 51 Zeilen hinzugefügt(+), 33 Zeilen entfernt(-)
 create mode 100644 hw/tmp105_regs.h

diff --git a/hw/tmp105.h b/hw/tmp105.h
index 51eff4b..982d1c9 100644
--- a/hw/tmp105.h
+++ b/hw/tmp105.h
@@ -15,39 +15,7 @@
 #define QEMU_TMP105_H
 
 #include "i2c.h"
-
-/**
- * TMP105Reg:
- * @TMP105_REG_TEMPERATURE: Temperature register
- * @TMP105_REG_CONFIG: Configuration register
- * @TMP105_REG_T_LOW: Low temperature register (also known as T_hyst)
- * @TMP105_REG_T_HIGH: High temperature register (also known as T_OS)
- *
- * The following temperature sensors are
- * compatible with the TMP105 registers:
- * - adt75
- * - ds1775
- * - ds75
- * - lm75
- * - lm75a
- * - max6625
- * - max6626
- * - mcp980x
- * - stds75
- * - tcn75
- * - tmp100
- * - tmp101
- * - tmp105
- * - tmp175
- * - tmp275
- * - tmp75
- **/
-typedef enum TMP105Reg {
-    TMP105_REG_TEMPERATURE = 0,
-    TMP105_REG_CONFIG,
-    TMP105_REG_T_LOW,
-    TMP105_REG_T_HIGH,
-} TMP105Reg;
+#include "tmp105_regs.h"
 
 /**
  * tmp105_set:
diff --git a/hw/tmp105_regs.h b/hw/tmp105_regs.h
new file mode 100644
index 0000000..9b55aba
--- /dev/null
+++ b/hw/tmp105_regs.h
@@ -0,0 +1,50 @@
+/*
+ * Texas Instruments TMP105 Temperature Sensor I2C messages
+ *
+ * Browse the data sheet:
+ *
+ *    http://www.ti.com/lit/gpn/tmp105
+ *
+ * Copyright (C) 2012 Alex Horn <alex.horn@cs.ox.ac.uk>
+ * Copyright (C) 2008-2012 Andrzej Zaborowski <balrogg@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_TMP105_MSGS_H
+#define QEMU_TMP105_MSGS_H
+
+/**
+ * TMP105Reg:
+ * @TMP105_REG_TEMPERATURE: Temperature register
+ * @TMP105_REG_CONFIG: Configuration register
+ * @TMP105_REG_T_LOW: Low temperature register (also known as T_hyst)
+ * @TMP105_REG_T_HIGH: High temperature register (also known as T_OS)
+ *
+ * The following temperature sensors are
+ * compatible with the TMP105 registers:
+ * - adt75
+ * - ds1775
+ * - ds75
+ * - lm75
+ * - lm75a
+ * - max6625
+ * - max6626
+ * - mcp980x
+ * - stds75
+ * - tcn75
+ * - tmp100
+ * - tmp101
+ * - tmp105
+ * - tmp175
+ * - tmp275
+ * - tmp75
+ **/
+typedef enum TMP105Reg {
+    TMP105_REG_TEMPERATURE = 0,
+    TMP105_REG_CONFIG,
+    TMP105_REG_T_LOW,
+    TMP105_REG_T_HIGH,
+} TMP105Reg;
+
+#endif
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 4/7] tests: Add tmp105 qtest test case

[Andreas Färber]

Exercise all four commands of the TMP105, testing for an issue in the
I2C TX path.

The test case uses the N800's OMAP I2C and is the first for ARM.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 tests/Makefile      |    2 ++
 tests/tmp105-test.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 Dateien geändert, 78 Zeilen hinzugefügt(+)
 create mode 100644 tests/tmp105-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 1ec41cb..a4e8ad3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -28,6 +28,7 @@ check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
+check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 
 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
 
@@ -78,6 +79,7 @@ tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
+tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(trace-obj-y)
 
 # QTest rules
 
diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
new file mode 100644
index 0000000..a6ad213
--- /dev/null
+++ b/tests/tmp105-test.c
@@ -0,0 +1,76 @@
+/*
+ * QTest testcase for the TMP105 temperature sensor
+ *
+ * Copyright (c) 2012 Andreas Färber
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "libqtest.h"
+#include "libi2c.h"
+#include "hw/tmp105_regs.h"
+
+#include <glib.h>
+
+#define OMAP2_I2C_1_BASE 0x48070000
+
+#define N8X0_ADDR 0x48
+
+static I2CAdapter *i2c;
+static uint8_t addr;
+
+static void send_and_receive(void)
+{
+    uint8_t cmd[3];
+    uint8_t resp[2];
+
+    cmd[0] = TMP105_REG_TEMPERATURE;
+    i2c_send(i2c, addr, cmd, 1);
+    i2c_recv(i2c, addr, resp, 2);
+    g_assert_cmpuint(((uint16_t)resp[0] << 8) | resp[1], ==, 0);
+
+    cmd[0] = TMP105_REG_CONFIG;
+    cmd[1] = 0x0; /* matches the reset value */
+    i2c_send(i2c, addr, cmd, 2);
+    i2c_recv(i2c, addr, resp, 1);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+
+    cmd[0] = TMP105_REG_T_LOW;
+    cmd[1] = 0x12;
+    cmd[2] = 0x34;
+    i2c_send(i2c, addr, cmd, 3);
+    i2c_recv(i2c, addr, resp, 2);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+    g_assert_cmphex(resp[1], ==, cmd[2]);
+
+    cmd[0] = TMP105_REG_T_HIGH;
+    cmd[1] = 0x42;
+    cmd[2] = 0x31;
+    i2c_send(i2c, addr, cmd, 3);
+    i2c_recv(i2c, addr, resp, 2);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+    g_assert_cmphex(resp[1], ==, cmd[2]);
+}
+
+int main(int argc, char **argv)
+{
+    QTestState *s = NULL;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    s = qtest_start("-display none -machine n800");
+    i2c = omap_i2c_create(OMAP2_I2C_1_BASE);
+    addr = N8X0_ADDR;
+
+    qtest_add_func("/tmp105/tx-rx", send_and_receive);
+
+    ret = g_test_run();
+
+    if (s) {
+        qtest_quit(s);
+    }
+    g_free(i2c);
+
+    return ret;
+}
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 5/7] tmp105: Fix I2C protocol bug

[Andreas Färber]

An early length postincrement in the TMP105's I2C TX path led to
transfers of more than one byte to place the second byte in the third
byte's place within the buffer and the third byte to get discarded.

Fix this by explictly incrementing the length after the checks but
before the callback is called, which again checks the length.

Adjust the Coding Style while at it.

Signed-off-by: Alex Horn <alex.horn@cs.ox.ac.uk>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/tmp105.c |    9 ++++++---
 1 Datei geändert, 6 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/hw/tmp105.c b/hw/tmp105.c
index 9c67e64..ff9f28b 100644
--- a/hw/tmp105.c
+++ b/hw/tmp105.c
@@ -153,11 +153,14 @@ static int tmp105_tx(I2CSlave *i2c, uint8_t data)
 {
     TMP105State *s = (TMP105State *) i2c;
 
-    if (!s->len ++)
+    if (s->len == 0) {
         s->pointer = data;
-    else {
-        if (s->len <= 2)
+        s->len++;
+    } else {
+        if (s->len <= 2) {
             s->buf[s->len - 1] = data;
+        }
+        s->len++;
         tmp105_write(s);
     }
 
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 6/7] tmp105: QOM'ify

[Andreas Färber]

Introduce TYPE_ constant and cast macro.
Move the state struct to the new header to allow for future embedding.

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

diff --git a/hw/tmp105.c b/hw/tmp105.c
index ff9f28b..a16f538 100644
--- a/hw/tmp105.c
+++ b/hw/tmp105.c
@@ -22,20 +22,6 @@
 #include "i2c.h"
 #include "tmp105.h"
 
-typedef struct {
-    I2CSlave i2c;
-    uint8_t len;
-    uint8_t buf[2];
-    qemu_irq pin;
-
-    uint8_t pointer;
-    uint8_t config;
-    int16_t temperature;
-    int16_t limit[2];
-    int faults;
-    uint8_t alarm;
-} TMP105State;
-
 static void tmp105_interrupt_update(TMP105State *s)
 {
     qemu_set_irq(s->pin, s->alarm ^ ((~s->config >> 2) & 1));	/* POL */
@@ -68,7 +54,7 @@ static void tmp105_alarm_update(TMP105State *s)
 /* Units are 0.001 centigrades relative to 0 C.  */
 void tmp105_set(I2CSlave *i2c, int temp)
 {
-    TMP105State *s = (TMP105State *) i2c;
+    TMP105State *s = TMP105(i2c);
 
     if (temp >= 128000 || temp < -128000) {
         fprintf(stderr, "%s: values is out of range (%i.%03i C)\n",
@@ -141,17 +127,18 @@ static void tmp105_write(TMP105State *s)
 
 static int tmp105_rx(I2CSlave *i2c)
 {
-    TMP105State *s = (TMP105State *) i2c;
+    TMP105State *s = TMP105(i2c);
 
-    if (s->len < 2)
+    if (s->len < 2) {
         return s->buf[s->len ++];
-    else
+    } else {
         return 0xff;
+    }
 }
 
 static int tmp105_tx(I2CSlave *i2c, uint8_t data)
 {
-    TMP105State *s = (TMP105State *) i2c;
+    TMP105State *s = TMP105(i2c);
 
     if (s->len == 0) {
         s->pointer = data;
@@ -169,10 +156,11 @@ static int tmp105_tx(I2CSlave *i2c, uint8_t data)
 
 static void tmp105_event(I2CSlave *i2c, enum i2c_event event)
 {
-    TMP105State *s = (TMP105State *) i2c;
+    TMP105State *s = TMP105(i2c);
 
-    if (event == I2C_START_RECV)
+    if (event == I2C_START_RECV) {
         tmp105_read(s);
+    }
 
     s->len = 0;
 }
@@ -208,7 +196,7 @@ static const VMStateDescription vmstate_tmp105 = {
 
 static void tmp105_reset(I2CSlave *i2c)
 {
-    TMP105State *s = (TMP105State *) i2c;
+    TMP105State *s = TMP105(i2c);
 
     s->temperature = 0;
     s->pointer = 0;
@@ -221,7 +209,7 @@ static void tmp105_reset(I2CSlave *i2c)
 
 static int tmp105_init(I2CSlave *i2c)
 {
-    TMP105State *s = FROM_I2C_SLAVE(TMP105State, i2c);
+    TMP105State *s = TMP105(i2c);
 
     qdev_init_gpio_out(&i2c->qdev, &s->pin, 1);
 
@@ -242,8 +230,8 @@ static void tmp105_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_tmp105;
 }
 
-static TypeInfo tmp105_info = {
-    .name          = "tmp105",
+static const TypeInfo tmp105_info = {
+    .name          = TYPE_TMP105,
     .parent        = TYPE_I2C_SLAVE,
     .instance_size = sizeof(TMP105State),
     .class_init    = tmp105_class_init,
diff --git a/hw/tmp105.h b/hw/tmp105.h
index 982d1c9..c21396f 100644
--- a/hw/tmp105.h
+++ b/hw/tmp105.h
@@ -17,6 +17,33 @@
 #include "i2c.h"
 #include "tmp105_regs.h"
 
+#define TYPE_TMP105 "tmp105"
+#define TMP105(obj) OBJECT_CHECK(TMP105State, (obj), TYPE_TMP105)
+
+/**
+ * TMP105State:
+ * @config: Bits 5 and 6 (value 32 and 64) determine the precision of the
+ * temperature. See Table 8 in the data sheet.
+ *
+ * @see_also: http://www.ti.com/lit/gpn/tmp105
+ */
+typedef struct TMP105State {
+    /*< private >*/
+    I2CSlave i2c;
+    /*< public >*/
+
+    uint8_t len;
+    uint8_t buf[2];
+    qemu_irq pin;
+
+    uint8_t pointer;
+    uint8_t config;
+    int16_t temperature;
+    int16_t limit[2];
+    int faults;
+    uint8_t alarm;
+} TMP105State;
+
 /**
  * tmp105_set:
  * @i2c: dispatcher to TMP105 hardware model
-- 
1.7.10.4

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

[Andreas Färber]

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

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

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

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
>

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

Re: [Qemu-devel] [PATCH v2 0/7] I2C libqos and tmp105 qtest support

[Andreas Färber]

Am 14.12.2012 12:34, schrieb Andreas Färber:
> Hello,
> 
> Here's I2C support for the libqos framework and an OMAP driver.
> The design was inspired by QEMU's i2c_bus and Linux' i2c_adapter.
> Seems like low hanging fruit (while we're still waiting for PCI support :)),
> hopefully motivating more people to contribute qtests for new devices or
> bug fixes.

Ping! See below...

> I found a bug in omap_i2c related to the SBD bit not getting cleared after
> a single-byte read; the patch has been dropped as we were not able to verify
> it against a TRM or hardware. The driver was therefore changed to ignore the
> SBD bit, like the Linux driver does.
> 
> I've split up the proposed tmp105.h header further to facilitate reuse of
> the register enum in qtest.
> 
> The test case itself initially fails; in lack of a v2 patch by Alex I'm
> appending my own fix proposal that makes the test pass.
> 
> I'm also appending a QOM'ish cleanup of the file, replacing the current API
> with a QOM property.
> To do a qom-set for testing a non-zero temperature reading, we'll need either
> a canonical path for the TMP105 or Jason's qtest_qmp_resp() to iterate over
> qom-list /machine/unassigned output looking for type "tmp105" with qom-get.
[...]
> v1 -> v2:
> * Factor out libqos API for I2C and an OMAP driver
> * Avoid casts by using uint8_t* in API, suggested by Blue
> * Ignore SBD bit in omap_i2c driver
> * Drop omap_i2c patch clearing SBD
> * Incorporate Alex' tmp105.h patch
> * Split out tmp105_regs.h for qtest, inspired by rtc-test
> * Append replacement for Alex' fix
> * Append QOM cleanup and turn setter function into QOM property
[...]
> Alex Horn (1):
>   tmp105: Create API for TMP105 temperature sensor

This was applied by Anthony meanwhile.

> Andreas Färber (6):
>   libqtest: Prepare I2C libqos
>   tmp105: Split out I2C message constants from header
>   tests: Add tmp105 qtest test case
>   tmp105: Fix I2C protocol bug
>   tmp105: QOM'ify
>   tmp105: Add temperature QOM property

These need to be rebased due to the header reorganization and gcov.
Anyone any feedback, especially on the libqos part? Anthony? Stefan?

Thanks,
Andreas

>  hw/i2c.h            |    3 -
>  hw/tmp105.c         |  101 ++++++++++++++++++-------------
>  hw/tmp105.h         |   47 +++++++++++++++
>  hw/tmp105_regs.h    |   50 ++++++++++++++++
>  tests/Makefile      |    3 +
>  tests/libi2c-omap.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libi2c.c      |   22 +++++++
>  tests/libi2c.h      |   30 ++++++++++
>  tests/tmp105-test.c |   76 +++++++++++++++++++++++
>  9 Dateien geändert, 453 Zeilen hinzugefügt(+), 45 Zeilen entfernt(-)
>  create mode 100644 hw/tmp105.h
>  create mode 100644 hw/tmp105_regs.h
>  create mode 100644 tests/libi2c-omap.c
>  create mode 100644 tests/libi2c.c
>  create mode 100644 tests/libi2c.h
>  create mode 100644 tests/tmp105-test.c

Re: [Qemu-devel] [PATCH v2 0/7] I2C libqos and tmp105 qtest support

[Anthony Liguori]

Andreas Färber <andreas.faerber@web.de> writes:

> Hello,
>
> Here's I2C support for the libqos framework and an OMAP driver.
> The design was inspired by QEMU's i2c_bus and Linux' i2c_adapter.
> Seems like low hanging fruit (while we're still waiting for PCI support :)),
> hopefully motivating more people to contribute qtests for new devices or
> bug fixes.
>
> I found a bug in omap_i2c related to the SBD bit not getting cleared after
> a single-byte read; the patch has been dropped as we were not able to verify
> it against a TRM or hardware. The driver was therefore changed to ignore the
> SBD bit, like the Linux driver does.
>
> I've split up the proposed tmp105.h header further to facilitate reuse of
> the register enum in qtest.
>
> The test case itself initially fails; in lack of a v2 patch by Alex I'm
> appending my own fix proposal that makes the test pass.
>
> I'm also appending a QOM'ish cleanup of the file, replacing the current API
> with a QOM property.
> To do a qom-set for testing a non-zero temperature reading, we'll need either
> a canonical path for the TMP105 or Jason's qtest_qmp_resp() to iterate over
> qom-list /machine/unassigned output looking for type "tmp105" with qom-get.
>
> Alex, if you could add a Tested-by (using object_property_set_int()) that
> would be appreciated.

Looks good.  Whole series:

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> Regards,
> Andreas
>
> v1 -> v2:
> * Factor out libqos API for I2C and an OMAP driver
> * Avoid casts by using uint8_t* in API, suggested by Blue
> * Ignore SBD bit in omap_i2c driver
> * Drop omap_i2c patch clearing SBD
> * Incorporate Alex' tmp105.h patch
> * Split out tmp105_regs.h for qtest, inspired by rtc-test
> * Append replacement for Alex' fix
> * Append QOM cleanup and turn setter function into QOM property
>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Alex Horn <alex.horn@cs.ox.ac.uk>
> Cc: Andrzej Zaborowski <balrogg@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Blue Swirl <blauwirbel@gmail.com>
>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
>
> Alex Horn (1):
>   tmp105: Create API for TMP105 temperature sensor
>
> Andreas Färber (6):
>   libqtest: Prepare I2C libqos
>   tmp105: Split out I2C message constants from header
>   tests: Add tmp105 qtest test case
>   tmp105: Fix I2C protocol bug
>   tmp105: QOM'ify
>   tmp105: Add temperature QOM property
>
>  hw/i2c.h            |    3 -
>  hw/tmp105.c         |  101 ++++++++++++++++++-------------
>  hw/tmp105.h         |   47 +++++++++++++++
>  hw/tmp105_regs.h    |   50 ++++++++++++++++
>  tests/Makefile      |    3 +
>  tests/libi2c-omap.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libi2c.c      |   22 +++++++
>  tests/libi2c.h      |   30 ++++++++++
>  tests/tmp105-test.c |   76 +++++++++++++++++++++++
>  9 Dateien geändert, 453 Zeilen hinzugefügt(+), 45 Zeilen entfernt(-)
>  create mode 100644 hw/tmp105.h
>  create mode 100644 hw/tmp105_regs.h
>  create mode 100644 tests/libi2c-omap.c
>  create mode 100644 tests/libi2c.c
>  create mode 100644 tests/libi2c.h
>  create mode 100644 tests/tmp105-test.c
>
> -- 
> 1.7.10.4

Re: [Qemu-devel] [PATCH v2 0/7] I2C libqos and tmp105 qtest support

[Anthony Liguori]

Andreas Färber <andreas.faerber@web.de> writes:

> Hello,
>
> Here's I2C support for the libqos framework and an OMAP driver.
> The design was inspired by QEMU's i2c_bus and Linux' i2c_adapter.
> Seems like low hanging fruit (while we're still waiting for PCI support :)),
> hopefully motivating more people to contribute qtests for new devices or
> bug fixes.
>
> I found a bug in omap_i2c related to the SBD bit not getting cleared after
> a single-byte read; the patch has been dropped as we were not able to verify
> it against a TRM or hardware. The driver was therefore changed to ignore the
> SBD bit, like the Linux driver does.
>
> I've split up the proposed tmp105.h header further to facilitate reuse of
> the register enum in qtest.
>
> The test case itself initially fails; in lack of a v2 patch by Alex I'm
> appending my own fix proposal that makes the test pass.
>
> I'm also appending a QOM'ish cleanup of the file, replacing the current API
> with a QOM property.
> To do a qom-set for testing a non-zero temperature reading, we'll need either
> a canonical path for the TMP105 or Jason's qtest_qmp_resp() to iterate over
> qom-list /machine/unassigned output looking for type "tmp105" with qom-get.
>
> Alex, if you could add a Tested-by (using object_property_set_int()) that
> would be appreciated.

BTW, this series doesn't apply anymore.  If you resubmit, I'll commit it.

Regards,

Anthony Liguori

>
> Regards,
> Andreas
>
> v1 -> v2:
> * Factor out libqos API for I2C and an OMAP driver
> * Avoid casts by using uint8_t* in API, suggested by Blue
> * Ignore SBD bit in omap_i2c driver
> * Drop omap_i2c patch clearing SBD
> * Incorporate Alex' tmp105.h patch
> * Split out tmp105_regs.h for qtest, inspired by rtc-test
> * Append replacement for Alex' fix
> * Append QOM cleanup and turn setter function into QOM property
>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Alex Horn <alex.horn@cs.ox.ac.uk>
> Cc: Andrzej Zaborowski <balrogg@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Blue Swirl <blauwirbel@gmail.com>
>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
>
> Alex Horn (1):
>   tmp105: Create API for TMP105 temperature sensor
>
> Andreas Färber (6):
>   libqtest: Prepare I2C libqos
>   tmp105: Split out I2C message constants from header
>   tests: Add tmp105 qtest test case
>   tmp105: Fix I2C protocol bug
>   tmp105: QOM'ify
>   tmp105: Add temperature QOM property
>
>  hw/i2c.h            |    3 -
>  hw/tmp105.c         |  101 ++++++++++++++++++-------------
>  hw/tmp105.h         |   47 +++++++++++++++
>  hw/tmp105_regs.h    |   50 ++++++++++++++++
>  tests/Makefile      |    3 +
>  tests/libi2c-omap.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libi2c.c      |   22 +++++++
>  tests/libi2c.h      |   30 ++++++++++
>  tests/tmp105-test.c |   76 +++++++++++++++++++++++
>  9 Dateien geändert, 453 Zeilen hinzugefügt(+), 45 Zeilen entfernt(-)
>  create mode 100644 hw/tmp105.h
>  create mode 100644 hw/tmp105_regs.h
>  create mode 100644 tests/libi2c-omap.c
>  create mode 100644 tests/libi2c.c
>  create mode 100644 tests/libi2c.h
>  create mode 100644 tests/tmp105-test.c
>
> -- 
> 1.7.10.4