All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] aspeed: extensions and fixes
@ 2019-11-19 14:11 Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 01/17] aspeed/i2c: Add support for pool buffer transfers Cédric Le Goater
                   ` (17 more replies)
  0 siblings, 18 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

Hi,

Here is a series adding :

 - support for pool buffer transfers in the I2C controller
 - fixes for the AST2600 and for the SMC controller
 - a new Tacoma BMC board using the AST2600 SoC.
 - misc small cleanups

Thanks,

C.


Cédric Le Goater (12):
  aspeed/i2c: Add support for pool buffer transfers
  aspeed/i2c: Check SRAM enablement on AST2500
  aspeed: Add a DRAM memory region at the SoC level
  aspeed/i2c: Add support for DMA transfers
  aspeed/i2c: Add trace events
  aspeed/smc: Restore default AHB window mapping at reset
  aspeed/smc: Do not map disabled segment on the AST2600
  aspeed/smc: Add AST2600 timings registers
  aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass
  aspeed: Add support for the tacoma-bmc board
  aspeed: Change the "scu" property definition
  aspeed: Change the "nic" property definition

Joel Stanley (4):
  aspeed/sdmc: Make ast2600 default 1G
  aspeed/scu: Fix W1C behavior
  watchdog/aspeed: Improve watchdog timeout message
  watchdog/aspeed: Fix AST2600 frequency behaviour

PanNengyuan (1):
  gpio: fix memory leak in aspeed_gpio_init()

 include/hw/arm/aspeed.h          |  24 +-
 include/hw/arm/aspeed_soc.h      |   1 +
 include/hw/i2c/aspeed_i2c.h      |  16 ++
 include/hw/ssi/aspeed_smc.h      |   1 +
 include/hw/watchdog/wdt_aspeed.h |   1 +
 hw/arm/aspeed.c                  | 259 ++++++++++--------
 hw/arm/aspeed_ast2600.c          |  25 +-
 hw/arm/aspeed_soc.c              |  22 +-
 hw/gpio/aspeed_gpio.c            |   1 +
 hw/i2c/aspeed_i2c.c              | 439 +++++++++++++++++++++++++++++--
 hw/misc/aspeed_scu.c             |  19 +-
 hw/misc/aspeed_sdmc.c            |   6 +-
 hw/net/ftgmac100.c               |  19 +-
 hw/ssi/aspeed_smc.c              |  63 +++--
 hw/timer/aspeed_timer.c          |  17 +-
 hw/watchdog/wdt_aspeed.c         |  41 ++-
 hw/i2c/trace-events              |   9 +
 17 files changed, 744 insertions(+), 219 deletions(-)

-- 
2.21.0



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

* [PATCH 01/17] aspeed/i2c: Add support for pool buffer transfers
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
@ 2019-11-19 14:11 ` Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 02/17] aspeed/i2c: Check SRAM enablement on AST2500 Cédric Le Goater
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jae Hyun Yoo, Andrew Jeffery, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

The Aspeed I2C controller can operate in different transfer modes :

  - Byte Buffer mode, using a dedicated register to transfer a
    byte. This is what the model supports today.

  - Pool Buffer mode, using an internal SRAM to transfer multiple
    bytes in the same command sequence.

Each SoC has different SRAM characteristics. On the AST2400, 2048
bytes of SRAM are available at offset 0x800 of the controller AHB
window. The pool buffer can be configured from 1 to 256 bytes per bus.

On the AST2500, the SRAM is at offset 0x200 and the pool buffer is of
16 bytes per bus.

On the AST2600, the SRAM is at offset 0xC00 and the pool buffer is of
32 bytes per bus. It can be splitted in two for TX and RX but the
current model does not add support for it as it it unused by known
drivers.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/i2c/aspeed_i2c.h |   8 ++
 hw/i2c/aspeed_i2c.c         | 197 ++++++++++++++++++++++++++++++++----
 2 files changed, 186 insertions(+), 19 deletions(-)

diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 13e01059189f..5313d07aa72f 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -32,6 +32,7 @@
     OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C)
 
 #define ASPEED_I2C_NR_BUSSES 16
+#define ASPEED_I2C_MAX_POOL_SIZE 0x800
 
 struct AspeedI2CState;
 
@@ -50,6 +51,7 @@ typedef struct AspeedI2CBus {
     uint32_t intr_status;
     uint32_t cmd;
     uint32_t buf;
+    uint32_t pool_ctrl;
 } AspeedI2CBus;
 
 typedef struct AspeedI2CState {
@@ -59,6 +61,8 @@ typedef struct AspeedI2CState {
     qemu_irq irq;
 
     uint32_t intr_status;
+    MemoryRegion pool_iomem;
+    uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE];
 
     AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
 } AspeedI2CState;
@@ -75,6 +79,10 @@ typedef struct AspeedI2CClass {
     uint8_t reg_size;
     uint8_t gap;
     qemu_irq (*bus_get_irq)(AspeedI2CBus *);
+
+    uint64_t pool_size;
+    hwaddr pool_base;
+    uint8_t *(*bus_pool_base)(AspeedI2CBus *);
 } AspeedI2CClass;
 
 I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 06c119f385b8..e21f45d96868 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -35,8 +35,7 @@
 /* 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_POOL_PAGE_SEL(x)            (((x) >> 20) & 0x7)  /* AST2400 */
 #define   I2CD_M_SDA_LOCK_EN               (0x1 << 16)
 #define   I2CD_MULTI_MASTER_DIS            (0x1 << 15)
 #define   I2CD_M_SCL_DRIVE_EN              (0x1 << 14)
@@ -113,10 +112,12 @@
 #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_RX_DMA_ENABLE               (0x1 << 9)
+#define   I2CD_TX_DMA_ENABLE               (0x1 << 8)
+#define   I2CD_RX_BUFF_ENABLE              (0x1 << 7)
+#define   I2CD_TX_BUFF_ENABLE              (0x1 << 6)
 #define   I2CD_M_STOP_CMD                  (0x1 << 5)
 #define   I2CD_M_S_RX_CMD_LAST             (0x1 << 4)
 #define   I2CD_M_RX_CMD                    (0x1 << 3)
@@ -125,7 +126,11 @@
 #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_POOL_CTRL_REG      0x1c       /* Pool Buffer Control */
+#define   I2CD_POOL_RX_COUNT(x)            (((x) >> 24) & 0xff)
+#define   I2CD_POOL_RX_SIZE(x)             ((((x) >> 16) & 0xff) + 1)
+#define   I2CD_POOL_TX_COUNT(x)            ((((x) >> 8) & 0xff) + 1)
+#define   I2CD_POOL_OFFSET(x)              (((x) & 0x3f) << 2)  /* AST2400 */
 #define I2CD_BYTE_BUF_REG       0x20       /* Transmit/Receive Byte Buffer */
 #define   I2CD_BYTE_BUF_TX_SHIFT           0
 #define   I2CD_BYTE_BUF_TX_MASK            0xff
@@ -170,6 +175,8 @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
         return bus->intr_ctrl;
     case I2CD_INTR_STS_REG:
         return bus->intr_status;
+    case I2CD_POOL_CTRL_REG:
+        return bus->pool_ctrl;
     case I2CD_BYTE_BUF_REG:
         return bus->buf;
     case I2CD_CMD_REG:
@@ -192,14 +199,58 @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus)
     return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK;
 }
 
-static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus)
+static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
+{
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
+    int ret = -1;
+    int i;
+
+    if (bus->cmd & I2CD_TX_BUFF_ENABLE) {
+        for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) {
+            uint8_t *pool_base = aic->bus_pool_base(bus);
+
+            ret = i2c_send(bus->bus, pool_base[i]);
+            if (ret) {
+                break;
+            }
+        }
+        bus->cmd &= ~I2CD_TX_BUFF_ENABLE;
+    } else {
+        ret = i2c_send(bus->bus, bus->buf);
+    }
+
+    return ret;
+}
+
+static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
 {
-    uint8_t ret;
+    AspeedI2CState *s = bus->controller;
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s);
+    uint8_t data;
+    int i;
+
+    if (bus->cmd & I2CD_RX_BUFF_ENABLE) {
+        uint8_t *pool_base = aic->bus_pool_base(bus);
 
+        for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) {
+            pool_base[i] = i2c_recv(bus->bus);
+        }
+
+        /* Update RX count */
+        bus->pool_ctrl &= ~(0xff << 24);
+        bus->pool_ctrl |= (i & 0xff) << 24;
+        bus->cmd &= ~I2CD_RX_BUFF_ENABLE;
+    } else {
+        data = i2c_recv(bus->bus);
+        bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
+    }
+}
+
+static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus)
+{
     aspeed_i2c_set_state(bus, I2CD_MRXD);
-    ret = i2c_recv(bus->bus);
+    aspeed_i2c_bus_recv(bus);
     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_S_RX_CMD_LAST) {
         i2c_nack(bus->bus);
     }
@@ -207,31 +258,66 @@ static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus)
     aspeed_i2c_set_state(bus, I2CD_MACTIVE);
 }
 
