qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers
@ 2016-06-17 12:15 Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property Cédric Le Goater
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Cédric Le Goater

Hello,

The following adds support for the AST2400 SMC controllers. The device
model does not implement all the HW features, linear addressing mode
is underway, but it should be complete enough for the OpenBMC distro.

Code is based on Andrew's SCU model : 

    https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg04420.html

Flash images can be grabbed here for testing :

    https://openpower.xyz/job/openbmc-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/palmetto/flash-palmetto
    https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/palmetto.pnor

Changes Since v1:

  - included Paolo's fix adding a "drive" property to the flash device
    model    
  - fixed drive definition in the test command line

Thanks,

C.

Cédric Le Goater (4):
  ast2400: add SMC controllers (FMC and SPI)
  ast2400: add SPI flash slave object
  ast2400: create SPI flash slaves
  tests: add a m25p80 test

Paolo Bonzini (1):
  m25p80: qdev-ify drive property

 hw/arm/ast2400.c                    |  31 +++
 hw/arm/palmetto-bmc.c               |   3 +
 hw/arm/xilinx_zynq.c                |   8 +-
 hw/arm/xlnx-ep108.c                 |   9 +-
 hw/block/m25p80.c                   |  10 +-
 hw/microblaze/petalogix_ml605_mmu.c |   9 +-
 hw/ssi/Makefile.objs                |   1 +
 hw/ssi/aspeed_smc.c                 | 460 ++++++++++++++++++++++++++++++++++++
 include/hw/arm/ast2400.h            |   3 +
 include/hw/ssi/aspeed_smc.h         | 105 ++++++++
 tests/Makefile.include              |   2 +
 tests/m25p80-test.c                 | 242 +++++++++++++++++++
 12 files changed, 872 insertions(+), 11 deletions(-)
 create mode 100644 hw/ssi/aspeed_smc.c
 create mode 100644 include/hw/ssi/aspeed_smc.h
 create mode 100644 tests/m25p80-test.c

-- 
2.1.4

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

* [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property
  2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
@ 2016-06-17 12:15 ` Cédric Le Goater
  2016-06-17 12:32   ` Paolo Bonzini
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Paolo Bonzini

From: Paolo Bonzini <pbonzini@redhat.com>

This allows specifying the property via -drive if=none and creating
the flash device with -device.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/xilinx_zynq.c                |  8 +++++++-
 hw/arm/xlnx-ep108.c                 |  9 ++++++++-
 hw/block/m25p80.c                   | 10 ++--------
 hw/microblaze/petalogix_ml605_mmu.c |  9 ++++++++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index aefebcfa6d8f..b0cabe2e4a67 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -138,7 +138,13 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
         spi = (SSIBus *)qdev_get_child_bus(dev, bus_name);
 
         for (j = 0; j < num_ss; ++j) {
-            flash_dev = ssi_create_slave(spi, "n25q128");
+            DriveInfo *dinfo = drive_get_next(IF_MTD);
+            flash_dev = ssi_create_slave_no_init(spi, "n25q128");
+            if (dinfo) {
+                qdev_prop_set_drive(flash_dev, "drive",
+                                    blk_by_legacy_dinfo(dinfo), &error_fatal);
+            }
+            qdev_init_nofail(flash_dev);
 
             cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
             sysbus_connect_irq(busdev, i * num_ss + j + 1, cs_line);
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 34b464171266..4ec590a25db5 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -88,12 +88,19 @@ static void xlnx_ep108_init(MachineState *machine)
         SSIBus *spi_bus;
         DeviceState *flash_dev;
         qemu_irq cs_line;
+        DriveInfo *dinfo = drive_get_next(IF_MTD);
         gchar *bus_name = g_strdup_printf("spi%d", i);
 
         spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), bus_name);
         g_free(bus_name);
 
