diff mbox

[2/2] tests: Add tmp105 unit test

Message ID 1355293773-18361-3-git-send-email-andreas.faerber@web.de
State New
Headers show

Commit Message

Andreas Färber Dec. 12, 2012, 6:29 a.m. UTC
Exercise all four commands of the TMP105, testing for an issue in the
I2C TX path. The test case is specific to the n800's OMAP I2C for now
and is the first test case for arm.

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

Comments

Alex Horn Dec. 12, 2012, 2:44 p.m. UTC | #1
Thanks so much for taking the initiative on creating functional tests
for the TMP105 hardware model. I am exciting to see these changes and
I am wondering if your QTests could be complemented with the following
standalone unit tests:

  https://github.com/ahorn/benchmarks/blob/965e571d92e677e8e64ad5faa0107f5dbd451981/qemu-hw/tmp105/tmp105-test.c

Their purpose is similar to yours except that unit tests focus on
checking the internal state of a hardware model without requiring a
bus implementation or a socket connection for the QTest client-server
infrastructure. That is, unit tests do not seek to replace QTests but
strengthen them (see also below).

In fact, I have tried including the above unit tests in QEMU. For
this, the following lines were added to tests/Makefile:

  check-unit-y += tests/test-tmp105$(EXESUF)
  tests/test-tmp105$(EXESUF): tests/test-tmp105.o hw/tmp105.o
$(qobject-obj-y) $(common-obj-y) $(qom-obj-y)

The last three $(...) variables are required to compile. However,
"make check" results in a NSS linking error:

/usr/bin/ld: libcacard/vcard_emul_nss.o: undefined reference to symbol
'CERT_GetDefaultCertDB@@NSS_3.2'
/usr/bin/ld: note: 'CERT_GetDefaultCertDB@@NSS_3.2' is defined in DSO
/usr/lib64/libnss3.so so try adding it to the linker command line
/usr/lib64/libnss3.so: could not read symbols: Invalid operation
collect2: ld returned 1 exit status
make: *** [tests/test-tmp105] Error 1

If these type of issues could be resolved, it would be possible to
encourage others to write unit tests for their hardware models.

The utility of these unit tests is manifold. Noteworthy, a certain
level of confidence can be achieved before integrating a hardware
model into a bus implementation (e.g. OMAP or SMBus for I2C). This
could also prove useful for overall regression testing and debugging.
Finally, unit tests encourage modular designs (e.g. [1]). Such
modularity could also be a catalyst for new QEMU use cases and a
potentially broader developer base.

With kind regards,
Alex

[1] http://en.wikipedia.org/wiki/Dependency_injection

