Il 09/06/2014 10:55, Marc Marí ha scritto:

+static void send_and_receive(void)
+{
+    uint8_t i;
+    uint8_t buf = 0;
+    uint64_t big_buf = 0;

Probably uint32_t since you only read 4 bytes.

+    for (i = EEPROM_TEST_ADDR; i < EEPROM_TEST_ADDR+8; ++i) {
+        i2c_recv(i2c, i, &buf, 1);
+        g_assert_cmphex(buf, ==, 0);
+
+        i2c_recv(i2c, i, (uint8_t *)&big_buf, 4);
+        g_assert_cmphex(big_buf, ==, 0);
+    }

Here you should be sending something before. The protocol is that you send the address, then send the offset, then receive bytes.

In fact, right now you're sending a zero here:

+    /* Clear data */
+    outb(s->addr + PIIX4_SMBUS_DAT0, 0);

in libqos.

The problem is that the piix4 smbus interface includes more "magic" than the i2c interface that is currently part of libqos. Basically libqos would have to implement the same state machine as hw/i2c/smbus.c in order to convert i2c commands (from the test case) to smbus commands (for the device). The opposite also makes sense if you want to have an smbus testcase API and you want to make it talk to "basic" i2c devices.

So this testcase by itself is not really meaningful. This is not your fault---I and Stefan aren't 100% comfortable with I2C either. :) Let's discuss it today on the chat.

+        while (len > 0) {
+            size = inb(s->addr + PIIX4_SMBUS_DAT0);
+            if (size == 0) {
+                break;
+            }
+            g_assert_cmpuint((len-size), <, 0);

Asserting len < size does not make much sense. Should you be asserting instead that size <= len here? Or even len == size (without the outer "while (len > 0)" loop)?

+            while (size > 0) {
+                buf[0] = readb(s->addr + PIIX4_SMBUS_BLKDAT);
+                ++buf;
+                --size;
+            }
+
+            len -= size;
+        }

size is zero here; the decrement should be before the inner while loop.

Paolo

Reply via email to