diff mbox

[3/3] qtest: Add PIIX4 SMBus support for libqos

Message ID 1402304133-29620-4-git-send-email-marc.mari.barcelo@gmail.com
State New
Headers show

Commit Message

Marc Marí June 9, 2014, 8:55 a.m. UTC 
Add support for the SMBus protocol on the PIIX4 chipset. Add a test for
EEPROMs using this interface.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/Makefile             |    5 +
 tests/eeprom-test.c        |   58 ++++++++++++
 tests/libqos/i2c.h         |    5 +-
 tests/libqos/smbus-piix4.c |  220 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 tests/eeprom-test.c
 create mode 100644 tests/libqos/smbus-piix4.c

Comments

Paolo Bonzini June 9, 2014, 10:33 a.m. UTC | #1 
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
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index 361bb7b..e49c779 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -152,6 +152,8 @@  gcov-files-i386-y += hw/pci-bridge/i82801b11.c
 check-qtest-i386-y += tests/ioh3420-test$(EXESUF)
 gcov-files-i386-y += hw/pci-bridge/ioh3420.c
 check-qtest-i386-y += tests/usb-hcd-ehci-test$(EXESUF)
+gcov-files-i386-y += hw/i2c/smbus_eeprom.c
+check-qtest-i386-y += tests/eeprom-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-ehci.c
 gcov-files-i386-y += hw/usb/hcd-uhci.c
 gcov-files-i386-y += hw/usb/dev-hid.c
@@ -281,6 +283,7 @@  libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
+libqos-piix4-obj-y = $(libqos-obj-y) tests/libqos/smbus-piix4.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
@@ -322,9 +325,11 @@  tests/es1370-test$(EXESUF): tests/es1370-test.o
 tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o
 tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-pc-obj-y)