+static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus)
+{
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
+
+    if (bus->cmd & I2CD_TX_BUFF_ENABLE) {
+        uint8_t *pool_base = aic->bus_pool_base(bus);
+
+        return pool_base[0];
+    } else {
+        return bus->buf;
+    }
+}
+
 /*
  * The state machine needs some refinement. It is only used to track
  * invalid STOP commands for the moment.
  */
 static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
 {
+    uint8_t pool_start = 0;
+
     bus->cmd &= ~0xFFFF;
     bus->cmd |= value & 0xFFFF;
 
     if (bus->cmd & I2CD_M_START_CMD) {
         uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ?
             I2CD_MSTARTR : I2CD_MSTART;
+        uint8_t addr;
 
         aspeed_i2c_set_state(bus, state);
 
-        if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7),
-                               extract32(bus->buf, 0, 1))) {
+        addr = aspeed_i2c_get_addr(bus);
+
+        if (i2c_start_transfer(bus->bus, extract32(addr, 1, 7),
+                               extract32(addr, 0, 1))) {
             bus->intr_status |= I2CD_INTR_TX_NAK;
         } else {
             bus->intr_status |= I2CD_INTR_TX_ACK;
         }
 
-        /* START command is also a TX command, as the slave address is
-         * sent on the bus */
-        bus->cmd &= ~(I2CD_M_START_CMD | I2CD_M_TX_CMD);
+        bus->cmd &= ~I2CD_M_START_CMD;
+
+        /*
+         * The START command is also a TX command, as the slave
+         * address is sent on the bus. Drop the TX flag if nothing
+         * else needs to be sent in this sequence.
+         */
+        if (bus->cmd & I2CD_TX_BUFF_ENABLE) {
+            if (I2CD_POOL_TX_COUNT(bus->pool_ctrl) == 1) {
+                bus->cmd &= ~I2CD_M_TX_CMD;
+            } else {
+                /*
+                 * Increase the start index in the TX pool buffer to
+                 * skip the address byte.
+                 */
+                pool_start++;
+            }
+        } else {
+            bus->cmd &= ~I2CD_M_TX_CMD;
+        }
 
         /* No slave found */
         if (!i2c_bus_busy(bus->bus)) {
@@ -242,7 +328,7 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
 
     if (bus->cmd & I2CD_M_TX_CMD) {
         aspeed_i2c_set_state(bus, I2CD_MTXD);
-        if (i2c_send(bus->bus, bus->buf)) {
+        if (aspeed_i2c_bus_send(bus, pool_start)) {
             bus->intr_status |= (I2CD_INTR_TX_NAK);
             i2c_end_transfer(bus->bus);
         } else {
@@ -313,6 +399,11 @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
         qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
                       __func__);
         break;
+    case I2CD_POOL_CTRL_REG:
+        bus->pool_ctrl &= ~0xffffff;
+        bus->pool_ctrl |= (value & 0xffffff);
+        break;
+
     case I2CD_BYTE_BUF_REG:
         bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT;
         break;
@@ -378,10 +469,45 @@ static const MemoryRegionOps aspeed_i2c_ctrl_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static uint64_t aspeed_i2c_pool_read(void *opaque, hwaddr offset,
+                                     unsigned size)
+{
+    AspeedI2CState *s = opaque;
+    uint64_t ret = 0;
+    int i;
+
+    for (i = 0; i < size; i++) {
+        ret |= (uint64_t) s->pool[offset + i] << (8 * i);
+    }
+
+    return ret;
+}
+
+static void aspeed_i2c_pool_write(void *opaque, hwaddr offset,
+                                  uint64_t value, unsigned size)
+{
+    AspeedI2CState *s = opaque;
+    int i;
+
+    for (i = 0; i < size; i++) {
+        s->pool[offset + i] = (value >> (8 * i)) & 0xFF;
+    }
+}
+
+static const MemoryRegionOps aspeed_i2c_pool_ops = {
+    .read = aspeed_i2c_pool_read,
+    .write = aspeed_i2c_pool_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
 static const VMStateDescription aspeed_i2c_bus_vmstate = {
     .name = TYPE_ASPEED_I2C,
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(id, AspeedI2CBus),
         VMSTATE_UINT32(ctrl, AspeedI2CBus),
@@ -390,19 +516,21 @@ static const VMStateDescription aspeed_i2c_bus_vmstate = {
         VMSTATE_UINT32(intr_status, AspeedI2CBus),
         VMSTATE_UINT32(cmd, AspeedI2CBus),
         VMSTATE_UINT32(buf, AspeedI2CBus),
+        VMSTATE_UINT32(pool_ctrl, AspeedI2CBus),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static const VMStateDescription aspeed_i2c_vmstate = {
     .name = TYPE_ASPEED_I2C,
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(intr_status, AspeedI2CState),
         VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState,
                              ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate,
                              AspeedI2CBus),
+        VMSTATE_UINT8_ARRAY(pool, AspeedI2CState, ASPEED_I2C_MAX_POOL_SIZE),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -472,6 +600,10 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
         memory_region_add_subregion(&s->iomem, aic->reg_size * (i + offset),
                                     &s->busses[i].mr);
     }
+
+    memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s,
+                          "aspeed.i2c-pool", aic->pool_size);
+    memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem);
 }
 
 static void aspeed_i2c_class_init(ObjectClass *klass, void *data)
@@ -498,6 +630,14 @@ static qemu_irq aspeed_2400_i2c_bus_get_irq(AspeedI2CBus *bus)
     return bus->controller->irq;
 }
 
+static uint8_t *aspeed_2400_i2c_bus_pool_base(AspeedI2CBus *bus)
+{
+    uint8_t *pool_page =
+        &bus->controller->pool[I2CD_POOL_PAGE_SEL(bus->ctrl) * 0x100];
+
+    return &pool_page[I2CD_POOL_OFFSET(bus->pool_ctrl)];
+}
+
 static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -509,6 +649,9 @@ static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data)
     aic->reg_size = 0x40;
     aic->gap = 7;
     aic->bus_get_irq = aspeed_2400_i2c_bus_get_irq;
+    aic->pool_size = 0x800;
+    aic->pool_base = 0x800;
+    aic->bus_pool_base = aspeed_2400_i2c_bus_pool_base;
 }
 
 static const TypeInfo aspeed_2400_i2c_info = {
@@ -522,6 +665,11 @@ static qemu_irq aspeed_2500_i2c_bus_get_irq(AspeedI2CBus *bus)
     return bus->controller->irq;
 }
 
+static uint8_t *aspeed_2500_i2c_bus_pool_base(AspeedI2CBus *bus)
+{
+    return &bus->controller->pool[bus->id * 0x10];
+}
+
 static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -533,6 +681,9 @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data)
     aic->reg_size = 0x40;
     aic->gap = 7;
     aic->bus_get_irq = aspeed_2500_i2c_bus_get_irq;
+    aic->pool_size = 0x100;
+    aic->pool_base = 0x200;
+    aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base;
 }
 
 static const TypeInfo aspeed_2500_i2c_info = {
@@ -546,6 +697,11 @@ static qemu_irq aspeed_2600_i2c_bus_get_irq(AspeedI2CBus *bus)
     return bus->irq;
 }
 
+static uint8_t *aspeed_2600_i2c_bus_pool_base(AspeedI2CBus *bus)
+{
+   return &bus->controller->pool[bus->id * 0x20];
+}
+
 static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -557,6 +713,9 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data)
     aic->reg_size = 0x80;
     aic->gap = -1; /* no gap */
     aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
+    aic->pool_size = 0x200;
+    aic->pool_base = 0xC00;
+    aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base;
 }
 
 static const TypeInfo aspeed_2600_i2c_info = {
-- 
2.21.0



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

* [PATCH 02/17] aspeed/i2c: Check SRAM enablement on AST2500
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 01/17] aspeed/i2c: Add support for pool buffer transfers Cédric Le Goater
@ 2019-11-19 14:11 ` Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 03/17] aspeed: Add a DRAM memory region at the SoC level Cédric Le Goater
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jae Hyun Yoo, Andrew Jeffery, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

The SRAM must be enabled before using the Buffer Pool mode or the DMA
mode. This is not required on other SoCs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/i2c/aspeed_i2c.h |  3 +++
 hw/i2c/aspeed_i2c.c         | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 5313d07aa72f..7a555072dfbf 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -61,6 +61,7 @@ typedef struct AspeedI2CState {
     qemu_irq irq;
 
     uint32_t intr_status;
+    uint32_t ctrl_global;
     MemoryRegion pool_iomem;
     uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE];
 
@@ -83,6 +84,8 @@ typedef struct AspeedI2CClass {
     uint64_t pool_size;
     hwaddr pool_base;
     uint8_t *(*bus_pool_base)(AspeedI2CBus *);
+    bool check_sram;
+
 } AspeedI2CClass;
 
 I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr);
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index e21f45d96868..c7929aa2850f 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -31,6 +31,8 @@
 #define I2C_CTRL_STATUS         0x00        /* Device Interrupt Status */
 #define I2C_CTRL_ASSIGN         0x08        /* Device Interrupt Target
                                                Assignment */
+#define I2C_CTRL_GLOBAL         0x0C        /* Global Control Register */
+#define   I2C_CTRL_SRAM_EN                 BIT(0)
 
 /* I2C Device (Bus) Register */
 
@@ -271,6 +273,29 @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus)
     }
 }
 
+static bool aspeed_i2c_check_sram(AspeedI2CBus *bus)
+{
+    AspeedI2CState *s = bus->controller;
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s);
+
+    if (!aic->check_sram) {
+        return true;
+    }
+
+    /*
+     * AST2500: SRAM must be enabled before using the Buffer Pool or
+     * DMA mode.
+     */
+    if (!(s->ctrl_global & I2C_CTRL_SRAM_EN) &&
+        (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE |
+                     I2CD_RX_BUFF_ENABLE | I2CD_TX_BUFF_ENABLE))) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: SRAM is not enabled\n", __func__);
+        return false;
+    }
+
+    return true;
+}
+
 /*
  * The state machine needs some refinement. It is only used to track
  * invalid STOP commands for the moment.
@@ -282,6 +307,10 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
     bus->cmd &= ~0xFFFF;
     bus->cmd |= value & 0xFFFF;
 
+    if (!aspeed_i2c_check_sram(bus)) {
+        return;
+    }
+
     if (bus->cmd & I2CD_M_START_CMD) {
         uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ?
             I2CD_MSTARTR : I2CD_MSTART;
@@ -436,6 +465,8 @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
     switch (offset) {
     case I2C_CTRL_STATUS:
         return s->intr_status;
+    case I2C_CTRL_GLOBAL:
+        return s->ctrl_global;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
                       __func__, offset);
@@ -448,7 +479,12 @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset,
 static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset,
                                   uint64_t value, unsigned size)
 {
+    AspeedI2CState *s = opaque;
+
     switch (offset) {
+    case I2C_CTRL_GLOBAL:
+        s->ctrl_global = value;
+        break;
     case I2C_CTRL_STATUS:
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
@@ -684,6 +720,7 @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data)
     aic->pool_size = 0x100;
     aic->pool_base = 0x200;
     aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base;
+    aic->check_sram = true;
 }
 
 static const TypeInfo aspeed_2500_i2c_info = {
-- 
2.21.0



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

* [PATCH 03/17] aspeed: Add a DRAM memory region at the SoC level
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 01/17] aspeed/i2c: Add support for pool buffer transfers Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 02/17] aspeed/i2c: Check SRAM enablement on AST2500 Cédric Le Goater
@ 2019-11-19 14:11 ` Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 04/17] aspeed/i2c: Add support for DMA transfers Cédric Le Goater
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jae Hyun Yoo, Andrew Jeffery, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