-        flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
+        flash_dev = ssi_create_slave_no_init(spi_bus, "sst25wf080");
+        if (dinfo) {
+            qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
+                                &error_fatal);
+        }
+        qdev_init_nofail(flash_dev);
+
         cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
 
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index e6f6e23fb71d..7d1c34a1a2d1 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -881,7 +881,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
 
 static void m25p80_realize(SSISlave *ss, Error **errp)
 {
-    DriveInfo *dinfo;
     Flash *s = M25P80(ss);
     M25P80Class *mc = M25P80_GET_CLASS(s);
 
@@ -890,14 +889,8 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
     s->size = s->pi->sector_size * s->pi->n_sectors;
     s->dirty_page = -1;
 
-    /* FIXME use a qdev drive property instead of drive_get_next() */
-    dinfo = drive_get_next(IF_MTD);
-
-    if (dinfo) {
+    if (s->blk) {
         DB_PRINT_L(0, "Binding to IF_MTD drive\n");
-        s->blk = blk_by_legacy_dinfo(dinfo);
-        blk_attach_dev_nofail(s->blk, s);
-
         s->storage = blk_blockalign(s->blk, s->size);
 
         if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
@@ -925,6 +918,7 @@ static void m25p80_pre_save(void *opaque)
 
 static Property m25p80_properties[] = {
     DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
+    DEFINE_PROP_DRIVE("drive", Flash, blk),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 07527b677b37..4968bdbb2821 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -191,9 +191,16 @@ petalogix_ml605_init(MachineState *machine)
         spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
 
         for (i = 0; i < NUM_SPI_FLASHES; i++) {
+            DriveInfo *dinfo = drive_get_next(IF_MTD);
             qemu_irq cs_line;
 
-            dev = ssi_create_slave(spi, "n25q128");
+            dev = ssi_create_slave_no_init(spi, "n25q128");
+            if (dinfo) {
+                qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+                                    &error_fatal);
+            }
+            qdev_init_nofail(dev);
+
             cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
             sysbus_connect_irq(busdev, i+1, cs_line);
         }
-- 
2.1.4

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

* [Qemu-devel] [PATCH v2 2/5] ast2400: add SMC controllers (FMC and SPI)
  2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property Cédric Le Goater
@ 2016-06-17 12:15 ` Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 3/5] ast2400: add SPI flash slave object Cédric Le Goater
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Cédric Le Goater

The Aspeed AST2400 soc includes a static memory controller for the BMC
which supports NOR, NAND and SPI flash memory modules. This controller
has two modes : the SMC for the legacy interface which supports only
one module and the FMC for the new interface which supports up to five
modules. The AST2400 also includes a SPI only controller used for the
host firmware, commonly called BIOS on Intel. It can be used in three
mode : a SPI master, SPI slave and SPI pass-through

Below is the initial framework for the SMC controller (FMC mode only)
and the SPI controller: the sysbus object, MMIO for registers
configuration and controls. Each controller has a SPI bus and a
configurable number of CS lines for SPI flash slaves.

The differences between the controllers are small, so they are
abstracted using indirections on the register numbers.

Only SPI flash modules are supported.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/ast2400.c            |  31 +++++
 hw/ssi/Makefile.objs        |   1 +
 hw/ssi/aspeed_smc.c         | 300 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/ast2400.h    |   3 +
 include/hw/ssi/aspeed_smc.h |  79 ++++++++++++
 5 files changed, 414 insertions(+)
 create mode 100644 hw/ssi/aspeed_smc.c
 create mode 100644 include/hw/ssi/aspeed_smc.h

diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
index fc0da5c38557..458c6d151fd2 100644
--- a/hw/arm/ast2400.c
+++ b/hw/arm/ast2400.c
@@ -23,6 +23,9 @@
 #define AST2400_UART_5_BASE      0x00184000
 #define AST2400_IOMEM_SIZE       0x00200000
 #define AST2400_IOMEM_BASE       0x1E600000
+#define AST2400_SMC_BASE         AST2400_IOMEM_BASE /* Legacy SMC */
+#define AST2400_FMC_BASE         0X1E620000
+#define AST2400_SPI_BASE         0X1E630000
 #define AST2400_VIC_BASE         0x1E6C0000
 #define AST2400_SCU_BASE         0x1E6E2000
 #define AST2400_TIMER_BASE       0x1E782000
@@ -121,6 +124,14 @@ static void ast2400_init(Object *obj)
         qdev_prop_set_uint32(DEVICE(&s->scu), propname, scu_reset[i].val);
         g_free(propname);
     }
+
+    object_initialize(&s->smc, sizeof(s->smc), "aspeed.smc.fmc");
+    object_property_add_child(obj, "smc", OBJECT(&s->smc), NULL);
+    qdev_set_parent_bus(DEVICE(&s->smc), sysbus_get_default());
+
+    object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
+    object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
+    qdev_set_parent_bus(DEVICE(&s->spi), sysbus_get_default());
 }
 
 static void ast2400_realize(DeviceState *dev, Error **errp)
@@ -183,6 +194,26 @@ static void ast2400_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, AST2400_I2C_BASE);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
                        qdev_get_gpio_in(DEVICE(&s->vic), 12));
+
+    /* SMC */
+    object_property_set_int(OBJECT(&s->smc), 1, "num-cs", &err);
+    object_property_set_bool(OBJECT(&s->smc), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, AST2400_FMC_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
+                       qdev_get_gpio_in(DEVICE(&s->vic), 19));
+
+    /* SPI */
+    object_property_set_int(OBJECT(&s->spi), 1, "num-cs", &err);
+    object_property_set_bool(OBJECT(&s->spi), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, AST2400_SPI_BASE);
 }
 
 static void ast2400_class_init(ObjectClass *oc, void *data)
diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index fcbb79ef0185..c79a8dcd86a9 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL022) += pl022.o
 common-obj-$(CONFIG_SSI) += ssi.o
 common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
 common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
+common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
 obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
new file mode 100644
index 000000000000..7937a9030903
--- /dev/null
+++ b/hw/ssi/aspeed_smc.c
@@ -0,0 +1,300 @@
+/*
+ * ASPEED AST2400 SMC Controller (SPI Flash Only)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+#include "include/qemu/error-report.h"
+#include "exec/address-spaces.h"
+
+#include "hw/ssi/aspeed_smc.h"
+
+/* CE Type Setting Register */
+#define R_CONF            (0x00 / 4)
+#define   CONF_LEGACY_DISABLE  (1 << 31)
+#define   CONF_ENABLE_W4       20
+#define   CONF_ENABLE_W3       19
+#define   CONF_ENABLE_W2       18
+#define   CONF_ENABLE_W1       17
+#define   CONF_ENABLE_W0       16
+#define   CONF_FLASH_TYPE4     9
+#define   CONF_FLASH_TYPE3     7
+#define   CONF_FLASH_TYPE2     5
+#define   CONF_FLASH_TYPE1     3
+#define   CONF_FLASH_TYPE0     1
+
+/* CE Control Register */
+#define R_CE_CTRL            (0x04 / 4)
+#define   CRTL_EXTENDED4       4  /* 32 bit addressing for SPI */
+#define   CRTL_EXTENDED3       3  /* 32 bit addressing for SPI */
+#define   CRTL_EXTENDED2       2  /* 32 bit addressing for SPI */
+#define   CRTL_EXTENDED1       1  /* 32 bit addressing for SPI */
+#define   CRTL_EXTENDED0       0  /* 32 bit addressing for SPI */
+
+/* Interrupt Control and Status Register */
+#define R_INTR_CTRL       (0x08 / 4)
+
+/* CEx Control Register */
+#define R_CTRL0           (0x10 / 4)
+#define   CTRL_CMD_SHIFT           16
+#define   CTRL_CMD_MASK            0xff
+#define   CTRL_CE_STOP_ACTIVE      (1 << 2)
+#define   CTRL_CMD_MODE_MASK       0x3
+#define     CTRL_READMODE          0x0
+#define     CTRL_FREADMODE         0x1
+#define     CTRL_WRITEMODE         0x2
+#define     CTRL_USERMODE          0x3
+#define R_CTRL1           (0x14 / 4)
+#define R_CTRL2           (0x18 / 4)
+#define R_CTRL3           (0x1C / 4)
+#define R_CTRL4           (0x20 / 4)
+
+/* CEx Segment Address Register */
+#define R_SEG_ADDR0       (0x30 / 4)
+#define   SEG_SIZE_SHIFT       24   /* 8MB units */
+#define   SEG_SIZE_MASK        0x7f
+#define   SEG_START_SHIFT      16   /* address bit [A29-A23] */
+#define   SEG_START_MASK       0x7f
+#define R_SEG_ADDR1       (0x34 / 4)
+#define R_SEG_ADDR2       (0x38 / 4)
+#define R_SEG_ADDR3       (0x3C / 4)
+#define R_SEG_ADDR4       (0x40 / 4)
+
+/* Misc Control Register #1 */
+#define R_MISC_CRTL1      (0x50 / 4)
+
+/* Misc Control Register #2 */
+#define R_MISC_CRTL2      (0x54 / 4)
+
+/* Misc Control Register #2 */
+#define R_TIMINGS         (0x94 / 4)
+
+/* SPI controller registers and bits */
+#define R_SPI_CONF        (0x00 / 4)
+#define   SPI_CONF_ENABLE_W0   0
+#define R_SPI_CTRL0       (0x4 / 4)
+#define R_SPI_MISC_CRTL   (0x10 / 4)
+#define R_SPI_TIMINGS     (0x14 / 4)
+
+static const AspeedSMCController controllers[] = {
+    { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+      CONF_ENABLE_W0, 5 },
+    { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+      CONF_ENABLE_W0, 5 },
+    { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
+      SPI_CONF_ENABLE_W0, 1 },
+};
+
+static bool aspeed_smc_is_ce_stop_active(AspeedSMCState *s, int cs)
+{
+    return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE;
+}
+
+static void aspeed_smc_update_cs(AspeedSMCState *s)
+{
+    int i;
+
+    for (i = 0; i < s->num_cs; ++i) {
+        qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i));
+    }
+}
+
+static void aspeed_smc_reset(DeviceState *d)
+{
+    AspeedSMCState *s = ASPEED_SMC(d);
+    int i;
+
+    memset(s->regs, 0, sizeof s->regs);
+
+    /* Unselect all slaves */
+    for (i = 0; i < s->num_cs; ++i) {
+        s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
+    }
+
+    aspeed_smc_update_cs(s);
+}
+
+static bool aspeed_smc_is_implemented(AspeedSMCState *s, hwaddr addr)
+{
+    return (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl ||
+            (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs));
+}
+
+static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    AspeedSMCState *s = ASPEED_SMC(opaque);
+
+    addr >>= 2;
+
+    if (addr >= ARRAY_SIZE(s->regs)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n",
+                      __func__, addr);
+        return 0;
+    }
+
+    if (!aspeed_smc_is_implemented(s, addr)) {
+        qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
+                __func__, addr);
+        return 0;
+    }
+
+    return s->regs[addr];
+}
+
+static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
+                             unsigned int size)
+{
+    AspeedSMCState *s = ASPEED_SMC(opaque);
+    uint32_t value = data;
+
+    addr >>= 2;
+
+    if (addr >= ARRAY_SIZE(s->regs)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n",
+                      __func__, addr);
+        return;
+    }
+
+    if (!aspeed_smc_is_implemented(s, addr)) {
+        qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
+                      __func__, addr);
+        return;
+    }
+
+    /*
+     * Not much to do apart from storing the value and set the cs
+     * lines if the register is a controlling one.
+     */
+    s->regs[addr] = value;
+    if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
+        aspeed_smc_update_cs(s);
+    }
+}
+
+static const MemoryRegionOps aspeed_smc_ops = {
+    .read = aspeed_smc_read,
+    .write = aspeed_smc_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.unaligned = true,
+};
+
+static int aspeed_smc_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    AspeedSMCState *s = ASPEED_SMC(dev);
+    AspeedSMCClass *mc = ASPEED_SMC_GET_CLASS(s);
+    int i;
+
+    s->ctrl = mc->ctrl;
+
+    /* keep a copy under AspeedSMCState to speed up accesses */
+    s->r_conf = s->ctrl->r_conf;
+    s->r_ce_ctrl = s->ctrl->r_ce_ctrl;
+    s->r_ctrl0 = s->ctrl->r_ctrl0;
+    s->r_timings = s->ctrl->r_timings;
+    s->conf_enable_w0 = s->ctrl->conf_enable_w0;
+
+    /* Enforce some real HW limits */
+    if (s->num_cs > s->ctrl->max_slaves) {
+        s->num_cs = s->ctrl->max_slaves;
+    }
+
+    s->spi = ssi_create_bus(dev, "spi");
+
+    /* Setup cs_lines for slaves */
+    sysbus_init_irq(sbd, &s->irq);
+    s->cs_lines = g_new0(qemu_irq, s->num_cs);
+    ssi_auto_connect_slaves(dev, s->cs_lines, s->spi);
+
+    for (i = 0; i < s->num_cs; ++i) {
+        sysbus_init_irq(sbd, &s->cs_lines[i]);
+    }
+
+    aspeed_smc_reset(dev);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
+                          s->ctrl->name, ASPEED_SMC_R_MAX * 4);
+    sysbus_init_mmio(sbd, &s->mmio);
+    return 0;
+}
+
+static const VMStateDescription vmstate_aspeed_smc = {
+    .name = "aspeed.smc",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(num_cs, AspeedSMCState),
+        VMSTATE_UINT8(r_conf, AspeedSMCState),
+        VMSTATE_UINT8(r_ctrl0, AspeedSMCState),
+        VMSTATE_UINT8(conf_enable_w0, AspeedSMCState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property aspeed_smc_properties[] = {
+    DEFINE_PROP_UINT8("num-cs", AspeedSMCState, num_cs, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void aspeed_smc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    AspeedSMCClass *mc = ASPEED_SMC_CLASS(klass);
+
+    k->init = aspeed_smc_init;
+    dc->reset = aspeed_smc_reset;
+    dc->props = aspeed_smc_properties;
+    dc->vmsd = &vmstate_aspeed_smc;
+    mc->ctrl = data;
+}
+
+static const TypeInfo aspeed_smc_info = {
+    .name           = TYPE_ASPEED_SMC,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(AspeedSMCState),
+    .class_size     = sizeof(AspeedSMCClass),
+    .abstract       = true,
+};
+
+static void aspeed_smc_register_types(void)
+{
+    int i;
+
+    type_register_static(&aspeed_smc_info);
+    for (i = 0; i < ARRAY_SIZE(controllers); ++i) {
+        TypeInfo ti = {
+            .name       = controllers[i].name,
+            .parent     = TYPE_ASPEED_SMC,
+            .class_init = aspeed_smc_class_init,
+            .class_data = (void *)&controllers[i],
+        };
+        type_register(&ti);
+    }
+}
+
+type_init(aspeed_smc_register_types)
diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
index f1a64fd3893d..7833bc716cd8 100644
--- a/include/hw/arm/ast2400.h
+++ b/include/hw/arm/ast2400.h
@@ -17,6 +17,7 @@
 #include "hw/misc/aspeed_scu.h"
 #include "hw/timer/aspeed_timer.h"
 #include "hw/i2c/aspeed_i2c.h"
+#include "hw/ssi/aspeed_smc.h"
 
 typedef struct AST2400State {
     /*< private >*/
@@ -29,6 +30,8 @@ typedef struct AST2400State {
     AspeedTimerCtrlState timerctrl;
     AspeedI2CState i2c;
     AspeedSCUState scu;
+    AspeedSMCState smc;
+    AspeedSMCState spi;
 } AST2400State;
 
 #define TYPE_AST2400 "ast2400"
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
new file mode 100644
index 000000000000..2d3e9f6b46d5
--- /dev/null
+++ b/include/hw/ssi/aspeed_smc.h
@@ -0,0 +1,79 @@
+/*
+ * ASPEED AST2400 SMC Controller (SPI Flash Only)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef ASPEED_SMC_H
+#define ASPEED_SMC_H
+
+#include "hw/ssi/ssi.h"
+
+typedef struct AspeedSMCController {
+    const char *name;
+    uint8_t r_conf;
+    uint8_t r_ce_ctrl;
+    uint8_t r_ctrl0;
+    uint8_t r_timings;
+    uint8_t conf_enable_w0;
+    uint8_t max_slaves;
+} AspeedSMCController;
+
+#define TYPE_ASPEED_SMC "aspeed.smc"
+#define ASPEED_SMC(obj) OBJECT_CHECK(AspeedSMCState, (obj), TYPE_ASPEED_SMC)
+#define ASPEED_SMC_CLASS(klass) \
+     OBJECT_CLASS_CHECK(AspeedSMCClass, (klass), TYPE_ASPEED_SMC)
+#define ASPEED_SMC_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(AspeedSMCClass, (obj), TYPE_ASPEED_SMC)
+
+typedef struct  AspeedSMCClass {
+    SysBusDevice parent_obj;
+    const AspeedSMCController *ctrl;
+}  AspeedSMCClass;
+
+#define ASPEED_SMC_R_MAX        (0x100 / 4)
+
+typedef struct AspeedSMCState {
+    SysBusDevice parent_obj;
+
+    const AspeedSMCController *ctrl;
+
+    MemoryRegion mmio;
+
+    qemu_irq irq;
+    int irqline;
+
+    uint8_t num_cs;
+    qemu_irq *cs_lines;
+
+    SSIBus *spi;
+
+    uint32_t regs[ASPEED_SMC_R_MAX];
+
+    /* depends on the controller type */
+    uint8_t r_conf;
+    uint8_t r_ce_ctrl;
+    uint8_t r_ctrl0;
+    uint8_t r_timings;
+    uint8_t conf_enable_w0;
+} AspeedSMCState;
+
+#endif /* ASPEED_SMC_H */
-- 
2.1.4

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

* [Qemu-devel] [PATCH v2 3/5] ast2400: add SPI flash slave object
  2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
@ 2016-06-17 12:15 ` Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves Cédric Le Goater
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 5/5] tests: add a m25p80 test Cédric Le Goater
  4 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Cédric Le Goater

Each SPI flash slave can operate in two modes: Command and User. When
in User mode, accesses to the memory segment of the slaves are
translated in SPI transfers. When in Command mode, the HW generates
the SPI commands automatically and the memory segment is accessed as
if doing a MMIO. Other SPI controllers call that mode linear
addressing mode.

This object is a model proposal for a SPI flash module, gathering SPI
slave properties and a MemoryRegion handling the memory accesses.
Only the User mode is supported for now but we are preparing ground
for the Command mode.

The framework below is sufficient to support Linux which only uses
User Mode.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c         | 97 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ssi/aspeed_smc.h | 16 ++++++++
 2 files changed, 113 insertions(+)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 7937a9030903..6a02906c8f97 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -112,6 +112,21 @@ static bool aspeed_smc_is_ce_stop_active(AspeedSMCState *s, int cs)
     return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE;
 }
 
+static inline int aspeed_smc_flash_mode(AspeedSMCState *s, int cs)
+{
+    return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK;
+}
+
+static inline bool aspeed_smc_is_usermode(AspeedSMCState *s, int cs)
+{
+    return (aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE);
+}
+
+static inline bool aspeed_smc_is_writable(AspeedSMCState *s, int cs)
+{
+    return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs));
+}
+
 static void aspeed_smc_update_cs(AspeedSMCState *s)
 {
     int i;
@@ -298,3 +313,85 @@ static void aspeed_smc_register_types(void)
 }
 
 type_init(aspeed_smc_register_types)