On 12 December 2012 06:29, Andreas Färber <andreas.faerber@web.de> wrote:
> Exercise all four commands of the TMP105, testing for an issue in the
> I2C TX path. The test case is specific to the n800's OMAP I2C for now
> and is the first test case for arm.
>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  tests/Makefile      |    2 +
>  tests/tmp105-test.c |  205 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 Dateien geändert, 207 Zeilen hinzugefügt(+)
>  create mode 100644 tests/tmp105-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index b60f0fb..69eff45 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..26d51f7
> --- /dev/null
> +++ b/tests/tmp105-test.c
> @@ -0,0 +1,205 @@
> +/*
> + * 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 <glib.h>
> +#include <string.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,
> +};
> +
> +#define OMAP2_I2C_1_BASE 0x48070000
> +
> +#define N8X0_ADDR 0x48
> +
> +static const uint32_t i2c_base = OMAP2_I2C_1_BASE;
> +
> +static void omap_i2c_init(void)
> +{
> +    uint16_t data;
> +
> +    /* verify the mmio address by looking for a known signature */
> +    memread(i2c_base + OMAP_I2C_REV, &data, 2);
> +    g_assert_cmphex(data, ==, 0x34);
> +}
> +
> +static void omap_i2c_set_slave_addr(uint8_t addr)
> +{
> +    uint16_t data = addr;
> +
> +    memwrite(i2c_base + OMAP_I2C_SA, &data, 2);
> +    memread(i2c_base + OMAP_I2C_SA, &data, 2);
> +    g_assert_cmphex(data, ==, addr);
> +}
> +
> +static void omap_i2c_send(uint8_t addr, const void *buf, uint16_t len)
> +{
> +    uint16_t data;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + 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(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) != 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    while (len > 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 2);
> +        buf = (uint8_t *)buf + 2;
> +        len -= 2;
> +    }
> +    if (len == 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 1);
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void omap_i2c_recv(uint8_t addr, void *buf, uint16_t len)
> +{
> +    uint16_t data, stat;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + OMAP_I2C_CNT, &data, 2);
> +
> +    data = OMAP_I2C_CON_I2C_EN |
> +           OMAP_I2C_CON_MST |
> +           OMAP_I2C_CON_STT |
> +           OMAP_I2C_CON_STP;
> +    memwrite(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_CNT, &data, 2);
> +    g_assert_cmpuint(data, ==, len);
> +
> +    while (len > 0) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_RRDY) != 0);
> +        g_assert((data & OMAP_I2C_STAT_ROVR) == 0);
> +
> +        memread(i2c_base + OMAP_I2C_DATA, &data, 2);
> +
> +        memread(i2c_base + OMAP_I2C_STAT, &stat, 2);
> +        if ((stat & OMAP_I2C_STAT_SBD) != 0) {
> +            *(uint8_t *)buf = data & 0xf;
> +            buf = (uint8_t *)buf + 1;
> +            len--;
> +        } else {
> +            memcpy(buf, &data, 2);
> +            buf = (uint8_t *)buf + 2;
> +            len -= 2;
> +        }
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void send_and_receive(void)
> +{
> +    const uint8_t addr = N8X0_ADDR;
> +    uint8_t cmd[3];
> +    uint8_t resp[2];
> +
> +    omap_i2c_init();
> +
> +    cmd[0] = 0;
> +    omap_i2c_send(addr, cmd, 1);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmpuint(((uint16_t)resp[0] << 8) | resp[1], ==, 0);
> +
> +    cmd[0] = 1;
> +    cmd[1] = 0x0;
> +    omap_i2c_send(addr, cmd, 2);
> +    omap_i2c_recv(addr, resp, 1);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +
> +    cmd[0] = 2;
> +    cmd[1] = 0x12;
> +    cmd[2] = 0x34;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +    g_assert_cmphex(resp[1], ==, cmd[2]);
> +
> +    cmd[0] = 3;
> +    cmd[1] = 0x42;
> +    cmd[2] = 0x31;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(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");
> +
> +    qtest_add_func("/tmp105/tx-rx", send_and_receive);
> +
> +    ret = g_test_run();
> +
> +    if (s) {
> +        qtest_quit(s);
> +    }
> +
> +    return ret;
> +}
> --
> 1.7.10.4
>
Blue Swirl Dec. 12, 2012, 7:43 p.m. UTC | #2
On Wed, Dec 12, 2012 at 6:29 AM, Andreas Färber <andreas.faerber@web.de> wrote:
> Exercise all four commands of the TMP105, testing for an issue in the
> I2C TX path. The test case is specific to the n800's OMAP I2C for now
> and is the first test case for arm.
>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  tests/Makefile      |    2 +
>  tests/tmp105-test.c |  205 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 Dateien geändert, 207 Zeilen hinzugefügt(+)
>  create mode 100644 tests/tmp105-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index b60f0fb..69eff45 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..26d51f7
> --- /dev/null
> +++ b/tests/tmp105-test.c
> @@ -0,0 +1,205 @@
> +/*
> + * 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 <glib.h>
> +#include <string.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,
> +};
> +
> +#define OMAP2_I2C_1_BASE 0x48070000
> +
> +#define N8X0_ADDR 0x48
> +
> +static const uint32_t i2c_base = OMAP2_I2C_1_BASE;
> +
> +static void omap_i2c_init(void)
> +{
> +    uint16_t data;
> +
> +    /* verify the mmio address by looking for a known signature */
> +    memread(i2c_base + OMAP_I2C_REV, &data, 2);
> +    g_assert_cmphex(data, ==, 0x34);
> +}
> +
> +static void omap_i2c_set_slave_addr(uint8_t addr)
> +{
> +    uint16_t data = addr;
> +
> +    memwrite(i2c_base + OMAP_I2C_SA, &data, 2);
> +    memread(i2c_base + OMAP_I2C_SA, &data, 2);
> +    g_assert_cmphex(data, ==, addr);
> +}
> +
> +static void omap_i2c_send(uint8_t addr, const void *buf, uint16_t len)

With const uint8_t *buf you could avoid a few casts below.

