All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Adds designware i2c module and adds it to virt arm
@ 2022-01-10 21:47 Patrick Venture
  2022-01-10 21:47 ` [PATCH 1/2] hw/i2c: Add designware i2c controller Patrick Venture
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Patrick Venture @ 2022-01-10 21:47 UTC (permalink / raw)
  To: crauer, peter.maydell, mst, imammedo, ani, shannon.zhaosl
  Cc: qemu-arm, qemu-devel, Patrick Venture

This patch series introduces a new i2c module, namely the designware one and further enables this (optionally) for the virt-arm machine.

Chris Rauer (2):
  hw/i2c: Add designware i2c controller.
  hw/arm: Enable smbus on arm virt machine.

 MAINTAINERS                     |   6 +
 docs/system/arm/virt.rst        |   4 +
 hw/arm/Kconfig                  |   1 +
 hw/arm/virt-acpi-build.c        |  24 +
 hw/arm/virt.c                   |  55 +++
 hw/i2c/Kconfig                  |   4 +
 hw/i2c/designware_i2c.c         | 821 ++++++++++++++++++++++++++++++++
 hw/i2c/meson.build              |   1 +
 include/hw/arm/virt.h           |   3 +
 include/hw/i2c/designware_i2c.h | 110 +++++
 10 files changed, 1029 insertions(+)
 create mode 100644 hw/i2c/designware_i2c.c
 create mode 100644 include/hw/i2c/designware_i2c.h

-- 
2.34.1.575.g55b058a8bb-goog



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

* [PATCH 1/2] hw/i2c: Add designware i2c controller.
  2022-01-10 21:47 [PATCH 0/2] Adds designware i2c module and adds it to virt arm Patrick Venture
@ 2022-01-10 21:47 ` Patrick Venture
  2022-01-10 21:47 ` [PATCH 2/2] hw/arm: Enable smbus on arm virt machine Patrick Venture
  2022-01-13 11:48 ` [PATCH 0/2] Adds designware i2c module and adds it to virt arm Peter Maydell
  2 siblings, 0 replies; 11+ messages in thread
From: Patrick Venture @ 2022-01-10 21:47 UTC (permalink / raw)
  To: crauer, peter.maydell, mst, imammedo, ani, shannon.zhaosl
  Cc: qemu-arm, qemu-devel, Hao Wu

From: Chris Rauer <crauer@google.com>

Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Chris Rauer <crauer@google.com>
---
 MAINTAINERS                     |   6 +
 hw/i2c/Kconfig                  |   4 +
 hw/i2c/designware_i2c.c         | 821 ++++++++++++++++++++++++++++++++
 hw/i2c/meson.build              |   1 +
 include/hw/i2c/designware_i2c.h | 110 +++++
 5 files changed, 942 insertions(+)
 create mode 100644 hw/i2c/designware_i2c.c
 create mode 100644 include/hw/i2c/designware_i2c.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f871d759fd..9aa34ea181 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2110,6 +2110,12 @@ S: Maintained
 F: hw/i2c/i2c_mux_pca954x.c
 F: include/hw/i2c/i2c_mux_pca954x.h
 
+DesignWare I2C
+M: Chris Rauer <crauer@google.com>
+S: Maintained
+F: hw/i2c/designware_i2c.c
+F: include/hw/i2c/designware_i2c.h
+
 Generic Loader
 M: Alistair Francis <alistair@alistair23.me>
 S: Maintained
diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
index 8217cb5041..0f3d86b869 100644
--- a/hw/i2c/Kconfig
+++ b/hw/i2c/Kconfig
@@ -13,6 +13,10 @@ config VERSATILE_I2C
     bool
     select BITBANG_I2C
 
+config DESIGNWARE_I2C
+    bool
+    select I2C
+
 config ACPI_SMBUS
     bool
     select SMBUS