+tests/eeprom-test$(EXESUF): tests/eeprom-test.o $(libqos-piix4-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
 
+
 # QTest rules
 
 TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
diff --git a/tests/eeprom-test.c b/tests/eeprom-test.c
new file mode 100644
index 0000000..6f01dd2
--- /dev/null
+++ b/tests/eeprom-test.c
@@ -0,0 +1,58 @@ 
+/*
+ * QTest testcase for the SMBus EEPROM device
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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 <glib.h>
+#include <stdio.h>
+
+#include "libqtest.h"
+#include "libqos/i2c.h"
+
+#define PIIX4_SMBUS_BASE 0x0000b100
+
+#define EEPROM_TEST_ID   "eeprom-test"
+#define EEPROM_TEST_ADDR 0x50
+
+static I2CAdapter *i2c;
+
+static void send_and_receive(void)
+{
+    uint8_t i;
+    uint8_t buf = 0;
+    uint64_t big_buf = 0;
+
+    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);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    QTestState *s = NULL;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    s = qtest_start(NULL);
+    i2c = piix4_smbus_create(PIIX4_SMBUS_BASE);
+
+    qtest_add_func("/eeprom/tx-rx", send_and_receive);
+
+    ret = g_test_run();
+
+    if (s) {
+        qtest_quit(s);
+    }
+    g_free(i2c);
+
+    return ret;
+}
diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h
index 1ce9af4..9fd3a1c 100644
--- a/tests/libqos/i2c.h
+++ b/tests/libqos/i2c.h
@@ -24,7 +24,10 @@  void i2c_send(I2CAdapter *i2c, uint8_t addr,
 void i2c_recv(I2CAdapter *i2c, uint8_t addr,
               uint8_t *buf, uint16_t len);
 
-/* libi2c-omap.c */
+/* i2c-omap.c */
 I2CAdapter *omap_i2c_create(uint64_t addr);
 
+/* smbus-piix4.c */
+I2CAdapter *piix4_smbus_create(uint64_t addr);
+
 #endif
diff --git a/tests/libqos/smbus-piix4.c b/tests/libqos/smbus-piix4.c
new file mode 100644
index 0000000..2e04968
--- /dev/null
+++ b/tests/libqos/smbus-piix4.c
@@ -0,0 +1,220 @@ 
+/*
+ * QTest I2C driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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 "libqos/i2c.h"
+
+#include <glib.h>
+
+#include "libqtest.h"
+
+#define SMBUS_TIMEOUT 100000
+
+enum PIIX4SMBUSRegisters {
+    PIIX4_SMBUS_BA = 0x90, /* Base address (in [15:4]). 32 bits */
+    PIIX4_SMBUS_STAT = 0x00, /* Status. 8 bits */
+    PIIX4_SMBUS_CNT = 0x02, /* Control. 8 bits */
+    PIIX4_SMBUS_CMD = 0x03, /* Command. 8 bits */
+    PIIX4_SMBUS_ADD = 0x04, /* Address. 8 bits */
+    PIIX4_SMBUS_DAT0 = 0x05, /* Data 0. 8 bits */
+    PIIX4_SMBUS_DAT1 = 0x06, /* Data 1. 8 bits */
+    PIIX4_SMBUS_BLKDAT = 0x07, /* Block data. 8 bits */
+};
+
+enum PIIX4SMBUSSTATBits {
+    PIIX4_SMBUS_CNT_INTEREN = 1 << 0, /* Enable interrupts */
+    PIIX4_SMBUS_CNT_KILL = 1 << 1, /* Stop the current transaction */
+    PIIX4_SMBUS_CNT_PROT = 7 << 2, /* Type of command */
+    PIIX4_SMBUS_CNT_START = 1 << 6, /* Start execution */
+    PIIX4_SMBUS_STAT_BUSY = 1 << 0, /* Host busy */
+};
+
+enum PIIX4SMBUSCommands {
+    PIIX4_SMBUS_QRW = 0x00, /* Quick read or write */
+    PIIX4_SMBUS_BRW = 0x04, /* Byte read or write */
+    PIIX4_SMBUS_BDRW = 0x08, /* Byte data read or write */
+    PIIX4_SMBUS_WDRW = 0x0C, /* Word data read or write */
+    PIIX4_SMBUS_BLRW = 0x14, /* Block read or write */
+    PIIX4_SMBUS_WR = 0x0, /* Write */
+    PIIX4_SMBUS_RD = 0x1, /* Read */
+};
+
+typedef struct {
+    I2CAdapter parent;
+    uint64_t addr;
+} PIIX4I2C;
+
+static int piix4_smbus_wait_ready(uint64_t addr)
+{
+    uint64_t loops;
+    uint8_t status;
+    loops = SMBUS_TIMEOUT;
+    do {
+        clock_step(10);
+        status = inb(addr + PIIX4_SMBUS_STAT);
+        if ((status & 0x1) == 0) {
+            break; /* It has ended */
+        }
+    } while (--loops);
+
+    if (loops != 0) {
+        return 0;
+    } else {
+        return -1;
+    }
+}
+
+static int piix4_smbus_wait_done(uint64_t addr)
+{
+    uint64_t loops;
+    uint8_t status;
+    loops = SMBUS_TIMEOUT;
+    do {
+        clock_step(10);
+        status = inb(addr + PIIX4_SMBUS_STAT);
+        if ((status & 0x1) != 0) {
+            continue; /* It has not started yet */
+        }
+        if (status & 0xfe) {
+            break; /* One of the interrupt flags is set */
+        }
+    } while (--loops);
+
+    if (loops != 0) {
+        return 0;
+    } else {
+        return -1;
+    }
+}
+
+static void piix4_smbus_recv(I2CAdapter *i2c, uint8_t addr,
+                          uint8_t *buf, uint16_t len)
+{
+    uint8_t stat;
+    uint8_t size;
+    PIIX4I2C *s = (PIIX4I2C *)i2c;
+
+    /* Wait to be ready */
+    g_assert_cmpint(piix4_smbus_wait_ready(s->addr), == , 0);
+
+    /* Clear interrupts */
+    outb(s->addr + PIIX4_SMBUS_STAT, 0x1e);
+
+    /* Write device */
+    outb(s->addr + PIIX4_SMBUS_ADD, (addr << 1) | PIIX4_SMBUS_RD);
+
+    /* Clear data */
+    outb(s->addr + PIIX4_SMBUS_DAT0, 0);
+
+    /* Start */
+    if (len == 1) {
+        outb(s->addr + PIIX4_SMBUS_CNT,
+                PIIX4_SMBUS_BRW | PIIX4_SMBUS_CNT_START);
+    } else {
+        outb(s->addr + PIIX4_SMBUS_CNT,
+                PIIX4_SMBUS_BLRW | PIIX4_SMBUS_CNT_START);
+    }
+
+    /* Wait to be done */
+    piix4_smbus_wait_done(s->addr);
+
+    /* Wait end */
+    stat = inb(s->addr + PIIX4_SMBUS_STAT);
+    g_assert_cmphex((stat & 0x3e), ==, 2); /* Only interrupt enabled
+
+    /* Read */
+    if (len == 1) {
+        buf[0] = inb(s->addr + PIIX4_SMBUS_DAT0);
+    } else {
+        while (len > 0) {
+            size = inb(s->addr + PIIX4_SMBUS_DAT0);
+            if (size == 0) {
+                break;
+            }
+            g_assert_cmpuint((len-size), <, 0);
+            while (size > 0) {
+                buf[0] = readb(s->addr + PIIX4_SMBUS_BLKDAT);
+                ++buf;
+                --size;
+            }
+
+            len -= size;
+        }
+    }
+}
+
+static void piix4_smbus_send(I2CAdapter *i2c, uint8_t addr,
+                          const uint8_t *buf, uint16_t len)
+{
+    uint8_t stat;
+    uint8_t size;
+    PIIX4I2C *s = (PIIX4I2C *)i2c;
+
+    /* Wait to be ready */
+    g_assert_cmpint(piix4_smbus_wait_ready(s->addr), ==, 0);
+
+    /* Clear interrupts */
+    outb(s->addr + PIIX4_SMBUS_STAT, 0x1e);
+
+    /* Write device */
+    outb(s->addr + PIIX4_SMBUS_ADD, (addr << 1) | PIIX4_SMBUS_WR);
+
+    if (len == 1) {
+        /* Write */
+        outb(s->addr + PIIX4_SMBUS_DAT0, buf[0]);
+
+        stat = inb(s->addr + PIIX4_SMBUS_DAT0);
+        g_assert_cmphex(buf[0], ==, stat);
+
+        /* Start */
+        outb(s->addr + PIIX4_SMBUS_CNT,
+                PIIX4_SMBUS_BRW | PIIX4_SMBUS_CNT_START);
+
+        /* Wait to be done */
+        piix4_smbus_wait_done(s->addr);
+
+        /* Wait end */
+        stat = inb(s->addr + PIIX4_SMBUS_STAT);
+        g_assert_cmphex((stat & 0x3e), ==, 2); /* Only interrupt enabled
+    } else {
+        /* Write 32 bytes */
+        while (len > 0) {
+            size = 0;
+            while (len > 0 && size < 32) {
+                outb(s->addr + PIIX4_SMBUS_BLKDAT, buf[0]);
+                ++buf;
+                ++size;
+                --len;
+            }
+            outb(s->addr + PIIX4_SMBUS_DAT0, size);
+
+            /* Start */
+            outb(s->addr + PIIX4_SMBUS_CNT,
+                    PIIX4_SMBUS_BLRW | PIIX4_SMBUS_CNT_START);
+
+            /* Wait to be done */
+            piix4_smbus_wait_done(s->addr);
+
+            /* Wait end */
+            stat = inb(s->addr + PIIX4_SMBUS_STAT);
+            g_assert_cmphex((stat & 0x3e), ==, 2);
+        }
+    }
+}
+
+I2CAdapter *piix4_smbus_create(uint64_t addr)
+{
+    PIIX4I2C *s = g_malloc0(sizeof(*s));
+    I2CAdapter *i2c = (I2CAdapter *)s;
+
+    s->addr = addr;
+
+    i2c->send = piix4_smbus_send;
+    i2c->recv = piix4_smbus_recv;
+
+    return i2c;
+}