Currently, we link the DRAM memory region to the FMC model (for DMAs)
through a property alias at the SoC level. The I2C model will need a
similar region for DMA support, add a DRAM region property at the SoC
level for both model to use.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/arm/aspeed_soc.h | 1 +
 hw/arm/aspeed_ast2600.c     | 7 +++++--
 hw/arm/aspeed_soc.c         | 9 +++++++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 495c08be1b84..e84380984f7b 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -40,6 +40,7 @@ typedef struct AspeedSoCState {
     ARMCPU cpu[ASPEED_CPUS_NUM];
     uint32_t num_cpus;
     A15MPPrivState     a7mpcore;
+    MemoryRegion *dram_mr;
     MemoryRegion sram;
     AspeedVICState vic;
     AspeedRtcState rtc;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 931887ac681f..a403c2aae067 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -158,8 +158,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
                           typename);
     object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
                               &error_abort);
-    object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram",
-                              &error_abort);
 
     for (i = 0; i < sc->spis_num; i++) {
         snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
@@ -362,6 +360,11 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     }
 
     /* FMC, The number of CS is set at the board level */
+    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
                             "sdram-base", &err);
     if (err) {
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index f4fe243458fd..dd1ee0e3336d 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -175,8 +175,6 @@ static void aspeed_soc_init(Object *obj)
                           typename);
     object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
                               &error_abort);
-    object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram",
-                              &error_abort);
 
     for (i = 0; i < sc->spis_num; i++) {
         snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
@@ -323,6 +321,11 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_I2C));
 
     /* FMC, The number of CS is set at the board level */
+    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
                             "sdram-base", &err);
     if (err) {
@@ -429,6 +432,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 }
 static Property aspeed_soc_properties[] = {
     DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
+    DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.21.0



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

* [PATCH 04/17] aspeed/i2c: Add support for DMA transfers
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (2 preceding siblings ...)
  2019-11-19 14:11 ` [PATCH 03/17] aspeed: Add a DRAM memory region at the SoC level Cédric Le Goater
@ 2019-11-19 14:11 ` Cédric Le Goater
  2019-11-19 14:11 ` [PATCH 05/17] aspeed/i2c: Add trace events Cédric Le Goater
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jae Hyun Yoo, Andrew Jeffery, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

The I2C controller of the Aspeed AST2500 and AST2600 SoCs supports DMA
transfers to and from DRAM.

A pair of registers defines the buffer address and the length of the
DMA transfer. The address should be aligned on 4 bytes and the maximum
length should not exceed 4K. The receive or transmit DMA transfer can
then be initiated with specific bits in the Command/Status register of
the controller.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/i2c/aspeed_i2c.h |   5 ++
 hw/arm/aspeed_ast2600.c     |   5 ++
 hw/arm/aspeed_soc.c         |   5 ++
 hw/i2c/aspeed_i2c.c         | 126 +++++++++++++++++++++++++++++++++++-
 4 files changed, 138 insertions(+), 3 deletions(-)

diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 7a555072dfbf..f1b9e5bf91e2 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -52,6 +52,8 @@ typedef struct AspeedI2CBus {
     uint32_t cmd;
     uint32_t buf;
     uint32_t pool_ctrl;
+    uint32_t dma_addr;
+    uint32_t dma_len;
 } AspeedI2CBus;
 
 typedef struct AspeedI2CState {
@@ -66,6 +68,8 @@ typedef struct AspeedI2CState {
     uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE];
 
     AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES];
+    MemoryRegion *dram_mr;
+    AddressSpace dram_as;
 } AspeedI2CState;
 
 #define ASPEED_I2C_CLASS(klass) \
@@ -85,6 +89,7 @@ typedef struct AspeedI2CClass {
     hwaddr pool_base;
     uint8_t *(*bus_pool_base)(AspeedI2CBus *);
     bool check_sram;
+    bool has_dma;
 
 } AspeedI2CClass;
 
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a403c2aae067..0881eb25983e 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -343,6 +343,11 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     }
 
     /* I2C */
+    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index dd1ee0e3336d..b01c97744196 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -311,6 +311,11 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     }
 
     /* I2C */
+    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
     object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index c7929aa2850f..030d9c56be65 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -23,8 +23,11 @@
 #include "migration/vmstate.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/i2c/aspeed_i2c.h"
 #include "hw/irq.h"
+#include "hw/qdev-properties.h"
 
 /* I2C Global Register */
 
@@ -138,7 +141,8 @@
 #define   I2CD_BYTE_BUF_TX_MASK            0xff
 #define   I2CD_BYTE_BUF_RX_SHIFT           8
 #define   I2CD_BYTE_BUF_RX_MASK            0xff
-
+#define I2CD_DMA_ADDR           0x24       /* DMA Buffer Address */
+#define I2CD_DMA_LEN            0x28       /* DMA Transfer Length < 4KB */
 
 static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
 {
@@ -165,6 +169,7 @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
                                     unsigned size)
 {
     AspeedI2CBus *bus = opaque;
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
 
     switch (offset) {
     case I2CD_FUN_CTRL_REG:
@@ -183,6 +188,18 @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
         return bus->buf;
     case I2CD_CMD_REG:
         return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
+    case I2CD_DMA_ADDR:
+        if (!aic->has_dma) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
+            return -1;
+        }
+        return bus->dma_addr;
+    case I2CD_DMA_LEN:
+        if (!aic->has_dma) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
+            return -1;
+        }
+        return bus->dma_len;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
@@ -201,6 +218,24 @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus)
     return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK;
 }
 
+static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data)
+{
+    MemTxResult result;
+    AspeedI2CState *s = bus->controller;
+
+    result = address_space_read(&s->dram_as, bus->dma_addr,
+                                MEMTXATTRS_UNSPECIFIED, data, 1);
+    if (result != MEMTX_OK) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n",
+                      __func__, bus->dma_addr);
+        return -1;
+    }
+
+    bus->dma_addr++;
+    bus->dma_len--;
+    return 0;
+}
+
 static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
 {
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
@@ -217,6 +252,16 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
             }
         }
         bus->cmd &= ~I2CD_TX_BUFF_ENABLE;
+    } else if (bus->cmd & I2CD_TX_DMA_ENABLE) {
+        while (bus->dma_len) {
+            uint8_t data;
+            aspeed_i2c_dma_read(bus, &data);
+            ret = i2c_send(bus->bus, data);
+            if (ret) {
+                break;
+            }
+        }
+        bus->cmd &= ~I2CD_TX_DMA_ENABLE;
     } else {
         ret = i2c_send(bus->bus, bus->buf);
     }
@@ -242,6 +287,24 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
         bus->pool_ctrl &= ~(0xff << 24);
         bus->pool_ctrl |= (i & 0xff) << 24;
         bus->cmd &= ~I2CD_RX_BUFF_ENABLE;
+    } else if (bus->cmd & I2CD_RX_DMA_ENABLE) {
+        uint8_t data;
+
+        while (bus->dma_len) {
+            MemTxResult result;
+
+            data = i2c_recv(bus->bus);
+            result = address_space_write(&s->dram_as, bus->dma_addr,
+                                         MEMTXATTRS_UNSPECIFIED, &data, 1);
+            if (result != MEMTX_OK) {
+                qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n",
+                              __func__, bus->dma_addr);
+                return;
+            }
+            bus->dma_addr++;
+            bus->dma_len--;
+        }
+        bus->cmd &= ~I2CD_RX_DMA_ENABLE;
     } else {
         data = i2c_recv(bus->bus);
         bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
@@ -268,6 +331,11 @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus)
         uint8_t *pool_base = aic->bus_pool_base(bus);
 
         return pool_base[0];
+    } else if (bus->cmd & I2CD_TX_DMA_ENABLE) {
+        uint8_t data;
+
+        aspeed_i2c_dma_read(bus, &data);
+        return data;
     } else {
         return bus->buf;
     }
@@ -344,6 +412,10 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
                  */
                 pool_start++;
             }
+        } else if (bus->cmd & I2CD_TX_DMA_ENABLE) {
+            if (bus->dma_len == 0) {
+                bus->cmd &= ~I2CD_M_TX_CMD;
+            }
         } else {
             bus->cmd &= ~I2CD_M_TX_CMD;
         }
@@ -447,9 +519,35 @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
             break;
         }
 
+        if (!aic->has_dma &&
+            value & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
+            break;
+        }
+
         aspeed_i2c_bus_handle_cmd(bus, value);
         aspeed_i2c_bus_raise_interrupt(bus);
         break;
+    case I2CD_DMA_ADDR:
+        if (!aic->has_dma) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
+            break;
+        }
+
+        bus->dma_addr = value & 0xfffffffc;
+        break;
+
+    case I2CD_DMA_LEN:
+        if (!aic->has_dma) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
+            break;
+        }
+
+        bus->dma_len = value & 0xfff;
+        if (!bus->dma_len) {
+            qemu_log_mask(LOG_UNIMP, "%s: invalid DMA length\n",  __func__);
+        }
+        break;
 
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
@@ -542,8 +640,8 @@ static const MemoryRegionOps aspeed_i2c_pool_ops = {
 
 static const VMStateDescription aspeed_i2c_bus_vmstate = {
     .name = TYPE_ASPEED_I2C,
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(id, AspeedI2CBus),
         VMSTATE_UINT32(ctrl, AspeedI2CBus),
@@ -553,6 +651,8 @@ static const VMStateDescription aspeed_i2c_bus_vmstate = {
         VMSTATE_UINT32(cmd, AspeedI2CBus),
         VMSTATE_UINT32(buf, AspeedI2CBus),
         VMSTATE_UINT32(pool_ctrl, AspeedI2CBus),
+        VMSTATE_UINT32(dma_addr, AspeedI2CBus),
+        VMSTATE_UINT32(dma_len, AspeedI2CBus),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -584,6 +684,8 @@ static void aspeed_i2c_reset(DeviceState *dev)
         s->busses[i].intr_status = 0;
         s->busses[i].cmd = 0;
         s->busses[i].buf = 0;
+        s->busses[i].dma_addr = 0;
+        s->busses[i].dma_len = 0;
         i2c_end_transfer(s->busses[i].bus);
     }
 }
@@ -640,14 +742,30 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s,
                           "aspeed.i2c-pool", aic->pool_size);
     memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem);
+
+    if (aic->has_dma) {
+        if (!s->dram_mr) {
+            error_setg(errp, TYPE_ASPEED_I2C ": 'dram' link not set");
+            return;
+        }
+
+        address_space_init(&s->dram_as, s->dram_mr, "dma-dram");
+    }
 }
 
+static Property aspeed_i2c_properties[] = {
+    DEFINE_PROP_LINK("dram", AspeedI2CState, dram_mr,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 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->props = aspeed_i2c_properties;
     dc->realize = aspeed_i2c_realize;
     dc->desc = "Aspeed I2C Controller";
 }
@@ -721,6 +839,7 @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data)
     aic->pool_base = 0x200;
     aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base;
     aic->check_sram = true;
+    aic->has_dma = true;
 }
 
 static const TypeInfo aspeed_2500_i2c_info = {
@@ -753,6 +872,7 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data)
     aic->pool_size = 0x200;
     aic->pool_base = 0xC00;
     aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base;
+    aic->has_dma = true;
 }
 
 static const TypeInfo aspeed_2600_i2c_info = {
-- 
2.21.0



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

* [PATCH 05/17] aspeed/i2c: Add trace events
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (3 preceding siblings ...)
  2019-11-19 14:11 ` [PATCH 04/17] aspeed/i2c: Add support for DMA transfers Cédric Le Goater
@ 2019-11-19 14:11 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 06/17] aspeed/sdmc: Make ast2600 default 1G Cédric Le Goater
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Jae Hyun Yoo, Andrew Jeffery, qemu-devel, qemu-arm,
	Cédric Le Goater, Philippe Mathieu-Daudé,
	Joel Stanley

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/i2c/aspeed_i2c.c | 93 ++++++++++++++++++++++++++++++++++++++-------
 hw/i2c/trace-events |  9 +++++
 2 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 030d9c56be65..2da04a4bff30 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -28,6 +28,7 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "trace.h"
 
 /* I2C Global Register */
 
@@ -158,6 +159,13 @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus)
 {
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
 
+    trace_aspeed_i2c_bus_raise_interrupt(bus->intr_status,
+          bus->intr_status & I2CD_INTR_TX_NAK ? "nak|" : "",
+          bus->intr_status & I2CD_INTR_TX_ACK ? "ack|" : "",
+          bus->intr_status & I2CD_INTR_RX_DONE ? "done|" : "",
+          bus->intr_status & I2CD_INTR_NORMAL_STOP ? "normal|" : "",
+          bus->intr_status & I2CD_INTR_ABNORMAL ? "abnormal" : "");
+
     bus->intr_status &= bus->intr_ctrl;
     if (bus->intr_status) {
         bus->controller->intr_status |= 1 << bus->id;
@@ -170,41 +178,57 @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset,
 {
     AspeedI2CBus *bus = opaque;
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
+    uint64_t value = -1;
 
     switch (offset) {
     case I2CD_FUN_CTRL_REG:
-        return bus->ctrl;
+        value = bus->ctrl;
+        break;
     case I2CD_AC_TIMING_REG1:
-        return bus->timing[0];
+        value = bus->timing[0];
+        break;
     case I2CD_AC_TIMING_REG2:
-        return bus->timing[1];
+        value = bus->timing[1];
+        break;
     case I2CD_INTR_CTRL_REG:
-        return bus->intr_ctrl;
+        value = bus->intr_ctrl;
+        break;
     case I2CD_INTR_STS_REG:
-        return bus->intr_status;
+        value = bus->intr_status;
+        break;
     case I2CD_POOL_CTRL_REG:
-        return bus->pool_ctrl;
+        value = bus->pool_ctrl;
+        break;
     case I2CD_BYTE_BUF_REG:
-        return bus->buf;
+        value = bus->buf;
+        break;
     case I2CD_CMD_REG:
-        return bus->cmd | (i2c_bus_busy(bus->bus) << 16);
+        value = bus->cmd | (i2c_bus_busy(bus->bus) << 16);
+        break;
     case I2CD_DMA_ADDR:
         if (!aic->has_dma) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
-            return -1;
+            break;
         }
-        return bus->dma_addr;
+        value = bus->dma_addr;
+        break;
     case I2CD_DMA_LEN:
         if (!aic->has_dma) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n",  __func__);
-            return -1;
+            break;
         }
-        return bus->dma_len;
+        value = bus->dma_len;
+        break;
+
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset);
-        return -1;
+        value = -1;
+        break;
     }
+
+    trace_aspeed_i2c_bus_read(bus->id, offset, size, value);
+    return value;
 }
 
 static void aspeed_i2c_set_state(AspeedI2CBus *bus, uint8_t state)
@@ -246,6 +270,9 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
         for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) {
             uint8_t *pool_base = aic->bus_pool_base(bus);
 
+            trace_aspeed_i2c_bus_send("BUF", i + 1,
+                                      I2CD_POOL_TX_COUNT(bus->pool_ctrl),
+                                      pool_base[i]);
             ret = i2c_send(bus->bus, pool_base[i]);
             if (ret) {
                 break;
@@ -256,6 +283,7 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
         while (bus->dma_len) {
             uint8_t data;
             aspeed_i2c_dma_read(bus, &data);
+            trace_aspeed_i2c_bus_send("DMA", bus->dma_len, bus->dma_len, data);
             ret = i2c_send(bus->bus, data);
             if (ret) {
                 break;
@@ -263,6 +291,7 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
         }
         bus->cmd &= ~I2CD_TX_DMA_ENABLE;
     } else {
+        trace_aspeed_i2c_bus_send("BYTE", pool_start, 1, bus->buf);
         ret = i2c_send(bus->bus, bus->buf);
     }
 
@@ -281,6 +310,9 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
 
         for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) {
             pool_base[i] = i2c_recv(bus->bus);
+            trace_aspeed_i2c_bus_recv("BUF", i + 1,
+                                      I2CD_POOL_RX_SIZE(bus->pool_ctrl),
+                                      pool_base[i]);
         }
 
         /* Update RX count */
@@ -294,6 +326,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
             MemTxResult result;
 
             data = i2c_recv(bus->bus);
+            trace_aspeed_i2c_bus_recv("DMA", bus->dma_len, bus->dma_len, data);
             result = address_space_write(&s->dram_as, bus->dma_addr,
                                          MEMTXATTRS_UNSPECIFIED, &data, 1);
             if (result != MEMTX_OK) {
@@ -307,6 +340,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
         bus->cmd &= ~I2CD_RX_DMA_ENABLE;
     } else {
         data = i2c_recv(bus->bus);
+        trace_aspeed_i2c_bus_recv("BYTE", 1, 1, bus->buf);
         bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT;
     }
 }
@@ -364,6 +398,33 @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus)
     return true;
 }
 
+static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus)
+{
+    g_autofree char *cmd_flags;
+    uint32_t count;
+
+    if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) {
+        count = I2CD_POOL_TX_COUNT(bus->pool_ctrl);
+    } else if (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_RX_DMA_ENABLE)) {
+        count = bus->dma_len;
+    } else { /* BYTE mode */
+        count = 1;
+    }
+
+    cmd_flags = g_strdup_printf("%s%s%s%s%s%s%s%s%s",
+                                bus->cmd & I2CD_M_START_CMD ? "start|" : "",
+                                bus->cmd & I2CD_RX_DMA_ENABLE ? "rxdma|" : "",
+                                bus->cmd & I2CD_TX_DMA_ENABLE ? "txdma|" : "",
+                                bus->cmd & I2CD_RX_BUFF_ENABLE ? "rxbuf|" : "",
+                                bus->cmd & I2CD_TX_BUFF_ENABLE ? "txbuf|" : "",
+                                bus->cmd & I2CD_M_TX_CMD ? "tx|" : "",
+                                bus->cmd & I2CD_M_RX_CMD ? "rx|" : "",
+                                bus->cmd & I2CD_M_S_RX_CMD_LAST ? "last|" : "",
+                                bus->cmd & I2CD_M_STOP_CMD ? "stop" : "");
+
+    trace_aspeed_i2c_bus_cmd(bus->cmd, cmd_flags, count, bus->intr_status);
+}
+
 /*
  * The state machine needs some refinement. It is only used to track
  * invalid STOP commands for the moment.
@@ -379,6 +440,10 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
         return;
     }
 
+    if (trace_event_get_state_backends(TRACE_ASPEED_I2C_BUS_CMD)) {
+        aspeed_i2c_bus_cmd_dump(bus);
+    }
+
     if (bus->cmd & I2CD_M_START_CMD) {
         uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ?
             I2CD_MSTARTR : I2CD_MSTART;
@@ -465,6 +530,8 @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset,
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
     bool handle_rx;
 
+    trace_aspeed_i2c_bus_write(bus->id, offset, size, value);
+
     switch (offset) {
     case I2CD_FUN_CTRL_REG:
         if (value & I2CD_SLAVE_EN) {
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index e1c810d5bd08..08db8fa68924 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -5,3 +5,12 @@
 i2c_event(const char *event, uint8_t address) "%s(addr:0x%02x)"
 i2c_send(uint8_t address, uint8_t data) "send(addr:0x%02x) data:0x%02x"
 i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x"
+
+# aspeed_i2c.c
+
+aspeed_i2c_bus_cmd(uint32_t cmd, const char *cmd_flags, uint32_t count, uint32_t intr_status) "handling cmd=0x%x %s count=%d intr=0x%x"
+aspeed_i2c_bus_raise_interrupt(uint32_t intr_status, const char *str1, const char *str2, const char *str3, const char *str4, const char *str5) "handled intr=0x%x %s%s%s%s%s"
+aspeed_i2c_bus_read(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64
+aspeed_i2c_bus_write(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64
+aspeed_i2c_bus_send(const char *mode, int i, int count, uint8_t byte) "%s send %d/%d 0x%02x"
+aspeed_i2c_bus_recv(const char *mode, int i, int count, uint8_t byte) "%s recv %d/%d 0x%02x"
-- 
2.21.0



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

* [PATCH 06/17] aspeed/sdmc: Make ast2600 default 1G
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (4 preceding siblings ...)
  2019-11-19 14:11 ` [PATCH 05/17] aspeed/i2c: Add trace events Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 07/17] aspeed/scu: Fix W1C behavior Cédric Le Goater
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, qemu-devel, qemu-arm, Cédric Le Goater,
	Alex Bennée, Joel Stanley

From: Joel Stanley <joel@jms.id.au>

Most boards have this much.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_sdmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index f3a63a2e01db..2df3244b53c8 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -208,10 +208,10 @@ static int ast2600_rambits(AspeedSDMCState *s)
     }
 
     /* use a common default */
-    warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M",
+    warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 1024M",
                 s->ram_size);
-    s->ram_size = 512 << 20;
-    return ASPEED_SDMC_AST2600_512MB;
+    s->ram_size = 1024 << 20;
+    return ASPEED_SDMC_AST2600_1024MB;
 }
 
 static void aspeed_sdmc_reset(DeviceState *dev)
-- 
2.21.0



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

* [PATCH 07/17] aspeed/scu: Fix W1C behavior
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (5 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 06/17] aspeed/sdmc: Make ast2600 default 1G Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 08/17] watchdog/aspeed: Improve watchdog timeout message Cédric Le Goater
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, qemu-devel, qemu-arm, Cédric Le Goater,
	Alex Bennée, Joel Stanley

From: Joel Stanley <joel@jms.id.au>

This models the clock write one to clear registers, and fixes up some
incorrect behavior in all of the write to clear registers.

There was also a typo in one of the register definitions.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: checkpatch.pl fixes ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_scu.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 717509bc5460..f62fa25e3474 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -98,7 +98,7 @@
 #define AST2600_CLK_STOP_CTRL     TO_REG(0x80)
 #define AST2600_CLK_STOP_CTRL_CLR TO_REG(0x84)
 #define AST2600_CLK_STOP_CTRL2     TO_REG(0x90)
-#define AST2600_CLK_STOP_CTR2L_CLR TO_REG(0x94)
+#define AST2600_CLK_STOP_CTRL2_CLR TO_REG(0x94)
 #define AST2600_SDRAM_HANDSHAKE   TO_REG(0x100)
 #define AST2600_HPLL_PARAM        TO_REG(0x200)
 #define AST2600_HPLL_EXT          TO_REG(0x204)
@@ -532,11 +532,13 @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset,
     return s->regs[reg];
 }
 
-static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data,
-                                     unsigned size)
+static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
+                                     uint64_t data64, unsigned size)
 {
     AspeedSCUState *s = ASPEED_SCU(opaque);
     int reg = TO_REG(offset);
+    /* Truncate here so bitwise operations below behave as expected */
+    uint32_t data = data64;
 
     if (reg >= ASPEED_AST2600_SCU_NR_REGS) {
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -563,15 +565,22 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data,
         /* fall through */
     case AST2600_SYS_RST_CTRL:
     case AST2600_SYS_RST_CTRL2:
+    case AST2600_CLK_STOP_CTRL:
+    case AST2600_CLK_STOP_CTRL2:
         /* W1S (Write 1 to set) registers */
         s->regs[reg] |= data;
         return;
     case AST2600_SYS_RST_CTRL_CLR:
     case AST2600_SYS_RST_CTRL2_CLR:
+    case AST2600_CLK_STOP_CTRL_CLR:
+    case AST2600_CLK_STOP_CTRL2_CLR:
     case AST2600_HW_STRAP1_CLR:
     case AST2600_HW_STRAP2_CLR:
-        /* W1C (Write 1 to clear) registers */
-        s->regs[reg] &= ~data;
+        /*
+         * W1C (Write 1 to clear) registers are offset by one address from
+         * the data register
+         */
+        s->regs[reg - 1] &= ~data;
         return;
 
     case AST2600_RNG_DATA:
-- 
2.21.0



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

* [PATCH 08/17] watchdog/aspeed: Improve watchdog timeout message
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (6 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 07/17] aspeed/scu: Fix W1C behavior Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 09/17] watchdog/aspeed: Fix AST2600 frequency behaviour Cédric Le Goater
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, qemu-devel, qemu-arm, Cédric Le Goater,
	Alex Bennée, Joel Stanley

From: Joel Stanley <joel@jms.id.au>

Users benefit from knowing which watchdog timer has expired. The address
of the watchdog's registers unambiguously indicates which has expired,
so log that.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/watchdog/wdt_aspeed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 145be6f99ce2..d283d07d6546 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -219,7 +219,8 @@ static void aspeed_wdt_timer_expired(void *dev)
         return;
     }
 
-    qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
+    qemu_log_mask(CPU_LOG_RESET, "Watchdog timer %" HWADDR_PRIx " expired.\n",
+                  s->iomem.addr);
     watchdog_perform_action();
     timer_del(s->timer);
 }
-- 
2.21.0



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

* [PATCH 09/17] watchdog/aspeed: Fix AST2600 frequency behaviour
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (7 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 08/17] watchdog/aspeed: Improve watchdog timeout message Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 10/17] aspeed/smc: Restore default AHB window mapping at reset Cédric Le Goater
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, qemu-devel, qemu-arm, Cédric Le Goater,
	Alex Bennée, Joel Stanley

From: Joel Stanley <joel@jms.id.au>

The AST2600 control register sneakily changed the meaning of bit 4
without anyone noticing. It no longer controls the 1MHz vs APB clock
select, and instead always runs at 1MHz.

The AST2500 was always 1MHz too, but it retained bit 4, making it read
only. We can model both using the same fixed 1MHz calculation.

Fixes: 6b2b2a703cad ("hw: wdt_aspeed: Add AST2600 support")
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/watchdog/wdt_aspeed.h |  1 +
 hw/watchdog/wdt_aspeed.c         | 21 +++++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
index dfedd7662dd1..819c22993a6e 100644
--- a/include/hw/watchdog/wdt_aspeed.h
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -47,6 +47,7 @@ typedef struct AspeedWDTClass {
     uint32_t ext_pulse_width_mask;
     uint32_t reset_ctrl_reg;
     void (*reset_pulse)(AspeedWDTState *s, uint32_t property);
+    void (*wdt_reload)(AspeedWDTState *s);
 }  AspeedWDTClass;
 
 #endif /* WDT_ASPEED_H */
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index d283d07d6546..122aa8daaadf 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -93,11 +93,11 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)
 
 }
 
-static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk)
+static void aspeed_wdt_reload(AspeedWDTState *s)
 {
     uint64_t reload;
 
-    if (pclk) {
+    if (!(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK)) {
         reload = muldiv64(s->regs[WDT_RELOAD_VALUE], NANOSECONDS_PER_SECOND,
                           s->pclk_freq);
     } else {
@@ -109,6 +109,16 @@ static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk)
     }
 }
 
+static void aspeed_wdt_reload_1mhz(AspeedWDTState *s)
+{
+    uint64_t reload = s->regs[WDT_RELOAD_VALUE] * 1000ULL;
+
+    if (aspeed_wdt_is_enabled(s)) {
+        timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + reload);
+    }
+}
+
+
 static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
                              unsigned size)
 {
@@ -130,13 +140,13 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
     case WDT_RESTART:
         if ((data & 0xFFFF) == WDT_RESTART_MAGIC) {
             s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE];
-            aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK));
+            awc->wdt_reload(s);
         }
         break;
     case WDT_CTRL:
         if (enable && !aspeed_wdt_is_enabled(s)) {
             s->regs[WDT_CTRL] = data;
-            aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK));
+            awc->wdt_reload(s);
         } else if (!enable && aspeed_wdt_is_enabled(s)) {
             s->regs[WDT_CTRL] = data;
             timer_del(s->timer);
@@ -283,6 +293,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data)
     awc->offset = 0x20;
     awc->ext_pulse_width_mask = 0xff;
     awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
+    awc->wdt_reload = aspeed_wdt_reload;
 }
 
 static const TypeInfo aspeed_2400_wdt_info = {
@@ -317,6 +328,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data)
     awc->ext_pulse_width_mask = 0xfffff;
     awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
     awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
+    awc->wdt_reload = aspeed_wdt_reload_1mhz;
 }
 
 static const TypeInfo aspeed_2500_wdt_info = {
@@ -336,6 +348,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data)
     awc->ext_pulse_width_mask = 0xfffff; /* TODO */
     awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
     awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
+    awc->wdt_reload = aspeed_wdt_reload_1mhz;
 }
 
 static const TypeInfo aspeed_2600_wdt_info = {
-- 
2.21.0



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

* [PATCH 10/17] aspeed/smc: Restore default AHB window mapping at reset
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (8 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 09/17] watchdog/aspeed: Fix AST2600 frequency behaviour Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 11/17] aspeed/smc: Do not map disabled segment on the AST2600 Cédric Le Goater
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

The current model only restores the Segment Register values but leaves
the previous CS mapping behind. Introduce a helper setting the
register value and mapping the region at the requested address. Use
this helper when a Segment register is set and at reset.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index f0c7bbbad302..955ec21852ac 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -475,10 +475,26 @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
     return false;
 }
 
+static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs,
+                                                uint64_t regval)
+{
+    AspeedSMCFlash *fl = &s->flashes[cs];
+    AspeedSegments seg;
+
+    s->ctrl->reg_to_segment(s, regval, &seg);
+
+    memory_region_transaction_begin();
+    memory_region_set_size(&fl->mmio, seg.size);
+    memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
+    memory_region_set_enabled(&fl->mmio, true);
+    memory_region_transaction_commit();
+
+    s->regs[R_SEG_ADDR0 + cs] = regval;
+}
+
 static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
                                          uint64_t new)
 {
-    AspeedSMCFlash *fl = &s->flashes[cs];
     AspeedSegments seg;
 
     s->ctrl->reg_to_segment(s, new, &seg);
@@ -529,13 +545,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
     aspeed_smc_flash_overlap(s, &seg, cs);
 
     /* All should be fine now to move the region */
-    memory_region_transaction_begin();
-    memory_region_set_size(&fl->mmio, seg.size);
-    memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
-    memory_region_set_enabled(&fl->mmio, true);
-    memory_region_transaction_commit();
-
-    s->regs[R_SEG_ADDR0 + cs] = new;
+    aspeed_smc_flash_set_segment_region(s, cs, new);
 }
 
 static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
@@ -897,10 +907,10 @@ static void aspeed_smc_reset(DeviceState *d)
         qemu_set_irq(s->cs_lines[i], true);
     }
 
-    /* setup default segment register values for all */
+    /* setup the default segment register values and regions for all */
     for (i = 0; i < s->ctrl->max_slaves; ++i) {
-        s->regs[R_SEG_ADDR0 + i] =
-            s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]);
+        aspeed_smc_flash_set_segment_region(s, i,
+                    s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]));
     }
 
     /* HW strapping flash type for the AST2600 controllers  */
-- 
2.21.0



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

* [PATCH 11/17] aspeed/smc: Do not map disabled segment on the AST2600
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (9 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 10/17] aspeed/smc: Restore default AHB window mapping at reset Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 12/17] aspeed/smc: Add AST2600 timings registers Cédric Le Goater
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

The segments can be disabled on the AST2600 (zero register value).
CS0 is open by default but not the other CS. This is closing the
access to the flash device in user mode and forbids scanning.

In the model, check the segment size and disable the associated region
when the value is zero.

Fixes: bcaa8ddd081c ("aspeed/smc: Add AST2600 support")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 955ec21852ac..86cadbe4cc00 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -444,8 +444,13 @@ static void aspeed_2600_smc_reg_to_segment(const AspeedSMCState *s,
     uint32_t start_offset = (reg << 16) & AST2600_SEG_ADDR_MASK;
     uint32_t end_offset = reg & AST2600_SEG_ADDR_MASK;
 
-    seg->addr = s->ctrl->flash_window_base + start_offset;
-    seg->size = end_offset + MiB - start_offset;
+    if (reg) {
+        seg->addr = s->ctrl->flash_window_base + start_offset;
+        seg->size = end_offset + MiB - start_offset;
+    } else {
+        seg->addr = s->ctrl->flash_window_base;
+        seg->size = 0;
+    }
 }
 
 static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
@@ -486,7 +491,7 @@ static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs,
     memory_region_transaction_begin();
     memory_region_set_size(&fl->mmio, seg.size);
     memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
-    memory_region_set_enabled(&fl->mmio, true);
+    memory_region_set_enabled(&fl->mmio, !!seg.size);
     memory_region_transaction_commit();
 
     s->regs[R_SEG_ADDR0 + cs] = regval;
@@ -526,8 +531,9 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
     }
 
     /* Keep the segment in the overall flash window */
-    if (seg.addr + seg.size <= s->ctrl->flash_window_base ||
-        seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) {
+    if (seg.size &&
+        (seg.addr + seg.size <= s->ctrl->flash_window_base ||
+         seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size)) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : "
                       "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
                       s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
-- 
2.21.0



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

* [PATCH 12/17] aspeed/smc: Add AST2600 timings registers
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (10 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 11/17] aspeed/smc: Do not map disabled segment on the AST2600 Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 13/17] aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass Cédric Le Goater
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

Each CS has its own Read Timing Compensation Register on newer SoCs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ssi/aspeed_smc.h |  1 +
 hw/ssi/aspeed_smc.c         | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 684d16e33613..6fbbb238f158 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -40,6 +40,7 @@ typedef struct AspeedSMCController {
     uint8_t r_ce_ctrl;
     uint8_t r_ctrl0;
     uint8_t r_timings;
+    uint8_t nregs_timings;
     uint8_t conf_enable_w0;
     uint8_t max_slaves;
     const AspeedSegments *segments;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 86cadbe4cc00..7755eca34976 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -137,7 +137,7 @@
 /* Checksum Calculation Result */
 #define R_DMA_CHECKSUM    (0x90 / 4)
 
-/* Misc Control Register #2 */
+/* Read Timing Compensation Register */
 #define R_TIMINGS         (0x94 / 4)
 
 /* SPI controller registers and bits (AST2400) */
@@ -256,6 +256,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 5,
         .segments          = aspeed_segments_legacy,
@@ -271,6 +272,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 5,
         .segments          = aspeed_segments_fmc,
@@ -288,6 +290,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = 0xff,
         .r_ctrl0           = R_SPI_CTRL0,
         .r_timings         = R_SPI_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = SPI_CONF_ENABLE_W0,
         .max_slaves        = 1,
         .segments          = aspeed_segments_spi,
@@ -303,6 +306,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 3,
         .segments          = aspeed_segments_ast2500_fmc,
@@ -320,6 +324,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 2,
         .segments          = aspeed_segments_ast2500_spi1,
@@ -335,6 +340,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 2,
         .segments          = aspeed_segments_ast2500_spi2,
@@ -350,6 +356,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 1,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 3,
         .segments          = aspeed_segments_ast2600_fmc,
@@ -365,6 +372,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 2,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 2,
         .segments          = aspeed_segments_ast2600_spi1,
@@ -380,6 +388,7 @@ static const AspeedSMCController controllers[] = {
         .r_ce_ctrl         = R_CE_CTRL,
         .r_ctrl0           = R_CTRL0,
         .r_timings         = R_TIMINGS,
+        .nregs_timings     = 3,
         .conf_enable_w0    = CONF_ENABLE_W0,
         .max_slaves        = 3,
         .segments          = aspeed_segments_ast2600_spi2,
@@ -951,7 +960,8 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
     addr >>= 2;
 
     if (addr == s->r_conf ||
-        addr == s->r_timings ||
+        (addr >= s->r_timings &&
+         addr < s->r_timings + s->ctrl->nregs_timings) ||
         addr == s->r_ce_ctrl ||
         addr == R_INTR_CTRL ||
         addr == R_DUMMY_DATA ||
@@ -1216,7 +1226,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
     addr >>= 2;
 
     if (addr == s->r_conf ||
-        addr == s->r_timings ||
+        (addr >= s->r_timings &&
+         addr < s->r_timings + s->ctrl->nregs_timings) ||
         addr == s->r_ce_ctrl) {
         s->regs[addr] = value;
     } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
-- 
2.21.0



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

* [PATCH 13/17] aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (11 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 12/17] aspeed/smc: Add AST2600 timings registers Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 14/17] aspeed: Add support for the tacoma-bmc board Cédric Le Goater
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

AspeedBoardConfig is a redundant way to define class attributes and it
complexifies the machine definition and initialization.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/arm/aspeed.h |  24 ++---
 hw/arm/aspeed.c         | 231 ++++++++++++++++++++++------------------
 2 files changed, 137 insertions(+), 118 deletions(-)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index f49bc7081e4d..4423cd0cda71 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -13,19 +13,6 @@
 
 typedef struct AspeedBoardState AspeedBoardState;
 
-typedef struct AspeedBoardConfig {
-    const char *name;
-    const char *desc;
-    const char *soc_name;
-    uint32_t hw_strap1;
-    uint32_t hw_strap2;
-    const char *fmc_model;
-    const char *spi_model;
-    uint32_t num_cs;
-    void (*i2c_init)(AspeedBoardState *bmc);
-    uint32_t ram;
-} AspeedBoardConfig;
-
 #define TYPE_ASPEED_MACHINE       MACHINE_TYPE_NAME("aspeed")
 #define ASPEED_MACHINE(obj) \
     OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE)
@@ -41,7 +28,16 @@ typedef struct AspeedMachine {
 
 typedef struct AspeedMachineClass {
     MachineClass parent_obj;
-    const AspeedBoardConfig *board;
+
+    const char *name;
+    const char *desc;
+    const char *soc_name;
+    uint32_t hw_strap1;
+    uint32_t hw_strap2;
+    const char *fmc_model;
+    const char *spi_model;
+    uint32_t num_cs;
+    void (*i2c_init)(AspeedBoardState *bmc);
 } AspeedMachineClass;
 
 
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 028191ff36fc..e34e6787430b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -167,10 +167,10 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
-static void aspeed_board_init(MachineState *machine,
-                              const AspeedBoardConfig *cfg)
+static void aspeed_machine_init(MachineState *machine)
 {
     AspeedBoardState *bmc;
+    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
     AspeedSoCClass *sc;
     DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
     ram_addr_t max_ram_size;
@@ -182,18 +182,18 @@ static void aspeed_board_init(MachineState *machine,
                        UINT32_MAX);
 
     object_initialize_child(OBJECT(machine), "soc", &bmc->soc,
-                            (sizeof(bmc->soc)), cfg->soc_name, &error_abort,
+                            (sizeof(bmc->soc)), amc->soc_name, &error_abort,
                             NULL);
 
     sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
 
     object_property_set_uint(OBJECT(&bmc->soc), ram_size, "ram-size",
                              &error_abort);
-    object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1",
+    object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap1, "hw-strap1",
                             &error_abort);
-    object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap2, "hw-strap2",
+    object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap2, "hw-strap2",
                             &error_abort);
-    object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs",
+    object_property_set_int(OBJECT(&bmc->soc), amc->num_cs, "num-cs",
                             &error_abort);
     object_property_set_int(OBJECT(&bmc->soc), machine->smp.cpus, "num-cpus",
                             &error_abort);
@@ -230,8 +230,8 @@ static void aspeed_board_init(MachineState *machine,
                           "max_ram", max_ram_size  - ram_size);
     memory_region_add_subregion(&bmc->ram_container, ram_size, &bmc->max_ram);
 
-    aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
-    aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort);
+    aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model, &error_abort);
+    aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model, &error_abort);
 
     /* Install first FMC flash content as a boot rom. */
     if (drive0) {
@@ -255,8 +255,8 @@ static void aspeed_board_init(MachineState *machine,
     aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM];
     aspeed_board_binfo.nb_cpus = bmc->soc.num_cpus;
 
-    if (cfg->i2c_init) {
-        cfg->i2c_init(bmc);
+    if (amc->i2c_init) {
+        amc->i2c_init(bmc);
     }
 
     for (i = 0; i < ARRAY_SIZE(bmc->soc.sdhci.slots); i++) {
@@ -383,118 +383,141 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
                      0x60);
 }
 
-static void aspeed_machine_init(MachineState *machine)
-{
-    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
-
-    aspeed_board_init(machine, amc->board);
-}
-
 static void aspeed_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
-    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
-    const AspeedBoardConfig *board = data;
 
-    mc->desc = board->desc;
     mc->init = aspeed_machine_init;
     mc->max_cpus = ASPEED_CPUS_NUM;
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->no_parallel = 1;
-    if (board->ram) {
-        mc->default_ram_size = board->ram;
-    }
-    amc->board = board;
 }
 
-static const TypeInfo aspeed_machine_type = {
-    .name = TYPE_ASPEED_MACHINE,
-    .parent = TYPE_MACHINE,
-    .instance_size = sizeof(AspeedMachine),
-    .class_size = sizeof(AspeedMachineClass),
-    .abstract = true,
+static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "OpenPOWER Palmetto BMC (ARM926EJ-S)";
+    amc->soc_name  = "ast2400-a1";
+    amc->hw_strap1 = PALMETTO_BMC_HW_STRAP1;
+    amc->fmc_model = "n25q256a";
+    amc->spi_model = "mx25l25635e";
+    amc->num_cs    = 1;
+    amc->i2c_init  = palmetto_bmc_i2c_init;
+    mc->default_ram_size       = 256 * MiB;
+};
+
+static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Aspeed AST2500 EVB (ARM1176)";
+    amc->soc_name  = "ast2500-a1";
+    amc->hw_strap1 = AST2500_EVB_HW_STRAP1;
+    amc->fmc_model = "w25q256";
+    amc->spi_model = "mx25l25635e";
+    amc->num_cs    = 1;
+    amc->i2c_init  = ast2500_evb_i2c_init;
+    mc->default_ram_size       = 512 * MiB;
+};
+
+static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "OpenPOWER Romulus BMC (ARM1176)";
+    amc->soc_name  = "ast2500-a1";
+    amc->hw_strap1 = ROMULUS_BMC_HW_STRAP1;
+    amc->fmc_model = "n25q256a";
+    amc->spi_model = "mx66l1g45g";
+    amc->num_cs    = 2;
+    amc->i2c_init  = romulus_bmc_i2c_init;
+    mc->default_ram_size       = 512 * MiB;
 };
 
-static const AspeedBoardConfig aspeed_boards[] = {
+static void aspeed_machine_swift_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "OpenPOWER Swift BMC (ARM1176)";
+    amc->soc_name  = "ast2500-a1";
+    amc->hw_strap1 = SWIFT_BMC_HW_STRAP1;
+    amc->fmc_model = "mx66l1g45g";
+    amc->spi_model = "mx66l1g45g";
+    amc->num_cs    = 2;
+    amc->i2c_init  = swift_bmc_i2c_init;
+    mc->default_ram_size       = 512 * MiB;
+};
+
+static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "OpenPOWER Witherspoon BMC (ARM1176)";
+    amc->soc_name  = "ast2500-a1";
+    amc->hw_strap1 = WITHERSPOON_BMC_HW_STRAP1;
+    amc->fmc_model = "mx25l25635e";
+    amc->spi_model = "mx66l1g45g";
+    amc->num_cs    = 2;
+    amc->i2c_init  = witherspoon_bmc_i2c_init;
+    mc->default_ram_size = 512 * MiB;
+};
+
+static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Aspeed AST2600 EVB (Cortex A7)";
+    amc->soc_name  = "ast2600-a0";
+    amc->hw_strap1 = AST2600_EVB_HW_STRAP1;
+    amc->hw_strap2 = AST2600_EVB_HW_STRAP2;
+    amc->fmc_model = "w25q512jv";
+    amc->spi_model = "mx66u51235f";
+    amc->num_cs    = 1;
+    amc->i2c_init  = ast2600_evb_i2c_init;
+    mc->default_ram_size = 1 * GiB;
+};
+
+static const TypeInfo aspeed_machine_types[] = {
     {
-        .name      = MACHINE_TYPE_NAME("palmetto-bmc"),
-        .desc      = "OpenPOWER Palmetto BMC (ARM926EJ-S)",
-        .soc_name  = "ast2400-a1",
-        .hw_strap1 = PALMETTO_BMC_HW_STRAP1,
-        .fmc_model = "n25q256a",
-        .spi_model = "mx25l25635e",
-        .num_cs    = 1,
-        .i2c_init  = palmetto_bmc_i2c_init,
-        .ram       = 256 * MiB,
+        .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_palmetto_class_init,
     }, {
-        .name      = MACHINE_TYPE_NAME("ast2500-evb"),
-        .desc      = "Aspeed AST2500 EVB (ARM1176)",
-        .soc_name  = "ast2500-a1",
-        .hw_strap1 = AST2500_EVB_HW_STRAP1,
-        .fmc_model = "w25q256",
-        .spi_model = "mx25l25635e",
-        .num_cs    = 1,
-        .i2c_init  = ast2500_evb_i2c_init,
-        .ram       = 512 * MiB,
+        .name          = MACHINE_TYPE_NAME("ast2500-evb"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_ast2500_evb_class_init,
     }, {
-        .name      = MACHINE_TYPE_NAME("romulus-bmc"),
-        .desc      = "OpenPOWER Romulus BMC (ARM1176)",
-        .soc_name  = "ast2500-a1",
-        .hw_strap1 = ROMULUS_BMC_HW_STRAP1,
-        .fmc_model = "n25q256a",
-        .spi_model = "mx66l1g45g",
-        .num_cs    = 2,
-        .i2c_init  = romulus_bmc_i2c_init,
-        .ram       = 512 * MiB,
+        .name          = MACHINE_TYPE_NAME("romulus-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_romulus_class_init,
     }, {
-        .name      = MACHINE_TYPE_NAME("swift-bmc"),
-        .desc      = "OpenPOWER Swift BMC (ARM1176)",
-        .soc_name  = "ast2500-a1",
-        .hw_strap1 = SWIFT_BMC_HW_STRAP1,
-        .fmc_model = "mx66l1g45g",
-        .spi_model = "mx66l1g45g",
-        .num_cs    = 2,
-        .i2c_init  = swift_bmc_i2c_init,
-        .ram       = 512 * MiB,
+        .name          = MACHINE_TYPE_NAME("swift-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_swift_class_init,
     }, {
-        .name      = MACHINE_TYPE_NAME("witherspoon-bmc"),
-        .desc      = "OpenPOWER Witherspoon BMC (ARM1176)",
-        .soc_name  = "ast2500-a1",
-        .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1,
-        .fmc_model = "mx25l25635e",
-        .spi_model = "mx66l1g45g",
-        .num_cs    = 2,
-        .i2c_init  = witherspoon_bmc_i2c_init,
-        .ram       = 512 * MiB,
+        .name          = MACHINE_TYPE_NAME("witherspoon-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_witherspoon_class_init,
     }, {
-        .name      = MACHINE_TYPE_NAME("ast2600-evb"),
-        .desc      = "Aspeed AST2600 EVB (Cortex A7)",
-        .soc_name  = "ast2600-a0",
-        .hw_strap1 = AST2600_EVB_HW_STRAP1,
-        .hw_strap2 = AST2600_EVB_HW_STRAP2,
-        .fmc_model = "w25q512jv",
-        .spi_model = "mx66u51235f",
-        .num_cs    = 1,
-        .i2c_init  = ast2600_evb_i2c_init,
-        .ram       = 1 * GiB,
-    },
-};
-
-static void aspeed_machine_types(void)
-{
-    int i;
-
-    type_register_static(&aspeed_machine_type);
-    for (i = 0; i < ARRAY_SIZE(aspeed_boards); ++i) {
-        TypeInfo ti = {
-            .name       = aspeed_boards[i].name,
-            .parent     = TYPE_ASPEED_MACHINE,
-            .class_init = aspeed_machine_class_init,
-            .class_data = (void *)&aspeed_boards[i],
-        };
-        type_register(&ti);
+        .name          = MACHINE_TYPE_NAME("ast2600-evb"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_ast2600_evb_class_init,
+    }, {
+        .name          = TYPE_ASPEED_MACHINE,
+        .parent        = TYPE_MACHINE,
+        .instance_size = sizeof(AspeedMachine),
+        .class_size    = sizeof(AspeedMachineClass),
+        .class_init    = aspeed_machine_class_init,
+        .abstract      = true,
     }
-}
+};
 
-type_init(aspeed_machine_types)
+DEFINE_TYPES(aspeed_machine_types)
-- 
2.21.0



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

* [PATCH 14/17] aspeed: Add support for the tacoma-bmc board
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (12 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 13/17] aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 15/17] gpio: fix memory leak in aspeed_gpio_init() Cédric Le Goater
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, Joel Stanley,
	qemu-devel

The Tacoma BMC board is replacement board for the BMC of the OpenPOWER
Witherspoon system. It uses a AST2600 SoC instead of a AST2500 and the
I2C layout is the same as it controls the same main board. Used for HW
bringup.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index e34e6787430b..cc06af4fbb3e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -92,6 +92,10 @@ struct AspeedBoardState {
 #define AST2600_EVB_HW_STRAP1 0x000000C0
 #define AST2600_EVB_HW_STRAP2 0x00000003
 
+/* Tacoma hardware value */
+#define TACOMA_BMC_HW_STRAP1  0x00000000
+#define TACOMA_BMC_HW_STRAP2  0x00000000
+
 /*
  * The max ram region is for firmwares that scan the address space
  * with load/store to guess how much RAM the SoC has.
@@ -363,6 +367,9 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
     AspeedSoCState *soc = &bmc->soc;
     uint8_t *eeprom_buf = g_malloc0(8 * 1024);
 
+    /* Bus 3: TODO bmp280@77 */
+    /* Bus 3: TODO max31785@52 */
+    /* Bus 3: TODO dps310@76 */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552,
                      0x60);
 
@@ -381,6 +388,7 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
                           eeprom_buf);
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552,
                      0x60);
+    /* Bus 11: TODO ucd90160@64 */
 }
 
 static void aspeed_machine_class_init(ObjectClass *oc, void *data)
@@ -485,6 +493,22 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
 };
 
+static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Aspeed AST2600 EVB (Cortex A7)";
+    amc->soc_name  = "ast2600-a0";
+    amc->hw_strap1 = TACOMA_BMC_HW_STRAP1;
+    amc->hw_strap2 = TACOMA_BMC_HW_STRAP2;
+    amc->fmc_model = "mx66l1g45g";
+    amc->spi_model = "mx66l1g45g";
+    amc->num_cs    = 2;
+    amc->i2c_init  = witherspoon_bmc_i2c_init; /* Same board layout */
+    mc->default_ram_size = 1 * GiB;
+};
+
 static const TypeInfo aspeed_machine_types[] = {
     {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -510,6 +534,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("ast2600-evb"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_ast2600_evb_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("tacoma-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_tacoma_class_init,
     }, {
         .name          = TYPE_ASPEED_MACHINE,
         .parent        = TYPE_MACHINE,
-- 
2.21.0



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

* [PATCH 15/17] gpio: fix memory leak in aspeed_gpio_init()
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (13 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 14/17] aspeed: Add support for the tacoma-bmc board Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 16/17] aspeed: Change the "scu" property definition Cédric Le Goater
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, PanNengyuan, qemu-devel, qemu-arm, Joel Stanley,
	Euler Robot, Cédric Le Goater

From: PanNengyuan <pannengyuan@huawei.com>

Address Sanitizer shows memory leak in hw/gpio/aspeed_gpio.c:875

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: PanNengyuan <pannengyuan@huawei.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/gpio/aspeed_gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 7acc5fa8e29a..41e11ea9b04d 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -876,6 +876,7 @@ static void aspeed_gpio_init(Object *obj)
                                pin_idx % GPIOS_PER_GROUP);
         object_property_add(obj, name, "bool", aspeed_gpio_get_pin,
                             aspeed_gpio_set_pin, NULL, NULL, NULL);
+        g_free(name);
     }
 }
 
-- 
2.21.0



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

* [PATCH 16/17] aspeed: Change the "scu" property definition
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (14 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 15/17] gpio: fix memory leak in aspeed_gpio_init() Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-11-19 14:12 ` [PATCH 17/17] aspeed: Change the "nic" " Cédric Le Goater
  2019-12-06 13:12 ` [PATCH 00/17] aspeed: extensions and fixes Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Greg Kurz, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

The Aspeed Watchdog and Timer models have a link pointing to the SCU
controller model of the machine.

Change the "scu" property definition so that it explicitly sets the
pointer. The property isn't optional : not being able to set the link
is a bug and QEMU should rather abort than exit in this case.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed_ast2600.c  |  8 ++++----
 hw/arm/aspeed_soc.c      |  8 ++++----
 hw/timer/aspeed_timer.c  | 17 +++++++++--------
 hw/watchdog/wdt_aspeed.c | 17 ++++++++---------
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0881eb25983e..810fd7de0c06 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -146,8 +146,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
     snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
     sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
                           sizeof(s->timerctrl), typename);
-    object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
-                                   OBJECT(&s->scu), &error_abort);
 
     snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
     sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
@@ -177,8 +175,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
         snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
         sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
                               sizeof(s->wdt[i]), typename);
-        object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
-                                       OBJECT(&s->scu), &error_abort);
     }
 
     for (i = 0; i < sc->macs_num; i++) {
@@ -323,6 +319,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_RTC));
 
     /* Timer */
+    object_property_set_link(OBJECT(&s->timerctrl),
+                             OBJECT(&s->scu), "scu", &error_abort);
     object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
@@ -415,6 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->wdts_num; i++) {
         AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
 
+        object_property_set_link(OBJECT(&s->wdt[i]),
+                                 OBJECT(&s->scu), "scu", &error_abort);
         object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index b01c97744196..a6237e594017 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -163,8 +163,6 @@ static void aspeed_soc_init(Object *obj)
     snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
     sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
                           sizeof(s->timerctrl), typename);
-    object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
-                                   OBJECT(&s->scu), &error_abort);
 
     snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
     sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
@@ -194,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
         snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
         sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
                               sizeof(s->wdt[i]), typename);
-        object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
-                                       OBJECT(&s->scu), &error_abort);
     }
 
     for (i = 0; i < sc->macs_num; i++) {
@@ -291,6 +287,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_RTC));
 
     /* Timer */
+    object_property_set_link(OBJECT(&s->timerctrl),
+                             OBJECT(&s->scu), "scu", &error_abort);
     object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
@@ -376,6 +374,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->wdts_num; i++) {
         AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
 
+        object_property_set_link(OBJECT(&s->wdt[i]),
+                                 OBJECT(&s->scu), "scu", &error_abort);
         object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index bcce2192a92a..a8c38cc1189b 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -19,6 +19,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "hw/qdev-properties.h"
 #include "trace.h"
 
 #define TIMER_NR_REGS 4
@@ -603,15 +604,8 @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp)
     int i;
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     AspeedTimerCtrlState *s = ASPEED_TIMER(dev);
-    Object *obj;
-    Error *err = NULL;
 
-    obj = object_property_get_link(OBJECT(dev), "scu", &err);
-    if (!obj) {
-        error_propagate_prepend(errp, err, "required link 'scu' not found: ");
-        return;
-    }
-    s->scu = ASPEED_SCU(obj);
+    assert(s->scu);
 
     for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
         aspeed_init_one_timer(s, i);
@@ -677,6 +671,12 @@ static const VMStateDescription vmstate_aspeed_timer_state = {
     }
 };
 
+static Property aspeed_timer_properties[] = {
+    DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU,
+                     AspeedSCUState *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void timer_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -685,6 +685,7 @@ static void timer_class_init(ObjectClass *klass, void *data)
     dc->reset = aspeed_timer_reset;
     dc->desc = "ASPEED Timer";
     dc->vmsd = &vmstate_aspeed_timer_state;
+    dc->props = aspeed_timer_properties;
 }
 
 static const TypeInfo aspeed_timer_info = {
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 122aa8daaadf..f50dab922e0f 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -241,16 +241,8 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     AspeedWDTState *s = ASPEED_WDT(dev);
-    Error *err = NULL;
-    Object *obj;
 
-    obj = object_property_get_link(OBJECT(dev), "scu", &err);
-    if (!obj) {
-        error_propagate(errp, err);
-        error_prepend(errp, "required link 'scu' not found: ");
-        return;
-    }
-    s->scu = ASPEED_SCU(obj);
+    assert(s->scu);
 
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev);
 
@@ -264,6 +256,12 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
+static Property aspeed_wdt_properties[] = {
+    DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU,
+                     AspeedSCUState *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -273,6 +271,7 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
     dc->reset = aspeed_wdt_reset;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->vmsd = &vmstate_aspeed_wdt;
+    dc->props = aspeed_wdt_properties;
 }
 
 static const TypeInfo aspeed_wdt_info = {
-- 
2.21.0



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

* [PATCH 17/17] aspeed: Change the "nic" property definition
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (15 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 16/17] aspeed: Change the "scu" property definition Cédric Le Goater
@ 2019-11-19 14:12 ` Cédric Le Goater
  2019-12-06 13:12 ` [PATCH 00/17] aspeed: extensions and fixes Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2019-11-19 14:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Andrew Jeffery, Greg Kurz, qemu-devel, qemu-arm,
	Cédric Le Goater, Joel Stanley

The Aspeed MII model has a link pointing to its associated FTGMAC100
NIC in the machine.

Change the "nic" property definition so that it explicitly sets the
pointer. The property isn't optional : not being able to set the link
is a bug and QEMU should rather abort than exit in this case.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed_ast2600.c |  5 ++---
 hw/net/ftgmac100.c      | 19 +++++++++----------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 810fd7de0c06..be88005dab8f 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -183,9 +183,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
         sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]),
                               TYPE_ASPEED_MII);
-        object_property_add_const_link(OBJECT(&s->mii[i]), "nic",
-                                       OBJECT(&s->ftgmac100[i]),
-                                       &error_abort);
     }
 
     sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
@@ -441,6 +438,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                            aspeed_soc_get_irq(s, ASPEED_ETH1 + i));
 
+        object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]),
+                                 "nic", &error_abort);
         object_property_set_bool(OBJECT(&s->mii[i]), true, "realized",
                                  &err);
         if (err) {
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index eb8b441461a1..86ac25894a89 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -1204,17 +1204,8 @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp)
 {
     AspeedMiiState *s = ASPEED_MII(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    Object *obj;
-    Error *local_err = NULL;
 
-    obj = object_property_get_link(OBJECT(dev), "nic", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'nic' not found: ");
-        return;
-    }
-
-    s->nic = FTGMAC100(obj);
+    assert(s->nic);
 
     memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s,
                           TYPE_ASPEED_MII, 0x8);
@@ -1231,6 +1222,13 @@ static const VMStateDescription vmstate_aspeed_mii = {
         VMSTATE_END_OF_LIST()
     }
 };
+
+static Property aspeed_mii_properties[] = {
+    DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100,
+                     FTGMAC100State *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void aspeed_mii_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1239,6 +1237,7 @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data)
     dc->reset = aspeed_mii_reset;
     dc->realize = aspeed_mii_realize;
     dc->desc = "Aspeed MII controller";
+    dc->props = aspeed_mii_properties;
 }
 
 static const TypeInfo aspeed_mii_info = {
-- 
2.21.0



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

* Re: [PATCH 00/17] aspeed: extensions and fixes
  2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
                   ` (16 preceding siblings ...)
  2019-11-19 14:12 ` [PATCH 17/17] aspeed: Change the "nic" " Cédric Le Goater
@ 2019-12-06 13:12 ` Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2019-12-06 13:12 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Andrew Jeffery, qemu-arm, Joel Stanley, QEMU Developers

On Tue, 19 Nov 2019 at 14:12, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hi,
>
> Here is a series adding :
>
>  - support for pool buffer transfers in the I2C controller
>  - fixes for the AST2600 and for the SMC controller
>  - a new Tacoma BMC board using the AST2600 SoC.
>  - misc small cleanups

Applied to target-arm.next for 5.0; thanks.

-- PMM


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

end of thread, other threads:[~2019-12-06 15:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 14:11 [PATCH 00/17] aspeed: extensions and fixes Cédric Le Goater
2019-11-19 14:11 ` [PATCH 01/17] aspeed/i2c: Add support for pool buffer transfers Cédric Le Goater
2019-11-19 14:11 ` [PATCH 02/17] aspeed/i2c: Check SRAM enablement on AST2500 Cédric Le Goater
2019-11-19 14:11 ` [PATCH 03/17] aspeed: Add a DRAM memory region at the SoC level Cédric Le Goater
2019-11-19 14:11 ` [PATCH 04/17] aspeed/i2c: Add support for DMA transfers Cédric Le Goater
2019-11-19 14:11 ` [PATCH 05/17] aspeed/i2c: Add trace events Cédric Le Goater
2019-11-19 14:12 ` [PATCH 06/17] aspeed/sdmc: Make ast2600 default 1G Cédric Le Goater
2019-11-19 14:12 ` [PATCH 07/17] aspeed/scu: Fix W1C behavior Cédric Le Goater
2019-11-19 14:12 ` [PATCH 08/17] watchdog/aspeed: Improve watchdog timeout message Cédric Le Goater
2019-11-19 14:12 ` [PATCH 09/17] watchdog/aspeed: Fix AST2600 frequency behaviour Cédric Le Goater
2019-11-19 14:12 ` [PATCH 10/17] aspeed/smc: Restore default AHB window mapping at reset Cédric Le Goater
2019-11-19 14:12 ` [PATCH 11/17] aspeed/smc: Do not map disabled segment on the AST2600 Cédric Le Goater
2019-11-19 14:12 ` [PATCH 12/17] aspeed/smc: Add AST2600 timings registers Cédric Le Goater
2019-11-19 14:12 ` [PATCH 13/17] aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass Cédric Le Goater
2019-11-19 14:12 ` [PATCH 14/17] aspeed: Add support for the tacoma-bmc board Cédric Le Goater
2019-11-19 14:12 ` [PATCH 15/17] gpio: fix memory leak in aspeed_gpio_init() Cédric Le Goater
2019-11-19 14:12 ` [PATCH 16/17] aspeed: Change the "scu" property definition Cédric Le Goater
2019-11-19 14:12 ` [PATCH 17/17] aspeed: Change the "nic" " Cédric Le Goater
2019-12-06 13:12 ` [PATCH 00/17] aspeed: extensions and fixes Peter Maydell

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.