diff --git a/hw/i2c/designware_i2c.c b/hw/i2c/designware_i2c.c
new file mode 100644
index 0000000000..b25c49e999
--- /dev/null
+++ b/hw/i2c/designware_i2c.c
@@ -0,0 +1,821 @@
+/*
+ * DesignWare I2C Module.
+ *
+ * Copyright 2021 Google LLC
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/i2c/designware_i2c.h"
+#include "migration/vmstate.h"
+#include "qemu/bitops.h"
+#include "qemu/guest-random.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+
+enum DesignWareI2CRegister {
+    DW_IC_CON                   = 0x00,
+    DW_IC_TAR                   = 0x04,
+    DW_IC_SAR                   = 0x08,
+    DW_IC_DATA_CMD              = 0x10,
+    DW_IC_SS_SCL_HCNT           = 0x14,
+    DW_IC_SS_SCL_LCNT           = 0x18,
+    DW_IC_FS_SCL_HCNT           = 0x1c,
+    DW_IC_FS_SCL_LCNT           = 0x20,
+    DW_IC_INTR_STAT             = 0x2c,
+    DW_IC_INTR_MASK             = 0x30,
+    DW_IC_RAW_INTR_STAT         = 0x34,
+    DW_IC_RX_TL                 = 0x38,
+    DW_IC_TX_TL                 = 0x3c,
+    DW_IC_CLR_INTR              = 0x40,
+    DW_IC_CLR_RX_UNDER          = 0x44,
+    DW_IC_CLR_RX_OVER           = 0x48,
+    DW_IC_CLR_TX_OVER           = 0x4c,
+    DW_IC_CLR_RD_REQ            = 0x50,
+    DW_IC_CLR_TX_ABRT           = 0x54,
+    DW_IC_CLR_RX_DONE           = 0x58,
+    DW_IC_CLR_ACTIVITY          = 0x5c,
+    DW_IC_CLR_STOP_DET          = 0x60,
+    DW_IC_CLR_START_DET         = 0x64,
+    DW_IC_CLR_GEN_CALL          = 0x68,
+    DW_IC_ENABLE                = 0x6c,
+    DW_IC_STATUS                = 0x70,
+    DW_IC_TXFLR                 = 0x74,
+    DW_IC_RXFLR                 = 0x78,
+    DW_IC_SDA_HOLD              = 0x7c,
+    DW_IC_TX_ABRT_SOURCE        = 0x80,
+    DW_IC_SLV_DATA_NACK_ONLY    = 0x84,
+    DW_IC_DMA_CR                = 0x88,
+    DW_IC_DMA_TDLR              = 0x8c,
+    DW_IC_DMA_RDLR              = 0x90,
+    DW_IC_SDA_SETUP             = 0x94,
+    DW_IC_ACK_GENERAL_CALL      = 0x98,
+    DW_IC_ENABLE_STATUS         = 0x9c,
+    DW_IC_FS_SPKLEN             = 0xa0,
+    DW_IC_CLR_RESTART_DET       = 0xa8,
+    DW_IC_COMP_PARAM_1          = 0xf4,
+    DW_IC_COMP_VERSION          = 0xf8,
+    DW_IC_COMP_TYPE             = 0xfc,
+};
+
+/* DW_IC_CON fields */
+#define DW_IC_CON_STOP_DET_IF_MASTER_ACTIV  BIT(10)
+#define DW_IC_CON_RX_FIFO_FULL_HLD_CTRL     BIT(9)
+#define DW_IC_CON_TX_EMPTY_CTRL             BIT(8)
+#define DW_IC_CON_STOP_IF_ADDRESSED         BIT(7)
+#define DW_IC_CON_SLAVE_DISABLE             BIT(6)
+#define DW_IC_CON_IC_RESTART_EN             BIT(5)
+#define DW_IC_CON_10BITADDR_MASTER          BIT(4)
+#define DW_IC_CON_10BITADDR_SLAVE           BIT(3)
+#define DW_IC_CON_SPEED(rv)                 extract32((rv), 1, 2)
+#define DW_IC_CON_MASTER_MODE               BIT(0)
+
+/* DW_IC_TAR fields */
+#define DW_IC_TAR_IC_10BITADDR_MASTER  BIT(12)
+#define DW_IC_TAR_SPECIAL              BIT(11)
+#define DW_IC_TAR_GC_OR_START          BIT(10)
+#define DW_IC_TAR_ADDRESS(rv)          extract32((rv), 0, 10)
+
+/* DW_IC_DATA_CMD fields */
+#define DW_IC_DATA_CMD_RESTART  BIT(10)
+#define DW_IC_DATA_CMD_STOP     BIT(9)
+#define DW_IC_DATA_CMD_CMD      BIT(8)
+#define DW_IC_DATA_CMD_DAT(rv)  extract32((rv), 0, 8)
+
+/* DW_IC_INTR_STAT/INTR_MASK/RAW_INTR_STAT fields */
+#define DW_IC_INTR_RESTART_DET  BIT(12)
+#define DW_IC_INTR_GEN_CALL     BIT(11)
+#define DW_IC_INTR_START_DET    BIT(10)
+#define DW_IC_INTR_STOP_DET     BIT(9)
+#define DW_IC_INTR_ACTIVITY     BIT(8)
+#define DW_IC_INTR_RX_DONE      BIT(7)
+#define DW_IC_INTR_TX_ABRT      BIT(6)
+#define DW_IC_INTR_RD_REQ       BIT(5)
+#define DW_IC_INTR_TX_EMPTY     BIT(4) /* Hardware clear only. */
+#define DW_IC_INTR_TX_OVER      BIT(3)
+#define DW_IC_INTR_RX_FULL      BIT(2) /* Hardware clear only. */
+#define DW_IC_INTR_RX_OVER      BIT(1)
+#define DW_IC_INTR_RX_UNDER     BIT(0)
+
+/* DW_IC_ENABLE fields */
+#define DW_IC_ENABLE_TX_CMD_BLOCK  BIT(2)
+#define DW_IC_ENABLE_ABORT         BIT(1)
+#define DW_IC_ENABLE_ENABLE        BIT(0)
+
+/* DW_IC_STATUS fields */
+#define DW_IC_STATUS_SLV_ACTIVITY  BIT(6)
+#define DW_IC_STATUS_MST_ACTIVITY  BIT(5)
+#define DW_IC_STATUS_RFF           BIT(4)
+#define DW_IC_STATUS_RFNE          BIT(3)
+#define DW_IC_STATUS_TFE           BIT(2)
+#define DW_IC_STATUS_TFNF          BIT(1)
+#define DW_IC_STATUS_ACTIVITY      BIT(0)
+
+/* DW_IC_TX_ABRT_SOURCE fields */
+#define DW_IC_TX_TX_FLUSH_CNT          extract32((rv), 23, 9)
+#define DW_IC_TX_ABRT_USER_ABRT        BIT(16)
+#define DW_IC_TX_ABRT_SLVRD_INTX       BIT(15)
+#define DW_IC_TX_ABRT_SLV_ARBLOST      BIT(14)
+#define DW_IC_TX_ABRT_SLVFLUSH_TXFIFO  BIT(13)
+#define DW_IC_TX_ARB_LOST              BIT(12)
+#define DW_IC_TX_ABRT_MASTER_DIS       BIT(11)
+#define DW_IC_TX_ABRT_10B_RD_NORSTRT   BIT(10)
+#define DW_IC_TX_ABRT_SBYTE_NORSTRT    BIT(9)
+#define DW_IC_TX_ABRT_HS_NORSTRT       BIT(8)
+#define DW_IC_TX_ABRT_SBYTE_ACKDET     BIT(7)
+#define DW_IC_TX_ABRT_HS_ACKDET        BIT(6)
+#define DW_IC_TX_ABRT_GCALL_READ       BIT(5)
+#define DW_IC_TX_ABRT_GCALL_NOACK      BIT(4)
+#define DW_IC_TX_ABRT_TXDATA_NOACK     BIT(3)
+#define DW_IC_TX_ABRT_10ADDR2_NOACK    BIT(2)
+#define DW_IC_TX_ABRT_10ADDR1_NOACK    BIT(1)
+#define DW_IC_TX_ABRT_7B_ADDR_NOACK    BIT(0)
+
+
+/* IC_ENABLE_STATUS fields */
+#define DW_IC_ENABLE_STATUS_SLV_RX_DATA_LOST         BIT(2)
+#define DW_IC_ENABLE_STATUS_SLV_DISABLED_WHILE_BUSY  BIT(1)
+#define DW_IC_ENABLE_STATUS_IC_EN                    BIT(0)
+
+/* Masks for writable registers. */
+#define DW_IC_CON_MASK          0x000003ff
+#define DW_IC_TAR_MASK          0x00000fff
+#define DW_IC_SAR_MASK          0x000003ff
+#define DW_IC_SS_SCL_HCNT_MASK  0x0000ffff
+#define DW_IC_SS_SCL_LCNT_MASK  0x0000ffff
+#define DW_IC_FS_SCL_HCNT_MASK  0x0000ffff
+#define DW_IC_FS_SCL_LCNT_MASK  0x0000ffff
+#define DW_IC_INTR_MASK_MASK    0x00001fff
+#define DW_IC_ENABLE_MASK       0x00000007
+#define DW_IC_SDA_HOLD_MASK     0x00ffffff
+#define DW_IC_SDA_SETUP_MASK    0x000000ff
+#define DW_IC_FS_SPKLEN_MASK    0x000000ff
+
+/* Reset values */
+#define DW_IC_CON_INIT_VAL          0x7d
+#define DW_IC_TAR_INIT_VAL          0x1055
+#define DW_IC_SAR_INIT_VAL          0x55
+#define DW_IC_SS_SCL_HCNT_INIT_VAL  0x190
+#define DW_IC_SS_SCL_LCNT_INIT_VAL  0x1d6
+#define DW_IC_FS_SCL_HCNT_INIT_VAL  0x3c
+#define DW_IC_FS_SCL_LCNT_INIT_VAL  0x82
+#define DW_IC_INTR_MASK_INIT_VAL    0x8ff
+#define DW_IC_STATUS_INIT_VAL       0x6
+#define DW_IC_SDA_HOLD_INIT_VAL     0x1
+#define DW_IC_SDA_SETUP_INIT_VAL    0x64
+#define DW_IC_FS_SPKLEN_INIT_VAL    0x2
+
+#define DW_IC_COMP_PARAM_1_HAS_ENCODED_PARAMS   BIT(7)
+#define DW_IC_COMP_PARAM_1_HAS_DMA              0 /* bit 6 - DMA disabled. */
+#define DW_IC_COMP_PARAM_1_INTR_IO              BIT(5)
+#define DW_IC_COMP_PARAM_1_HC_COUNT_VAL         0 /* bit 4 - disabled */
+#define DW_IC_COMP_PARAM_1_HIGH_SPEED_MODE      (BIT(2) | BIT(3))
+#define DW_IC_COMP_PARAM_1_APB_DATA_WIDTH_32    BIT(1) /* bits 0, 1 */
+#define DW_IC_COMP_PARAM_1_INIT_VAL             \
+    (DW_IC_COMP_PARAM_1_APB_DATA_WIDTH_32 | \
+    DW_IC_COMP_PARAM_1_HIGH_SPEED_MODE | \
+    DW_IC_COMP_PARAM_1_HC_COUNT_VAL | \
+    DW_IC_COMP_PARAM_1_INTR_IO | \
+    DW_IC_COMP_PARAM_1_HAS_DMA | \
+    DW_IC_COMP_PARAM_1_HAS_ENCODED_PARAMS | \
+    ((DESIGNWARE_I2C_RX_FIFO_SIZE - 1) << 8) | \
+    ((DESIGNWARE_I2C_TX_FIFO_SIZE - 1) << 16))
+#define DW_IC_COMP_VERSION_INIT_VAL             0x3132302a
+#define DW_IC_COMP_TYPE_INIT_VAL                0x44570140
+
+static void dw_i2c_update_irq(DesignWareI2CState *s)
+{
+    int level;
+    uint32_t intr = s->ic_raw_intr_stat & s->ic_intr_mask;
+
+    level = !!((intr & DW_IC_INTR_RX_UNDER) |
+        (intr & DW_IC_INTR_RX_OVER) |
+        (intr & DW_IC_INTR_RX_FULL) |
+        (intr & DW_IC_INTR_TX_OVER) |
+        (intr & DW_IC_INTR_TX_EMPTY) |
+        (intr & DW_IC_INTR_RD_REQ) |
+        (intr & DW_IC_INTR_TX_ABRT) |
+        (intr & DW_IC_INTR_RX_DONE) |
+        (intr & DW_IC_INTR_ACTIVITY) |
+        (intr & DW_IC_INTR_STOP_DET) |
+        (intr & DW_IC_INTR_START_DET) |
+        (intr & DW_IC_INTR_GEN_CALL) |
+        (intr & DW_IC_INTR_RESTART_DET)
+        );
+    qemu_set_irq(s->irq, level);
+}
+
+static uint32_t dw_i2c_read_ic_data_cmd(DesignWareI2CState *s)
+{
+    uint32_t value = s->rx_fifo[s->rx_cur];
+
+    if (s->status != DW_I2C_STATUS_RECEIVING) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Attempted to read from RX fifo when not in receive "
+                      "state.\n", DEVICE(s)->canonical_path);
+        if (s->status != DW_I2C_STATUS_IDLE) {
+            s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_UNDER;
+            dw_i2c_update_irq(s);
+        }
+        return 0;
+    }
+
+    s->rx_cur = (s->rx_cur + 1) % DESIGNWARE_I2C_RX_FIFO_SIZE;
+
+    if (s->ic_rxflr > 0) {
+        s->ic_rxflr--;
+    } else {
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_UNDER;
+        dw_i2c_update_irq(s);
+        return 0;
+    }
+
+    if (s->ic_rxflr <= s->ic_rx_tl) {
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_FULL;
+        dw_i2c_update_irq(s);
+    }
+
+    return value;
+}
+
+static uint64_t dw_i2c_read(void *opaque, hwaddr offset, unsigned size)
+{
+    uint64_t value = 0;
+
+    DesignWareI2CState *s = opaque;
+
+    switch (offset) {
+    case DW_IC_CON:
+        value = s->ic_con;
+        break;
+    case DW_IC_TAR:
+        value = s->ic_tar;
+        break;
+    case DW_IC_SAR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported read - ic_sar\n",
+                      DEVICE(s)->canonical_path);
+        value = s->ic_sar;
+        break;
+    case DW_IC_DATA_CMD:
+        value = dw_i2c_read_ic_data_cmd(s);
+        break;
+    case DW_IC_SS_SCL_HCNT:
+        value = s->ic_ss_scl_hcnt;
+        break;
+    case DW_IC_SS_SCL_LCNT:
+        value = s->ic_ss_scl_lcnt;
+        break;
+    case DW_IC_FS_SCL_HCNT:
+        value = s->ic_fs_scl_hcnt;
+        break;
+    case DW_IC_FS_SCL_LCNT:
+        value = s->ic_fs_scl_lcnt;
+        break;
+    case DW_IC_INTR_STAT:
+        value = s->ic_raw_intr_stat & s->ic_intr_mask;
+        break;
+    case DW_IC_INTR_MASK:
+        value = s->ic_intr_mask;
+        break;
+    case DW_IC_RAW_INTR_STAT:
+        value = s->ic_raw_intr_stat;
+        break;
+    case DW_IC_RX_TL:
+        value = s->ic_rx_tl;
+        break;
+    case DW_IC_TX_TL:
+        value = s->ic_tx_tl;
+        break;
+    case DW_IC_CLR_INTR:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_GEN_CALL |
+            DW_IC_INTR_RESTART_DET |
+            DW_IC_INTR_START_DET |
+            DW_IC_INTR_STOP_DET |
+            DW_IC_INTR_ACTIVITY |
+            DW_IC_INTR_RX_DONE |
+            DW_IC_INTR_TX_ABRT |
+            DW_IC_INTR_RD_REQ |
+            DW_IC_INTR_TX_OVER |
+            DW_IC_INTR_RX_OVER |
+            DW_IC_INTR_RX_UNDER);
+        s->ic_tx_abrt_source = 0;
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_RX_UNDER:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_RX_UNDER);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_RX_OVER:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_RX_OVER);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_TX_OVER:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_TX_OVER);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_RD_REQ:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_RD_REQ);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_TX_ABRT:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_TX_ABRT);
+        s->ic_tx_abrt_source = 0;
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_RX_DONE:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_RX_DONE);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_ACTIVITY:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_ACTIVITY);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_STOP_DET:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_STOP_DET);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_START_DET:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_START_DET);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_CLR_GEN_CALL:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_GEN_CALL);
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_ENABLE:
+        value = s->ic_enable;
+        break;
+    case DW_IC_STATUS:
+        value = s->ic_status;
+        break;
+    case DW_IC_TXFLR:
+        value = s->ic_txflr;
+        break;
+    case DW_IC_RXFLR:
+        value = s->ic_rxflr;
+        break;
+    case DW_IC_SDA_HOLD:
+        value = s->ic_sda_hold;
+        break;
+    case DW_IC_TX_ABRT_SOURCE:
+        value = s->ic_tx_abrt_source;
+        break;
+    case DW_IC_SLV_DATA_NACK_ONLY:
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: unsupported read - ic_slv_data_nack_only\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_CR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported read - ic_dma_cr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_TDLR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported read - ic_dma_tdlr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_RDLR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported read - ic_dma_rdlr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_SDA_SETUP:
+        value = s->ic_sda_setup;
+        break;
+    case DW_IC_ACK_GENERAL_CALL:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported read - ic_ack_general_call\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_ENABLE_STATUS:
+        value = s->ic_enable_status;
+        break;
+    case DW_IC_FS_SPKLEN:
+        value = s->ic_fs_spklen;
+        break;
+    case DW_IC_CLR_RESTART_DET:
+        s->ic_raw_intr_stat &= ~(DW_IC_INTR_RESTART_DET);
+        break;
+    case DW_IC_COMP_PARAM_1:
+        value = s->ic_comp_param_1;
+        break;
+    case DW_IC_COMP_VERSION:
+        value = s->ic_comp_version;
+        break;
+    case DW_IC_COMP_TYPE:
+        value = s->ic_comp_type;
+        break;
+
+    /* This register is invalid at this point. */
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: read from invalid offset 0x%" HWADDR_PRIx "\n",
+                      DEVICE(s)->canonical_path, offset);
+        break;
+    }
+
+    return value;
+}
+
+static void dw_i2c_write_ic_con(DesignWareI2CState *s, uint32_t value)
+{
+    if (value & DW_IC_CON_RX_FIFO_FULL_HLD_CTRL) {
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: unsupported ic_con flag - RX_FIFO_FULL_HLD_CTRL\n",
+                      DEVICE(s)->canonical_path);
+    }
+
+    if (!(s->ic_enable & DW_IC_ENABLE_ENABLE)) {
+        s->ic_con = value & DW_IC_CON_MASK;
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid setting to ic_con %d when ic_enable[0]==1\n",
+                      DEVICE(s)->canonical_path, value);
+    }
+}
+
+static void dw_i2c_reset_to_idle(DesignWareI2CState *s)
+{
+        s->ic_enable_status &= ~DW_IC_ENABLE_STATUS_IC_EN;
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_TX_EMPTY;
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_FULL;
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_UNDER;
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_OVER;
+        s->ic_rxflr = 0;
+        s->ic_status &= ~DW_IC_STATUS_ACTIVITY;
+        s->status = DW_I2C_STATUS_IDLE;
+        dw_i2c_update_irq(s);
+}
+
+static void dw_ic_tx_abort(DesignWareI2CState *s, uint32_t src)
+{
+    s->ic_tx_abrt_source |= src;
+    s->ic_raw_intr_stat |= DW_IC_INTR_TX_ABRT;
+    dw_i2c_reset_to_idle(s);
+    dw_i2c_update_irq(s);
+}
+
+static void dw_i2c_write_ic_data_cmd(DesignWareI2CState *s, uint32_t value)
+{
+    int recv = !!(value & DW_IC_DATA_CMD_CMD);
+
+    if (s->status == DW_I2C_STATUS_IDLE ||
+        s->ic_raw_intr_stat & DW_IC_INTR_TX_ABRT) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Attempted to write to TX fifo when it is held in "
+                      "reset.\n", DEVICE(s)->canonical_path);
+        return;
+    }
+
+    /* Send the address if it hasn't been sent yet. */
+    if (s->status == DW_I2C_STATUS_SENDING_ADDRESS) {
+        int rv = i2c_start_transfer(s->bus, DW_IC_TAR_ADDRESS(s->ic_tar), recv);
+        if (rv) {
+            dw_ic_tx_abort(s, DW_IC_TX_ABRT_7B_ADDR_NOACK);
+            return;
+        }
+        s->status = recv ? DW_I2C_STATUS_RECEIVING : DW_I2C_STATUS_SENDING;
+    }
+
+    /* Send data */
+    if (!recv) {
+        int rv = i2c_send(s->bus, DW_IC_DATA_CMD_DAT(value));
+        if (rv) {
+            i2c_end_transfer(s->bus);
+            dw_ic_tx_abort(s, DW_IC_TX_ABRT_TXDATA_NOACK);
+            return;
+        }
+        dw_i2c_update_irq(s);
+    }
+
+    /* Restart command */
+    if (value & DW_IC_DATA_CMD_RESTART && s->ic_con & DW_IC_CON_IC_RESTART_EN) {
+        s->ic_raw_intr_stat |= DW_IC_INTR_RESTART_DET |
+                               DW_IC_INTR_START_DET |
+                               DW_IC_INTR_ACTIVITY;
+        s->ic_status |= DW_IC_STATUS_ACTIVITY;
+        dw_i2c_update_irq(s);
+
+        if (i2c_start_transfer(s->bus, DW_IC_TAR_ADDRESS(s->ic_tar), recv)) {
+            dw_ic_tx_abort(s, DW_IC_TX_ABRT_7B_ADDR_NOACK);
+            return;
+        }
+
+        s->status = recv ? DW_I2C_STATUS_RECEIVING : DW_I2C_STATUS_SENDING;
+    }
+
+    /* Receive data */
+    if (recv) {
+        uint8_t pos = (s->rx_cur + s->ic_rxflr) % DESIGNWARE_I2C_RX_FIFO_SIZE;
+
+        if (s->ic_rxflr < DESIGNWARE_I2C_RX_FIFO_SIZE) {
+            s->rx_fifo[pos] = i2c_recv(s->bus);
+            s->ic_rxflr++;
+        } else {
+            s->ic_raw_intr_stat |= DW_IC_INTR_RX_OVER;
+            dw_i2c_update_irq(s);
+        }
+
+        if (s->ic_rxflr > s->ic_rx_tl) {
+            s->ic_raw_intr_stat |= DW_IC_INTR_RX_FULL;
+            dw_i2c_update_irq(s);
+        }
+        if (value & DW_IC_DATA_CMD_STOP) {
+            i2c_nack(s->bus);
+        }
+    }
+
+    /* Stop command */
+    if (value & DW_IC_DATA_CMD_STOP) {
+        s->ic_raw_intr_stat |= DW_IC_INTR_STOP_DET;
+        s->ic_status &= ~DW_IC_STATUS_ACTIVITY;
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_TX_EMPTY;
+        i2c_end_transfer(s->bus);
+        dw_i2c_update_irq(s);
+    }
+}
+
+static void dw_i2c_write_ic_enable(DesignWareI2CState *s, uint32_t value)
+{
+    if (value & DW_IC_ENABLE_ENABLE && !(s->ic_con & DW_IC_CON_SLAVE_DISABLE)) {
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: Designware I2C slave mode is not supported.\n",
+                      DEVICE(s)->canonical_path);
+        return;
+    }
+
+    s->ic_enable = value & DW_IC_ENABLE_MASK;
+
+    if (value & DW_IC_ENABLE_ABORT || value & DW_IC_ENABLE_TX_CMD_BLOCK) {
+        dw_ic_tx_abort(s, DW_IC_TX_ABRT_USER_ABRT);
+        return;
+    }
+
+    if (value & DW_IC_ENABLE_ENABLE) {
+        s->ic_enable_status |= DW_IC_ENABLE_STATUS_IC_EN;
+        s->ic_status |= DW_IC_STATUS_ACTIVITY;
+        s->ic_raw_intr_stat |= DW_IC_INTR_ACTIVITY |
+                               DW_IC_INTR_START_DET |
+                               DW_IC_INTR_TX_EMPTY;
+        s->status = DW_I2C_STATUS_SENDING_ADDRESS;
+        dw_i2c_update_irq(s);
+    } else if ((value & DW_IC_ENABLE_ENABLE) == 0) {
+        dw_i2c_reset_to_idle(s);
+    }
+
+}
+
+static void dw_i2c_write_ic_rx_tl(DesignWareI2CState *s, uint32_t value)
+{
+    /* Note that a value of 0 for ic_rx_tl indicates a threashold of 1. */
+    if (value > DESIGNWARE_I2C_RX_FIFO_SIZE - 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid setting to ic_rx_tl %d\n",
+                      DEVICE(s)->canonical_path, value);
+        s->ic_rx_tl = DESIGNWARE_I2C_RX_FIFO_SIZE - 1;
+    } else {
+        s->ic_rx_tl = value;
+    }
+
+    if (s->ic_rxflr > s->ic_rx_tl && s->ic_enable & DW_IC_ENABLE_ENABLE) {
+        s->ic_raw_intr_stat |= DW_IC_INTR_RX_FULL;
+    } else {
+        s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_FULL;
+    }
+    dw_i2c_update_irq(s);
+}
+
+static void dw_i2c_write_ic_tx_tl(DesignWareI2CState *s, uint32_t value)
+{
+    /*
+     * Note that a value of 0 for ic_tx_tl indicates a threashold of 1.
+     * However, the tx threshold is not used in the model because commands are
+     * always sent out as soon as they are written.
+     */
+    if (value > DESIGNWARE_I2C_TX_FIFO_SIZE - 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid setting to ic_tx_tl %d\n",
+                      DEVICE(s)->canonical_path, value);
+        s->ic_tx_tl = DESIGNWARE_I2C_TX_FIFO_SIZE - 1;
+    } else {
+        s->ic_tx_tl = value;
+    }
+}
+
+static void dw_i2c_write(void *opaque, hwaddr offset, uint64_t value,
+                              unsigned size)
+{
+    DesignWareI2CState *s = opaque;
+
+    /* The order of the registers are their order in memory. */
+    switch (offset) {
+    case DW_IC_CON:
+        dw_i2c_write_ic_con(s, value);
+        break;
+    case DW_IC_TAR:
+        s->ic_tar = value & DW_IC_TAR_MASK;
+        break;
+    case DW_IC_SAR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported write - ic_sar\n",
+                      DEVICE(s)->canonical_path);
+        s->ic_sar = value & DW_IC_SAR_MASK;
+        break;
+    case DW_IC_DATA_CMD:
+        dw_i2c_write_ic_data_cmd(s, value);
+        break;
+    case DW_IC_SS_SCL_HCNT:
+        s->ic_ss_scl_hcnt = value & DW_IC_SS_SCL_HCNT_MASK;
+        break;
+    case DW_IC_SS_SCL_LCNT:
+        s->ic_ss_scl_lcnt = value & DW_IC_SS_SCL_LCNT_MASK;
+        break;
+    case DW_IC_FS_SCL_HCNT:
+        s->ic_fs_scl_hcnt = value & DW_IC_FS_SCL_HCNT_MASK;
+        break;
+    case DW_IC_FS_SCL_LCNT:
+        s->ic_fs_scl_lcnt = value & DW_IC_FS_SCL_LCNT_MASK;
+        break;
+    case DW_IC_INTR_MASK:
+        s->ic_intr_mask = value & DW_IC_INTR_MASK_MASK;
+        dw_i2c_update_irq(s);
+        break;
+    case DW_IC_RX_TL:
+        dw_i2c_write_ic_rx_tl(s, value);
+        break;
+    case DW_IC_TX_TL:
+        dw_i2c_write_ic_tx_tl(s, value);
+        break;
+    case DW_IC_ENABLE:
+        dw_i2c_write_ic_enable(s, value);
+        break;
+    case DW_IC_SDA_HOLD:
+        s->ic_sda_hold = value & DW_IC_SDA_HOLD_MASK;
+        break;
+    case DW_IC_SLV_DATA_NACK_ONLY:
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: unsupported write - ic_slv_data_nack_only\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_CR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported write - ic_dma_cr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_TDLR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported write - ic_dma_tdlr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_DMA_RDLR:
+        qemu_log_mask(LOG_UNIMP, "%s: unsupported write - ic_dma_rdlr\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_SDA_SETUP:
+        s->ic_sda_setup = value & DW_IC_SDA_SETUP_MASK;
+        break;
+    case DW_IC_ACK_GENERAL_CALL:
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: unsupported write - ic_ack_general_call\n",
+                      DEVICE(s)->canonical_path);
+        break;
+    case DW_IC_FS_SPKLEN:
+        s->ic_fs_spklen = value & DW_IC_FS_SPKLEN_MASK;
+        break;
+
+    /* This register is invalid at this point. */
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: write to invalid offset or readonly register 0x%"
+                      HWADDR_PRIx "\n",
+                      DEVICE(s)->canonical_path, offset);
+        break;
+    }
+}
+
+static const MemoryRegionOps designware_i2c_ops = {
+    .read = dw_i2c_read,
+    .write = dw_i2c_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+        .unaligned = false,
+    },
+};
+
+static void designware_i2c_enter_reset(Object *obj, ResetType type)
+{
+    DesignWareI2CState *s = DESIGNWARE_I2C(obj);
+
+    s->ic_con = DW_IC_CON_INIT_VAL;
+    s->ic_tar = DW_IC_TAR_INIT_VAL;
+    s->ic_sar = DW_IC_SAR_INIT_VAL;
+    s->ic_ss_scl_hcnt = DW_IC_SS_SCL_HCNT_INIT_VAL;
+    s->ic_ss_scl_lcnt = DW_IC_SS_SCL_LCNT_INIT_VAL;
+    s->ic_fs_scl_hcnt = DW_IC_FS_SCL_HCNT_INIT_VAL;
+    s->ic_fs_scl_lcnt = DW_IC_FS_SCL_LCNT_INIT_VAL;
+    s->ic_intr_mask = DW_IC_INTR_MASK_INIT_VAL;
+    s->ic_raw_intr_stat = 0;
+    s->ic_rx_tl = 0;
+    s->ic_tx_tl = 0;
+    s->ic_enable = 0;
+    s->ic_status = DW_IC_STATUS_INIT_VAL;
+    s->ic_txflr = 0;
+    s->ic_rxflr = 0;
+    s->ic_sda_hold = DW_IC_SDA_HOLD_INIT_VAL;
+    s->ic_tx_abrt_source = 0;
+    s->ic_sda_setup = DW_IC_SDA_SETUP_INIT_VAL;
+    s->ic_enable_status = 0;
+    s->ic_fs_spklen = DW_IC_FS_SPKLEN_INIT_VAL;
+    s->ic_comp_param_1 = DW_IC_COMP_PARAM_1_INIT_VAL;
+    s->ic_comp_version = DW_IC_COMP_VERSION_INIT_VAL;
+    s->ic_comp_type = DW_IC_COMP_TYPE_INIT_VAL;
+
+    s->rx_cur = 0;
+    s->status = DW_I2C_STATUS_IDLE;
+}
+
+static void designware_i2c_hold_reset(Object *obj)
+{
+    DesignWareI2CState *s = DESIGNWARE_I2C(obj);
+
+    qemu_irq_lower(s->irq);
+}
+
+static const VMStateDescription vmstate_designware_i2c = {
+    .name = TYPE_DESIGNWARE_I2C,
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(ic_con, DesignWareI2CState),
+        VMSTATE_UINT32(ic_tar, DesignWareI2CState),
+        VMSTATE_UINT32(ic_sar, DesignWareI2CState),
+        VMSTATE_UINT32(ic_ss_scl_hcnt, DesignWareI2CState),
+        VMSTATE_UINT32(ic_ss_scl_lcnt, DesignWareI2CState),
+        VMSTATE_UINT32(ic_fs_scl_hcnt, DesignWareI2CState),
+        VMSTATE_UINT32(ic_fs_scl_lcnt, DesignWareI2CState),
+        VMSTATE_UINT32(ic_intr_mask, DesignWareI2CState),
+        VMSTATE_UINT32(ic_raw_intr_stat, DesignWareI2CState),
+        VMSTATE_UINT32(ic_rx_tl, DesignWareI2CState),
+        VMSTATE_UINT32(ic_tx_tl, DesignWareI2CState),
+        VMSTATE_UINT32(ic_enable, DesignWareI2CState),
+        VMSTATE_UINT32(ic_status, DesignWareI2CState),
+        VMSTATE_UINT32(ic_txflr, DesignWareI2CState),
+        VMSTATE_UINT32(ic_rxflr, DesignWareI2CState),
+        VMSTATE_UINT32(ic_sda_hold, DesignWareI2CState),
+        VMSTATE_UINT32(ic_tx_abrt_source, DesignWareI2CState),
+        VMSTATE_UINT32(ic_sda_setup, DesignWareI2CState),
+        VMSTATE_UINT32(ic_enable_status, DesignWareI2CState),
+        VMSTATE_UINT32(ic_fs_spklen, DesignWareI2CState),
+        VMSTATE_UINT32(ic_comp_param_1, DesignWareI2CState),
+        VMSTATE_UINT32(ic_comp_version, DesignWareI2CState),
+        VMSTATE_UINT32(ic_comp_type, DesignWareI2CState),
+        VMSTATE_UINT32(status, DesignWareI2CState),
+        VMSTATE_UINT8_ARRAY(rx_fifo, DesignWareI2CState,
+                        DESIGNWARE_I2C_RX_FIFO_SIZE),
+        VMSTATE_UINT8(rx_cur, DesignWareI2CState),
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+static void designware_i2c_smbus_init(Object *obj)
+{
+    DesignWareI2CState *s = DESIGNWARE_I2C(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->iomem, obj, &designware_i2c_ops, s,
+                          "regs", 4 * KiB);
+    sysbus_init_mmio(sbd, &s->iomem);
+
+    s->bus = i2c_init_bus(DEVICE(s), "i2c-bus");
+}
+
+static void designware_i2c_class_init(ObjectClass *klass, void *data)
+{
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "Designware I2C";
+    dc->vmsd = &vmstate_designware_i2c;
+    rc->phases.enter = designware_i2c_enter_reset;
+    rc->phases.hold = designware_i2c_hold_reset;
+}
+
+static const TypeInfo designware_i2c_types[] = {
+    {
+        .name = TYPE_DESIGNWARE_I2C,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(DesignWareI2CState),
+        .class_init = designware_i2c_class_init,
+        .instance_init = designware_i2c_smbus_init,
+    },
+};
+DEFINE_TYPES(designware_i2c_types);
diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
index d3df273251..80e03c830f 100644
--- a/hw/i2c/meson.build
+++ b/hw/i2c/meson.build
@@ -10,6 +10,7 @@ i2c_ss.add(when: 'CONFIG_IMX_I2C', if_true: files('imx_i2c.c'))
 i2c_ss.add(when: 'CONFIG_MPC_I2C', if_true: files('mpc_i2c.c'))
 i2c_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('microbit_i2c.c'))
 i2c_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_smbus.c'))
+i2c_ss.add(when: 'CONFIG_DESIGNWARE_I2C', if_true: files('designware_i2c.c'))
 i2c_ss.add(when: 'CONFIG_SMBUS_EEPROM', if_true: files('smbus_eeprom.c'))
 i2c_ss.add(when: 'CONFIG_VERSATILE_I2C', if_true: files('versatile_i2c.c'))
 i2c_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_i2c.c'))
diff --git a/include/hw/i2c/designware_i2c.h b/include/hw/i2c/designware_i2c.h
new file mode 100644
index 0000000000..4dcb39afe5
--- /dev/null
+++ b/include/hw/i2c/designware_i2c.h
@@ -0,0 +1,110 @@
+/*
+ * DesignWare I2C Module.
+ *
+ * Copyright 2021 Google LLC
+ *
+ * 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.
+ */
+#ifndef DESIGNWARE_I2C_H
+#define DESIGNWARE_I2C_H
+
+#include "exec/memory.h"
+#include "hw/i2c/i2c.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+
+/* Size of the FIFO buffers. */
+#define DESIGNWARE_I2C_RX_FIFO_SIZE 16
+#define DESIGNWARE_I2C_TX_FIFO_SIZE 16
+
+typedef enum DesignWareI2CStatus {
+    DW_I2C_STATUS_IDLE,
+    DW_I2C_STATUS_SENDING_ADDRESS,
+    DW_I2C_STATUS_SENDING,
+    DW_I2C_STATUS_RECEIVING,
+} DesignWareI2CStatus;
+
+/*
+ * struct DesignWareI2CState - DesignWare I2C device state.
+ * @bus: The underlying I2C Bus
+ * @irq: GIC interrupt line to fire on events
+ * @ic_con: : I2C control register
+ * @ic_tar: I2C target address register
+ * @ic_sar: I2C slave address register
+ * @ic_ss_scl_hcnt: Standard speed i2c clock scl high count register
+ * @ic_ss_scl_lcnt: Standard speed i2c clock scl low count register
+ * @ic_fs_scl_hcnt: Fast mode or fast mode plus i2c clock scl high count
+ *                  register
+ * @ic_fs_scl_lcnt:Fast mode or fast mode plus i2c clock scl low count
+ *                  register
+ * @ic_intr_mask: I2C Interrupt Mask Register
+ * @ic_raw_intr_stat: I2C raw interrupt status register
+ * @ic_rx_tl: I2C receive FIFO threshold register
+ * @ic_tx_tl: I2C transmit FIFO threshold register
+ * @ic_enable: I2C enable register
+ * @ic_status: I2C status register
+ * @ic_txflr: I2C transmit fifo level register
+ * @ic_rxflr: I2C receive fifo level register
+ * @ic_sda_hold: I2C SDA hold time length register
+ * @ic_tx_abrt_source: The I2C transmit abort source register
+ * @ic_sda_setup: I2C SDA setup register
+ * @ic_enable_status: I2C enable status register
+ * @ic_fs_spklen: I2C SS, FS or FM+ spike suppression limit
+ * @ic_comp_param_1: Component parameter register
+ * @ic_comp_version: I2C component version register
+ * @ic_comp_type: I2C component type register
+ * @rx_fifo: The FIFO buffer for receiving in FIFO mode.
+ * @rx_cur: The current position of rx_fifo.
+ * @status: The current status of the SMBus.
+ */
+typedef struct DesignWareI2CState {
+    SysBusDevice parent;
+
+    MemoryRegion iomem;
+
+    I2CBus      *bus;
+    qemu_irq     irq;
+
+    uint32_t ic_con;
+    uint32_t ic_tar;
+    uint32_t ic_sar;
+    uint32_t ic_ss_scl_hcnt;
+    uint32_t ic_ss_scl_lcnt;
+    uint32_t ic_fs_scl_hcnt;
+    uint32_t ic_fs_scl_lcnt;
+    uint32_t ic_intr_mask;
+    uint32_t ic_raw_intr_stat;
+    uint32_t ic_rx_tl;
+    uint32_t ic_tx_tl;
+    uint32_t ic_enable;
+    uint32_t ic_status;
+    uint32_t ic_txflr;
+    uint32_t ic_rxflr;
+    uint32_t ic_sda_hold;
+    uint32_t ic_tx_abrt_source;
+    uint32_t ic_sda_setup;
+    uint32_t ic_enable_status;
+    uint32_t ic_fs_spklen;
+    uint32_t ic_comp_param_1;
+    uint32_t ic_comp_version;
+    uint32_t ic_comp_type;
+
+    uint8_t      rx_fifo[DESIGNWARE_I2C_RX_FIFO_SIZE];
+    uint8_t      rx_cur;
+
+    DesignWareI2CStatus status;
+} DesignWareI2CState;
+
+#define TYPE_DESIGNWARE_I2C "designware-i2c"
+#define DESIGNWARE_I2C(obj) OBJECT_CHECK(DesignWareI2CState, (obj), \
+                                        TYPE_DESIGNWARE_I2C)
+
+#endif /* DESIGNWARE_I2C_H */
-- 
2.34.1.575.g55b058a8bb-goog



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