+
+static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
+{
+    AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(opaque);
+    AspeedSMCState *s = fl->controller;
+    uint64_t ret = 0;
+    int i;
+
+    if (aspeed_smc_is_usermode(s, fl->id)) {
+        for (i = 0; i < size; i++) {
+            ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
+        }
+    } else {
+        error_report("%s: flash not in usermode", __func__);
+        ret = -1;
+    }
+
+    return ret;
+}
+
+static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
+                           unsigned size)
+{
+    AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(opaque);
+    AspeedSMCState *s = fl->controller;
+    int i;
+
+    if (!aspeed_smc_is_writable(s, fl->id)) {
+        error_report("%s: flash is not writable", __func__);
+        return;
+    }
+
+    if (!aspeed_smc_is_usermode(s, fl->id)) {
+        error_report("%s: flash not in usermode", __func__);
+        return;
+    }
+
+    for (i = 0; i < size; i++) {
+        ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
+    }
+}
+
+static const MemoryRegionOps aspeed_smc_flash_ops = {
+    .read = aspeed_smc_flash_read,
+    .write = aspeed_smc_flash_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
+static const VMStateDescription vmstate_aspeed_smc_flash = {
+    .name = "aspeed.smc_flash",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(id, AspeedSMCFlashState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void aspeed_smc_flash_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_aspeed_smc_flash;
+}
+
+static const TypeInfo aspeed_smc_flash_info = {
+    .name           = TYPE_ASPEED_SMC_FLASH,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(AspeedSMCState),
+    .class_init     = aspeed_smc_flash_class_init,
+};
+
+static void aspeed_smc_flash_register_types(void)
+{
+    type_register_static(&aspeed_smc_flash_info);
+}
+
+type_init(aspeed_smc_flash_register_types)
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 2d3e9f6b46d5..abd0005b01c2 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -27,6 +27,22 @@
 
 #include "hw/ssi/ssi.h"
 
+struct AspeedSMCState;
+
+typedef struct AspeedSMCFlashState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+    uint8_t id;
+    size_t size;
+    struct AspeedSMCState *controller;
+    DeviceState *flash;
+} AspeedSMCFlashState;
+
+#define TYPE_ASPEED_SMC_FLASH "aspeed.smc.flash"
+#define ASPEED_SMC_FLASH(obj) \
+    OBJECT_CHECK(AspeedSMCFlashState, (obj), TYPE_ASPEED_SMC_FLASH)
+
 typedef struct AspeedSMCController {
     const char *name;
     uint8_t r_conf;
-- 
2.1.4

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

* [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves
  2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
                   ` (2 preceding siblings ...)
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 3/5] ast2400: add SPI flash slave object Cédric Le Goater
@ 2016-06-17 12:15 ` Cédric Le Goater
  2016-06-20 15:38   ` Peter Maydell
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 5/5] tests: add a m25p80 test Cédric Le Goater
  4 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Cédric Le Goater

A set of SPI flash slaves is attached under the flash controllers of
the palmetto platform. "n25q256a" flash modules are used for the BMC
and "mx25l25635e" for the host. These types are common in the
OpenPower ecosystem.

The segment addresses used for the memory mappings are the defaults
provided by the specs. They can be changed with the Segment Address
Register but this is not supported in the current implementation.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:
 
 - use the "drive" property to define the flash backend 

 hw/arm/palmetto-bmc.c       |  3 ++
 hw/ssi/aspeed_smc.c         | 69 +++++++++++++++++++++++++++++++++++++++++++--
 include/hw/ssi/aspeed_smc.h | 10 +++++++
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
index a51d960510ee..0aed97a7d9cf 100644
--- a/hw/arm/palmetto-bmc.c
+++ b/hw/arm/palmetto-bmc.c
@@ -47,6 +47,9 @@ static void palmetto_bmc_init(MachineState *machine)
     object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
                              &error_abort);
 
+    aspeed_smc_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
+    aspeed_smc_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
+
     palmetto_bmc_binfo.kernel_filename = machine->kernel_filename;
     palmetto_bmc_binfo.initrd_filename = machine->initrd_filename;
     palmetto_bmc_binfo.kernel_cmdline = machine->kernel_cmdline;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 6a02906c8f97..a8337eb81975 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -28,6 +28,8 @@
 #include "qemu/log.h"
 #include "include/qemu/error-report.h"
 #include "exec/address-spaces.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
 
 #include "hw/ssi/aspeed_smc.h"
 
@@ -98,13 +100,32 @@
 #define R_SPI_MISC_CRTL   (0x10 / 4)
 #define R_SPI_TIMINGS     (0x14 / 4)
 
+/*
+ * Default segments mappings and size for each slave
+ */
+static const AspeedSegments aspeed_segments_legacy[] = {
+    { 0x14000000, 32 * 1024 * 1024 },
+};
+
+static const AspeedSegments aspeed_segments_fmc[] = {
+    { 0x20000000, 64 * 1024 * 1024 },
+    { 0x24000000, 32 * 1024 * 1024 },
+    { 0x26000000, 32 * 1024 * 1024 },
+    { 0x28000000, 32 * 1024 * 1024 },
+    { 0x2A000000, 32 * 1024 * 1024 }
+};
+
+static const AspeedSegments aspeed_segments_spi[] = {
+    { 0x30000000, 64 * 1024 * 1024 },
+};
+
 static const AspeedSMCController controllers[] = {
     { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
-      CONF_ENABLE_W0, 5 },
+      CONF_ENABLE_W0, 5, aspeed_segments_legacy },
     { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
-      CONF_ENABLE_W0, 5 },
+      CONF_ENABLE_W0, 5, aspeed_segments_fmc },
     { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
-      SPI_CONF_ENABLE_W0, 1 },
+      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi },
 };
 
 static bool aspeed_smc_is_ce_stop_active(AspeedSMCState *s, int cs)
@@ -254,6 +275,8 @@ static int aspeed_smc_init(SysBusDevice *sbd)
     memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
                           s->ctrl->name, ASPEED_SMC_R_MAX * 4);
     sysbus_init_mmio(sbd, &s->mmio);
+
+    s->flashes = g_new0(AspeedSMCFlashState *, s->num_cs);
     return 0;
 }
 
@@ -395,3 +418,43 @@ static void aspeed_smc_flash_register_types(void)
 }
 
 type_init(aspeed_smc_flash_register_types)
+
+void aspeed_smc_init_flashes(AspeedSMCState *s, const char *flashtype,
+                             Error **errp)
+{
+    int i ;
+    char name[32];
+
+    for (i = 0; i < s->num_cs; ++i) {
+        Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
+        AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
+        DriveInfo *dinfo = drive_get_next(IF_MTD);
+        qemu_irq cs_line;
+
+        s->flashes[i] = fl;
+
+        snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
+
+        fl->id = i;
+        fl->controller = s;
+        fl->size = s->ctrl->segments[i].size;
+
+        /* backing region */
+        memory_region_init_io(&fl->mmio, obj, &aspeed_smc_flash_ops, fl, name,
+                              fl->size);
+        sysbus_init_mmio(SYS_BUS_DEVICE(fl), &fl->mmio);
+
+        /* SPI Flash module */
+        fl->flash = ssi_create_slave_no_init(s->spi, flashtype);
+        if (dinfo) {
+            qdev_prop_set_drive(fl->flash, "drive", blk_by_legacy_dinfo(dinfo),
+                                errp);
+        }
+        qdev_init_nofail(fl->flash);
+
+        cs_line = qdev_get_gpio_in_named(fl->flash, SSI_GPIO_CS, 0);
+        sysbus_connect_irq(SYS_BUS_DEVICE(s), i + 1, cs_line);
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(fl), 0, s->ctrl->segments[i].addr);
+    }
+}
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index abd0005b01c2..2636c775c90f 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -43,6 +43,11 @@ typedef struct AspeedSMCFlashState {
 #define ASPEED_SMC_FLASH(obj) \
     OBJECT_CHECK(AspeedSMCFlashState, (obj), TYPE_ASPEED_SMC_FLASH)
 
+typedef struct AspeedSegments {
+    hwaddr addr;
+    uint32_t size;
+} AspeedSegments;
+
 typedef struct AspeedSMCController {
     const char *name;
     uint8_t r_conf;
@@ -51,6 +56,7 @@ typedef struct AspeedSMCController {
     uint8_t r_timings;
     uint8_t conf_enable_w0;
     uint8_t max_slaves;
+    const AspeedSegments *segments;
 } AspeedSMCController;
 
 #define TYPE_ASPEED_SMC "aspeed.smc"
@@ -90,6 +96,10 @@ typedef struct AspeedSMCState {
     uint8_t r_ctrl0;
     uint8_t r_timings;
     uint8_t conf_enable_w0;
+
+    AspeedSMCFlashState **flashes;
 } AspeedSMCState;
 
+extern void aspeed_smc_init_flashes(AspeedSMCState *s, const char *flashtype,
+                                    Error **errp);
 #endif /* ASPEED_SMC_H */
-- 
2.1.4

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

* [Qemu-devel] [PATCH v2 5/5] tests: add a m25p80 test
  2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
                   ` (3 preceding siblings ...)
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves Cédric Le Goater
@ 2016-06-17 12:15 ` Cédric Le Goater
  4 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:15 UTC (permalink / raw)
  To: Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley,
	Cédric Le Goater

This test uses the palmetto platform and the AST2400 SPI controller to
test the m25p80 flash module device model. The flash model is defined
by the platform (n25q256a) and it would be nice to find way to control
it, using a property probably.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:

 - fixed guest args to use -drive and not -mtdblock
 
 tests/Makefile.include |   2 +
 tests/m25p80-test.c    | 242 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 244 insertions(+)
 create mode 100644 tests/m25p80-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6135875c37f6..81264fbdf7e7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -252,6 +252,7 @@ gcov-files-sparc-y += hw/timer/m48t59.c
 gcov-files-sparc64-y += hw/timer/m48t59.c
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 check-qtest-arm-y = tests/ds1338-test$(EXESUF)
+check-qtest-arm-y = tests/m25p80-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
 check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
@@ -562,6 +563,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
+tests/m25p80-test$(EXESUF): tests/m25p80-test.o
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
 tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c
new file mode 100644
index 000000000000..ddc2f20721ba
--- /dev/null
+++ b/tests/m25p80-test.c
@@ -0,0 +1,242 @@
+/*
+ * QTest testcase for the M25P80 Flash (Using the AST2400 SPI Controller)
+ *
+ * Copyright (C) 2016 IBM Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "libqtest.h"
+
+/*
+ * AST2400 SPI Controller registers
+ */
+#define R_CONF              0x00
+#define   CONF_ENABLE_W0       (1 << 16)
+#define R_CE_CTRL           0x04
+#define   CRTL_EXTENDED0       0  /* 32 bit addressing for SPI */
+#define R_CTRL0             0x10
+#define   CTRL_CE_STOP_ACTIVE  (1 << 2)
+#define   CTRL_USERMODE        0x3
+
+#define AST2400_FMC_BASE    0x1E620000
+#define AST2400_FLASH_BASE  0x20000000
+
+/*
+ * Flash commands
+ */
+enum {
+    JEDEC_READ = 0x9f,
+    BULK_ERASE = 0xc7,
+    READ = 0x03,
+    PP = 0x02,
+    WREN = 0x6,
+    EN_4BYTE_ADDR = 0xB7,
+    ERASE_SECTOR = 0xd8,
+};
+
+#define FLASH_JEDEC         0x20ba19  /* n25q256a */
+#define FLASH_SIZE          (32 * 1024 * 1024)
+
+#define PAGE_SIZE           256
+
+static void spi_conf(uint32_t value)
+{
+    uint32_t conf = readl(AST2400_FMC_BASE + R_CONF);
+
+    conf |= value;
+    writel(AST2400_FMC_BASE + R_CONF, conf);
+}
+
+static void spi_ctrl_start_user(void)
+{
+    uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
+
+    ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
+    writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+
+    ctrl &= ~CTRL_CE_STOP_ACTIVE;
+    writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+}
+
+static void spi_ctrl_stop_user(void)
+{
+    uint32_t ctrl = readl(AST2400_FMC_BASE + R_CTRL0);
+
+    ctrl |= CTRL_USERMODE | CTRL_CE_STOP_ACTIVE;
+    writel(AST2400_FMC_BASE + R_CTRL0, ctrl);
+}
+
+static void test_read_jedec(void)
+{
+    uint32_t jedec = 0x0;
+
+    spi_conf(CONF_ENABLE_W0);
+
+    spi_ctrl_start_user();
+    writeb(AST2400_FLASH_BASE, JEDEC_READ);
+    jedec |= readb(AST2400_FLASH_BASE) << 16;
+    jedec |= readb(AST2400_FLASH_BASE) << 8;
+    jedec |= readb(AST2400_FLASH_BASE);
+    spi_ctrl_stop_user();
+
+    g_assert_cmphex(jedec, ==, FLASH_JEDEC);
+}
+
+static void read_page(uint32_t addr, uint32_t *page)
+{
+    int i;
+
+    spi_ctrl_start_user();
+
+    writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+    writeb(AST2400_FLASH_BASE, READ);
+    writel(AST2400_FLASH_BASE, cpu_to_be32(addr));
+
+    /* Continuous read are supported */
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        page[i] = be32_to_cpu(readl(AST2400_FLASH_BASE));
+    }
+    spi_ctrl_stop_user();
+}
+
+static void test_erase_sector(void)
+{
+    uint32_t some_page_addr = 0x600 * PAGE_SIZE;
+    uint32_t page[PAGE_SIZE / 4];
+    int i;
+
+    spi_conf(CONF_ENABLE_W0);
+
+    spi_ctrl_start_user();
+    writeb(AST2400_FLASH_BASE, WREN);
+    writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+    writeb(AST2400_FLASH_BASE, ERASE_SECTOR);
+    writel(AST2400_FLASH_BASE, cpu_to_be32(some_page_addr));
+    spi_ctrl_stop_user();
+
+    /* Previous page should be full of zeroes as backend is not
+     * initialized */
+    read_page(some_page_addr - PAGE_SIZE, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0x0);
+    }
+
+    /* But this one was erased */
+    read_page(some_page_addr, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0xffffffff);
+    }
+}
+
+static void test_erase_all(void)
+{
+    uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
+    uint32_t page[PAGE_SIZE / 4];
+    int i;
+
+    spi_conf(CONF_ENABLE_W0);
+
+    /* Check some random page. Should be full of zeroes as backend is
+     * not initialized */
+    read_page(some_page_addr, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0x0);
+    }
+
+    spi_ctrl_start_user();
+    writeb(AST2400_FLASH_BASE, WREN);
+    writeb(AST2400_FLASH_BASE, BULK_ERASE);
+    spi_ctrl_stop_user();
+
+    /* Recheck that some random page */
+    read_page(some_page_addr, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0xffffffff);
+    }
+}
+
+static void test_write_page(void)
+{
+    uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */
+    uint32_t some_page_addr = 0x15000 * PAGE_SIZE;
+    uint32_t page[PAGE_SIZE / 4];
+    int i;
+
+    spi_conf(CONF_ENABLE_W0);
+
+    spi_ctrl_start_user();
+    writeb(AST2400_FLASH_BASE, EN_4BYTE_ADDR);
+    writeb(AST2400_FLASH_BASE, PP);
+    writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr));
+
+    /* Fill the page with its own addresses */
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        writel(AST2400_FLASH_BASE, cpu_to_be32(my_page_addr + i * 4));
+    }
+    spi_ctrl_stop_user();
+
+    /* Check what was written */
+    read_page(my_page_addr, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, my_page_addr + i * 4);
+    }
+
+    /* Check some other page. It should be full of 0xff */
+    read_page(some_page_addr, page);
+    for (i = 0; i < PAGE_SIZE / 4; i++) {
+        g_assert_cmphex(page[i], ==, 0xffffffff);
+    }
+}
+
+static char tmp_path[] = "/tmp/qtest.XXXXXX";
+
+int main(int argc, char **argv)
+{
+    int ret;
+    int fd;
+    char *args;
+
+    g_test_init(&argc, &argv, NULL);
+
+    fd = mkstemp(tmp_path);
+    g_assert(fd >= 0);
+    ret = ftruncate(fd, FLASH_SIZE);
+    g_assert(ret == 0);
+    close(fd);
+
+    args = g_strdup_printf("-M 256 -machine palmetto-bmc "
+                           "-drive file=%s,format=raw,if=mtd",
+                           tmp_path);
+    qtest_start(args);
+
+    qtest_add_func("/m25p80/read_jedec", test_read_jedec);
+    qtest_add_func("/m25p80/erase_sector", test_erase_sector);
+    qtest_add_func("/m25p80/erase_all",  test_erase_all);
+    qtest_add_func("/m25p80/write_page", test_write_page);
+
+    ret = g_test_run();
+
+    qtest_quit(global_qtest);
+    unlink(tmp_path);
+    g_free(args);
+    return ret;
+}
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property Cédric Le Goater
@ 2016-06-17 12:32   ` Paolo Bonzini
  2016-06-17 13:07     ` Cédric Le Goater
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2016-06-17 12:32 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley



On 17/06/2016 14:15, Cédric Le Goater wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> This allows specifying the property via -drive if=none and creating
> the flash device with -device.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

You need to add your Signed-off-by line too.  You should have used "git
am -s" for that, but in this case just replace to my mail with the
signoff and the maintainer will collect it.

Thanks for picking up the patch. :)

Paolo

> ---
>  hw/arm/xilinx_zynq.c                |  8 +++++++-
>  hw/arm/xlnx-ep108.c                 |  9 ++++++++-
>  hw/block/m25p80.c                   | 10 ++--------
>  hw/microblaze/petalogix_ml605_mmu.c |  9 ++++++++-
>  4 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index aefebcfa6d8f..b0cabe2e4a67 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -138,7 +138,13 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>          spi = (SSIBus *)qdev_get_child_bus(dev, bus_name);
>  
>          for (j = 0; j < num_ss; ++j) {
> -            flash_dev = ssi_create_slave(spi, "n25q128");
> +            DriveInfo *dinfo = drive_get_next(IF_MTD);
> +            flash_dev = ssi_create_slave_no_init(spi, "n25q128");
> +            if (dinfo) {
> +                qdev_prop_set_drive(flash_dev, "drive",
> +                                    blk_by_legacy_dinfo(dinfo), &error_fatal);
> +            }
> +            qdev_init_nofail(flash_dev);
>  
>              cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
>              sysbus_connect_irq(busdev, i * num_ss + j + 1, cs_line);
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> index 34b464171266..4ec590a25db5 100644
> --- a/hw/arm/xlnx-ep108.c
> +++ b/hw/arm/xlnx-ep108.c
> @@ -88,12 +88,19 @@ static void xlnx_ep108_init(MachineState *machine)
>          SSIBus *spi_bus;
>          DeviceState *flash_dev;
>          qemu_irq cs_line;
> +        DriveInfo *dinfo = drive_get_next(IF_MTD);
>          gchar *bus_name = g_strdup_printf("spi%d", i);
>  
>          spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), bus_name);
>          g_free(bus_name);
>  
> -        flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
> +        flash_dev = ssi_create_slave_no_init(spi_bus, "sst25wf080");
> +        if (dinfo) {
> +            qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
> +                                &error_fatal);
> +        }
> +        qdev_init_nofail(flash_dev);
> +
>          cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
>  
>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index e6f6e23fb71d..7d1c34a1a2d1 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -881,7 +881,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
>  
>  static void m25p80_realize(SSISlave *ss, Error **errp)
>  {
> -    DriveInfo *dinfo;
>      Flash *s = M25P80(ss);
>      M25P80Class *mc = M25P80_GET_CLASS(s);
>  
> @@ -890,14 +889,8 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>      s->size = s->pi->sector_size * s->pi->n_sectors;
>      s->dirty_page = -1;
>  
> -    /* FIXME use a qdev drive property instead of drive_get_next() */
> -    dinfo = drive_get_next(IF_MTD);
> -
> -    if (dinfo) {
> +    if (s->blk) {
>          DB_PRINT_L(0, "Binding to IF_MTD drive\n");
> -        s->blk = blk_by_legacy_dinfo(dinfo);
> -        blk_attach_dev_nofail(s->blk, s);
> -
>          s->storage = blk_blockalign(s->blk, s->size);
>  
>          if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
> @@ -925,6 +918,7 @@ static void m25p80_pre_save(void *opaque)
>  
>  static Property m25p80_properties[] = {
>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
> +    DEFINE_PROP_DRIVE("drive", Flash, blk),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index 07527b677b37..4968bdbb2821 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -191,9 +191,16 @@ petalogix_ml605_init(MachineState *machine)
>          spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
>  
>          for (i = 0; i < NUM_SPI_FLASHES; i++) {
> +            DriveInfo *dinfo = drive_get_next(IF_MTD);
>              qemu_irq cs_line;
>  
> -            dev = ssi_create_slave(spi, "n25q128");
> +            dev = ssi_create_slave_no_init(spi, "n25q128");
> +            if (dinfo) {
> +                qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> +                                    &error_fatal);
> +            }
> +            qdev_init_nofail(dev);
> +
>              cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
>              sysbus_connect_irq(busdev, i+1, cs_line);
>          }
> 

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

* Re: [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property
  2016-06-17 12:32   ` Paolo Bonzini
@ 2016-06-17 13:07     ` Cédric Le Goater
  0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-17 13:07 UTC (permalink / raw)
  To: Paolo Bonzini, Peter Maydell, Peter Crosthwaite
  Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery, Joel Stanley

On 06/17/2016 02:32 PM, Paolo Bonzini wrote:
> 
> 
> On 17/06/2016 14:15, Cédric Le Goater wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> This allows specifying the property via -drive if=none and creating
>> the flash device with -device.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> You need to add your Signed-off-by line too.  

Yes. Sorry about that :/

Signed-off-by: Cédric Le Goater <clg@kaod.org> 

I will next round. 

> You should have used "git
> am -s" for that, but in this case just replace to my mail with the
> signoff and the maintainer will collect it.
> 
> Thanks for picking up the patch. :)

That was needed and it should help us for the next item we want 
to work on. We need more control on the flash storage as we
should be mapping it in memory.

Thanks,

C.

> Paolo
> 
>> ---
>>  hw/arm/xilinx_zynq.c                |  8 +++++++-
>>  hw/arm/xlnx-ep108.c                 |  9 ++++++++-
>>  hw/block/m25p80.c                   | 10 ++--------
>>  hw/microblaze/petalogix_ml605_mmu.c |  9 ++++++++-
>>  4 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> index aefebcfa6d8f..b0cabe2e4a67 100644
>> --- a/hw/arm/xilinx_zynq.c
>> +++ b/hw/arm/xilinx_zynq.c
>> @@ -138,7 +138,13 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>>          spi = (SSIBus *)qdev_get_child_bus(dev, bus_name);
>>  
>>          for (j = 0; j < num_ss; ++j) {
>> -            flash_dev = ssi_create_slave(spi, "n25q128");
>> +            DriveInfo *dinfo = drive_get_next(IF_MTD);
>> +            flash_dev = ssi_create_slave_no_init(spi, "n25q128");
>> +            if (dinfo) {
>> +                qdev_prop_set_drive(flash_dev, "drive",
>> +                                    blk_by_legacy_dinfo(dinfo), &error_fatal);
>> +            }
>> +            qdev_init_nofail(flash_dev);
>>  
>>              cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
>>              sysbus_connect_irq(busdev, i * num_ss + j + 1, cs_line);
>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>> index 34b464171266..4ec590a25db5 100644
>> --- a/hw/arm/xlnx-ep108.c
>> +++ b/hw/arm/xlnx-ep108.c
>> @@ -88,12 +88,19 @@ static void xlnx_ep108_init(MachineState *machine)
>>          SSIBus *spi_bus;
>>          DeviceState *flash_dev;
>>          qemu_irq cs_line;
>> +        DriveInfo *dinfo = drive_get_next(IF_MTD);
>>          gchar *bus_name = g_strdup_printf("spi%d", i);
>>  
>>          spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), bus_name);
>>          g_free(bus_name);
>>  
>> -        flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
>> +        flash_dev = ssi_create_slave_no_init(spi_bus, "sst25wf080");
>> +        if (dinfo) {
>> +            qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
>> +                                &error_fatal);
>> +        }
>> +        qdev_init_nofail(flash_dev);
>> +
>>          cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
>>  
>>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index e6f6e23fb71d..7d1c34a1a2d1 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -881,7 +881,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
>>  
>>  static void m25p80_realize(SSISlave *ss, Error **errp)
>>  {
>> -    DriveInfo *dinfo;
>>      Flash *s = M25P80(ss);
>>      M25P80Class *mc = M25P80_GET_CLASS(s);
>>  
>> @@ -890,14 +889,8 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>>      s->size = s->pi->sector_size * s->pi->n_sectors;
>>      s->dirty_page = -1;
>>  
>> -    /* FIXME use a qdev drive property instead of drive_get_next() */
>> -    dinfo = drive_get_next(IF_MTD);
>> -
>> -    if (dinfo) {
>> +    if (s->blk) {
>>          DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>> -        s->blk = blk_by_legacy_dinfo(dinfo);
>> -        blk_attach_dev_nofail(s->blk, s);
>> -
>>          s->storage = blk_blockalign(s->blk, s->size);
>>  
>>          if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>> @@ -925,6 +918,7 @@ static void m25p80_pre_save(void *opaque)
>>  
>>  static Property m25p80_properties[] = {
>>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
>> +    DEFINE_PROP_DRIVE("drive", Flash, blk),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
>> index 07527b677b37..4968bdbb2821 100644
>> --- a/hw/microblaze/petalogix_ml605_mmu.c
>> +++ b/hw/microblaze/petalogix_ml605_mmu.c
>> @@ -191,9 +191,16 @@ petalogix_ml605_init(MachineState *machine)
>>          spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
>>  
>>          for (i = 0; i < NUM_SPI_FLASHES; i++) {
>> +            DriveInfo *dinfo = drive_get_next(IF_MTD);
>>              qemu_irq cs_line;
>>  
>> -            dev = ssi_create_slave(spi, "n25q128");
>> +            dev = ssi_create_slave_no_init(spi, "n25q128");
>> +            if (dinfo) {
>> +                qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
>> +                                    &error_fatal);
>> +            }
>> +            qdev_init_nofail(dev);
>> +
>>              cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
>>              sysbus_connect_irq(busdev, i+1, cs_line);
>>          }
>>

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

* Re: [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves
  2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves Cédric Le Goater
@ 2016-06-20 15:38   ` Peter Maydell
  2016-06-20 16:02     ` Cédric Le Goater
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2016-06-20 15:38 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
	Markus Armbruster, Andrew Jeffery, Joel Stanley

On 17 June 2016 at 13:15, Cédric Le Goater <clg@kaod.org> wrote:
> A set of SPI flash slaves is attached under the flash controllers of
> the palmetto platform. "n25q256a" flash modules are used for the BMC
> and "mx25l25635e" for the host. These types are common in the
> OpenPower ecosystem.
>
> The segment addresses used for the memory mappings are the defaults
> provided by the specs. They can be changed with the Segment Address
> Register but this is not supported in the current implementation.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 6a02906c8f97..a8337eb81975 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c

> +void aspeed_smc_init_flashes(AspeedSMCState *s, const char *flashtype,
> +                             Error **errp)
> +{
> +    int i ;
> +    char name[32];
> +
> +    for (i = 0; i < s->num_cs; ++i) {
> +        Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
> +        AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
> +        DriveInfo *dinfo = drive_get_next(IF_MTD);

You don't want to be calling drive_get_next() in code in
hw/ssi -- that should be done at the board level, and then
the board creates the flash device and sets its drive property
and connects the flash device up to the SSI controller.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves
  2016-06-20 15:38   ` Peter Maydell
@ 2016-06-20 16:02     ` Cédric Le Goater
  0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-06-20 16:02 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
	Markus Armbruster, Andrew Jeffery, Joel Stanley

On 06/20/2016 05:38 PM, Peter Maydell wrote:
> On 17 June 2016 at 13:15, Cédric Le Goater <clg@kaod.org> wrote:
>> A set of SPI flash slaves is attached under the flash controllers of
>> the palmetto platform. "n25q256a" flash modules are used for the BMC
>> and "mx25l25635e" for the host. These types are common in the
>> OpenPower ecosystem.
>>
>> The segment addresses used for the memory mappings are the defaults
>> provided by the specs. They can be changed with the Segment Address
>> Register but this is not supported in the current implementation.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
> 
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 6a02906c8f97..a8337eb81975 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
> 
>> +void aspeed_smc_init_flashes(AspeedSMCState *s, const char *flashtype,
>> +                             Error **errp)
>> +{
>> +    int i ;
>> +    char name[32];
>> +
>> +    for (i = 0; i < s->num_cs; ++i) {
>> +        Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
>> +        AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
>> +        DriveInfo *dinfo = drive_get_next(IF_MTD);
> 
> You don't want to be calling drive_get_next() in code in
> hw/ssi -- that should be done at the board level, and then
> the board creates the flash device and sets its drive property
> and connects the flash device up to the SSI controller.

OK. I will rework that part.
 
Thanks,

C.

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

end of thread, other threads:[~2016-06-20 16:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-17 12:15 [Qemu-devel] [PATCH v2 0/5] ast2400: SMC controllers Cédric Le Goater
2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 1/5] m25p80: qdev-ify drive property Cédric Le Goater
2016-06-17 12:32   ` Paolo Bonzini
2016-06-17 13:07     ` Cédric Le Goater
2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 3/5] ast2400: add SPI flash slave object Cédric Le Goater
2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 4/5] ast2400: create SPI flash slaves Cédric Le Goater
2016-06-20 15:38   ` Peter Maydell
2016-06-20 16:02     ` Cédric Le Goater
2016-06-17 12:15 ` [Qemu-devel] [PATCH v2 5/5] tests: add a m25p80 test Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).