> +{
> +    uint16_t data;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + 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(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) != 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    while (len > 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 2);
> +        buf = (uint8_t *)buf + 2;
> +        len -= 2;
> +    }
> +    if (len == 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 1);
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void omap_i2c_recv(uint8_t addr, void *buf, uint16_t len)
> +{
> +    uint16_t data, stat;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + OMAP_I2C_CNT, &data, 2);
> +
> +    data = OMAP_I2C_CON_I2C_EN |
> +           OMAP_I2C_CON_MST |
> +           OMAP_I2C_CON_STT |
> +           OMAP_I2C_CON_STP;
> +    memwrite(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_CNT, &data, 2);
> +    g_assert_cmpuint(data, ==, len);
> +
> +    while (len > 0) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_RRDY) != 0);
> +        g_assert((data & OMAP_I2C_STAT_ROVR) == 0);
> +
> +        memread(i2c_base + OMAP_I2C_DATA, &data, 2);
> +
> +        memread(i2c_base + OMAP_I2C_STAT, &stat, 2);
> +        if ((stat & OMAP_I2C_STAT_SBD) != 0) {
> +            *(uint8_t *)buf = data & 0xf;
> +            buf = (uint8_t *)buf + 1;
> +            len--;
> +        } else {
> +            memcpy(buf, &data, 2);
> +            buf = (uint8_t *)buf + 2;
> +            len -= 2;
> +        }
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void send_and_receive(void)
> +{
> +    const uint8_t addr = N8X0_ADDR;
> +    uint8_t cmd[3];
> +    uint8_t resp[2];
> +
> +    omap_i2c_init();
> +
> +    cmd[0] = 0;
> +    omap_i2c_send(addr, cmd, 1);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmpuint(((uint16_t)resp[0] << 8) | resp[1], ==, 0);
> +
> +    cmd[0] = 1;
> +    cmd[1] = 0x0;
> +    omap_i2c_send(addr, cmd, 2);
> +    omap_i2c_recv(addr, resp, 1);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +
> +    cmd[0] = 2;
> +    cmd[1] = 0x12;
> +    cmd[2] = 0x34;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +    g_assert_cmphex(resp[1], ==, cmd[2]);
> +
> +    cmd[0] = 3;
> +    cmd[1] = 0x42;
> +    cmd[2] = 0x31;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(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");
> +
> +    qtest_add_func("/tmp105/tx-rx", send_and_receive);

A register fuzzing test (like rtc-test.c) would be nice too.

> +
> +    ret = g_test_run();
> +
> +    if (s) {
> +        qtest_quit(s);
> +    }
> +
> +    return ret;
> +}
> --
> 1.7.10.4
>
Andreas Färber Dec. 12, 2012, 11:17 p.m. UTC | #3
Am 12.12.2012 20:43, schrieb Blue Swirl:
> On Wed, Dec 12, 2012 at 6:29 AM, Andreas Färber <andreas.faerber@web.de> wrote:
>> +static void omap_i2c_send(uint8_t addr, const void *buf, uint16_t len)
> 
> With const uint8_t *buf you could avoid a few casts below.

Good idea, thanks. I was working with other data types before, where
void* saved casts.

>> +    qtest_add_func("/tmp105/tx-rx", send_and_receive);
> 
> A register fuzzing test (like rtc-test.c) would be nice too.

I'm aware of your register fuzzing tests. However we do not directly
access registers of the tmp105, so those should go into an
omap_i2c-test.c file instead if someone wants to ever test that
implementation.

Alex H. has pointed to some more real-world test cases which I'd invite
him to add here as follow-ups.

My quest for tonight will be to generalize the I2C API for libqos by
comparing the tegra_i2c implementation. I hope my omap_i2c_* API is
pretty close already if we add one parameter for an adapter struct.

Andreas
Andreas Färber Dec. 15, 2012, 5:44 p.m. UTC | #4
Am 12.12.2012 15:44, schrieb Alex Horn:
> Thanks so much for taking the initiative on creating functional tests
> for the TMP105 hardware model. I am exciting to see these changes and
> I am wondering if your QTests could be complemented with the following
> standalone unit tests:
> 
>   https://github.com/ahorn/benchmarks/blob/965e571d92e677e8e64ad5faa0107f5dbd451981/qemu-hw/tmp105/tmp105-test.c

Honestly I consider that test a gross hack for three reasons:
* You ignore any global state that devices may rely on, i.e. some
operations like hotplug may succeed that would normally fail.
* You completely bypass QOM/qdev infrastructure by instantiating random
structs on the stack. This may lead to devices not properly being
initialized.
* You ignore any endianness swizzling our Memory API takes care of. (Did
you verify your tests on a Big Endian host?)

All this would be quite a lot of work to duplicate into a testing
environment and it occasionally changes, which is why we are reusing the
"original" infrastructure for testing in a special "qtest" mode.

What would be appreciated though is if you could add some of your tests
to our test infrastructure as a follow-up to my patches - assuming my
proposal gets adopted. For testing the alarm, Paolo's IRQs interception
API may be useful (in rtc-test IIRC).

> Their purpose is similar to yours except that unit tests focus on
> checking the internal state of a hardware model without requiring a
> bus implementation or a socket connection for the QTest client-server
> infrastructure. That is, unit tests do not seek to replace QTests but
> strengthen them (see also below).

I am the wrong person to argue about this, Anthony has been setting up
this test infrastructure. Unlike C++ with its friend classes we can't
test any static C functions anyway, so the interface for testing the way
you propose is very limited.

What I have been demonstrating is that it is too trivial to do it the
expected qtest way (one evening a prototype for bus+device plus one
evening a bus abstraction) to try working around it. :)

Regards,
Andreas
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index b60f0fb..69eff45 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..26d51f7
--- /dev/null
+++ b/tests/tmp105-test.c
@@ -0,0 +1,205 @@ 
+/*
+ * 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 <glib.h>
+#include <string.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,
+};
+
+#define OMAP2_I2C_1_BASE 0x48070000
+
+#define N8X0_ADDR 0x48
+
+static const uint32_t i2c_base = OMAP2_I2C_1_BASE;
+
+static void omap_i2c_init(void)
+{
+    uint16_t data;
+
+    /* verify the mmio address by looking for a known signature */
+    memread(i2c_base + OMAP_I2C_REV, &data, 2);
+    g_assert_cmphex(data, ==, 0x34);
+}
+
+static void omap_i2c_set_slave_addr(uint8_t addr)
+{
+    uint16_t data = addr;
+
+    memwrite(i2c_base + OMAP_I2C_SA, &data, 2);
+    memread(i2c_base + OMAP_I2C_SA, &data, 2);
+    g_assert_cmphex(data, ==, addr);
+}
+
+static void omap_i2c_send(uint8_t addr, const void *buf, uint16_t len)
+{
+    uint16_t data;
+
+    omap_i2c_set_slave_addr(addr);
+
+    data = len;
+    memwrite(i2c_base + 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(i2c_base + OMAP_I2C_CON, &data, 2);
+    memread(i2c_base + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) != 0);
+
+    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
+    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
+
+    while (len > 1) {
+        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
+
+        memwrite(i2c_base + OMAP_I2C_DATA, buf, 2);
+        buf = (uint8_t *)buf + 2;
+        len -= 2;
+    }
+    if (len == 1) {
+        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
+
+        memwrite(i2c_base + OMAP_I2C_DATA, buf, 1);
+    }
+
+    memread(i2c_base + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+}
+
+static void omap_i2c_recv(uint8_t addr, void *buf, uint16_t len)
+{
+    uint16_t data, stat;
+
+    omap_i2c_set_slave_addr(addr);
+
+    data = len;
+    memwrite(i2c_base + OMAP_I2C_CNT, &data, 2);
+
+    data = OMAP_I2C_CON_I2C_EN |
+           OMAP_I2C_CON_MST |
+           OMAP_I2C_CON_STT |
+           OMAP_I2C_CON_STP;
+    memwrite(i2c_base + OMAP_I2C_CON, &data, 2);
+    memread(i2c_base + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+
+    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
+    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
+
+    memread(i2c_base + OMAP_I2C_CNT, &data, 2);
+    g_assert_cmpuint(data, ==, len);
+
+    while (len > 0) {
+        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
+        g_assert((data & OMAP_I2C_STAT_RRDY) != 0);
+        g_assert((data & OMAP_I2C_STAT_ROVR) == 0);
+
+        memread(i2c_base + OMAP_I2C_DATA, &data, 2);
+
+        memread(i2c_base + OMAP_I2C_STAT, &stat, 2);
+        if ((stat & OMAP_I2C_STAT_SBD) != 0) {
+            *(uint8_t *)buf = data & 0xf;
+            buf = (uint8_t *)buf + 1;
+            len--;
+        } else {
+            memcpy(buf, &data, 2);
+            buf = (uint8_t *)buf + 2;
+            len -= 2;
+        }
+    }
+
+    memread(i2c_base + OMAP_I2C_CON, &data, 2);
+    g_assert((data & OMAP_I2C_CON_STP) == 0);
+}
+
+static void send_and_receive(void)
+{
+    const uint8_t addr = N8X0_ADDR;
+    uint8_t cmd[3];
+    uint8_t resp[2];
+
+    omap_i2c_init();
+
+    cmd[0] = 0;
+    omap_i2c_send(addr, cmd, 1);
+    omap_i2c_recv(addr, resp, 2);
+    g_assert_cmpuint(((uint16_t)resp[0] << 8) | resp[1], ==, 0);
+
+    cmd[0] = 1;
+    cmd[1] = 0x0;
+    omap_i2c_send(addr, cmd, 2);
+    omap_i2c_recv(addr, resp, 1);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+
+    cmd[0] = 2;
+    cmd[1] = 0x12;
+    cmd[2] = 0x34;
+    omap_i2c_send(addr, cmd, 3);
+    omap_i2c_recv(addr, resp, 2);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+    g_assert_cmphex(resp[1], ==, cmd[2]);
+
+    cmd[0] = 3;
+    cmd[1] = 0x42;
+    cmd[2] = 0x31;
+    omap_i2c_send(addr, cmd, 3);
+    omap_i2c_recv(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");
+
+    qtest_add_func("/tmp105/tx-rx", send_and_receive);
+
+    ret = g_test_run();
+
+    if (s) {
+        qtest_quit(s);
+    }
+
+    return ret;
+}