* [PATCH 2/2] hw/arm: Enable smbus on arm virt machine.
  2022-01-10 21:47 [PATCH 0/2] Adds designware i2c module and adds it to virt arm Patrick Venture
  2022-01-10 21:47 ` [PATCH 1/2] hw/i2c: Add designware i2c controller Patrick Venture
@ 2022-01-10 21:47 ` Patrick Venture
  2022-01-13 11:48 ` [PATCH 0/2] Adds designware i2c module and adds it to virt arm Peter Maydell
  2 siblings, 0 replies; 11+ messages in thread
From: Patrick Venture @ 2022-01-10 21:47 UTC (permalink / raw)
  To: crauer, peter.maydell, mst, imammedo, ani, shannon.zhaosl
  Cc: qemu-arm, qemu-devel, Hao Wu

From: Chris Rauer <crauer@google.com>

Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Chris Rauer <crauer@google.com>
---
 docs/system/arm/virt.rst |  4 +++
 hw/arm/Kconfig           |  1 +
 hw/arm/virt-acpi-build.c | 24 ++++++++++++++++++
 hw/arm/virt.c            | 55 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h    |  3 +++
 5 files changed, 87 insertions(+)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 850787495b..e357143f6b 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -30,6 +30,7 @@ The virt board supports:
 - An RTC
 - The fw_cfg device that allows a guest to obtain data from QEMU
 - A PL061 GPIO controller
+- A DesignWare I2C controller
 - An optional SMMUv3 IOMMU
 - hotpluggable DIMMs
 - hotpluggable NVDIMMs
@@ -121,6 +122,9 @@ ras
   Set ``on``/``off`` to enable/disable reporting host memory errors to a guest
   using ACPI and guest external abort exceptions. The default is off.
 
+smbus
+  Set ``on``/``off`` to enable/disable smbus controller. The default is ``off``.
+
 Linux guest kernel configuration
 """"""""""""""""""""""""""""""""
 
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e652590943..692af8c265 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM_VIRT
     select ACPI_HW_REDUCED
     select ACPI_APEI
     select ACPI_VIOT
+    select DESIGNWARE_I2C
 
 config CHEETAH
     bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index d0f4867fdf..310cd6fb5b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -92,6 +92,26 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
     aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_smbus(Aml *scope, const MemMapEntry *smbus_memmap,
+                                           uint32_t i2c_irq, I2CBus *smbus)
+{
+    Aml *dev = aml_device("SMB0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("APMC0D0F")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_STA", aml_int(0x0F)));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(smbus_memmap->base,
+                                       smbus_memmap->size, AML_READ_WRITE));
+    aml_append(crs,
+               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                             AML_EXCLUSIVE, &i2c_irq, 1));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
 {
     Aml *dev = aml_device("FWCF");
@@ -862,6 +882,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_dsdt_add_cpus(scope, vms);
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
+    if (vms->smbus_enabled) {
+        acpi_dsdt_add_smbus(scope, &memmap[VIRT_SMBUS],
+                            (irqmap[VIRT_SMBUS] + ARM_SPI_BASE), vms->smbus);
+    }
     if (vmc->acpi_expose_flash) {
         acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
     }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4593fea1ce..dd2219424d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -76,6 +76,7 @@
 #include "hw/acpi/generic_event_device.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
+#include "hw/i2c/designware_i2c.h"
 #include "qemu/guest-random.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
@@ -152,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
     [VIRT_PVTIME] =             { 0x090a0000, 0x00010000 },
     [VIRT_SECURE_GPIO] =        { 0x090b0000, 0x00001000 },
+    [VIRT_SMBUS] =              { 0x090c0000, 0x00001000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -188,6 +190,7 @@ static const int a15irqmap[] = {
     [VIRT_GPIO] = 7,
     [VIRT_SECURE_UART] = 8,
     [VIRT_ACPI_GED] = 9,
+    [VIRT_SMBUS] = 10,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
     [VIRT_SMMU] = 74,    /* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -878,6 +881,32 @@ static void create_rtc(const VirtMachineState *vms)
     g_free(nodename);
 }
 
+static void create_smbus(VirtMachineState *vms)
+{
+    char *nodename;
+    hwaddr base = vms->memmap[VIRT_SMBUS].base;
+    hwaddr size = vms->memmap[VIRT_SMBUS].size;
+    int irq = vms->irqmap[VIRT_SMBUS];
+    const char compat[] = "snps,designware-i2c";
+    MachineState *ms = MACHINE(vms);
+    DeviceState *dev;
+
+    dev = sysbus_create_simple(TYPE_DESIGNWARE_I2C, base,
+                               qdev_get_gpio_in(vms->gic, irq));
+    vms->smbus = (I2CBus *)qdev_get_child_bus(dev, "i2c-bus");
+
+    nodename = g_strdup_printf("/i2c@%" PRIx64, base);
+    qemu_fdt_add_subnode(ms->fdt, nodename);
+    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 2, base, 2, size);
+    qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    qemu_fdt_setprop_cell(ms->fdt, nodename, "clocks", vms->clock_phandle);
+    qemu_fdt_setprop_string(ms->fdt, nodename, "clock-names", "apb_pclk");
+    g_free(nodename);
+}
+
 static DeviceState *gpio_key_dev;
 static void virt_powerdown_req(Notifier *n, void *opaque)
 {
@@ -2125,6 +2154,10 @@ static void machvirt_init(MachineState *machine)
 
     vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
 
+    if (vms->smbus_enabled) {
+        create_smbus(vms);
+    }
+
     create_rtc(vms);
 
     create_pcie(vms);
@@ -2400,6 +2433,20 @@ static void virt_set_default_bus_bypass_iommu(Object *obj, bool value,
     vms->default_bus_bypass_iommu = value;
 }
 
+static bool virt_machine_get_smbus(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->smbus_enabled;
+}
+
+static void virt_machine_set_smbus(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->smbus_enabled = value;
+}
+
 static CpuInstanceProperties
 virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 {
@@ -2781,6 +2828,11 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
                                           "in ACPI table header."
                                           "The string may be up to 8 bytes in size");
 
+    object_class_property_add_bool(oc, "smbus",
+                                   virt_machine_get_smbus,
+                                   virt_machine_set_smbus);
+    object_class_property_set_description(oc, "smbus",
+                                          "Enable SMBUS controller. ");
 }
 
 static void virt_instance_init(Object *obj)
@@ -2828,6 +2880,9 @@ static void virt_instance_init(Object *obj)
     /* MTE is disabled by default.  */
     vms->mte = false;
 
+    /* smbus is disabled by default.  */
+    vms->smbus_enabled = false;
+
     vms->irqmap = a15irqmap;
 
     virt_flash_create(vms);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index dc6b66ffc8..d28de89695 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -86,6 +86,7 @@ enum {
     VIRT_ACPI_GED,
     VIRT_NVDIMM_ACPI,
     VIRT_PVTIME,
+    VIRT_SMBUS,
     VIRT_LOWMEMMAP_LAST,
 };
 
@@ -148,6 +149,7 @@ struct VirtMachineState {
     bool virt;
     bool ras;
     bool mte;
+    bool smbus_enabled;
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
@@ -169,6 +171,7 @@ struct VirtMachineState {
     DeviceState *acpi_dev;
     Notifier powerdown_notifier;
     PCIBus *bus;
+    I2CBus *smbus;
     char *oem_id;
     char *oem_table_id;
 };
-- 
2.34.1.575.g55b058a8bb-goog



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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-10 21:47 [PATCH 0/2] Adds designware i2c module and adds it to virt arm Patrick Venture
  2022-01-10 21:47 ` [PATCH 1/2] hw/i2c: Add designware i2c controller Patrick Venture
  2022-01-10 21:47 ` [PATCH 2/2] hw/arm: Enable smbus on arm virt machine Patrick Venture
@ 2022-01-13 11:48 ` Peter Maydell
  2022-01-13 11:51   ` Peter Maydell
  2 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2022-01-13 11:48 UTC (permalink / raw)
  To: Patrick Venture
  Cc: mst, qemu-devel, shannon.zhaosl, qemu-arm, ani, imammedo, crauer

On Mon, 10 Jan 2022 at 21:47, Patrick Venture <venture@google.com> wrote:
>
> This patch series introduces a new i2c module, namely the designware one and further enables this (optionally) for the virt-arm machine.
>
> Chris Rauer (2):
>   hw/i2c: Add designware i2c controller.
>   hw/arm: Enable smbus on arm virt machine.

I need to see a pretty strong justification for why we
should be adding new kinds of devices to the virt machine,
given that it increases complexity and potential attack
surface for using it with KVM; this cover letter doesn't
seem to provide any...

thanks
-- PMM


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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-13 11:48 ` [PATCH 0/2] Adds designware i2c module and adds it to virt arm Peter Maydell
@ 2022-01-13 11:51   ` Peter Maydell
  2022-01-26 17:12     ` Chris Rauer
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2022-01-13 11:51 UTC (permalink / raw)
  To: Patrick Venture
  Cc: mst, qemu-devel, shannon.zhaosl, qemu-arm, ani, imammedo, crauer

On Thu, 13 Jan 2022 at 11:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 10 Jan 2022 at 21:47, Patrick Venture <venture@google.com> wrote:
> >
> > This patch series introduces a new i2c module, namely the designware one and further enables this (optionally) for the virt-arm machine.
> >
> > Chris Rauer (2):
> >   hw/i2c: Add designware i2c controller.
> >   hw/arm: Enable smbus on arm virt machine.
>
> I need to see a pretty strong justification for why we
> should be adding new kinds of devices to the virt machine,
> given that it increases complexity and potential attack
> surface for using it with KVM; this cover letter doesn't
> seem to provide any...

Forgot to mention, but my prefered approach for providing
an i2c controller on the virt board would be to have a
PCI i2c controller: that way users who do need it can plug it
in with a -device command line option, and users who don't
need it never have to worry about it. (We seem to have
an ICH9-SMB PCI device already; I have no idea if it's suitable.)

thanks
-- PMM


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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-13 11:51   ` Peter Maydell
@ 2022-01-26 17:12     ` Chris Rauer
  2022-01-26 18:03       ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Rauer @ 2022-01-26 17:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Patrick Venture, mst, imammedo, ani, shannon.zhaosl, qemu-arm,
	qemu-devel

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

>I need to see a pretty strong justification for why we should be adding
new kinds of devices to the virt machine,
The designware i2c controller is a very common controller on many ARM
SoCs.  It has device tree bindings and ACPI bindings which makes it ideal
for this platform.

>Forgot to mention, but my prefered approach for providing
>an i2c controller on the virt board would be to have a
>PCI i2c controller: that way users who do need it can plug it
>in with a -device command line option, and users who don't
>need it never have to worry about it.
The device is enabled by a machine parameter, “-machine virt,smbus=true”,
and is disabled by default.

> (We seem to have an ICH9-SMB PCI device already; I have no idea if it's
suitable.)
I didn't find that device suitable because it is part of the Intel
Southbridge, which may have some Intel platform quirks, and we don't need
all the things in that IO hub.

-Chris

[-- Attachment #2: Type: text/html, Size: 1032 bytes --]

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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-26 17:12     ` Chris Rauer
@ 2022-01-26 18:03       ` Peter Maydell
  2022-01-26 22:01         ` Chris Rauer
  2022-01-26 23:42         ` Philippe Mathieu-Daudé via
  0 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2022-01-26 18:03 UTC (permalink / raw)
  To: Chris Rauer
  Cc: mst, Patrick Venture, qemu-devel, shannon.zhaosl, qemu-arm, ani,
	imammedo

On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote:
>
>> I need to see a pretty strong justification for why we should be
>> adding new kinds of devices to the virt machine,
>
> The designware i2c controller is a very common controller on many
>  ARM SoCs.  It has device tree bindings and ACPI bindings which
> makes it ideal for this platform.

No, I mean, why do we need an i2c controller on the virt
board at all ?

> >Forgot to mention, but my prefered approach for providing
> >an i2c controller on the virt board would be to have a
> >PCI i2c controller: that way users who do need it can plug it
> >in with a -device command line option, and users who don't
> >need it never have to worry about it.

> > (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.)
> I didn't find that device suitable because it is part of the Intel
> Southbridge, which may have some Intel platform quirks, and
> we don't need all the things in that IO hub.

That's a pity. Is there a different PCI I2C controller we could model ?

thanks
-- PMM


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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-26 18:03       ` Peter Maydell
@ 2022-01-26 22:01         ` Chris Rauer
  2022-01-26 23:42         ` Philippe Mathieu-Daudé via
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Rauer @ 2022-01-26 22:01 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Patrick Venture, mst, imammedo, ani, shannon.zhaosl, qemu-arm,
	qemu-devel

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

>>> I need to see a pretty strong justification for why we should be

>>> adding new kinds of devices to the virt machine,

>>

>> The designware i2c controller is a very common controller on many

>>  ARM SoCs.  It has device tree bindings and ACPI bindings which

>> makes it ideal for this platform.

>

>No, I mean, why do we need an i2c controller on the virt

>board at all ?

The use case we are interested in is to enable IPMI SSIF on the arm virt
machine which is why I was interested in something with ACPI support.  That
particular IPMI SSIF enablement patch is not in this series but I think it
could be pulled into the series if necessary.

> > >Forgot to mention, but my prefered approach for providing

> > >an i2c controller on the virt board would be to have a

> > >PCI i2c controller: that way users who do need it can plug it

> > >in with a -device command line option, and users who don't

> > >need it never have to worry about it.

>

> > > (We seem to have an ICH9-SMB PCI device already; I have no idea if
it's suitable.)

> > I didn't find that device suitable because it is part of the Intel

> > Southbridge, which may have some Intel platform quirks, and

> > we don't need all the things in that IO hub.

>

> That's a pity. Is there a different PCI I2C controller we could model ?

I’m not aware of any standalone PCI I2C controllers.  I’ve seen I2C
controllers on PCI devices with other things but I don’t think those could
be used for IPMI SSIF or other general I2C use cases.   Do you know of a
particular device I should take a look at?


-Chris


On Wed, Jan 26, 2022 at 10:03 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote:
> >
> >> I need to see a pretty strong justification for why we should be
> >> adding new kinds of devices to the virt machine,
> >
> > The designware i2c controller is a very common controller on many
> >  ARM SoCs.  It has device tree bindings and ACPI bindings which
> > makes it ideal for this platform.
>
> No, I mean, why do we need an i2c controller on the virt
> board at all ?
>
> > >Forgot to mention, but my prefered approach for providing
> > >an i2c controller on the virt board would be to have a
> > >PCI i2c controller: that way users who do need it can plug it
> > >in with a -device command line option, and users who don't
> > >need it never have to worry about it.
>
> > > (We seem to have an ICH9-SMB PCI device already; I have no idea if
> it's suitable.)
> > I didn't find that device suitable because it is part of the Intel
> > Southbridge, which may have some Intel platform quirks, and
> > we don't need all the things in that IO hub.
>
> That's a pity. Is there a different PCI I2C controller we could model ?
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 9348 bytes --]

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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-26 18:03       ` Peter Maydell
  2022-01-26 22:01         ` Chris Rauer
@ 2022-01-26 23:42         ` Philippe Mathieu-Daudé via
  2022-02-21 17:47           ` Chris Rauer
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-26 23:42 UTC (permalink / raw)
  To: Peter Maydell, Chris Rauer
  Cc: mst, Patrick Venture, qemu-devel, shannon.zhaosl, qemu-arm, ani,
	imammedo, Enrico Weigelt

+Enrico Weigelt

On 26/1/22 19:03, Peter Maydell wrote:
> On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote:
>>
>>> I need to see a pretty strong justification for why we should be
>>> adding new kinds of devices to the virt machine,
>>
>> The designware i2c controller is a very common controller on many
>>   ARM SoCs.  It has device tree bindings and ACPI bindings which
>> makes it ideal for this platform.
> 
> No, I mean, why do we need an i2c controller on the virt
> board at all ?
> 
>>> Forgot to mention, but my prefered approach for providing
>>> an i2c controller on the virt board would be to have a
>>> PCI i2c controller: that way users who do need it can plug it
>>> in with a -device command line option, and users who don't
>>> need it never have to worry about it.
> 
>>> (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.)
>> I didn't find that device suitable because it is part of the Intel
>> Southbridge, which may have some Intel platform quirks, and
>> we don't need all the things in that IO hub.
> 
> That's a pity. Is there a different PCI I2C controller we could model ?

What about using virtio-gpio & bitbang I2C?

- virtio-gpio
https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/

- bitbang I2C already in: hw/i2c/bitbang_i2c.c

Regards,

Phil.


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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-01-26 23:42         ` Philippe Mathieu-Daudé via
@ 2022-02-21 17:47           ` Chris Rauer
  2022-02-22 17:05             ` Corey Minyard
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Rauer @ 2022-02-21 17:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, mst, Patrick Venture, qemu-devel, shannon.zhaosl,
	qemu-arm, ani, imammedo, Enrico Weigelt

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

Hi Phil,

> What about using virtio-gpio & bitbang I2C?
>
> - virtio-gpio
> https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/
>
> - bitbang I2C already in: hw/i2c/bitbang_i2c.c
Sorry for the delay.

That looks like it might be doable with a bit more work creating the ACPI
entries for the bitbang I2C.  This definitely seems more appropriate on the
ARM_VIRT platform instead of putting a specific controller in.

For my uses, I will have to stick with the designware controller since it
matches the system I'm emulating a little more closely.  We can hold back
the designware stuff until another SoC platform is interested in using this
controller (since it seems like it is a common one).  Hopefully someone
will find another use for the controller patches someday.

Thanks again for looking at our patches.

-Chris


On Wed, Jan 26, 2022 at 3:42 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> +Enrico Weigelt
>
> On 26/1/22 19:03, Peter Maydell wrote:
> > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote:
> >>
> >>> I need to see a pretty strong justification for why we should be
> >>> adding new kinds of devices to the virt machine,
> >>
> >> The designware i2c controller is a very common controller on many
> >>   ARM SoCs.  It has device tree bindings and ACPI bindings which
> >> makes it ideal for this platform.
> >
> > No, I mean, why do we need an i2c controller on the virt
> > board at all ?
> >
> >>> Forgot to mention, but my prefered approach for providing
> >>> an i2c controller on the virt board would be to have a
> >>> PCI i2c controller: that way users who do need it can plug it
> >>> in with a -device command line option, and users who don't
> >>> need it never have to worry about it.
> >
> >>> (We seem to have an ICH9-SMB PCI device already; I have no idea if
> it's suitable.)
> >> I didn't find that device suitable because it is part of the Intel
> >> Southbridge, which may have some Intel platform quirks, and
> >> we don't need all the things in that IO hub.
> >
> > That's a pity. Is there a different PCI I2C controller we could model ?
>
> What about using virtio-gpio & bitbang I2C?
>
> - virtio-gpio
> https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/
>
> - bitbang I2C already in: hw/i2c/bitbang_i2c.c
>
> Regards,
>
> Phil.
>

[-- Attachment #2: Type: text/html, Size: 3404 bytes --]

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

* Re: [PATCH 0/2] Adds designware i2c module and adds it to virt arm
  2022-02-21 17:47           ` Chris Rauer
@ 2022-02-22 17:05             ` Corey Minyard
  0 siblings, 0 replies; 11+ messages in thread
From: Corey Minyard @ 2022-02-22 17:05 UTC (permalink / raw)
  To: Chris Rauer
  Cc: Peter Maydell, mst, Patrick Venture, qemu-devel,
	Philippe Mathieu-Daudé,
	shannon.zhaosl, qemu-arm, ani, imammedo, Enrico Weigelt

On Mon, Feb 21, 2022 at 09:47:27AM -0800, Chris Rauer wrote:
> Hi Phil,
> 
> > What about using virtio-gpio & bitbang I2C?
> >
> > - virtio-gpio
> > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/
> >
> > - bitbang I2C already in: hw/i2c/bitbang_i2c.c
> Sorry for the delay.
> 
> That looks like it might be doable with a bit more work creating the ACPI
> entries for the bitbang I2C.  This definitely seems more appropriate on the
> ARM_VIRT platform instead of putting a specific controller in.

I would think the efficiency of this would be horrible.

> 
> For my uses, I will have to stick with the designware controller since it
> matches the system I'm emulating a little more closely.  We can hold back
> the designware stuff until another SoC platform is interested in using this
> controller (since it seems like it is a common one).  Hopefully someone
> will find another use for the controller patches someday.

I should have chimed in sooner.

The i.MX i2c device has ACPI and OF support, I believe, and it's already
available in qemu.  I don't think the Intel smbus device would work.

The designware one is a pretty common and general one, it's a little
suprising that it hasn't already been done on qemu.  I would be ok with
this but Peter has the big picture here.

-corey

> 
> Thanks again for looking at our patches.
> 
> -Chris
> 
> 
> On Wed, Jan 26, 2022 at 3:42 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
> 
> > +Enrico Weigelt
> >
> > On 26/1/22 19:03, Peter Maydell wrote:
> > > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote:
> > >>
> > >>> I need to see a pretty strong justification for why we should be
> > >>> adding new kinds of devices to the virt machine,
> > >>
> > >> The designware i2c controller is a very common controller on many
> > >>   ARM SoCs.  It has device tree bindings and ACPI bindings which
> > >> makes it ideal for this platform.
> > >
> > > No, I mean, why do we need an i2c controller on the virt
> > > board at all ?
> > >
> > >>> Forgot to mention, but my prefered approach for providing
> > >>> an i2c controller on the virt board would be to have a
> > >>> PCI i2c controller: that way users who do need it can plug it
> > >>> in with a -device command line option, and users who don't
> > >>> need it never have to worry about it.
> > >
> > >>> (We seem to have an ICH9-SMB PCI device already; I have no idea if
> > it's suitable.)
> > >> I didn't find that device suitable because it is part of the Intel
> > >> Southbridge, which may have some Intel platform quirks, and
> > >> we don't need all the things in that IO hub.
> > >
> > > That's a pity. Is there a different PCI I2C controller we could model ?
> >
> > What about using virtio-gpio & bitbang I2C?
> >
> > - virtio-gpio
> > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/
> >
> > - bitbang I2C already in: hw/i2c/bitbang_i2c.c
> >
> > Regards,
> >
> > Phil.
> >


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

end of thread, other threads:[~2022-02-22 17:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 21:47 [PATCH 0/2] Adds designware i2c module and adds it to virt arm Patrick Venture
2022-01-10 21:47 ` [PATCH 1/2] hw/i2c: Add designware i2c controller Patrick Venture
2022-01-10 21:47 ` [PATCH 2/2] hw/arm: Enable smbus on arm virt machine Patrick Venture
2022-01-13 11:48 ` [PATCH 0/2] Adds designware i2c module and adds it to virt arm Peter Maydell
2022-01-13 11:51   ` Peter Maydell
2022-01-26 17:12     ` Chris Rauer
2022-01-26 18:03       ` Peter Maydell
2022-01-26 22:01         ` Chris Rauer
2022-01-26 23:42         ` Philippe Mathieu-Daudé via
2022-02-21 17:47           ` Chris Rauer
2022-02-22 17:05             ` Corey Minyard

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.