All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH qemu 0/3] add i2c support
@ 2016-05-17 19:24 Cédric Le Goater
  2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-17 19:24 UTC (permalink / raw)
  To: openbmc

This serie adds an ASPEED I2C crontroller device model to qemu and a
couple of I2C slaves. The device are a little different from the real
hardware so you will have to recompile the palmetto dtb with the right
models.

Patches are also available here :

	https://github.com/legoater/qemu/commits/aspeed

Based on 'aspeed-integration' of https://github.com/amboar/qemu

Thanks,

Cédric Le Goater (3):
  i2c: add aspeed i2c crontroller
  ast2400: add a rtc device on I2C bus 0
  ast2400: add a temp sensor device on I2C bus 3

 hw/arm/ast2400.c            |  21 +++
 hw/i2c/Makefile.objs        |   1 +
 hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/ast2400.h    |   2 +
 include/hw/i2c/aspeed_i2c.h |  62 ++++++
 5 files changed, 534 insertions(+)
 create mode 100644 hw/i2c/aspeed_i2c.c
 create mode 100644 include/hw/i2c/aspeed_i2c.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-17 19:24 [PATCH qemu 0/3] add i2c support Cédric Le Goater
@ 2016-05-17 19:25 ` Cédric Le Goater
  2016-05-18 13:26   ` Joel Stanley
  2016-05-19  1:41   ` Andrew Jeffery
  2016-05-17 19:25 ` [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0 Cédric Le Goater
  2016-05-17 19:25 ` [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3 Cédric Le Goater
  2 siblings, 2 replies; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-17 19:25 UTC (permalink / raw)
  To: openbmc

The Aspeed AST2400 integrates a set of 14 I2C/SMBus bus controllers
directly connected to APB bus which can be programmed as a master or
slave.

This patch offers a device model for the master mode only, slave mode
is not supported.

On the TODO list, we also have :

 - improve and harden the state machine. This is really a first
   version.
 - bus recovery support (used by the Linux driver).
 - transfer mode state machine bits. this is not strictly necessary as
   it is mostly used for debug. The bus busy bit is deducted from the
   I2C core engine of qemu.
 - support of the pool buffer: 2048 bytes of internal SRAM (not used
   by the Linux driver).

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/ast2400.c            |  16 ++
 hw/i2c/Makefile.objs        |   1 +
 hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/ast2400.h    |   2 +
 include/hw/i2c/aspeed_i2c.h |  62 ++++++
 5 files changed, 529 insertions(+)
 create mode 100644 hw/i2c/aspeed_i2c.c
 create mode 100644 include/hw/i2c/aspeed_i2c.h

diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index cf9f37e6a301..4576e67cd483 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -18,6 +18,7 @@
 #include "hw/arm/ast2400.h"
 #include "hw/char/serial.h"
 #include "hw/boards.h"
+#include "hw/i2c/aspeed_i2c.h"
 
 #define AST2400_UART_5_BASE      0x00184000
 #define AST2400_IOMEM_SIZE       0x00200000
@@ -25,6 +26,7 @@
 #define AST2400_VIC_BASE         0x1E6C0000
 #define AST2400_SCU_BASE         0x1E6E2000
 #define AST2400_TIMER_BASE       0x1E782000
+#define AST2400_I2C_BASE         0x1E78A000
 
 static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
 static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
@@ -71,6 +73,10 @@ static void ast2400_init(Object *obj)
     object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
     object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
     qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
+
+    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
+    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
+    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
 }
 
 static void ast2400_realize(DeviceState *dev, Error **errp)
@@ -143,6 +149,16 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
         serial_mm_init(&s->iomem, AST2400_UART_5_BASE, 2,
                        uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
     }
+
+    /* I2C */
+    object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
+                       qdev_get_gpio_in(DEVICE(&s->vic), 12));
 }
 
 static void ast2400_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
index aeb8f38d70f1..1fd54edf4c23 100644
--- a/hw/i2c/Makefile.objs
+++ b/hw/i2c/Makefile.objs
@@ -5,4 +5,5 @@ common-obj-$(CONFIG_APM) += pm_smbus.o
 common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
 common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
 common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
+common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o
 obj-$(CONFIG_OMAP) += omap_i2c.o
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
new file mode 100644
index 000000000000..f512e64d827a
--- /dev/null
+++ b/hw/i2c/aspeed_i2c.c
@@ -0,0 +1,448 @@
+/*
+ * ARM Aspeed I2C controller
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/i2c/aspeed_i2c.h"
+
+/* I2C Global Register */
+
+#define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
+#define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
+                                               Assignment */
+
+/* I2C Device (Bus) Register */
+
+#define I2CD_FUN_CTRL_REG       0x00       /* I2CD Function Control  */
+#define   I2CD_BUFF_SEL_MASK               (0x7 << 20)
+#define   I2CD_BUFF_SEL(x)                 (x << 20)
+#define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
+#define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
+#define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
+#define   I2CD_MSB_STS                     (0x1 << 9)
+#define   I2CD_SDA_DRIVE_1T_EN             (0x1 << 8)
+#define   I2CD_M_SDA_DRIVE_1T_EN           (0x1 << 7)
+#define   I2CD_M_HIGH_SPEED_EN             (0x1 << 6)
+#define   I2CD_DEF_ADDR_EN                 (0x1 << 5)
+#define   I2CD_DEF_ALERT_EN                (0x1 << 4)
+#define   I2CD_DEF_ARP_EN                  (0x1 << 3)
+#define   I2CD_DEF_GCALL_EN                (0x1 << 2)
+#define   I2CD_SLAVE_EN                    (0x1 << 1)
+#define   I2CD_MASTER_EN                   (0x1)
+
+#define I2CD_AC_TIMING_REG1     0x04       /* Clock and AC Timing Control #1 */
+#define I2CD_AC_TIMING_REG2     0x08       /* Clock and AC Timing Control #1 */
+#define I2CD_INTR_CTRL_REG      0x0c       /* I2CD Interrupt Control */
+#define I2CD_INTR_STS_REG       0x10       /* I2CD Interrupt Status */
+#define   I2CD_INTR_SDA_DL_TIMEOUT         (0x1 << 14)
+#define   I2CD_INTR_BUS_RECOVER_DONE       (0x1 << 13)
+#define   I2CD_INTR_SMBUS_ALERT            (0x1 << 12) /* Bus [0-3] only */
+#define   I2CD_INTR_SMBUS_ARP_ADDR         (0x1 << 11) /* Removed */
+#define   I2CD_INTR_SMBUS_DEV_ALERT_ADDR   (0x1 << 10) /* Removed */
+#define   I2CD_INTR_SMBUS_DEF_ADDR         (0x1 << 9)  /* Removed */
+#define   I2CD_INTR_GCALL_ADDR             (0x1 << 8)  /* Removed */
+#define   I2CD_INTR_SLAVE_MATCH            (0x1 << 7)  /* use RX_DONE */
+#define   I2CD_INTR_SCL_TIMEOUT            (0x1 << 6)
+#define   I2CD_INTR_ABNORMAL               (0x1 << 5)
+#define   I2CD_INTR_NORMAL_STOP            (0x1 << 4)
+#define   I2CD_INTR_ARBIT_LOSS             (0x1 << 3)
+#define   I2CD_INTR_RX_DONE                (0x1 << 2)
+#define   I2CD_INTR_TX_NAK                 (0x1 << 1)
+#define   I2CD_INTR_TX_ACK                 (0x1 << 0)
+
+#define I2CD_CMD_REG            0x14       /* I2CD Command/Status */
+#define   I2CD_SDA_OE                      (0x1 << 28)
+#define   I2CD_SDA_O                       (0x1 << 27)
+#define   I2CD_SCL_OE                      (0x1 << 26)
+#define   I2CD_SCL_O                       (0x1 << 25)
+#define   I2CD_TX_TIMING                   (0x1 << 24)
+#define   I2CD_TX_STATUS                   (0x1 << 23)
+
+#define   I2CD_TX_STATE_SHIFT              19 /* Tx State Machine */
+#define   I2CD_TX_STATE_MASK                  0xf
+#define     I2CD_IDLE                         0x0
+#define     I2CD_MACTIVE                      0x8
+#define     I2CD_MSTART                       0x9
+#define     I2CD_MSTARTR                      0xa
+#define     I2CD_MSTOP                        0xb
+#define     I2CD_MTXD                         0xc
+#define     I2CD_MRXACK                       0xd
+#define     I2CD_MRXD                         0xe
+#define     I2CD_MTXACK                       0xf
+#define     I2CD_SWAIT                        0x1
+#define     I2CD_SRXD                         0x4
+#define     I2CD_STXACK                       0x5
+#define     I2CD_STXD                         0x6
+#define     I2CD_SRXACK                       0x7
+#define     I2CD_RECOVER                      0x3
+
+#define   I2CD_SCL_LINE_STS                (0x1 << 18)
+#define   I2CD_SDA_LINE_STS                (0x1 << 17)
+#define   I2CD_BUS_BUSY_STS                (0x1 << 16)
+#define   I2CD_SDA_OE_OUT_DIR              (0x1 << 15)
+#define   I2CD_SDA_O_OUT_DIR               (0x1 << 14)
+#define   I2CD_SCL_OE_OUT_DIR              (0x1 << 13)
+#define   I2CD_SCL_O_OUT_DIR               (0x1 << 12)
+#define   I2CD_BUS_RECOVER_CMD_EN          (0x1 << 11)
+#define   I2CD_S_ALT_EN                    (0x1 << 10)
+#define   I2CD_RX_DMA_ENABLE               (0x1 << 9)
+#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
+
+/* Command Bit */
+#define   I2CD_M_STOP_CMD                  (0x1 << 5)
+#define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
+#define   I2CD_M_RX_CMD                    (0x1 << 3)
+#define   I2CD_S_TX_CMD                    (0x1 << 2)
+#define   I2CD_M_TX_CMD                    (0x1 << 1)
+#define   I2CD_M_START_CMD                 (0x1)
+
+#define I2CD_DEV_ADDR_REG       0x18       /* Slave Device Address */
+#define I2CD_BUF_CTRL_REG       0x1c       /* Pool Buffer Control */
+#define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
+#define   I2CD_BYTE_BUF_TX_SHIFT           0
+#define   I2CD_BYTE_BUF_TX_MASK            0xff
+#define   I2CD_BYTE_BUF_RX_SHIFT           8
+#define   I2CD_BYTE_BUF_RX_MASK            0xff
+
+
+static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
+{
+    return bus->ctrl & I2CD_MASTER_EN;
+}
+
+static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus *bus)
+{
+    return bus->ctrl & (I2CD_MASTER_EN | I2CD_SLAVE_EN);
+}
+
+static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
+{
+    bus->intr_status &= bus->intr_ctrl;
+    if (bus->intr_status) {
+        bus->controller->intr_status |= 1 << bus->id;
+        qemu_irq_raise(bus->controller->irq);
+    }
+}
+
+static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
+                                    unsigned size)
+{
+    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
+
+    switch (offset) {
+    case I2CD_FUN_CTRL_REG:
+        return bus->ctrl;
+    case I2CD_AC_TIMING_REG1:
+        return bus->timing[0];
+    case I2CD_AC_TIMING_REG2:
+        return bus->timing[1];
+    case I2CD_INTR_CTRL_REG:
+        return bus->intr_ctrl;
+    case I2CD_INTR_STS_REG:
+        return bus->intr_status;
+    case I2CD_BYTE_BUF_REG:
+        return bus->buf;
+    case I2CD_CMD_REG:
+        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+        return -1;
+    }
+}
+
+static inline uint64_t aspeed_i2c_bus_get_state(AspeedI2CBus *bus)
+{
+    return bus->cmd >> 19 & 0xF;
+}
+
+static inline void aspeed_i2c_bus_set_state(AspeedI2CBus *bus, uint64_t value)
+{
+    bus->cmd |= (value & 0xF) << 19;
+}
+
+static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
+{
+    bus->cmd |= value & 0xFFFF;
+    bus->intr_status = 0;
+
+    if (bus->cmd & I2CD_M_START_CMD) {
+        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
+                               extract32(bus->buf, 0, 1))) {
+            bus->intr_status |= I2CD_INTR_TX_NAK;
+        } else {
+            bus->intr_status |= I2CD_INTR_TX_ACK;
+        }
+
+    } else if (bus->cmd & I2CD_M_TX_CMD) {
+        if (i2c_send(bus->bus, bus->buf)) {
+            bus->intr_status |= (I2CD_INTR_TX_NAK | I2CD_INTR_ABNORMAL);
+            i2c_end_transfer(bus->bus);
+        } else {
+            bus->intr_status |= I2CD_INTR_TX_ACK;
+        }
+
+    } else if (bus->cmd & I2CD_M_RX_CMD) {
+        int ret = i2c_recv(bus->bus);
+        if (ret < 0) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__);
+            ret = 0xff;
+        } else {
+            bus->intr_status |= I2CD_INTR_RX_DONE;
+        }
+        bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
+    }
+
+    if (bus->cmd & (I2CD_M_STOP_CMD | I2CD_M_S_RX_CMD_LAST)) {
+        if (!i2c_bus_busy(bus->bus)) {
+            bus->intr_status |= I2CD_INTR_ABNORMAL;
+        } else {
+            i2c_end_transfer(bus->bus);
+            bus->intr_status |= I2CD_INTR_NORMAL_STOP;
+        }
+    }
+
+    /* command is handled, reset it and check for interrupts  */
+    bus->cmd &= ~0xFFFF;
+    aspeed_i2c_bus_raise_interrupt(bus);
+}
+
+static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
+                                 uint64_t value, unsigned size)
+{
+    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
+
+    switch (offset) {
+    case I2CD_FUN_CTRL_REG:
+        if (value & I2CD_SLAVE_EN) {
+            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
+                          __func__);
+            break;
+        }
+        bus->ctrl = value & 0x0071C3FF;
+        break;
+    case I2CD_AC_TIMING_REG1:
+        bus->timing[0] = value & 0xFFFFF0F;
+        break;
+    case I2CD_AC_TIMING_REG2:
+        bus->timing[1] = value & 0x7;
+        break;
+    case I2CD_INTR_CTRL_REG:
+        bus->intr_ctrl = value & 0x7FFF;
+        break;
+    case I2CD_INTR_STS_REG:
+        bus->intr_status &= ~(value & 0x7FFF);
+        bus->controller->intr_status &= ~(1 << bus->id);
+        qemu_irq_lower(bus->controller->irq);
+        break;
+    case I2CD_DEV_ADDR_REG:
+        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
+                      __func__);
+        break;
+    case I2CD_BYTE_BUF_REG:
+        bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
+        break;
+    case I2CD_CMD_REG:
+        if (!aspeed_i2c_bus_is_enabled(bus)) {
+            break;
+        }
+
+        if (!aspeed_i2c_bus_is_master(bus)) {
+            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
+                          __func__);
+            break;
+        }
+
+        aspeed_i2c_bus_handle_cmd(bus, value);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+    }
+}
+
+static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
+                                   unsigned size)
+{
+    AspeedI2CState *s = (AspeedI2CState *)opaque;
+
+    switch (offset) {
+    case I2C_CTRL_STATUS:
+        return s->intr_status;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        break;
+    }
+
+    return -1;
+}
+
+static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
+                                  uint64_t value, unsigned size)
+{
+    switch (offset) {
+    case I2C_CTRL_STATUS:
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        break;
+    }
+}
+
+static const MemoryRegionOps aspeed_i2c_bus_ops = {
+    .read = aspeed_i2c_bus_read,
+    .write = aspeed_i2c_bus_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
+    .read = aspeed_i2c_ctrl_read,
+    .write = aspeed_i2c_ctrl_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const VMStateDescription aspeed_i2c_bus_vmstate = {
+    .name = TYPE_ASPEED_I2C,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(id, AspeedI2CBus),
+        VMSTATE_UINT32(ctrl, AspeedI2CBus),
+        VMSTATE_UINT32_ARRAY(timing, AspeedI2CBus, 2),
+        VMSTATE_UINT32(intr_ctrl, AspeedI2CBus),
+        VMSTATE_UINT32(intr_status, AspeedI2CBus),
+        VMSTATE_UINT32(cmd, AspeedI2CBus),
+        VMSTATE_UINT32(buf, AspeedI2CBus),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription aspeed_i2c_vmstate = {
+    .name = TYPE_ASPEED_I2C,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(intr_status, AspeedI2CState),
+        VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
+                             ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
+                             AspeedI2CBus),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void aspeed_i2c_reset(DeviceState *dev)
+{
+    int i;
+    AspeedI2CState *s = ASPEED_I2C(dev);
+
+    s->intr_status = 0;
+
+    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
+        s->busses[i].intr_status = 0;
+        s->busses[i].cmd = 0;
+        s->busses[i].buf = 0;
+        i2c_end_transfer(s->busses[i].bus);
+    }
+}
+
+/*
+ * Address Definitions
+ *
+ *   0x000 ... 0x03F: Global Register
+ *   0x040 ... 0x07F: Device 1
+ *   0x080 ... 0x0BF: Device 2
+ *   0x0C0 ... 0x0FF: Device 3
+ *   0x100 ... 0x13F: Device 4
+ *   0x140 ... 0x17F: Device 5
+ *   0x180 ... 0x1BF: Device 6
+ *   0x1C0 ... 0x1FF: Device 7
+ *   0x200 ... 0x2FF: Buffer Pool  (unused in linux driver)
+ *   0x300 ... 0x33F: Device 8
+ *   0x340 ... 0x37F: Device 9
+ *   0x380 ... 0x3BF: Device 10
+ *   0x3C0 ... 0x3FF: Device 11
+ *   0x400 ... 0x43F: Device 12
+ *   0x440 ... 0x47F: Device 13
+ *   0x480 ... 0x4BF: Device 14
+ *   0x800 ... 0xFFF: Buffer Pool  (unused in linux driver)
+ */
+static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
+{
+    int i;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedI2CState *s = ASPEED_I2C(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i2c_ctrl_ops, s,
+                          "aspeed.i2c", 0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+
+    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
+        char name[16];
+        int offset = i < 7 ? 1 : 5;
+        snprintf(name, sizeof(name), "aspeed.i2c.%d", i);
+        s->busses[i].controller = s;
+        s->busses[i].id = i;
+        s->busses[i].bus = i2c_init_bus(dev, name);
+        memory_region_init_io(&s->busses[i].mr, OBJECT(dev),
+                              &aspeed_i2c_bus_ops, &s->busses[i], name, 0x40);
+        memory_region_add_subregion(&s->iomem, 0x40 * (i + offset),
+                                    &s->busses[i].mr);
+    }
+}
+
+static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &aspeed_i2c_vmstate;
+    dc->reset = aspeed_i2c_reset;
+    dc->realize = aspeed_i2c_realize;
+    dc->desc = "Aspeed I2C Controller";
+}
+
+static const TypeInfo aspeed_i2c_info = {
+    .name          = TYPE_ASPEED_I2C,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(AspeedI2CState),
+    .class_init    = aspeed_i2c_class_init,
+};
+
+static void aspeed_i2c_register_types(void)
+{
+    type_register_static(&aspeed_i2c_info);
+}
+
+type_init(aspeed_i2c_register_types)
+
+
+I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr)
+{
+    AspeedI2CState *s = ASPEED_I2C(dev);
+    I2CBus *bus = NULL;
+
+    if (busnr >= 0 && busnr < ASPEED_I2C_NR_BUSSES) {
+        bus = s->busses[busnr].bus;
+    }
+
+    return bus;
+}
diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
index cf7102145355..e96e3db3fbea 100644
--- a/include/hw/arm/ast2400.h
+++ b/include/hw/arm/ast2400.h
@@ -16,6 +16,7 @@
 #include "hw/intc/aspeed_vic.h"
 #include "hw/misc/aspeed_scu.h"
 #include "hw/timer/aspeed_timer.h"
+#include "hw/i2c/aspeed_i2c.h"
 
 typedef struct AST2400State {
     /*< private >*/
@@ -28,6 +29,7 @@ typedef struct AST2400State {
     AspeedVICState vic;
     AspeedTimerCtrlState timerctrl;
     AspeedSCUState scu;
+    AspeedI2CState i2c;
 } AST2400State;
 
 #define TYPE_AST2400 "ast2400"
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
new file mode 100644
index 000000000000..f9020acdef30
--- /dev/null
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -0,0 +1,62 @@
+/*
+ *  ASPEED AST2400 I2C Controller
+ *
+ *  Copyright (C) 2016 IBM Corp.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef ASPEED_I2C_H
+#define ASPEED_I2C_H
+
+#include "hw/i2c/i2c.h"
+
+#define TYPE_ASPEED_I2C "aspeed.i2c"
+#define ASPEED_I2C(obj) \
+    OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
+
+#define ASPEED_I2C_NR_BUSSES 14
+
+struct AspeedI2CState;
+
+typedef struct AspeedI2CBus {
+    struct AspeedI2CState *controller;
+
+    MemoryRegion mr;
+
+    I2CBus *bus;
+    uint8_t id;
+
+    uint32_t ctrl;
+    uint32_t timing[2];
+    uint32_t intr_ctrl;
+    uint32_t intr_status;
+    uint32_t cmd;
+    uint32_t buf;
+} AspeedI2CBus;
+
+typedef struct AspeedI2CState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    uint32_t intr_status;
+
+    AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
+} AspeedI2CState;
+
+I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
+
+#endif /* ASPEED_I2C_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0
  2016-05-17 19:24 [PATCH qemu 0/3] add i2c support Cédric Le Goater
  2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
@ 2016-05-17 19:25 ` Cédric Le Goater
  2016-05-18 13:33   ` Joel Stanley
  2016-05-17 19:25 ` [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3 Cédric Le Goater
  2 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-17 19:25 UTC (permalink / raw)
  To: openbmc

The palmetto platform has a ds3231 RTC device but not qemu. Let's use
a ds1338 instead.

The aspeed-bmc-opp-palmetto dts file needs to be changed consequently
with :

	rtc@68 {
		compatible = "dallas,ds1338";
		reg = <0x68>;
	};

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/ast2400.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index 4576e67cd483..41bfc74493fa 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -159,6 +159,8 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
                        qdev_get_gpio_in(DEVICE(&s->vic), 12));
+
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 0), "ds1338", 0x68);
 }
 
 static void ast2400_class_init(ObjectClass *oc, void *data)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3
  2016-05-17 19:24 [PATCH qemu 0/3] add i2c support Cédric Le Goater
  2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
  2016-05-17 19:25 ` [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0 Cédric Le Goater
@ 2016-05-17 19:25 ` Cédric Le Goater
  2016-05-18 13:29   ` Joel Stanley
  2 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-17 19:25 UTC (permalink / raw)
  To: openbmc

The palmetto platform has a tmp421 device. Qemu has not (yet). Let's
use a tmp105 instead.

The aspeed-bmc-opp-palmetto dts file should to be changed consequently
with :

	i2c2: i2c-bus@c0 {
		tmp423@4c {
			compatible = "ti,tmp105";
			reg = <0x4c>;
		};
	};

Temperature can be changed from the monitor with :

	(qemu) qom-set /machine/unattached/device[2] temperature 12000

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/ast2400.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index 41bfc74493fa..d231d98a88e1 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -161,6 +161,9 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
                        qdev_get_gpio_in(DEVICE(&s->vic), 12));
 
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 0), "ds1338", 0x68);
+    dev = i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 2),
+                           "tmp105", 0x4c);
+    object_property_set_int(OBJECT(dev), 25000, "temperature", &err);
 }
 
 static void ast2400_class_init(ObjectClass *oc, void *data)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
@ 2016-05-18 13:26   ` Joel Stanley
  2016-05-19 16:56     ` Cédric Le Goater
  2016-05-19  1:41   ` Andrew Jeffery
  1 sibling, 1 reply; 14+ messages in thread
From: Joel Stanley @ 2016-05-18 13:26 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

Hey Cedric,

On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
> The Aspeed AST2400 integrates a set of 14 I2C/SMBus bus controllers
> directly connected to APB bus which can be programmed as a master or
> slave.

Nice work!

>
> This patch offers a device model for the master mode only, slave mode
> is not supported.

Cool. That's good enough for us to emulate a barreleye or palemtto.

We should look to extend it to support this as well as multi-byte
transfers using the pool buffer soon; both the model and the driver.

I had a read of the code and it looks good to me for a first pass.
I'll let Andrew comment further and merge it into our staging tree.

> On the TODO list, we also have :
>
>  - improve and harden the state machine. This is really a first
>    version.
>  - bus recovery support (used by the Linux driver).
>  - transfer mode state machine bits. this is not strictly necessary as
>    it is mostly used for debug. The bus busy bit is deducted from the
>    I2C core engine of qemu.
>  - support of the pool buffer: 2048 bytes of internal SRAM (not used
>    by the Linux driver).
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Acked-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/arm/ast2400.c            |  16 ++
>  hw/i2c/Makefile.objs        |   1 +
>  hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/ast2400.h    |   2 +
>  include/hw/i2c/aspeed_i2c.h |  62 ++++++
>  5 files changed, 529 insertions(+)
>  create mode 100644 hw/i2c/aspeed_i2c.c
>  create mode 100644 include/hw/i2c/aspeed_i2c.h
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index cf9f37e6a301..4576e67cd483 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -18,6 +18,7 @@
>  #include "hw/arm/ast2400.h"
>  #include "hw/char/serial.h"
>  #include "hw/boards.h"
> +#include "hw/i2c/aspeed_i2c.h"
>
>  #define AST2400_UART_5_BASE      0x00184000
>  #define AST2400_IOMEM_SIZE       0x00200000
> @@ -25,6 +26,7 @@
>  #define AST2400_VIC_BASE         0x1E6C0000
>  #define AST2400_SCU_BASE         0x1E6E2000
>  #define AST2400_TIMER_BASE       0x1E782000
> +#define AST2400_I2C_BASE         0x1E78A000
>
>  static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
> @@ -71,6 +73,10 @@ static void ast2400_init(Object *obj)
>      object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
>      object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
>      qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
> +
> +    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
> +    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
>  }
>
>  static void ast2400_realize(DeviceState *dev, Error **errp)
> @@ -143,6 +149,16 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>          serial_mm_init(&s->iomem, AST2400_UART_5_BASE, 2,
>                         uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
>      }
> +
> +    /* I2C */
> +    object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->vic), 12));
>  }
>
>  static void ast2400_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
> index aeb8f38d70f1..1fd54edf4c23 100644
> --- a/hw/i2c/Makefile.objs
> +++ b/hw/i2c/Makefile.objs
> @@ -5,4 +5,5 @@ common-obj-$(CONFIG_APM) += pm_smbus.o
>  common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
>  common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
>  common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o
>  obj-$(CONFIG_OMAP) += omap_i2c.o
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> new file mode 100644
> index 000000000000..f512e64d827a
> --- /dev/null
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -0,0 +1,448 @@
> +/*
> + * ARM Aspeed I2C controller
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/i2c/aspeed_i2c.h"
> +
> +/* I2C Global Register */
> +
> +#define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
> +#define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
> +                                               Assignment */
> +
> +/* I2C Device (Bus) Register */
> +
> +#define I2CD_FUN_CTRL_REG       0x00       /* I2CD Function Control  */
> +#define   I2CD_BUFF_SEL_MASK               (0x7 << 20)
> +#define   I2CD_BUFF_SEL(x)                 (x << 20)
> +#define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
> +#define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
> +#define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
> +#define   I2CD_MSB_STS                     (0x1 << 9)
> +#define   I2CD_SDA_DRIVE_1T_EN             (0x1 << 8)
> +#define   I2CD_M_SDA_DRIVE_1T_EN           (0x1 << 7)
> +#define   I2CD_M_HIGH_SPEED_EN             (0x1 << 6)
> +#define   I2CD_DEF_ADDR_EN                 (0x1 << 5)
> +#define   I2CD_DEF_ALERT_EN                (0x1 << 4)
> +#define   I2CD_DEF_ARP_EN                  (0x1 << 3)
> +#define   I2CD_DEF_GCALL_EN                (0x1 << 2)
> +#define   I2CD_SLAVE_EN                    (0x1 << 1)
> +#define   I2CD_MASTER_EN                   (0x1)
> +
> +#define I2CD_AC_TIMING_REG1     0x04       /* Clock and AC Timing Control #1 */
> +#define I2CD_AC_TIMING_REG2     0x08       /* Clock and AC Timing Control #1 */
> +#define I2CD_INTR_CTRL_REG      0x0c       /* I2CD Interrupt Control */
> +#define I2CD_INTR_STS_REG       0x10       /* I2CD Interrupt Status */
> +#define   I2CD_INTR_SDA_DL_TIMEOUT         (0x1 << 14)
> +#define   I2CD_INTR_BUS_RECOVER_DONE       (0x1 << 13)
> +#define   I2CD_INTR_SMBUS_ALERT            (0x1 << 12) /* Bus [0-3] only */
> +#define   I2CD_INTR_SMBUS_ARP_ADDR         (0x1 << 11) /* Removed */
> +#define   I2CD_INTR_SMBUS_DEV_ALERT_ADDR   (0x1 << 10) /* Removed */
> +#define   I2CD_INTR_SMBUS_DEF_ADDR         (0x1 << 9)  /* Removed */
> +#define   I2CD_INTR_GCALL_ADDR             (0x1 << 8)  /* Removed */
> +#define   I2CD_INTR_SLAVE_MATCH            (0x1 << 7)  /* use RX_DONE */
> +#define   I2CD_INTR_SCL_TIMEOUT            (0x1 << 6)
> +#define   I2CD_INTR_ABNORMAL               (0x1 << 5)
> +#define   I2CD_INTR_NORMAL_STOP            (0x1 << 4)
> +#define   I2CD_INTR_ARBIT_LOSS             (0x1 << 3)
> +#define   I2CD_INTR_RX_DONE                (0x1 << 2)
> +#define   I2CD_INTR_TX_NAK                 (0x1 << 1)
> +#define   I2CD_INTR_TX_ACK                 (0x1 << 0)
> +
> +#define I2CD_CMD_REG            0x14       /* I2CD Command/Status */
> +#define   I2CD_SDA_OE                      (0x1 << 28)
> +#define   I2CD_SDA_O                       (0x1 << 27)
> +#define   I2CD_SCL_OE                      (0x1 << 26)
> +#define   I2CD_SCL_O                       (0x1 << 25)
> +#define   I2CD_TX_TIMING                   (0x1 << 24)
> +#define   I2CD_TX_STATUS                   (0x1 << 23)
> +
> +#define   I2CD_TX_STATE_SHIFT              19 /* Tx State Machine */
> +#define   I2CD_TX_STATE_MASK                  0xf
> +#define     I2CD_IDLE                         0x0
> +#define     I2CD_MACTIVE                      0x8
> +#define     I2CD_MSTART                       0x9
> +#define     I2CD_MSTARTR                      0xa
> +#define     I2CD_MSTOP                        0xb
> +#define     I2CD_MTXD                         0xc
> +#define     I2CD_MRXACK                       0xd
> +#define     I2CD_MRXD                         0xe
> +#define     I2CD_MTXACK                       0xf
> +#define     I2CD_SWAIT                        0x1
> +#define     I2CD_SRXD                         0x4
> +#define     I2CD_STXACK                       0x5
> +#define     I2CD_STXD                         0x6
> +#define     I2CD_SRXACK                       0x7
> +#define     I2CD_RECOVER                      0x3
> +
> +#define   I2CD_SCL_LINE_STS                (0x1 << 18)
> +#define   I2CD_SDA_LINE_STS                (0x1 << 17)
> +#define   I2CD_BUS_BUSY_STS                (0x1 << 16)
> +#define   I2CD_SDA_OE_OUT_DIR              (0x1 << 15)
> +#define   I2CD_SDA_O_OUT_DIR               (0x1 << 14)
> +#define   I2CD_SCL_OE_OUT_DIR              (0x1 << 13)
> +#define   I2CD_SCL_O_OUT_DIR               (0x1 << 12)
> +#define   I2CD_BUS_RECOVER_CMD_EN          (0x1 << 11)
> +#define   I2CD_S_ALT_EN                    (0x1 << 10)
> +#define   I2CD_RX_DMA_ENABLE               (0x1 << 9)
> +#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
> +
> +/* Command Bit */
> +#define   I2CD_M_STOP_CMD                  (0x1 << 5)
> +#define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
> +#define   I2CD_M_RX_CMD                    (0x1 << 3)
> +#define   I2CD_S_TX_CMD                    (0x1 << 2)
> +#define   I2CD_M_TX_CMD                    (0x1 << 1)
> +#define   I2CD_M_START_CMD                 (0x1)
> +
> +#define I2CD_DEV_ADDR_REG       0x18       /* Slave Device Address */
> +#define I2CD_BUF_CTRL_REG       0x1c       /* Pool Buffer Control */
> +#define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
> +#define   I2CD_BYTE_BUF_TX_SHIFT           0
> +#define   I2CD_BYTE_BUF_TX_MASK            0xff
> +#define   I2CD_BYTE_BUF_RX_SHIFT           8
> +#define   I2CD_BYTE_BUF_RX_MASK            0xff
> +
> +
> +static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
> +{
> +    return bus->ctrl & I2CD_MASTER_EN;
> +}
> +
> +static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus *bus)
> +{
> +    return bus->ctrl & (I2CD_MASTER_EN | I2CD_SLAVE_EN);
> +}
> +
> +static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
> +{
> +    bus->intr_status &= bus->intr_ctrl;
> +    if (bus->intr_status) {
> +        bus->controller->intr_status |= 1 << bus->id;
> +        qemu_irq_raise(bus->controller->irq);
> +    }
> +}
> +
> +static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
> +                                    unsigned size)
> +{
> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
> +
> +    switch (offset) {
> +    case I2CD_FUN_CTRL_REG:
> +        return bus->ctrl;
> +    case I2CD_AC_TIMING_REG1:
> +        return bus->timing[0];
> +    case I2CD_AC_TIMING_REG2:
> +        return bus->timing[1];
> +    case I2CD_INTR_CTRL_REG:
> +        return bus->intr_ctrl;
> +    case I2CD_INTR_STS_REG:
> +        return bus->intr_status;
> +    case I2CD_BYTE_BUF_REG:
> +        return bus->buf;
> +    case I2CD_CMD_REG:
> +        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
> +        return -1;
> +    }
> +}
> +
> +static inline uint64_t aspeed_i2c_bus_get_state(AspeedI2CBus *bus)
> +{
> +    return bus->cmd >> 19 & 0xF;
> +}
> +
> +static inline void aspeed_i2c_bus_set_state(AspeedI2CBus *bus, uint64_t value)
> +{
> +    bus->cmd |= (value & 0xF) << 19;
> +}
> +
> +static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
> +{
> +    bus->cmd |= value & 0xFFFF;
> +    bus->intr_status = 0;
> +
> +    if (bus->cmd & I2CD_M_START_CMD) {
> +        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
> +                               extract32(bus->buf, 0, 1))) {
> +            bus->intr_status |= I2CD_INTR_TX_NAK;
> +        } else {
> +            bus->intr_status |= I2CD_INTR_TX_ACK;
> +        }
> +
> +    } else if (bus->cmd & I2CD_M_TX_CMD) {
> +        if (i2c_send(bus->bus, bus->buf)) {
> +            bus->intr_status |= (I2CD_INTR_TX_NAK | I2CD_INTR_ABNORMAL);
> +            i2c_end_transfer(bus->bus);
> +        } else {
> +            bus->intr_status |= I2CD_INTR_TX_ACK;
> +        }
> +
> +    } else if (bus->cmd & I2CD_M_RX_CMD) {
> +        int ret = i2c_recv(bus->bus);
> +        if (ret < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__);
> +            ret = 0xff;
> +        } else {
> +            bus->intr_status |= I2CD_INTR_RX_DONE;
> +        }
> +        bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
> +    }
> +
> +    if (bus->cmd & (I2CD_M_STOP_CMD | I2CD_M_S_RX_CMD_LAST)) {
> +        if (!i2c_bus_busy(bus->bus)) {
> +            bus->intr_status |= I2CD_INTR_ABNORMAL;
> +        } else {
> +            i2c_end_transfer(bus->bus);
> +            bus->intr_status |= I2CD_INTR_NORMAL_STOP;
> +        }
> +    }
> +
> +    /* command is handled, reset it and check for interrupts  */
> +    bus->cmd &= ~0xFFFF;
> +    aspeed_i2c_bus_raise_interrupt(bus);
> +}
> +
> +static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
> +                                 uint64_t value, unsigned size)
> +{
> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
> +
> +    switch (offset) {
> +    case I2CD_FUN_CTRL_REG:
> +        if (value & I2CD_SLAVE_EN) {
> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                          __func__);
> +            break;
> +        }
> +        bus->ctrl = value & 0x0071C3FF;
> +        break;
> +    case I2CD_AC_TIMING_REG1:
> +        bus->timing[0] = value & 0xFFFFF0F;
> +        break;
> +    case I2CD_AC_TIMING_REG2:
> +        bus->timing[1] = value & 0x7;
> +        break;
> +    case I2CD_INTR_CTRL_REG:
> +        bus->intr_ctrl = value & 0x7FFF;
> +        break;
> +    case I2CD_INTR_STS_REG:
> +        bus->intr_status &= ~(value & 0x7FFF);
> +        bus->controller->intr_status &= ~(1 << bus->id);
> +        qemu_irq_lower(bus->controller->irq);
> +        break;
> +    case I2CD_DEV_ADDR_REG:
> +        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                      __func__);
> +        break;
> +    case I2CD_BYTE_BUF_REG:
> +        bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
> +        break;
> +    case I2CD_CMD_REG:
> +        if (!aspeed_i2c_bus_is_enabled(bus)) {
> +            break;
> +        }
> +
> +        if (!aspeed_i2c_bus_is_master(bus)) {
> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                          __func__);
> +            break;
> +        }
> +
> +        aspeed_i2c_bus_handle_cmd(bus, value);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +    }
> +}
> +
> +static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
> +                                   unsigned size)
> +{
> +    AspeedI2CState *s = (AspeedI2CState *)opaque;
> +
> +    switch (offset) {
> +    case I2C_CTRL_STATUS:
> +        return s->intr_status;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        break;
> +    }
> +
> +    return -1;
> +}
> +
> +static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
> +                                  uint64_t value, unsigned size)
> +{
> +    switch (offset) {
> +    case I2C_CTRL_STATUS:
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps aspeed_i2c_bus_ops = {
> +    .read = aspeed_i2c_bus_read,
> +    .write = aspeed_i2c_bus_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
> +    .read = aspeed_i2c_ctrl_read,
> +    .write = aspeed_i2c_ctrl_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription aspeed_i2c_bus_vmstate = {
> +    .name = TYPE_ASPEED_I2C,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(id, AspeedI2CBus),
> +        VMSTATE_UINT32(ctrl, AspeedI2CBus),
> +        VMSTATE_UINT32_ARRAY(timing, AspeedI2CBus, 2),
> +        VMSTATE_UINT32(intr_ctrl, AspeedI2CBus),
> +        VMSTATE_UINT32(intr_status, AspeedI2CBus),
> +        VMSTATE_UINT32(cmd, AspeedI2CBus),
> +        VMSTATE_UINT32(buf, AspeedI2CBus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription aspeed_i2c_vmstate = {
> +    .name = TYPE_ASPEED_I2C,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(intr_status, AspeedI2CState),
> +        VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
> +                             ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
> +                             AspeedI2CBus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void aspeed_i2c_reset(DeviceState *dev)
> +{
> +    int i;
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +
> +    s->intr_status = 0;
> +
> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
> +        s->busses[i].intr_status = 0;
> +        s->busses[i].cmd = 0;
> +        s->busses[i].buf = 0;
> +        i2c_end_transfer(s->busses[i].bus);
> +    }
> +}
> +
> +/*
> + * Address Definitions
> + *
> + *   0x000 ... 0x03F: Global Register
> + *   0x040 ... 0x07F: Device 1
> + *   0x080 ... 0x0BF: Device 2
> + *   0x0C0 ... 0x0FF: Device 3
> + *   0x100 ... 0x13F: Device 4
> + *   0x140 ... 0x17F: Device 5
> + *   0x180 ... 0x1BF: Device 6
> + *   0x1C0 ... 0x1FF: Device 7
> + *   0x200 ... 0x2FF: Buffer Pool  (unused in linux driver)
> + *   0x300 ... 0x33F: Device 8
> + *   0x340 ... 0x37F: Device 9
> + *   0x380 ... 0x3BF: Device 10
> + *   0x3C0 ... 0x3FF: Device 11
> + *   0x400 ... 0x43F: Device 12
> + *   0x440 ... 0x47F: Device 13
> + *   0x480 ... 0x4BF: Device 14
> + *   0x800 ... 0xFFF: Buffer Pool  (unused in linux driver)
> + */
> +static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
> +{
> +    int i;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i2c_ctrl_ops, s,
> +                          "aspeed.i2c", 0x1000);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +
> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
> +        char name[16];
> +        int offset = i < 7 ? 1 : 5;
> +        snprintf(name, sizeof(name), "aspeed.i2c.%d", i);
> +        s->busses[i].controller = s;
> +        s->busses[i].id = i;
> +        s->busses[i].bus = i2c_init_bus(dev, name);
> +        memory_region_init_io(&s->busses[i].mr, OBJECT(dev),
> +                              &aspeed_i2c_bus_ops, &s->busses[i], name, 0x40);
> +        memory_region_add_subregion(&s->iomem, 0x40 * (i + offset),
> +                                    &s->busses[i].mr);
> +    }
> +}
> +
> +static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &aspeed_i2c_vmstate;
> +    dc->reset = aspeed_i2c_reset;
> +    dc->realize = aspeed_i2c_realize;
> +    dc->desc = "Aspeed I2C Controller";
> +}
> +
> +static const TypeInfo aspeed_i2c_info = {
> +    .name          = TYPE_ASPEED_I2C,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AspeedI2CState),
> +    .class_init    = aspeed_i2c_class_init,
> +};
> +
> +static void aspeed_i2c_register_types(void)
> +{
> +    type_register_static(&aspeed_i2c_info);
> +}
> +
> +type_init(aspeed_i2c_register_types)
> +
> +
> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr)
> +{
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +    I2CBus *bus = NULL;
> +
> +    if (busnr >= 0 && busnr < ASPEED_I2C_NR_BUSSES) {
> +        bus = s->busses[busnr].bus;
> +    }
> +
> +    return bus;
> +}
> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
> index cf7102145355..e96e3db3fbea 100644
> --- a/include/hw/arm/ast2400.h
> +++ b/include/hw/arm/ast2400.h
> @@ -16,6 +16,7 @@
>  #include "hw/intc/aspeed_vic.h"
>  #include "hw/misc/aspeed_scu.h"
>  #include "hw/timer/aspeed_timer.h"
> +#include "hw/i2c/aspeed_i2c.h"
>
>  typedef struct AST2400State {
>      /*< private >*/
> @@ -28,6 +29,7 @@ typedef struct AST2400State {
>      AspeedVICState vic;
>      AspeedTimerCtrlState timerctrl;
>      AspeedSCUState scu;
> +    AspeedI2CState i2c;
>  } AST2400State;
>
>  #define TYPE_AST2400 "ast2400"
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> new file mode 100644
> index 000000000000..f9020acdef30
> --- /dev/null
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -0,0 +1,62 @@
> +/*
> + *  ASPEED AST2400 I2C Controller
> + *
> + *  Copyright (C) 2016 IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +#ifndef ASPEED_I2C_H
> +#define ASPEED_I2C_H
> +
> +#include "hw/i2c/i2c.h"
> +
> +#define TYPE_ASPEED_I2C "aspeed.i2c"
> +#define ASPEED_I2C(obj) \
> +    OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
> +
> +#define ASPEED_I2C_NR_BUSSES 14
> +
> +struct AspeedI2CState;
> +
> +typedef struct AspeedI2CBus {
> +    struct AspeedI2CState *controller;
> +
> +    MemoryRegion mr;
> +
> +    I2CBus *bus;
> +    uint8_t id;
> +
> +    uint32_t ctrl;
> +    uint32_t timing[2];
> +    uint32_t intr_ctrl;
> +    uint32_t intr_status;
> +    uint32_t cmd;
> +    uint32_t buf;
> +} AspeedI2CBus;
> +
> +typedef struct AspeedI2CState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t intr_status;
> +
> +    AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
> +} AspeedI2CState;
> +
> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
> +
> +#endif /* ASPEED_I2C_H */
> --
> 2.1.4
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3
  2016-05-17 19:25 ` [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3 Cédric Le Goater
@ 2016-05-18 13:29   ` Joel Stanley
  2016-05-18 13:35     ` Cédric Le Goater
  0 siblings, 1 reply; 14+ messages in thread
From: Joel Stanley @ 2016-05-18 13:29 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

Hey Cedric,

On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
> The palmetto platform has a tmp421 device. Qemu has not (yet). Let's
> use a tmp105 instead.
>
> The aspeed-bmc-opp-palmetto dts file should to be changed consequently
> with :
>
>         i2c2: i2c-bus@c0 {
>                 tmp423@4c {
>                         compatible = "ti,tmp105";
>                         reg = <0x4c>;
>                 };
>         };
>
> Temperature can be changed from the monitor with :
>
>         (qemu) qom-set /machine/unattached/device[2] temperature 12000

This takes us back to the discussion we had with the Ethernet device.
We need to make the call between emulating something that is close to
a BMC in order for us to test the OpenBMC userspace stack, and making
the Qemu model strictly follow the palmetto and barreleye machines so
we can test both the kernel and userspace.

How much does the tmp105 differ from the tmp421? Could we add a model
for the 421?

Cheers,

Joel

>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/arm/ast2400.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index 41bfc74493fa..d231d98a88e1 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -161,6 +161,9 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>                         qdev_get_gpio_in(DEVICE(&s->vic), 12));
>
>      i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 0), "ds1338", 0x68);
> +    dev = i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 2),
> +                           "tmp105", 0x4c);
> +    object_property_set_int(OBJECT(dev), 25000, "temperature", &err);
>  }
>
>  static void ast2400_class_init(ObjectClass *oc, void *data)
> --
> 2.1.4
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0
  2016-05-17 19:25 ` [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0 Cédric Le Goater
@ 2016-05-18 13:33   ` Joel Stanley
  2016-05-18 13:46     ` Cédric Le Goater
  0 siblings, 1 reply; 14+ messages in thread
From: Joel Stanley @ 2016-05-18 13:33 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

Hey Cedric,

On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
> The palmetto platform has a ds3231 RTC device but not qemu. Let's use
> a ds1338 instead.
>
> The aspeed-bmc-opp-palmetto dts file needs to be changed consequently
> with :
>
>         rtc@68 {
>                 compatible = "dallas,ds1338";
>                 reg = <0x68>;
>         };

Please see my comments for the tmp device.

How much does the register layout differ between the 1338 and the 3231?

Cheers,

Joel

>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/arm/ast2400.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index 4576e67cd483..41bfc74493fa 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -159,6 +159,8 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>                         qdev_get_gpio_in(DEVICE(&s->vic), 12));
> +
> +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&s->i2c), 0), "ds1338", 0x68);
>  }
>
>  static void ast2400_class_init(ObjectClass *oc, void *data)
> --
> 2.1.4
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3
  2016-05-18 13:29   ` Joel Stanley
@ 2016-05-18 13:35     ` Cédric Le Goater
  0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-18 13:35 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Andrew Jeffery

On 05/18/2016 03:29 PM, Joel Stanley wrote:
> 
> How much does the tmp105 differ from the tmp421? Could we add a model
> for the 421?

tmp421_{g,s}et_temperature should be a little different, so will be the 
register read routine. nothing complex really. This tmp421 is on my TODO 
list.

C. 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0
  2016-05-18 13:33   ` Joel Stanley
@ 2016-05-18 13:46     ` Cédric Le Goater
  2016-05-18 14:06       ` Joel Stanley
  0 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-18 13:46 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Andrew Jeffery

On 05/18/2016 03:33 PM, Joel Stanley wrote:
> Hey Cedric,
> 
> On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
>> The palmetto platform has a ds3231 RTC device but not qemu. Let's use
>> a ds1338 instead.
>>
>> The aspeed-bmc-opp-palmetto dts file needs to be changed consequently
>> with :
>>
>>         rtc@68 {
>>                 compatible = "dallas,ds1338";
>>                 reg = <0x68>;
>>         };
> 
> Please see my comments for the tmp device.
> 
> How much does the register layout differ between the 1338 and the 3231?

Will tell you soon.

But on my palmetto running OpenBMC, I see : 

	[    5.630000] rtc-ds1307: probe of 0-0068 failed with error -5

So do we really have a RTC available on this bus ? 

	root@palmetto:~# i2cdetect -y 0
	     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
	00:          -- -- -- -- -- -- -- -- -- -- -- -- -- 
	10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	50: UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
	70: -- -- -- -- -- -- -- --                         

There are plenty of other devices on the other busses though, but 
none with Ox68.

There is also a RTC directly connected to APB bus. I guess we need a 
model for this one also. 

Cheers, 

C.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0
  2016-05-18 13:46     ` Cédric Le Goater
@ 2016-05-18 14:06       ` Joel Stanley
  0 siblings, 0 replies; 14+ messages in thread
From: Joel Stanley @ 2016-05-18 14:06 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: OpenBMC Maillist, Andrew Jeffery

On Wed, May 18, 2016 at 11:16 PM, Cédric Le Goater <clg@kaod.org> wrote:
> On 05/18/2016 03:33 PM, Joel Stanley wrote:
>> Hey Cedric,
>>
>> On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>> The palmetto platform has a ds3231 RTC device but not qemu. Let's use
>>> a ds1338 instead.
>>>
>>> The aspeed-bmc-opp-palmetto dts file needs to be changed consequently
>>> with :
>>>
>>>         rtc@68 {
>>>                 compatible = "dallas,ds1338";
>>>                 reg = <0x68>;
>>>         };
>>
>> Please see my comments for the tmp device.
>>
>> How much does the register layout differ between the 1338 and the 3231?
>
> Will tell you soon.
>
> But on my palmetto running OpenBMC, I see :
>
>         [    5.630000] rtc-ds1307: probe of 0-0068 failed with error -5
>
> So do we really have a RTC available on this bus ?

The schematic I have suggests it's at  0x0e on the Palmetto. But I am
sure we've used that device tree as-is and the clock works on the
hardware. I will check tomorrow.

>         root@palmetto:~# i2cdetect -y 0
>              0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>         00:          -- -- -- -- -- -- -- -- -- -- -- -- --
>         10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         50: UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>         70: -- -- -- -- -- -- -- --
>
> There are plenty of other devices on the other busses though, but
> none with Ox68.
>
> There is also a RTC directly connected to APB bus. I guess we need a
> model for this one also.

That one in the SoC isn't battery backed. It's currently disabled in
the device tree for openbmc machines to avoid confusion with the
battery backed i2c device.

I'd suggest it's a low priority to model the internal one.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
  2016-05-18 13:26   ` Joel Stanley
@ 2016-05-19  1:41   ` Andrew Jeffery
  2016-05-19 16:54     ` Cédric Le Goater
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Jeffery @ 2016-05-19  1:41 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

[-- Attachment #1: Type: text/plain, Size: 27244 bytes --]

Hi Cédric,

Looks good to me as well. I have made a few comments.

On Tue, 2016-05-17 at 21:25 +0200, Cédric Le Goater wrote:
> The Aspeed AST2400 integrates a set of 14 I2C/SMBus bus controllers
> directly connected to APB bus which can be programmed as a master or
> slave.
> 
> This patch offers a device model for the master mode only, slave mode
> is not supported.
> 
> On the TODO list, we also have :
> 
>  - improve and harden the state machine. This is really a first
>    version.

What are your thoughts here? Validating the transitions? Something
more?

>  - bus recovery support (used by the Linux driver).
>  - transfer mode state machine bits. this is not strictly necessary as
>    it is mostly used for debug. The bus busy bit is deducted from the
>    I2C core engine of qemu.
>  - support of the pool buffer: 2048 bytes of internal SRAM (not used
>    by the Linux driver).
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/arm/ast2400.c            |  16 ++
>  hw/i2c/Makefile.objs        |   1 +
>  hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/ast2400.h    |   2 +
>  include/hw/i2c/aspeed_i2c.h |  62 ++++++
>  5 files changed, 529 insertions(+)
>  create mode 100644 hw/i2c/aspeed_i2c.c
>  create mode 100644 include/hw/i2c/aspeed_i2c.h
> 
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index cf9f37e6a301..4576e67cd483 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -18,6 +18,7 @@
>  #include "hw/arm/ast2400.h"
>  #include "hw/char/serial.h"
>  #include "hw/boards.h"
> +#include "hw/i2c/aspeed_i2c.h"
>  
>  #define AST2400_UART_5_BASE      0x00184000
>  #define AST2400_IOMEM_SIZE       0x00200000
> @@ -25,6 +26,7 @@
>  #define AST2400_VIC_BASE         0x1E6C0000
>  #define AST2400_SCU_BASE         0x1E6E2000
>  #define AST2400_TIMER_BASE       0x1E782000
> +#define AST2400_I2C_BASE         0x1E78A000
>  
>  static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
> @@ -71,6 +73,10 @@ static void ast2400_init(Object *obj)
>      object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
>      object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
>      qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
> +
> +    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
> +    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
>  }
>  
>  static void ast2400_realize(DeviceState *dev, Error **errp)
> @@ -143,6 +149,16 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>          serial_mm_init(&s->iomem, AST2400_UART_5_BASE, 2,
>                         uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
>      }
> +
> +    /* I2C */
> +    object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->vic), 12));
>  }
>  
>  static void ast2400_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
> index aeb8f38d70f1..1fd54edf4c23 100644
> --- a/hw/i2c/Makefile.objs
> +++ b/hw/i2c/Makefile.objs
> @@ -5,4 +5,5 @@ common-obj-$(CONFIG_APM) += pm_smbus.o
>  common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
>  common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
>  common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o
>  obj-$(CONFIG_OMAP) += omap_i2c.o
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> new file mode 100644
> index 000000000000..f512e64d827a
> --- /dev/null
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -0,0 +1,448 @@
> +/*
> + * ARM Aspeed I2C controller
> + *
> + * Copyright (C) 2016 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see .
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/i2c/aspeed_i2c.h"
> +
> +/* I2C Global Register */
> +
> +#define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
> +#define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
> +                                               Assignment */
> +
> +/* I2C Device (Bus) Register */
> +
> +#define I2CD_FUN_CTRL_REG       0x00       /* I2CD Function Control  */
> +#define   I2CD_BUFF_SEL_MASK               (0x7 << 20)
> +#define   I2CD_BUFF_SEL(x)                 (x << 20)
> +#define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
> +#define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
> +#define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
> +#define   I2CD_MSB_STS                     (0x1 << 9)
> +#define   I2CD_SDA_DRIVE_1T_EN             (0x1 << 8)
> +#define   I2CD_M_SDA_DRIVE_1T_EN           (0x1 << 7)
> +#define   I2CD_M_HIGH_SPEED_EN             (0x1 << 6)
> +#define   I2CD_DEF_ADDR_EN                 (0x1 << 5)
> +#define   I2CD_DEF_ALERT_EN                (0x1 << 4)
> +#define   I2CD_DEF_ARP_EN                  (0x1 << 3)
> +#define   I2CD_DEF_GCALL_EN                (0x1 << 2)
> +#define   I2CD_SLAVE_EN                    (0x1 << 1)
> +#define   I2CD_MASTER_EN                   (0x1)
> +
> +#define I2CD_AC_TIMING_REG1     0x04       /* Clock and AC Timing Control #1 */
> +#define I2CD_AC_TIMING_REG2     0x08       /* Clock and AC Timing Control #1 */
> +#define I2CD_INTR_CTRL_REG      0x0c       /* I2CD Interrupt Control */
> +#define I2CD_INTR_STS_REG       0x10       /* I2CD Interrupt Status */
> +#define   I2CD_INTR_SDA_DL_TIMEOUT         (0x1 << 14)
> +#define   I2CD_INTR_BUS_RECOVER_DONE       (0x1 << 13)
> +#define   I2CD_INTR_SMBUS_ALERT            (0x1 << 12) /* Bus [0-3] only */
> +#define   I2CD_INTR_SMBUS_ARP_ADDR         (0x1 << 11) /* Removed */
> +#define   I2CD_INTR_SMBUS_DEV_ALERT_ADDR   (0x1 << 10) /* Removed */
> +#define   I2CD_INTR_SMBUS_DEF_ADDR         (0x1 << 9)  /* Removed */
> +#define   I2CD_INTR_GCALL_ADDR             (0x1 << 8)  /* Removed */
> +#define   I2CD_INTR_SLAVE_MATCH            (0x1 << 7)  /* use RX_DONE */
> +#define   I2CD_INTR_SCL_TIMEOUT            (0x1 << 6)
> +#define   I2CD_INTR_ABNORMAL               (0x1 << 5)
> +#define   I2CD_INTR_NORMAL_STOP            (0x1 << 4)
> +#define   I2CD_INTR_ARBIT_LOSS             (0x1 << 3)
> +#define   I2CD_INTR_RX_DONE                (0x1 << 2)
> +#define   I2CD_INTR_TX_NAK                 (0x1 << 1)
> +#define   I2CD_INTR_TX_ACK                 (0x1 << 0)
> +
> +#define I2CD_CMD_REG            0x14       /* I2CD Command/Status */
> +#define   I2CD_SDA_OE                      (0x1 << 28)
> +#define   I2CD_SDA_O                       (0x1 << 27)
> +#define   I2CD_SCL_OE                      (0x1 << 26)
> +#define   I2CD_SCL_O                       (0x1 << 25)
> +#define   I2CD_TX_TIMING                   (0x1 << 24)
> +#define   I2CD_TX_STATUS                   (0x1 << 23)
> +
> +#define   I2CD_TX_STATE_SHIFT              19 /* Tx State Machine */
> +#define   I2CD_TX_STATE_MASK                  0xf
> +#define     I2CD_IDLE                         0x0
> +#define     I2CD_MACTIVE                      0x8
> +#define     I2CD_MSTART                       0x9
> +#define     I2CD_MSTARTR                      0xa
> +#define     I2CD_MSTOP                        0xb
> +#define     I2CD_MTXD                         0xc
> +#define     I2CD_MRXACK                       0xd
> +#define     I2CD_MRXD                         0xe
> +#define     I2CD_MTXACK                       0xf
> +#define     I2CD_SWAIT                        0x1
> +#define     I2CD_SRXD                         0x4
> +#define     I2CD_STXACK                       0x5
> +#define     I2CD_STXD                         0x6
> +#define     I2CD_SRXACK                       0x7
> +#define     I2CD_RECOVER                      0x3
> +
> +#define   I2CD_SCL_LINE_STS                (0x1 << 18)
> +#define   I2CD_SDA_LINE_STS                (0x1 << 17)
> +#define   I2CD_BUS_BUSY_STS                (0x1 << 16)
> +#define   I2CD_SDA_OE_OUT_DIR              (0x1 << 15)
> +#define   I2CD_SDA_O_OUT_DIR               (0x1 << 14)
> +#define   I2CD_SCL_OE_OUT_DIR              (0x1 << 13)
> +#define   I2CD_SCL_O_OUT_DIR               (0x1 << 12)
> +#define   I2CD_BUS_RECOVER_CMD_EN          (0x1 << 11)
> +#define   I2CD_S_ALT_EN                    (0x1 << 10)
> +#define   I2CD_RX_DMA_ENABLE               (0x1 << 9)
> +#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
> +
> +/* Command Bit */
> +#define   I2CD_M_STOP_CMD                  (0x1 << 5)
> +#define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
> +#define   I2CD_M_RX_CMD                    (0x1 << 3)
> +#define   I2CD_S_TX_CMD                    (0x1 << 2)
> +#define   I2CD_M_TX_CMD                    (0x1 << 1)
> +#define   I2CD_M_START_CMD                 (0x1)
> +
> +#define I2CD_DEV_ADDR_REG       0x18       /* Slave Device Address */
> +#define I2CD_BUF_CTRL_REG       0x1c       /* Pool Buffer Control */
> +#define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
> +#define   I2CD_BYTE_BUF_TX_SHIFT           0
> +#define   I2CD_BYTE_BUF_TX_MASK            0xff
> +#define   I2CD_BYTE_BUF_RX_SHIFT           8
> +#define   I2CD_BYTE_BUF_RX_MASK            0xff
> +
> +
> +static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
> +{
> +    return bus->ctrl & I2CD_MASTER_EN;
> +}
> +
> +static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus *bus)
> +{
> +    return bus->ctrl & (I2CD_MASTER_EN | I2CD_SLAVE_EN);
> +}
> +
> +static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
> +{
> +    bus->intr_status &= bus->intr_ctrl;
> +    if (bus->intr_status) {
> +        bus->controller->intr_status |= 1 << bus->id;
> +        qemu_irq_raise(bus->controller->irq);
> +    }
> +}
> +
> +static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
> +                                    unsigned size)
> +{
> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;

Nitpick - we can avoid the cast here.

> +
> +    switch (offset) {
> +    case I2CD_FUN_CTRL_REG:
> +        return bus->ctrl;
> +    case I2CD_AC_TIMING_REG1:
> +        return bus->timing[0];
> +    case I2CD_AC_TIMING_REG2:
> +        return bus->timing[1];
> +    case I2CD_INTR_CTRL_REG:
> +        return bus->intr_ctrl;
> +    case I2CD_INTR_STS_REG:
> +        return bus->intr_status;
> +    case I2CD_BYTE_BUF_REG:
> +        return bus->buf;
> +    case I2CD_CMD_REG:
> +        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
> +        return -1;
> +    }
> +}
> +
> +static inline uint64_t aspeed_i2c_bus_get_state(AspeedI2CBus *bus)
> +{
> +    return bus->cmd >> 19 & 0xF;
> +}
> +
> +static inline void aspeed_i2c_bus_set_state(AspeedI2CBus *bus, uint64_t value)
> +{
> +    bus->cmd |= (value & 0xF) << 19;
> +}
> +
> +static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
> +{
> +    bus->cmd |= value & 0xFFFF;
> +    bus->intr_status = 0;
> +
> +    if (bus->cmd & I2CD_M_START_CMD) {
> +        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
> +                               extract32(bus->buf, 0, 1))) {
> +            bus->intr_status |= I2CD_INTR_TX_NAK;
> +        } else {
> +            bus->intr_status |= I2CD_INTR_TX_ACK;
> +        }
> +
> +    } else if (bus->cmd & I2CD_M_TX_CMD) {
> +        if (i2c_send(bus->bus, bus->buf)) {
> +            bus->intr_status |= (I2CD_INTR_TX_NAK | I2CD_INTR_ABNORMAL);
> +            i2c_end_transfer(bus->bus);
> +        } else {
> +            bus->intr_status |= I2CD_INTR_TX_ACK;
> +        }
> +
> +    } else if (bus->cmd & I2CD_M_RX_CMD) {
> +        int ret = i2c_recv(bus->bus);
> +        if (ret < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__);
> +            ret = 0xff;
> +        } else {
> +            bus->intr_status |= I2CD_INTR_RX_DONE;
> +        }
> +        bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
> +    }
> +
> +    if (bus->cmd & (I2CD_M_STOP_CMD | I2CD_M_S_RX_CMD_LAST)) {
> +        if (!i2c_bus_busy(bus->bus)) {
> +            bus->intr_status |= I2CD_INTR_ABNORMAL;
> +        } else {
> +            i2c_end_transfer(bus->bus);
> +            bus->intr_status |= I2CD_INTR_NORMAL_STOP;
> +        }
> +    }
> +
> +    /* command is handled, reset it and check for interrupts  */
> +    bus->cmd &= ~0xFFFF;
> +    aspeed_i2c_bus_raise_interrupt(bus);
> +}
> +
> +static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
> +                                 uint64_t value, unsigned size)
> +{
> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;

As above

> +
> +    switch (offset) {
> +    case I2CD_FUN_CTRL_REG:
> +        if (value & I2CD_SLAVE_EN) {
> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                          __func__);
> +            break;
> +        }
> +        bus->ctrl = value & 0x0071C3FF;
> +        break;
> +    case I2CD_AC_TIMING_REG1:
> +        bus->timing[0] = value & 0xFFFFF0F;
> +        break;
> +    case I2CD_AC_TIMING_REG2:
> +        bus->timing[1] = value & 0x7;
> +        break;
> +    case I2CD_INTR_CTRL_REG:
> +        bus->intr_ctrl = value & 0x7FFF;
> +        break;
> +    case I2CD_INTR_STS_REG:
> +        bus->intr_status &= ~(value & 0x7FFF);
> +        bus->controller->intr_status &= ~(1 << bus->id);
> +        qemu_irq_lower(bus->controller->irq);
> +        break;
> +    case I2CD_DEV_ADDR_REG:
> +        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                      __func__);
> +        break;
> +    case I2CD_BYTE_BUF_REG:
> +        bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
> +        break;
> +    case I2CD_CMD_REG:
> +        if (!aspeed_i2c_bus_is_enabled(bus)) {
> +            break;
> +        }
> +
> +        if (!aspeed_i2c_bus_is_master(bus)) {
> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> +                          __func__);
> +            break;
> +        }
> +
> +        aspeed_i2c_bus_handle_cmd(bus, value);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +    }
> +}
> +
> +static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
> +                                   unsigned size)
> +{
> +    AspeedI2CState *s = (AspeedI2CState *)opaque;

As above

> +
> +    switch (offset) {
> +    case I2C_CTRL_STATUS:
> +        return s->intr_status;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        break;
> +    }
> +
> +    return -1;
> +}
> +
> +static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
> +                                  uint64_t value, unsigned size)
> +{
> +    switch (offset) {
> +    case I2C_CTRL_STATUS:
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps aspeed_i2c_bus_ops = {
> +    .read = aspeed_i2c_bus_read,
> +    .write = aspeed_i2c_bus_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

Alexey suggested I make these DEVICE_LITTLE_ENDIAN when he looked at my
timer and intc devices. We should be consistent one way or the other -
what's the right approach?

> +};
> +
> +static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
> +    .read = aspeed_i2c_ctrl_read,
> +    .write = aspeed_i2c_ctrl_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription aspeed_i2c_bus_vmstate = {
> +    .name = TYPE_ASPEED_I2C,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(id, AspeedI2CBus),
> +        VMSTATE_UINT32(ctrl, AspeedI2CBus),
> +        VMSTATE_UINT32_ARRAY(timing, AspeedI2CBus, 2),
> +        VMSTATE_UINT32(intr_ctrl, AspeedI2CBus),
> +        VMSTATE_UINT32(intr_status, AspeedI2CBus),
> +        VMSTATE_UINT32(cmd, AspeedI2CBus),
> +        VMSTATE_UINT32(buf, AspeedI2CBus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription aspeed_i2c_vmstate = {
> +    .name = TYPE_ASPEED_I2C,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(intr_status, AspeedI2CState),
> +        VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
> +                             ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
> +                             AspeedI2CBus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void aspeed_i2c_reset(DeviceState *dev)
> +{
> +    int i;
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +
> +    s->intr_status = 0;
> +
> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
> +        s->busses[i].intr_status = 0;

Should we also be resetting .intr_ctrl = 0? Datasheet says that's the
init value.

> +        s->busses[i].cmd = 0;
> +        s->busses[i].buf = 0;
> +        i2c_end_transfer(s->busses[i].bus);
> +    }
> +}
> +
> +/*
> + * Address Definitions
> + *
> + *   0x000 ... 0x03F: Global Register
> + *   0x040 ... 0x07F: Device 1
> + *   0x080 ... 0x0BF: Device 2
> + *   0x0C0 ... 0x0FF: Device 3
> + *   0x100 ... 0x13F: Device 4
> + *   0x140 ... 0x17F: Device 5
> + *   0x180 ... 0x1BF: Device 6
> + *   0x1C0 ... 0x1FF: Device 7
> + *   0x200 ... 0x2FF: Buffer Pool  (unused in linux driver)
> + *   0x300 ... 0x33F: Device 8
> + *   0x340 ... 0x37F: Device 9
> + *   0x380 ... 0x3BF: Device 10
> + *   0x3C0 ... 0x3FF: Device 11
> + *   0x400 ... 0x43F: Device 12
> + *   0x440 ... 0x47F: Device 13
> + *   0x480 ... 0x4BF: Device 14
> + *   0x800 ... 0xFFF: Buffer Pool  (unused in linux driver)
> + */
> +static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
> +{
> +    int i;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +
> +    sysbus_init_irq(sbd, &s->irq);
> +    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i2c_ctrl_ops, s,
> +                          "aspeed.i2c", 0x1000);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +
> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
> +        char name[16];
> +        int offset = i < 7 ? 1 : 5;
> +        snprintf(name, sizeof(name), "aspeed.i2c.%d", i);
> +        s->busses[i].controller = s;
> +        s->busses[i].id = i;
> +        s->busses[i].bus = i2c_init_bus(dev, name);
> +        memory_region_init_io(&s->busses[i].mr, OBJECT(dev),
> +                              &aspeed_i2c_bus_ops, &s->busses[i], name, 0x40);
> +        memory_region_add_subregion(&s->iomem, 0x40 * (i + offset),
> +                                    &s->busses[i].mr);
> +    }
> +}
> +
> +static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &aspeed_i2c_vmstate;
> +    dc->reset = aspeed_i2c_reset;
> +    dc->realize = aspeed_i2c_realize;
> +    dc->desc = "Aspeed I2C Controller";
> +}
> +
> +static const TypeInfo aspeed_i2c_info = {
> +    .name          = TYPE_ASPEED_I2C,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AspeedI2CState),
> +    .class_init    = aspeed_i2c_class_init,
> +};
> +
> +static void aspeed_i2c_register_types(void)
> +{
> +    type_register_static(&aspeed_i2c_info);
> +}
> +
> +type_init(aspeed_i2c_register_types)
> +
> +
> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr)
> +{
> +    AspeedI2CState *s = ASPEED_I2C(dev);
> +    I2CBus *bus = NULL;
> +
> +    if (busnr >= 0 && busnr < ASPEED_I2C_NR_BUSSES) {
> +        bus = s->busses[busnr].bus;
> +    }
> +
> +    return bus;
> +}
> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
> index cf7102145355..e96e3db3fbea 100644
> --- a/include/hw/arm/ast2400.h
> +++ b/include/hw/arm/ast2400.h
> @@ -16,6 +16,7 @@
>  #include "hw/intc/aspeed_vic.h"
>  #include "hw/misc/aspeed_scu.h"
>  #include "hw/timer/aspeed_timer.h"
> +#include "hw/i2c/aspeed_i2c.h"
>  
>  typedef struct AST2400State {
>      /*< private >*/
> @@ -28,6 +29,7 @@ typedef struct AST2400State {
>      AspeedVICState vic;
>      AspeedTimerCtrlState timerctrl;
>      AspeedSCUState scu;
> +    AspeedI2CState i2c;
>  } AST2400State;
>  
>  #define TYPE_AST2400 "ast2400"
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> new file mode 100644
> index 000000000000..f9020acdef30
> --- /dev/null
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -0,0 +1,62 @@
> +/*
> + *  ASPEED AST2400 I2C Controller
> + *
> + *  Copyright (C) 2016 IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +#ifndef ASPEED_I2C_H
> +#define ASPEED_I2C_H
> +
> +#include "hw/i2c/i2c.h"
> +
> +#define TYPE_ASPEED_I2C "aspeed.i2c"
> +#define ASPEED_I2C(obj) \
> +    OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
> +
> +#define ASPEED_I2C_NR_BUSSES 14
> +
> +struct AspeedI2CState;
> +
> +typedef struct AspeedI2CBus {
> +    struct AspeedI2CState *controller;
> +
> +    MemoryRegion mr;
> +
> +    I2CBus *bus;
> +    uint8_t id;
> +
> +    uint32_t ctrl;
> +    uint32_t timing[2];
> +    uint32_t intr_ctrl;
> +    uint32_t intr_status;
> +    uint32_t cmd;
> +    uint32_t buf;
> +} AspeedI2CBus;
> +
> +typedef struct AspeedI2CState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    uint32_t intr_status;
> +
> +    AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
> +} AspeedI2CState;
> +
> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
> +
> +#endif /* ASPEED_I2C_H */

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-19  1:41   ` Andrew Jeffery
@ 2016-05-19 16:54     ` Cédric Le Goater
  2016-05-20  6:43       ` Andrew Jeffery
  0 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-19 16:54 UTC (permalink / raw)
  To: andrew, openbmc

Hello Andrew,

On 05/19/2016 03:41 AM, Andrew Jeffery wrote:
> Hi Cédric,
> 
> Looks good to me as well. I have made a few comments.
> 
> On Tue, 2016-05-17 at 21:25 +0200, Cédric Le Goater wrote:
>> The Aspeed AST2400 integrates a set of 14 I2C/SMBus bus controllers
>> directly connected to APB bus which can be programmed as a master or
>> slave.
>>
>> This patch offers a device model for the master mode only, slave mode
>> is not supported.
>>
>> On the TODO list, we also have :
>>
>>  - improve and harden the state machine. This is really a first
>>    version.
> 
> What are your thoughts here? Validating the transitions? Something
> more?

Well, this was tested with the linux driver with known and unknown 
devices. But to be complete, the device model and the transfer state 
machine should be stressed a little more with really buggy devices and 
drivers. 

I should have addressed your comments on : 

	https://github.com/legoater/qemu/commits/aspeed

in which I also added a device model for the TMP42{1,2,3} temperature 
sensors to fit the palmetto dts. 

Tell me when you want a resend.

Thanks,

C. 

>>  - bus recovery support (used by the Linux driver).
>>  - transfer mode state machine bits. this is not strictly necessary as
>>    it is mostly used for debug. The bus busy bit is deducted from the
>>    I2C core engine of qemu.
>>  - support of the pool buffer: 2048 bytes of internal SRAM (not used
>>    by the Linux driver).
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/arm/ast2400.c            |  16 ++
>>  hw/i2c/Makefile.objs        |   1 +
>>  hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/ast2400.h    |   2 +
>>  include/hw/i2c/aspeed_i2c.h |  62 ++++++
>>  5 files changed, 529 insertions(+)
>>  create mode 100644 hw/i2c/aspeed_i2c.c
>>  create mode 100644 include/hw/i2c/aspeed_i2c.h
>>
>> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
>> index cf9f37e6a301..4576e67cd483 100644
>> --- a/hw/arm/ast2400.c
>> +++ b/hw/arm/ast2400.c
>> @@ -18,6 +18,7 @@
>>  #include "hw/arm/ast2400.h"
>>  #include "hw/char/serial.h"
>>  #include "hw/boards.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>>  
>>  #define AST2400_UART_5_BASE      0x00184000
>>  #define AST2400_IOMEM_SIZE       0x00200000
>> @@ -25,6 +26,7 @@
>>  #define AST2400_VIC_BASE         0x1E6C0000
>>  #define AST2400_SCU_BASE         0x1E6E2000
>>  #define AST2400_TIMER_BASE       0x1E782000
>> +#define AST2400_I2C_BASE         0x1E78A000
>>  
>>  static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>> @@ -71,6 +73,10 @@ static void ast2400_init(Object *obj)
>>      object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
>>      object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
>>      qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
>> +
>> +    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
>> +    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
>> +    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
>>  }
>>  
>>  static void ast2400_realize(DeviceState *dev, Error **errp)
>> @@ -143,6 +149,16 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>>          serial_mm_init(&s->iomem, AST2400_UART_5_BASE, 2,
>>                         uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
>>      }
>> +
>> +    /* I2C */
>> +    object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>> +                       qdev_get_gpio_in(DEVICE(&s->vic), 12));
>>  }
>>  
>>  static void ast2400_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
>> index aeb8f38d70f1..1fd54edf4c23 100644
>> --- a/hw/i2c/Makefile.objs
>> +++ b/hw/i2c/Makefile.objs
>> @@ -5,4 +5,5 @@ common-obj-$(CONFIG_APM) += pm_smbus.o
>>  common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
>>  common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
>>  common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
>> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o
>>  obj-$(CONFIG_OMAP) += omap_i2c.o
>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
>> new file mode 100644
>> index 000000000000..f512e64d827a
>> --- /dev/null
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -0,0 +1,448 @@
>> +/*
>> + * ARM Aspeed I2C controller
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see .
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>> +
>> +/* I2C Global Register */
>> +
>> +#define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
>> +#define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
>> +                                               Assignment */
>> +
>> +/* I2C Device (Bus) Register */
>> +
>> +#define I2CD_FUN_CTRL_REG       0x00       /* I2CD Function Control  */
>> +#define   I2CD_BUFF_SEL_MASK               (0x7 << 20)
>> +#define   I2CD_BUFF_SEL(x)                 (x << 20)
>> +#define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
>> +#define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
>> +#define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
>> +#define   I2CD_MSB_STS                     (0x1 << 9)
>> +#define   I2CD_SDA_DRIVE_1T_EN             (0x1 << 8)
>> +#define   I2CD_M_SDA_DRIVE_1T_EN           (0x1 << 7)
>> +#define   I2CD_M_HIGH_SPEED_EN             (0x1 << 6)
>> +#define   I2CD_DEF_ADDR_EN                 (0x1 << 5)
>> +#define   I2CD_DEF_ALERT_EN                (0x1 << 4)
>> +#define   I2CD_DEF_ARP_EN                  (0x1 << 3)
>> +#define   I2CD_DEF_GCALL_EN                (0x1 << 2)
>> +#define   I2CD_SLAVE_EN                    (0x1 << 1)
>> +#define   I2CD_MASTER_EN                   (0x1)
>> +
>> +#define I2CD_AC_TIMING_REG1     0x04       /* Clock and AC Timing Control #1 */
>> +#define I2CD_AC_TIMING_REG2     0x08       /* Clock and AC Timing Control #1 */
>> +#define I2CD_INTR_CTRL_REG      0x0c       /* I2CD Interrupt Control */
>> +#define I2CD_INTR_STS_REG       0x10       /* I2CD Interrupt Status */
>> +#define   I2CD_INTR_SDA_DL_TIMEOUT         (0x1 << 14)
>> +#define   I2CD_INTR_BUS_RECOVER_DONE       (0x1 << 13)
>> +#define   I2CD_INTR_SMBUS_ALERT            (0x1 << 12) /* Bus [0-3] only */
>> +#define   I2CD_INTR_SMBUS_ARP_ADDR         (0x1 << 11) /* Removed */
>> +#define   I2CD_INTR_SMBUS_DEV_ALERT_ADDR   (0x1 << 10) /* Removed */
>> +#define   I2CD_INTR_SMBUS_DEF_ADDR         (0x1 << 9)  /* Removed */
>> +#define   I2CD_INTR_GCALL_ADDR             (0x1 << 8)  /* Removed */
>> +#define   I2CD_INTR_SLAVE_MATCH            (0x1 << 7)  /* use RX_DONE */
>> +#define   I2CD_INTR_SCL_TIMEOUT            (0x1 << 6)
>> +#define   I2CD_INTR_ABNORMAL               (0x1 << 5)
>> +#define   I2CD_INTR_NORMAL_STOP            (0x1 << 4)
>> +#define   I2CD_INTR_ARBIT_LOSS             (0x1 << 3)
>> +#define   I2CD_INTR_RX_DONE                (0x1 << 2)
>> +#define   I2CD_INTR_TX_NAK                 (0x1 << 1)
>> +#define   I2CD_INTR_TX_ACK                 (0x1 << 0)
>> +
>> +#define I2CD_CMD_REG            0x14       /* I2CD Command/Status */
>> +#define   I2CD_SDA_OE                      (0x1 << 28)
>> +#define   I2CD_SDA_O                       (0x1 << 27)
>> +#define   I2CD_SCL_OE                      (0x1 << 26)
>> +#define   I2CD_SCL_O                       (0x1 << 25)
>> +#define   I2CD_TX_TIMING                   (0x1 << 24)
>> +#define   I2CD_TX_STATUS                   (0x1 << 23)
>> +
>> +#define   I2CD_TX_STATE_SHIFT              19 /* Tx State Machine */
>> +#define   I2CD_TX_STATE_MASK                  0xf
>> +#define     I2CD_IDLE                         0x0
>> +#define     I2CD_MACTIVE                      0x8
>> +#define     I2CD_MSTART                       0x9
>> +#define     I2CD_MSTARTR                      0xa
>> +#define     I2CD_MSTOP                        0xb
>> +#define     I2CD_MTXD                         0xc
>> +#define     I2CD_MRXACK                       0xd
>> +#define     I2CD_MRXD                         0xe
>> +#define     I2CD_MTXACK                       0xf
>> +#define     I2CD_SWAIT                        0x1
>> +#define     I2CD_SRXD                         0x4
>> +#define     I2CD_STXACK                       0x5
>> +#define     I2CD_STXD                         0x6
>> +#define     I2CD_SRXACK                       0x7
>> +#define     I2CD_RECOVER                      0x3
>> +
>> +#define   I2CD_SCL_LINE_STS                (0x1 << 18)
>> +#define   I2CD_SDA_LINE_STS                (0x1 << 17)
>> +#define   I2CD_BUS_BUSY_STS                (0x1 << 16)
>> +#define   I2CD_SDA_OE_OUT_DIR              (0x1 << 15)
>> +#define   I2CD_SDA_O_OUT_DIR               (0x1 << 14)
>> +#define   I2CD_SCL_OE_OUT_DIR              (0x1 << 13)
>> +#define   I2CD_SCL_O_OUT_DIR               (0x1 << 12)
>> +#define   I2CD_BUS_RECOVER_CMD_EN          (0x1 << 11)
>> +#define   I2CD_S_ALT_EN                    (0x1 << 10)
>> +#define   I2CD_RX_DMA_ENABLE               (0x1 << 9)
>> +#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
>> +
>> +/* Command Bit */
>> +#define   I2CD_M_STOP_CMD                  (0x1 << 5)
>> +#define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
>> +#define   I2CD_M_RX_CMD                    (0x1 << 3)
>> +#define   I2CD_S_TX_CMD                    (0x1 << 2)
>> +#define   I2CD_M_TX_CMD                    (0x1 << 1)
>> +#define   I2CD_M_START_CMD                 (0x1)
>> +
>> +#define I2CD_DEV_ADDR_REG       0x18       /* Slave Device Address */
>> +#define I2CD_BUF_CTRL_REG       0x1c       /* Pool Buffer Control */
>> +#define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
>> +#define   I2CD_BYTE_BUF_TX_SHIFT           0
>> +#define   I2CD_BYTE_BUF_TX_MASK            0xff
>> +#define   I2CD_BYTE_BUF_RX_SHIFT           8
>> +#define   I2CD_BYTE_BUF_RX_MASK            0xff
>> +
>> +
>> +static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
>> +{
>> +    return bus->ctrl & I2CD_MASTER_EN;
>> +}
>> +
>> +static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus *bus)
>> +{
>> +    return bus->ctrl & (I2CD_MASTER_EN | I2CD_SLAVE_EN);
>> +}
>> +
>> +static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
>> +{
>> +    bus->intr_status &= bus->intr_ctrl;
>> +    if (bus->intr_status) {
>> +        bus->controller->intr_status |= 1 << bus->id;
>> +        qemu_irq_raise(bus->controller->irq);
>> +    }
>> +}
>> +
>> +static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
>> +                                    unsigned size)
>> +{
>> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
> 
> Nitpick - we can avoid the cast here.
> 
>> +
>> +    switch (offset) {
>> +    case I2CD_FUN_CTRL_REG:
>> +        return bus->ctrl;
>> +    case I2CD_AC_TIMING_REG1:
>> +        return bus->timing[0];
>> +    case I2CD_AC_TIMING_REG2:
>> +        return bus->timing[1];
>> +    case I2CD_INTR_CTRL_REG:
>> +        return bus->intr_ctrl;
>> +    case I2CD_INTR_STS_REG:
>> +        return bus->intr_status;
>> +    case I2CD_BYTE_BUF_REG:
>> +        return bus->buf;
>> +    case I2CD_CMD_REG:
>> +        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
>> +        return -1;
>> +    }
>> +}
>> +
>> +static inline uint64_t aspeed_i2c_bus_get_state(AspeedI2CBus *bus)
>> +{
>> +    return bus->cmd >> 19 & 0xF;
>> +}
>> +
>> +static inline void aspeed_i2c_bus_set_state(AspeedI2CBus *bus, uint64_t value)
>> +{
>> +    bus->cmd |= (value & 0xF) << 19;
>> +}
>> +
>> +static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>> +{
>> +    bus->cmd |= value & 0xFFFF;
>> +    bus->intr_status = 0;
>> +
>> +    if (bus->cmd & I2CD_M_START_CMD) {
>> +        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
>> +                               extract32(bus->buf, 0, 1))) {
>> +            bus->intr_status |= I2CD_INTR_TX_NAK;
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_TX_ACK;
>> +        }
>> +
>> +    } else if (bus->cmd & I2CD_M_TX_CMD) {
>> +        if (i2c_send(bus->bus, bus->buf)) {
>> +            bus->intr_status |= (I2CD_INTR_TX_NAK | I2CD_INTR_ABNORMAL);
>> +            i2c_end_transfer(bus->bus);
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_TX_ACK;
>> +        }
>> +
>> +    } else if (bus->cmd & I2CD_M_RX_CMD) {
>> +        int ret = i2c_recv(bus->bus);
>> +        if (ret < 0) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__);
>> +            ret = 0xff;
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_RX_DONE;
>> +        }
>> +        bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
>> +    }
>> +
>> +    if (bus->cmd & (I2CD_M_STOP_CMD | I2CD_M_S_RX_CMD_LAST)) {
>> +        if (!i2c_bus_busy(bus->bus)) {
>> +            bus->intr_status |= I2CD_INTR_ABNORMAL;
>> +        } else {
>> +            i2c_end_transfer(bus->bus);
>> +            bus->intr_status |= I2CD_INTR_NORMAL_STOP;
>> +        }
>> +    }
>> +
>> +    /* command is handled, reset it and check for interrupts  */
>> +    bus->cmd &= ~0xFFFF;
>> +    aspeed_i2c_bus_raise_interrupt(bus);
>> +}
>> +
>> +static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
>> +                                 uint64_t value, unsigned size)
>> +{
>> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
> 
> As above
> 
>> +
>> +    switch (offset) {
>> +    case I2CD_FUN_CTRL_REG:
>> +        if (value & I2CD_SLAVE_EN) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                          __func__);
>> +            break;
>> +        }
>> +        bus->ctrl = value & 0x0071C3FF;
>> +        break;
>> +    case I2CD_AC_TIMING_REG1:
>> +        bus->timing[0] = value & 0xFFFFF0F;
>> +        break;
>> +    case I2CD_AC_TIMING_REG2:
>> +        bus->timing[1] = value & 0x7;
>> +        break;
>> +    case I2CD_INTR_CTRL_REG:
>> +        bus->intr_ctrl = value & 0x7FFF;
>> +        break;
>> +    case I2CD_INTR_STS_REG:
>> +        bus->intr_status &= ~(value & 0x7FFF);
>> +        bus->controller->intr_status &= ~(1 << bus->id);
>> +        qemu_irq_lower(bus->controller->irq);
>> +        break;
>> +    case I2CD_DEV_ADDR_REG:
>> +        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                      __func__);
>> +        break;
>> +    case I2CD_BYTE_BUF_REG:
>> +        bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
>> +        break;
>> +    case I2CD_CMD_REG:
>> +        if (!aspeed_i2c_bus_is_enabled(bus)) {
>> +            break;
>> +        }
>> +
>> +        if (!aspeed_i2c_bus_is_master(bus)) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                          __func__);
>> +            break;
>> +        }
>> +
>> +        aspeed_i2c_bus_handle_cmd(bus, value);
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +    }
>> +}
>> +
>> +static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
>> +                                   unsigned size)
>> +{
>> +    AspeedI2CState *s = (AspeedI2CState *)opaque;
> 
> As above
> 
>> +
>> +    switch (offset) {
>> +    case I2C_CTRL_STATUS:
>> +        return s->intr_status;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +        break;
>> +    }
>> +
>> +    return -1;
>> +}
>> +
>> +static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
>> +                                  uint64_t value, unsigned size)
>> +{
>> +    switch (offset) {
>> +    case I2C_CTRL_STATUS:
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_i2c_bus_ops = {
>> +    .read = aspeed_i2c_bus_read,
>> +    .write = aspeed_i2c_bus_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
> 
> Alexey suggested I make these DEVICE_LITTLE_ENDIAN when he looked at my
> timer and intc devices. We should be consistent one way or the other -
> what's the right approach?
> 
>> +};
>> +
>> +static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
>> +    .read = aspeed_i2c_ctrl_read,
>> +    .write = aspeed_i2c_ctrl_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static const VMStateDescription aspeed_i2c_bus_vmstate = {
>> +    .name = TYPE_ASPEED_I2C,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT8(id, AspeedI2CBus),
>> +        VMSTATE_UINT32(ctrl, AspeedI2CBus),
>> +        VMSTATE_UINT32_ARRAY(timing, AspeedI2CBus, 2),
>> +        VMSTATE_UINT32(intr_ctrl, AspeedI2CBus),
>> +        VMSTATE_UINT32(intr_status, AspeedI2CBus),
>> +        VMSTATE_UINT32(cmd, AspeedI2CBus),
>> +        VMSTATE_UINT32(buf, AspeedI2CBus),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static const VMStateDescription aspeed_i2c_vmstate = {
>> +    .name = TYPE_ASPEED_I2C,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(intr_status, AspeedI2CState),
>> +        VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
>> +                             ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
>> +                             AspeedI2CBus),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void aspeed_i2c_reset(DeviceState *dev)
>> +{
>> +    int i;
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +
>> +    s->intr_status = 0;
>> +
>> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
>> +        s->busses[i].intr_status = 0;
> 
> Should we also be resetting .intr_ctrl = 0? Datasheet says that's the
> init value.
> 
>> +        s->busses[i].cmd = 0;
>> +        s->busses[i].buf = 0;
>> +        i2c_end_transfer(s->busses[i].bus);
>> +    }
>> +}
>> +
>> +/*
>> + * Address Definitions
>> + *
>> + *   0x000 ... 0x03F: Global Register
>> + *   0x040 ... 0x07F: Device 1
>> + *   0x080 ... 0x0BF: Device 2
>> + *   0x0C0 ... 0x0FF: Device 3
>> + *   0x100 ... 0x13F: Device 4
>> + *   0x140 ... 0x17F: Device 5
>> + *   0x180 ... 0x1BF: Device 6
>> + *   0x1C0 ... 0x1FF: Device 7
>> + *   0x200 ... 0x2FF: Buffer Pool  (unused in linux driver)
>> + *   0x300 ... 0x33F: Device 8
>> + *   0x340 ... 0x37F: Device 9
>> + *   0x380 ... 0x3BF: Device 10
>> + *   0x3C0 ... 0x3FF: Device 11
>> + *   0x400 ... 0x43F: Device 12
>> + *   0x440 ... 0x47F: Device 13
>> + *   0x480 ... 0x4BF: Device 14
>> + *   0x800 ... 0xFFF: Buffer Pool  (unused in linux driver)
>> + */
>> +static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
>> +{
>> +    int i;
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +
>> +    sysbus_init_irq(sbd, &s->irq);
>> +    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i2c_ctrl_ops, s,
>> +                          "aspeed.i2c", 0x1000);
>> +    sysbus_init_mmio(sbd, &s->iomem);
>> +
>> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
>> +        char name[16];
>> +        int offset = i < 7 ? 1 : 5;
>> +        snprintf(name, sizeof(name), "aspeed.i2c.%d", i);
>> +        s->busses[i].controller = s;
>> +        s->busses[i].id = i;
>> +        s->busses[i].bus = i2c_init_bus(dev, name);
>> +        memory_region_init_io(&s->busses[i].mr, OBJECT(dev),
>> +                              &aspeed_i2c_bus_ops, &s->busses[i], name, 0x40);
>> +        memory_region_add_subregion(&s->iomem, 0x40 * (i + offset),
>> +                                    &s->busses[i].mr);
>> +    }
>> +}
>> +
>> +static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->vmsd = &aspeed_i2c_vmstate;
>> +    dc->reset = aspeed_i2c_reset;
>> +    dc->realize = aspeed_i2c_realize;
>> +    dc->desc = "Aspeed I2C Controller";
>> +}
>> +
>> +static const TypeInfo aspeed_i2c_info = {
>> +    .name          = TYPE_ASPEED_I2C,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(AspeedI2CState),
>> +    .class_init    = aspeed_i2c_class_init,
>> +};
>> +
>> +static void aspeed_i2c_register_types(void)
>> +{
>> +    type_register_static(&aspeed_i2c_info);
>> +}
>> +
>> +type_init(aspeed_i2c_register_types)
>> +
>> +
>> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr)
>> +{
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +    I2CBus *bus = NULL;
>> +
>> +    if (busnr >= 0 && busnr < ASPEED_I2C_NR_BUSSES) {
>> +        bus = s->busses[busnr].bus;
>> +    }
>> +
>> +    return bus;
>> +}
>> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
>> index cf7102145355..e96e3db3fbea 100644
>> --- a/include/hw/arm/ast2400.h
>> +++ b/include/hw/arm/ast2400.h
>> @@ -16,6 +16,7 @@
>>  #include "hw/intc/aspeed_vic.h"
>>  #include "hw/misc/aspeed_scu.h"
>>  #include "hw/timer/aspeed_timer.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>>  
>>  typedef struct AST2400State {
>>      /*< private >*/
>> @@ -28,6 +29,7 @@ typedef struct AST2400State {
>>      AspeedVICState vic;
>>      AspeedTimerCtrlState timerctrl;
>>      AspeedSCUState scu;
>> +    AspeedI2CState i2c;
>>  } AST2400State;
>>  
>>  #define TYPE_AST2400 "ast2400"
>> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
>> new file mode 100644
>> index 000000000000..f9020acdef30
>> --- /dev/null
>> +++ b/include/hw/i2c/aspeed_i2c.h
>> @@ -0,0 +1,62 @@
>> +/*
>> + *  ASPEED AST2400 I2C Controller
>> + *
>> + *  Copyright (C) 2016 IBM Corp.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +#ifndef ASPEED_I2C_H
>> +#define ASPEED_I2C_H
>> +
>> +#include "hw/i2c/i2c.h"
>> +
>> +#define TYPE_ASPEED_I2C "aspeed.i2c"
>> +#define ASPEED_I2C(obj) \
>> +    OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
>> +
>> +#define ASPEED_I2C_NR_BUSSES 14
>> +
>> +struct AspeedI2CState;
>> +
>> +typedef struct AspeedI2CBus {
>> +    struct AspeedI2CState *controller;
>> +
>> +    MemoryRegion mr;
>> +
>> +    I2CBus *bus;
>> +    uint8_t id;
>> +
>> +    uint32_t ctrl;
>> +    uint32_t timing[2];
>> +    uint32_t intr_ctrl;
>> +    uint32_t intr_status;
>> +    uint32_t cmd;
>> +    uint32_t buf;
>> +} AspeedI2CBus;
>> +
>> +typedef struct AspeedI2CState {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    uint32_t intr_status;
>> +
>> +    AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
>> +} AspeedI2CState;
>> +
>> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
>> +
>> +#endif /* ASPEED_I2C_H */
> 
> Cheers,
> 
> Andrew
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-18 13:26   ` Joel Stanley
@ 2016-05-19 16:56     ` Cédric Le Goater
  0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2016-05-19 16:56 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Andrew Jeffery

Hello Joel,

On 05/18/2016 03:26 PM, Joel Stanley wrote:
> Hey Cedric,
> 
> On Wed, May 18, 2016 at 4:55 AM, Cédric Le Goater <clg@kaod.org> wrote:
>> The Aspeed AST2400 integrates a set of 14 I2C/SMBus bus controllers
>> directly connected to APB bus which can be programmed as a master or
>> slave.
> 
> Nice work!
> 
>>
>> This patch offers a device model for the master mode only, slave mode
>> is not supported.
> 
> Cool. That's good enough for us to emulate a barreleye or palemtto.
> 
> We should look to extend it to support this as well as multi-byte
> transfers using the pool buffer soon; both the model and the driver.

Yes. That will help for the transfers with the OCC, which are quite
heavy.

Thanks,

C.

> 
> I had a read of the code and it looks good to me for a first pass.
> I'll let Andrew comment further and merge it into our staging tree.
> 
>> On the TODO list, we also have :
>>
>>  - improve and harden the state machine. This is really a first
>>    version.
>>  - bus recovery support (used by the Linux driver).
>>  - transfer mode state machine bits. this is not strictly necessary as
>>    it is mostly used for debug. The bus busy bit is deducted from the
>>    I2C core engine of qemu.
>>  - support of the pool buffer: 2048 bytes of internal SRAM (not used
>>    by the Linux driver).
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Acked-by: Joel Stanley <joel@jms.id.au>
> 
>> ---
>>  hw/arm/ast2400.c            |  16 ++
>>  hw/i2c/Makefile.objs        |   1 +
>>  hw/i2c/aspeed_i2c.c         | 448 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/arm/ast2400.h    |   2 +
>>  include/hw/i2c/aspeed_i2c.h |  62 ++++++
>>  5 files changed, 529 insertions(+)
>>  create mode 100644 hw/i2c/aspeed_i2c.c
>>  create mode 100644 include/hw/i2c/aspeed_i2c.h
>>
>> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
>> index cf9f37e6a301..4576e67cd483 100644
>> --- a/hw/arm/ast2400.c
>> +++ b/hw/arm/ast2400.c
>> @@ -18,6 +18,7 @@
>>  #include "hw/arm/ast2400.h"
>>  #include "hw/char/serial.h"
>>  #include "hw/boards.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>>
>>  #define AST2400_UART_5_BASE      0x00184000
>>  #define AST2400_IOMEM_SIZE       0x00200000
>> @@ -25,6 +26,7 @@
>>  #define AST2400_VIC_BASE         0x1E6C0000
>>  #define AST2400_SCU_BASE         0x1E6E2000
>>  #define AST2400_TIMER_BASE       0x1E782000
>> +#define AST2400_I2C_BASE         0x1E78A000
>>
>>  static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>> @@ -71,6 +73,10 @@ static void ast2400_init(Object *obj)
>>      object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
>>      object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
>>      qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
>> +
>> +    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
>> +    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
>> +    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
>>  }
>>
>>  static void ast2400_realize(DeviceState *dev, Error **errp)
>> @@ -143,6 +149,16 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
>>          serial_mm_init(&s->iomem, AST2400_UART_5_BASE, 2,
>>                         uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
>>      }
>> +
>> +    /* I2C */
>> +    object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>> +                       qdev_get_gpio_in(DEVICE(&s->vic), 12));
>>  }
>>
>>  static void ast2400_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
>> index aeb8f38d70f1..1fd54edf4c23 100644
>> --- a/hw/i2c/Makefile.objs
>> +++ b/hw/i2c/Makefile.objs
>> @@ -5,4 +5,5 @@ common-obj-$(CONFIG_APM) += pm_smbus.o
>>  common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
>>  common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
>>  common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
>> +common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o
>>  obj-$(CONFIG_OMAP) += omap_i2c.o
>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
>> new file mode 100644
>> index 000000000000..f512e64d827a
>> --- /dev/null
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -0,0 +1,448 @@
>> +/*
>> + * ARM Aspeed I2C controller
>> + *
>> + * Copyright (C) 2016 IBM Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>> +
>> +/* I2C Global Register */
>> +
>> +#define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
>> +#define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
>> +                                               Assignment */
>> +
>> +/* I2C Device (Bus) Register */
>> +
>> +#define I2CD_FUN_CTRL_REG       0x00       /* I2CD Function Control  */
>> +#define   I2CD_BUFF_SEL_MASK               (0x7 << 20)
>> +#define   I2CD_BUFF_SEL(x)                 (x << 20)
>> +#define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
>> +#define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
>> +#define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
>> +#define   I2CD_MSB_STS                     (0x1 << 9)
>> +#define   I2CD_SDA_DRIVE_1T_EN             (0x1 << 8)
>> +#define   I2CD_M_SDA_DRIVE_1T_EN           (0x1 << 7)
>> +#define   I2CD_M_HIGH_SPEED_EN             (0x1 << 6)
>> +#define   I2CD_DEF_ADDR_EN                 (0x1 << 5)
>> +#define   I2CD_DEF_ALERT_EN                (0x1 << 4)
>> +#define   I2CD_DEF_ARP_EN                  (0x1 << 3)
>> +#define   I2CD_DEF_GCALL_EN                (0x1 << 2)
>> +#define   I2CD_SLAVE_EN                    (0x1 << 1)
>> +#define   I2CD_MASTER_EN                   (0x1)
>> +
>> +#define I2CD_AC_TIMING_REG1     0x04       /* Clock and AC Timing Control #1 */
>> +#define I2CD_AC_TIMING_REG2     0x08       /* Clock and AC Timing Control #1 */
>> +#define I2CD_INTR_CTRL_REG      0x0c       /* I2CD Interrupt Control */
>> +#define I2CD_INTR_STS_REG       0x10       /* I2CD Interrupt Status */
>> +#define   I2CD_INTR_SDA_DL_TIMEOUT         (0x1 << 14)
>> +#define   I2CD_INTR_BUS_RECOVER_DONE       (0x1 << 13)
>> +#define   I2CD_INTR_SMBUS_ALERT            (0x1 << 12) /* Bus [0-3] only */
>> +#define   I2CD_INTR_SMBUS_ARP_ADDR         (0x1 << 11) /* Removed */
>> +#define   I2CD_INTR_SMBUS_DEV_ALERT_ADDR   (0x1 << 10) /* Removed */
>> +#define   I2CD_INTR_SMBUS_DEF_ADDR         (0x1 << 9)  /* Removed */
>> +#define   I2CD_INTR_GCALL_ADDR             (0x1 << 8)  /* Removed */
>> +#define   I2CD_INTR_SLAVE_MATCH            (0x1 << 7)  /* use RX_DONE */
>> +#define   I2CD_INTR_SCL_TIMEOUT            (0x1 << 6)
>> +#define   I2CD_INTR_ABNORMAL               (0x1 << 5)
>> +#define   I2CD_INTR_NORMAL_STOP            (0x1 << 4)
>> +#define   I2CD_INTR_ARBIT_LOSS             (0x1 << 3)
>> +#define   I2CD_INTR_RX_DONE                (0x1 << 2)
>> +#define   I2CD_INTR_TX_NAK                 (0x1 << 1)
>> +#define   I2CD_INTR_TX_ACK                 (0x1 << 0)
>> +
>> +#define I2CD_CMD_REG            0x14       /* I2CD Command/Status */
>> +#define   I2CD_SDA_OE                      (0x1 << 28)
>> +#define   I2CD_SDA_O                       (0x1 << 27)
>> +#define   I2CD_SCL_OE                      (0x1 << 26)
>> +#define   I2CD_SCL_O                       (0x1 << 25)
>> +#define   I2CD_TX_TIMING                   (0x1 << 24)
>> +#define   I2CD_TX_STATUS                   (0x1 << 23)
>> +
>> +#define   I2CD_TX_STATE_SHIFT              19 /* Tx State Machine */
>> +#define   I2CD_TX_STATE_MASK                  0xf
>> +#define     I2CD_IDLE                         0x0
>> +#define     I2CD_MACTIVE                      0x8
>> +#define     I2CD_MSTART                       0x9
>> +#define     I2CD_MSTARTR                      0xa
>> +#define     I2CD_MSTOP                        0xb
>> +#define     I2CD_MTXD                         0xc
>> +#define     I2CD_MRXACK                       0xd
>> +#define     I2CD_MRXD                         0xe
>> +#define     I2CD_MTXACK                       0xf
>> +#define     I2CD_SWAIT                        0x1
>> +#define     I2CD_SRXD                         0x4
>> +#define     I2CD_STXACK                       0x5
>> +#define     I2CD_STXD                         0x6
>> +#define     I2CD_SRXACK                       0x7
>> +#define     I2CD_RECOVER                      0x3
>> +
>> +#define   I2CD_SCL_LINE_STS                (0x1 << 18)
>> +#define   I2CD_SDA_LINE_STS                (0x1 << 17)
>> +#define   I2CD_BUS_BUSY_STS                (0x1 << 16)
>> +#define   I2CD_SDA_OE_OUT_DIR              (0x1 << 15)
>> +#define   I2CD_SDA_O_OUT_DIR               (0x1 << 14)
>> +#define   I2CD_SCL_OE_OUT_DIR              (0x1 << 13)
>> +#define   I2CD_SCL_O_OUT_DIR               (0x1 << 12)
>> +#define   I2CD_BUS_RECOVER_CMD_EN          (0x1 << 11)
>> +#define   I2CD_S_ALT_EN                    (0x1 << 10)
>> +#define   I2CD_RX_DMA_ENABLE               (0x1 << 9)
>> +#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
>> +
>> +/* Command Bit */
>> +#define   I2CD_M_STOP_CMD                  (0x1 << 5)
>> +#define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
>> +#define   I2CD_M_RX_CMD                    (0x1 << 3)
>> +#define   I2CD_S_TX_CMD                    (0x1 << 2)
>> +#define   I2CD_M_TX_CMD                    (0x1 << 1)
>> +#define   I2CD_M_START_CMD                 (0x1)
>> +
>> +#define I2CD_DEV_ADDR_REG       0x18       /* Slave Device Address */
>> +#define I2CD_BUF_CTRL_REG       0x1c       /* Pool Buffer Control */
>> +#define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
>> +#define   I2CD_BYTE_BUF_TX_SHIFT           0
>> +#define   I2CD_BYTE_BUF_TX_MASK            0xff
>> +#define   I2CD_BYTE_BUF_RX_SHIFT           8
>> +#define   I2CD_BYTE_BUF_RX_MASK            0xff
>> +
>> +
>> +static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
>> +{
>> +    return bus->ctrl & I2CD_MASTER_EN;
>> +}
>> +
>> +static inline bool aspeed_i2c_bus_is_enabled(AspeedI2CBus *bus)
>> +{
>> +    return bus->ctrl & (I2CD_MASTER_EN | I2CD_SLAVE_EN);
>> +}
>> +
>> +static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
>> +{
>> +    bus->intr_status &= bus->intr_ctrl;
>> +    if (bus->intr_status) {
>> +        bus->controller->intr_status |= 1 << bus->id;
>> +        qemu_irq_raise(bus->controller->irq);
>> +    }
>> +}
>> +
>> +static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
>> +                                    unsigned size)
>> +{
>> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
>> +
>> +    switch (offset) {
>> +    case I2CD_FUN_CTRL_REG:
>> +        return bus->ctrl;
>> +    case I2CD_AC_TIMING_REG1:
>> +        return bus->timing[0];
>> +    case I2CD_AC_TIMING_REG2:
>> +        return bus->timing[1];
>> +    case I2CD_INTR_CTRL_REG:
>> +        return bus->intr_ctrl;
>> +    case I2CD_INTR_STS_REG:
>> +        return bus->intr_status;
>> +    case I2CD_BYTE_BUF_REG:
>> +        return bus->buf;
>> +    case I2CD_CMD_REG:
>> +        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
>> +        return -1;
>> +    }
>> +}
>> +
>> +static inline uint64_t aspeed_i2c_bus_get_state(AspeedI2CBus *bus)
>> +{
>> +    return bus->cmd >> 19 & 0xF;
>> +}
>> +
>> +static inline void aspeed_i2c_bus_set_state(AspeedI2CBus *bus, uint64_t value)
>> +{
>> +    bus->cmd |= (value & 0xF) << 19;
>> +}
>> +
>> +static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>> +{
>> +    bus->cmd |= value & 0xFFFF;
>> +    bus->intr_status = 0;
>> +
>> +    if (bus->cmd & I2CD_M_START_CMD) {
>> +        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
>> +                               extract32(bus->buf, 0, 1))) {
>> +            bus->intr_status |= I2CD_INTR_TX_NAK;
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_TX_ACK;
>> +        }
>> +
>> +    } else if (bus->cmd & I2CD_M_TX_CMD) {
>> +        if (i2c_send(bus->bus, bus->buf)) {
>> +            bus->intr_status |= (I2CD_INTR_TX_NAK | I2CD_INTR_ABNORMAL);
>> +            i2c_end_transfer(bus->bus);
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_TX_ACK;
>> +        }
>> +
>> +    } else if (bus->cmd & I2CD_M_RX_CMD) {
>> +        int ret = i2c_recv(bus->bus);
>> +        if (ret < 0) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__);
>> +            ret = 0xff;
>> +        } else {
>> +            bus->intr_status |= I2CD_INTR_RX_DONE;
>> +        }
>> +        bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
>> +    }
>> +
>> +    if (bus->cmd & (I2CD_M_STOP_CMD | I2CD_M_S_RX_CMD_LAST)) {
>> +        if (!i2c_bus_busy(bus->bus)) {
>> +            bus->intr_status |= I2CD_INTR_ABNORMAL;
>> +        } else {
>> +            i2c_end_transfer(bus->bus);
>> +            bus->intr_status |= I2CD_INTR_NORMAL_STOP;
>> +        }
>> +    }
>> +
>> +    /* command is handled, reset it and check for interrupts  */
>> +    bus->cmd &= ~0xFFFF;
>> +    aspeed_i2c_bus_raise_interrupt(bus);
>> +}
>> +
>> +static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
>> +                                 uint64_t value, unsigned size)
>> +{
>> +    AspeedI2CBus *bus = (AspeedI2CBus *) opaque;
>> +
>> +    switch (offset) {
>> +    case I2CD_FUN_CTRL_REG:
>> +        if (value & I2CD_SLAVE_EN) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                          __func__);
>> +            break;
>> +        }
>> +        bus->ctrl = value & 0x0071C3FF;
>> +        break;
>> +    case I2CD_AC_TIMING_REG1:
>> +        bus->timing[0] = value & 0xFFFFF0F;
>> +        break;
>> +    case I2CD_AC_TIMING_REG2:
>> +        bus->timing[1] = value & 0x7;
>> +        break;
>> +    case I2CD_INTR_CTRL_REG:
>> +        bus->intr_ctrl = value & 0x7FFF;
>> +        break;
>> +    case I2CD_INTR_STS_REG:
>> +        bus->intr_status &= ~(value & 0x7FFF);
>> +        bus->controller->intr_status &= ~(1 << bus->id);
>> +        qemu_irq_lower(bus->controller->irq);
>> +        break;
>> +    case I2CD_DEV_ADDR_REG:
>> +        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                      __func__);
>> +        break;
>> +    case I2CD_BYTE_BUF_REG:
>> +        bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
>> +        break;
>> +    case I2CD_CMD_REG:
>> +        if (!aspeed_i2c_bus_is_enabled(bus)) {
>> +            break;
>> +        }
>> +
>> +        if (!aspeed_i2c_bus_is_master(bus)) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
>> +                          __func__);
>> +            break;
>> +        }
>> +
>> +        aspeed_i2c_bus_handle_cmd(bus, value);
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +    }
>> +}
>> +
>> +static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
>> +                                   unsigned size)
>> +{
>> +    AspeedI2CState *s = (AspeedI2CState *)opaque;
>> +
>> +    switch (offset) {
>> +    case I2C_CTRL_STATUS:
>> +        return s->intr_status;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +        break;
>> +    }
>> +
>> +    return -1;
>> +}
>> +
>> +static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
>> +                                  uint64_t value, unsigned size)
>> +{
>> +    switch (offset) {
>> +    case I2C_CTRL_STATUS:
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
>> +                      __func__, offset);
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_i2c_bus_ops = {
>> +    .read = aspeed_i2c_bus_read,
>> +    .write = aspeed_i2c_bus_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
>> +    .read = aspeed_i2c_ctrl_read,
>> +    .write = aspeed_i2c_ctrl_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static const VMStateDescription aspeed_i2c_bus_vmstate = {
>> +    .name = TYPE_ASPEED_I2C,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT8(id, AspeedI2CBus),
>> +        VMSTATE_UINT32(ctrl, AspeedI2CBus),
>> +        VMSTATE_UINT32_ARRAY(timing, AspeedI2CBus, 2),
>> +        VMSTATE_UINT32(intr_ctrl, AspeedI2CBus),
>> +        VMSTATE_UINT32(intr_status, AspeedI2CBus),
>> +        VMSTATE_UINT32(cmd, AspeedI2CBus),
>> +        VMSTATE_UINT32(buf, AspeedI2CBus),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static const VMStateDescription aspeed_i2c_vmstate = {
>> +    .name = TYPE_ASPEED_I2C,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(intr_status, AspeedI2CState),
>> +        VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
>> +                             ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
>> +                             AspeedI2CBus),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void aspeed_i2c_reset(DeviceState *dev)
>> +{
>> +    int i;
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +
>> +    s->intr_status = 0;
>> +
>> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
>> +        s->busses[i].intr_status = 0;
>> +        s->busses[i].cmd = 0;
>> +        s->busses[i].buf = 0;
>> +        i2c_end_transfer(s->busses[i].bus);
>> +    }
>> +}
>> +
>> +/*
>> + * Address Definitions
>> + *
>> + *   0x000 ... 0x03F: Global Register
>> + *   0x040 ... 0x07F: Device 1
>> + *   0x080 ... 0x0BF: Device 2
>> + *   0x0C0 ... 0x0FF: Device 3
>> + *   0x100 ... 0x13F: Device 4
>> + *   0x140 ... 0x17F: Device 5
>> + *   0x180 ... 0x1BF: Device 6
>> + *   0x1C0 ... 0x1FF: Device 7
>> + *   0x200 ... 0x2FF: Buffer Pool  (unused in linux driver)
>> + *   0x300 ... 0x33F: Device 8
>> + *   0x340 ... 0x37F: Device 9
>> + *   0x380 ... 0x3BF: Device 10
>> + *   0x3C0 ... 0x3FF: Device 11
>> + *   0x400 ... 0x43F: Device 12
>> + *   0x440 ... 0x47F: Device 13
>> + *   0x480 ... 0x4BF: Device 14
>> + *   0x800 ... 0xFFF: Buffer Pool  (unused in linux driver)
>> + */
>> +static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
>> +{
>> +    int i;
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +
>> +    sysbus_init_irq(sbd, &s->irq);
>> +    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_i2c_ctrl_ops, s,
>> +                          "aspeed.i2c", 0x1000);
>> +    sysbus_init_mmio(sbd, &s->iomem);
>> +
>> +    for (i = 0; i < ASPEED_I2C_NR_BUSSES; i++) {
>> +        char name[16];
>> +        int offset = i < 7 ? 1 : 5;
>> +        snprintf(name, sizeof(name), "aspeed.i2c.%d", i);
>> +        s->busses[i].controller = s;
>> +        s->busses[i].id = i;
>> +        s->busses[i].bus = i2c_init_bus(dev, name);
>> +        memory_region_init_io(&s->busses[i].mr, OBJECT(dev),
>> +                              &aspeed_i2c_bus_ops, &s->busses[i], name, 0x40);
>> +        memory_region_add_subregion(&s->iomem, 0x40 * (i + offset),
>> +                                    &s->busses[i].mr);
>> +    }
>> +}
>> +
>> +static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->vmsd = &aspeed_i2c_vmstate;
>> +    dc->reset = aspeed_i2c_reset;
>> +    dc->realize = aspeed_i2c_realize;
>> +    dc->desc = "Aspeed I2C Controller";
>> +}
>> +
>> +static const TypeInfo aspeed_i2c_info = {
>> +    .name          = TYPE_ASPEED_I2C,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(AspeedI2CState),
>> +    .class_init    = aspeed_i2c_class_init,
>> +};
>> +
>> +static void aspeed_i2c_register_types(void)
>> +{
>> +    type_register_static(&aspeed_i2c_info);
>> +}
>> +
>> +type_init(aspeed_i2c_register_types)
>> +
>> +
>> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr)
>> +{
>> +    AspeedI2CState *s = ASPEED_I2C(dev);
>> +    I2CBus *bus = NULL;
>> +
>> +    if (busnr >= 0 && busnr < ASPEED_I2C_NR_BUSSES) {
>> +        bus = s->busses[busnr].bus;
>> +    }
>> +
>> +    return bus;
>> +}
>> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
>> index cf7102145355..e96e3db3fbea 100644
>> --- a/include/hw/arm/ast2400.h
>> +++ b/include/hw/arm/ast2400.h
>> @@ -16,6 +16,7 @@
>>  #include "hw/intc/aspeed_vic.h"
>>  #include "hw/misc/aspeed_scu.h"
>>  #include "hw/timer/aspeed_timer.h"
>> +#include "hw/i2c/aspeed_i2c.h"
>>
>>  typedef struct AST2400State {
>>      /*< private >*/
>> @@ -28,6 +29,7 @@ typedef struct AST2400State {
>>      AspeedVICState vic;
>>      AspeedTimerCtrlState timerctrl;
>>      AspeedSCUState scu;
>> +    AspeedI2CState i2c;
>>  } AST2400State;
>>
>>  #define TYPE_AST2400 "ast2400"
>> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
>> new file mode 100644
>> index 000000000000..f9020acdef30
>> --- /dev/null
>> +++ b/include/hw/i2c/aspeed_i2c.h
>> @@ -0,0 +1,62 @@
>> +/*
>> + *  ASPEED AST2400 I2C Controller
>> + *
>> + *  Copyright (C) 2016 IBM Corp.
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License along
>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>> + *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +#ifndef ASPEED_I2C_H
>> +#define ASPEED_I2C_H
>> +
>> +#include "hw/i2c/i2c.h"
>> +
>> +#define TYPE_ASPEED_I2C "aspeed.i2c"
>> +#define ASPEED_I2C(obj) \
>> +    OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
>> +
>> +#define ASPEED_I2C_NR_BUSSES 14
>> +
>> +struct AspeedI2CState;
>> +
>> +typedef struct AspeedI2CBus {
>> +    struct AspeedI2CState *controller;
>> +
>> +    MemoryRegion mr;
>> +
>> +    I2CBus *bus;
>> +    uint8_t id;
>> +
>> +    uint32_t ctrl;
>> +    uint32_t timing[2];
>> +    uint32_t intr_ctrl;
>> +    uint32_t intr_status;
>> +    uint32_t cmd;
>> +    uint32_t buf;
>> +} AspeedI2CBus;
>> +
>> +typedef struct AspeedI2CState {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    uint32_t intr_status;
>> +
>> +    AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
>> +} AspeedI2CState;
>> +
>> +I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
>> +
>> +#endif /* ASPEED_I2C_H */
>> --
>> 2.1.4
>>
>> _______________________________________________
>> openbmc mailing list
>> openbmc@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/openbmc

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH qemu 1/3] i2c: add aspeed i2c crontroller
  2016-05-19 16:54     ` Cédric Le Goater
@ 2016-05-20  6:43       ` Andrew Jeffery
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Jeffery @ 2016-05-20  6:43 UTC (permalink / raw)
  To: Cédric Le Goater, openbmc

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On Thu, 2016-05-19 at 18:54 +0200, Cédric Le Goater wrote:
> I should have addressed your comments on : 
> 
>         https://github.com/legoater/qemu/commits/aspeed
> 
> in which I also added a device model for the TMP42{1,2,3} temperature 
> sensors to fit the palmetto dts. 
> 
> Tell me when you want a resend.

I've fetched down the aspeed branch from github and had a play. Looks
good to me, so I'll push the patches to openbmc qemu/master shortly.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-05-20  6:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 19:24 [PATCH qemu 0/3] add i2c support Cédric Le Goater
2016-05-17 19:25 ` [PATCH qemu 1/3] i2c: add aspeed i2c crontroller Cédric Le Goater
2016-05-18 13:26   ` Joel Stanley
2016-05-19 16:56     ` Cédric Le Goater
2016-05-19  1:41   ` Andrew Jeffery
2016-05-19 16:54     ` Cédric Le Goater
2016-05-20  6:43       ` Andrew Jeffery
2016-05-17 19:25 ` [PATCH qemu 2/3] ast2400: add a rtc device on I2C bus 0 Cédric Le Goater
2016-05-18 13:33   ` Joel Stanley
2016-05-18 13:46     ` Cédric Le Goater
2016-05-18 14:06       ` Joel Stanley
2016-05-17 19:25 ` [PATCH qemu 3/3] ast2400: add a temp sensor device on I2C bus 3 Cédric Le Goater
2016-05-18 13:29   ` Joel Stanley
2016-05-18 13:35     ` Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.