Re: [Qemu-devel] [PATCH v2 4/4] Add qtest support for i.MX I2C device emulation.

["Andreas Färber" <afaerber@suse.de>]

Am 04.05.2013 16:09, schrieb Jean-Christophe DUBOIS:
> This is using a ds1338 RTC chip on the i2c bus. This RTC
> chip is nop present on the real board
> 
> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
> ---
>  tests/Makefile         |   3 +
>  tests/ds1338-test.c    |  64 ++++++++++++++
>  tests/libqos/i2c-imx.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/i2c.h     |   3 +
>  4 files changed, 294 insertions(+)
>  create mode 100644 tests/ds1338-test.c
>  create mode 100644 tests/libqos/i2c-imx.c
[...]

The qtest itself looks fine, thanks.

> diff --git a/tests/libqos/i2c-imx.c b/tests/libqos/i2c-imx.c
> new file mode 100644
> index 0000000..da7316f
> --- /dev/null
> +++ b/tests/libqos/i2c-imx.c
> @@ -0,0 +1,224 @@
> +/*
> + * QTest i.MX I2C driver
> + *
> + * Copyright (c) 2013 Jean-Christophe Dubois
> + *
> + * 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 <string.h>
> +
> +#include "qemu/osdep.h"

> +#include "qemu/bswap.h"

Is this one needed?

> +#include "libqtest.h"
> +

> +enum IMXI2CRegisters {
> +    IMX_I2C_IADR = 0x00,
> +    IMX_I2C_IFDR = 0x04,
> +    IMX_I2C_I2CR = 0x08,
> +    IMX_I2C_I2SR = 0x0c,
> +    IMX_I2C_I2DR = 0x10,
> +};
> +
> +enum IMXI2CCRBits {
> +    IMX_I2C_I2CR_IEN  = 1 << 7,
> +    IMX_I2C_I2CR_IIEN = 1 << 6,
> +    IMX_I2C_I2CR_MSTA = 1 << 5,
> +    IMX_I2C_I2CR_MTX  = 1 << 4,
> +    IMX_I2C_I2CR_TXAK = 1 << 3,
> +    IMX_I2C_I2CR_RSTA = 1 << 2,
> +};
> +
> +enum IMXI2CSRBits {
> +    IMX_I2C_I2SR_ICF  = 1 << 7,
> +    IMX_I2C_I2SR_IAAF = 1 << 6,
> +    IMX_I2C_I2SR_IBB  = 1 << 5,
> +    IMX_I2C_I2SR_IAL  = 1 << 4,
> +    IMX_I2C_I2SR_SRW  = 1 << 2,
> +    IMX_I2C_I2SR_IIF  = 1 << 1,
> +    IMX_I2C_I2SR_RXAK = 1 << 0,
> +};
> +
> +enum IMXI2CDirection {
> +    IMX_I2C_READ,
> +    IMX_I2C_WRITE,
> +};

libqos/i2c-omap.c was a driver for an unmaintained legacy device.
i.MX I2C however is being added by you in 2/4, so it would be better to
put these constants in a header in 2/4 for reuse here (i2c/imx_regs.h?).

Otherwise looking fine!

Regards,
Andreas

> +
> +typedef struct IMXI2C {
> +    I2CAdapter parent;
> +
> +    uint64_t addr;
> +} IMXI2C;
> +
> +
> +static void imx_i2c_set_slave_addr(IMXI2C *s, uint8_t addr,
> +                                   enum IMXI2CDirection direction)
> +{
> +    writeb(s->addr + IMX_I2C_I2DR, (addr << 1) |
> +           (direction == IMX_I2C_READ ? 1 : 0));
> +}
> +
> +static void imx_i2c_send(I2CAdapter *i2c, uint8_t addr,
> +                         const uint8_t *buf, uint16_t len)
> +{
> +    IMXI2C *s = (IMXI2C *)i2c;
> +    uint8_t data;
> +    uint8_t status;
> +    uint16_t size = 0;
> +
> +    if (!len) {
> +        return;
> +    }
> +
> +    /* set the bus for write */
> +    data = IMX_I2C_I2CR_IEN |
> +           IMX_I2C_I2CR_IIEN |
> +           IMX_I2C_I2CR_MSTA |
> +           IMX_I2C_I2CR_MTX |
> +           IMX_I2C_I2CR_TXAK;
> +
> +    writeb(s->addr + IMX_I2C_I2CR, data);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IBB) != 0);
> +
> +    /* set the slave address */
> +    imx_i2c_set_slave_addr(s, addr, IMX_I2C_WRITE);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) != 0);
> +    g_assert((status & IMX_I2C_I2SR_RXAK) == 0);
> +
> +    /* ack the interrupt */
> +    writeb(s->addr + IMX_I2C_I2SR, 0);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) == 0);
> +
> +    while (size < len) {
> +        /* check we are still busy */
> +        status = readb(s->addr + IMX_I2C_I2SR);
> +        g_assert((status & IMX_I2C_I2SR_IBB) != 0);
> +
> +        /* write the data */
> +        writeb(s->addr + IMX_I2C_I2DR, buf[size]);
> +        status = readb(s->addr + IMX_I2C_I2SR);
> +        g_assert((status & IMX_I2C_I2SR_IIF) != 0);
> +        g_assert((status & IMX_I2C_I2SR_RXAK) == 0);
> +
> +        /* ack the interrupt */
> +        writeb(s->addr + IMX_I2C_I2SR, 0);
> +        status = readb(s->addr + IMX_I2C_I2SR);
> +        g_assert((status & IMX_I2C_I2SR_IIF) == 0);
> +
> +        size++;
> +    }
> +
> +    /* release the bus */
> +    data &= ~(IMX_I2C_I2CR_MSTA | IMX_I2C_I2CR_MTX);
> +    writeb(s->addr + IMX_I2C_I2CR, data);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IBB) == 0);
> +}
> +
> +static void imx_i2c_recv(I2CAdapter *i2c, uint8_t addr,
> +                         uint8_t *buf, uint16_t len)
> +{
> +    IMXI2C *s = (IMXI2C *)i2c;
> +    uint8_t data;
> +    uint8_t status;
> +    uint16_t size = 0;
> +
> +    if (!len) {
> +        return;
> +    }
> +
> +    /* set the bus for write */
> +    data = IMX_I2C_I2CR_IEN |
> +           IMX_I2C_I2CR_IIEN |
> +           IMX_I2C_I2CR_MSTA |
> +           IMX_I2C_I2CR_MTX |
> +           IMX_I2C_I2CR_TXAK;
> +
> +    writeb(s->addr + IMX_I2C_I2CR, data);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IBB) != 0);
> +
> +    /* set the slave address */
> +    imx_i2c_set_slave_addr(s, addr, IMX_I2C_READ);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) != 0);
> +    g_assert((status & IMX_I2C_I2SR_RXAK) == 0);
> +
> +    /* ack the interrupt */
> +    writeb(s->addr + IMX_I2C_I2SR, 0);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) == 0);
> +
> +    /* set the bus for read */
> +    data &= ~IMX_I2C_I2CR_MTX;
> +    /* if only one byte don't ack */
> +    if (len != 1) {
> +        data &= ~IMX_I2C_I2CR_TXAK;
> +    }
> +    writeb(s->addr + IMX_I2C_I2CR, data);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IBB) != 0);
> +
> +    /* dummy read */
> +    readb(s->addr + IMX_I2C_I2DR);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) != 0);
> +
> +    /* ack the interrupt */
> +    writeb(s->addr + IMX_I2C_I2SR, 0);
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IIF) == 0);
> +
> +    while (size < len) {
> +        /* check we are still busy */
> +        status = readb(s->addr + IMX_I2C_I2SR);
> +        g_assert((status & IMX_I2C_I2SR_IBB) != 0);
> +
> +        if (size == (len - 1)) {
> +            /* stop the read transaction */
> +            data &= ~(IMX_I2C_I2CR_MSTA | IMX_I2C_I2CR_MTX);
> +        } else {
> +            /* ack the data read */
> +            data |= IMX_I2C_I2CR_TXAK;
> +        }
> +        writeb(s->addr + IMX_I2C_I2CR, data);
> +
> +        /* read the data */
> +        buf[size] = readb(s->addr + IMX_I2C_I2DR);
> +
> +        if (size != (len - 1)) {
> +            status = readb(s->addr + IMX_I2C_I2SR);
> +            g_assert((status & IMX_I2C_I2SR_IIF) != 0);
> +
> +            /* ack the interrupt */
> +            writeb(s->addr + IMX_I2C_I2SR, 0);
> +        }
> +
> +        status = readb(s->addr + IMX_I2C_I2SR);
> +        g_assert((status & IMX_I2C_I2SR_IIF) == 0);
> +
> +        size++;
> +    }
> +
> +    status = readb(s->addr + IMX_I2C_I2SR);
> +    g_assert((status & IMX_I2C_I2SR_IBB) == 0);
> +}
> +
> +I2CAdapter *imx_i2c_create(uint64_t addr)
> +{
> +    IMXI2C *s = g_malloc0(sizeof(*s));
> +    I2CAdapter *i2c = (I2CAdapter *)s;
> +
> +    s->addr = addr;
> +
> +    i2c->send = imx_i2c_send;
> +    i2c->recv = imx_i2c_recv;
> +
> +    return i2c;
> +}
> diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h
> index 1ce9af4..c21f1dc 100644
> --- a/tests/libqos/i2c.h
> +++ b/tests/libqos/i2c.h
> @@ -27,4 +27,7 @@ void i2c_recv(I2CAdapter *i2c, uint8_t addr,
>  /* libi2c-omap.c */
>  I2CAdapter *omap_i2c_create(uint64_t addr);
>  
> +/* libi2c-imx.c */
> +I2CAdapter *imx_i2c_create(uint64_t addr);
> +
>  #endif
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg