* [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers
@ 2016-06-23 16:33 Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property Cédric Le Goater
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
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 (v2) :
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg06626.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 v2:
- switched to a realize ops
- moved the initialization of the flash modules under the palmetto
platform
- changed mkstemp() path prefix
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é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 | 28 +++
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 | 437 ++++++++++++++++++++++++++++++++++++
include/hw/arm/ast2400.h | 3 +
include/hw/ssi/aspeed_smc.h | 103 +++++++++
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] 16+ messages in thread
* [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
@ 2016-06-23 16:33 ` Cédric Le Goater
2016-06-23 18:35 ` Peter Maydell
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
Paolo Bonzini, Cédric Le Goater
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>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
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] 16+ messages in thread
* [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI)
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property Cédric Le Goater
@ 2016-06-23 16:33 ` Cédric Le Goater
2016-06-23 18:43 ` Peter Maydell
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object Cédric Le Goater
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
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>
---
Changes since v2:
- switched to a realize ops to handle errors.
hw/arm/ast2400.c | 31 +++++
hw/ssi/Makefile.objs | 1 +
hw/ssi/aspeed_smc.c | 298 ++++++++++++++++++++++++++++++++++++++++++++
include/hw/arm/ast2400.h | 3 +
include/hw/ssi/aspeed_smc.h | 79 ++++++++++++
5 files changed, 412 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 1a26d74e695c..c2665c0c6ead 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
@@ -81,6 +84,14 @@ static void ast2400_init(Object *obj)
"hw-strap1", &error_abort);
object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
"hw-strap2", &error_abort);
+
+ 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)
@@ -143,6 +154,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..6b52123a67bb
--- /dev/null
+++ b/hw/ssi/aspeed_smc.c
@@ -0,0 +1,298 @@
+/*
+ * 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 void aspeed_smc_realize(DeviceState *dev, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ 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);
+}
+
+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);
+ AspeedSMCClass *mc = ASPEED_SMC_CLASS(klass);
+
+ dc->realize = aspeed_smc_realize;
+ 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] 16+ messages in thread
* [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
@ 2016-06-23 16:33 ` Cédric Le Goater
2016-06-23 18:56 ` Peter Maydell
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test Cédric Le Goater
4 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
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 6b52123a67bb..d97c077565c3 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;
@@ -296,3 +311,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] 16+ messages in thread
* [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
` (2 preceding siblings ...)
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object Cédric Le Goater
@ 2016-06-23 16:33 ` Cédric Le Goater
2016-06-23 18:48 ` Peter Maydell
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test Cédric Le Goater
4 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
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 v2 :
- moved the initialization of the flash modules under the palmetto
platform
hw/arm/palmetto-bmc.c | 28 ++++++++++++++++++++++++++
hw/ssi/aspeed_smc.c | 48 ++++++++++++++++++++++++++++++++++++++++++---
include/hw/ssi/aspeed_smc.h | 8 ++++++++
3 files changed, 81 insertions(+), 3 deletions(-)
diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
index b8eed21348d8..d4abf3ca61ab 100644
--- a/hw/arm/palmetto-bmc.c
+++ b/hw/arm/palmetto-bmc.c
@@ -18,6 +18,8 @@
#include "hw/arm/ast2400.h"
#include "hw/boards.h"
#include "qemu/log.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
static struct arm_boot_info palmetto_bmc_binfo = {
.loader_start = AST2400_SDRAM_BASE,
@@ -30,6 +32,29 @@ typedef struct PalmettoBMCState {
MemoryRegion ram;
} PalmettoBMCState;
+static void palmetto_bmc_init_flashes(AspeedSMCState *s, const char *flashtype,
+ Error **errp)
+{
+ int i ;
+
+ for (i = 0; i < s->num_cs; ++i) {
+ AspeedSMCFlashState *fl = s->flashes[i];
+ DriveInfo *dinfo = drive_get_next(IF_MTD);
+ qemu_irq cs_line;
+
+ /* 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);
+ }
+}
+
static void palmetto_bmc_init(MachineState *machine)
{
PalmettoBMCState *bmc;
@@ -49,6 +74,9 @@ static void palmetto_bmc_init(MachineState *machine)
object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
&error_abort);
+ palmetto_bmc_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
+ palmetto_bmc_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 d97c077565c3..6be911ed36d0 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -98,13 +98,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)
@@ -217,6 +236,8 @@ static const MemoryRegionOps aspeed_smc_ops = {
.valid.unaligned = true,
};
+static const MemoryRegionOps aspeed_smc_flash_ops;
+
static void aspeed_smc_realize(DeviceState *dev, Error **errp)
{
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
@@ -254,6 +275,27 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
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);
+
+ for (i = 0; i < s->num_cs; ++i) {
+ Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
+ AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
+ char name[32];
+
+ 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;
+
+ memory_region_init_io(&fl->mmio, obj, &aspeed_smc_flash_ops, fl, name,
+ fl->size);
+ sysbus_init_mmio(SYS_BUS_DEVICE(fl), &fl->mmio);
+ sysbus_mmio_map(SYS_BUS_DEVICE(fl), 0, s->ctrl->segments[i].addr);
+ }
}
static const VMStateDescription vmstate_aspeed_smc = {
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index abd0005b01c2..10b4083952cf 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,8 @@ typedef struct AspeedSMCState {
uint8_t r_ctrl0;
uint8_t r_timings;
uint8_t conf_enable_w0;
+
+ AspeedSMCFlashState **flashes;
} AspeedSMCState;
#endif /* ASPEED_SMC_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
` (3 preceding siblings ...)
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves Cédric Le Goater
@ 2016-06-23 16:33 ` Cédric Le Goater
2016-06-23 18:52 ` Peter Maydell
4 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 16:33 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: qemu-devel, qemu-arm, kwolf, armbru, Andrew Jeffery,
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 v2:
- changed mkstemp() path prefix
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 fd2dba49a741..c64bc2097e44 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
@@ -568,6 +569,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..d5fb0b20283e
--- /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.m25p80.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] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property Cédric Le Goater
@ 2016-06-23 18:35 ` Peter Maydell
2016-06-24 11:49 ` Cédric Le Goater
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-06-23 18:35 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery, Paolo Bonzini
On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> 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>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 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(-)
hw/arm/sabrelite.c also creates one of these devices and needs updating.
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI)
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
@ 2016-06-23 18:43 ` Peter Maydell
2016-06-24 13:30 ` Cédric Le Goater
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-06-23 18:43 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
> 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>
> ---
>
> Changes since v2:
>
> - switched to a realize ops to handle errors.
>
> hw/arm/ast2400.c | 31 +++++
> hw/ssi/Makefile.objs | 1 +
> hw/ssi/aspeed_smc.c | 298 ++++++++++++++++++++++++++++++++++++++++++++
> include/hw/arm/ast2400.h | 3 +
> include/hw/ssi/aspeed_smc.h | 79 ++++++++++++
> 5 files changed, 412 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 1a26d74e695c..c2665c0c6ead 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
> @@ -81,6 +84,14 @@ static void ast2400_init(Object *obj)
> "hw-strap1", &error_abort);
> object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
> "hw-strap2", &error_abort);
> +
> + 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)
> @@ -143,6 +154,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;
> + }
You can't pass the same err to two functions in a row
without checking it like this. See the comments on usage in
include/qapi/error.h.
> + 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..6b52123a67bb
> --- /dev/null
> +++ b/hw/ssi/aspeed_smc.c
> @@ -0,0 +1,298 @@
> +/*
> + * 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 */
Presumably these should be CTRL, not CRTL.
> +
> +/* 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)
Is this really spelt CRTL and not CTRL ?
> +
> +/* 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));
> +}
This is one of those borderline cases which is "this is an
odd way to do this but not so odd that I'm going to ask you
to change it" :-)
> +
> +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 void aspeed_smc_realize(DeviceState *dev, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + 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;
Should this be a "fail the realize" instead? (I don't know the
hardware, so genuine question.)
> + }
> +
> + 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);
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves Cédric Le Goater
@ 2016-06-23 18:48 ` Peter Maydell
2016-06-24 13:42 ` Cédric Le Goater
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-06-23 18:48 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 23 June 2016 at 17:33, 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 d97c077565c3..6be911ed36d0 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -98,13 +98,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)
> @@ -217,6 +236,8 @@ static const MemoryRegionOps aspeed_smc_ops = {
> .valid.unaligned = true,
> };
>
> +static const MemoryRegionOps aspeed_smc_flash_ops;
> +
> static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> @@ -254,6 +275,27 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
> 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);
> +
> + for (i = 0; i < s->num_cs; ++i) {
> + Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
> + AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
> + char name[32];
> +
> + 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;
> +
> + memory_region_init_io(&fl->mmio, obj, &aspeed_smc_flash_ops, fl, name,
> + fl->size);
> + sysbus_init_mmio(SYS_BUS_DEVICE(fl), &fl->mmio);
> + sysbus_mmio_map(SYS_BUS_DEVICE(fl), 0, s->ctrl->segments[i].addr);
Device code shouldn't call sysbus_mmio_map() -- the system memory map is
the board's responsibility.
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test Cédric Le Goater
@ 2016-06-23 18:52 ` Peter Maydell
2016-06-23 19:09 ` Cédric Le Goater
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-06-23 18:52 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
> 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>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Are you in a position to check this test passes on both a
bigendian and little endian host? I have no specific reason
to think it will fail, but it seems worth checking.
(If not, it'll get tested when I do a merge eventually.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object Cédric Le Goater
@ 2016-06-23 18:56 ` Peter Maydell
2016-06-24 13:47 ` Cédric Le Goater
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-06-23 18:56 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
> 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 6b52123a67bb..d97c077565c3 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);
You don't need these brackets.
> +}
> +
> +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;
> @@ -296,3 +311,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__);
This should probably be a LOG_UNIMP or LOG_GUEST_ERROR qemu_log
message. (Similarly below.)
> + 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),
I can see where we read this, but not where we write it.
If it's read only, it doesn't need to be migrated. If it's
writeable, the device needs a reset function to reset it.
("currently r/o but will become r/w when later functionality
is added, and don't want to make a migration break later" is
ok, but we should be consistent about whether we treat it as
r/w for reset and migration purposes.)
> + 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;
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test
2016-06-23 18:52 ` Peter Maydell
@ 2016-06-23 19:09 ` Cédric Le Goater
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-23 19:09 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 06/23/2016 08:52 PM, Peter Maydell wrote:
> On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
>> 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>
>> ---
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Are you in a position to check this test passes on both a
> bigendian and little endian host? I have no specific reason
> to think it will fail, but it seems worth checking.
> (If not, it'll get tested when I do a merge eventually.)
I can do bigendian on a p7/ppc64 host. I will check that when
I have addressed all your comments.
Thanks for the review.
C.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property
2016-06-23 18:35 ` Peter Maydell
@ 2016-06-24 11:49 ` Cédric Le Goater
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-24 11:49 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery, Paolo Bonzini
On 06/23/2016 08:35 PM, Peter Maydell wrote:
> On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> 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>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> 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(-)
>
> hw/arm/sabrelite.c also creates one of these devices and needs updating.
OK. I will extend Paolo's patch then.
Thanks,
C.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI)
2016-06-23 18:43 ` Peter Maydell
@ 2016-06-24 13:30 ` Cédric Le Goater
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-24 13:30 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 06/23/2016 08:43 PM, Peter Maydell wrote:
> On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
>> 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>
>> ---
>>
>> Changes since v2:
>>
>> - switched to a realize ops to handle errors.
>>
>> hw/arm/ast2400.c | 31 +++++
>> hw/ssi/Makefile.objs | 1 +
>> hw/ssi/aspeed_smc.c | 298 ++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/arm/ast2400.h | 3 +
>> include/hw/ssi/aspeed_smc.h | 79 ++++++++++++
>> 5 files changed, 412 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 1a26d74e695c..c2665c0c6ead 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
>> @@ -81,6 +84,14 @@ static void ast2400_init(Object *obj)
>> "hw-strap1", &error_abort);
>> object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
>> "hw-strap2", &error_abort);
>> +
>> + 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)
>> @@ -143,6 +154,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;
>> + }
>
> You can't pass the same err to two functions in a row
> without checking it like this. See the comments on usage in
> include/qapi/error.h.
ah yes. this a copy paste without thinking ...
>> + 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..6b52123a67bb
>> --- /dev/null
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -0,0 +1,298 @@
>> +/*
>> + * 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 */
>
> Presumably these should be CTRL, not CRTL.
CTRL.
>> +
>> +/* 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)
>
> Is this really spelt CRTL and not CTRL ?
This is really a CTRL :) I will fix that.
>> +
>> +/* 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));
>> +}
>
> This is one of those borderline cases which is "this is an
> odd way to do this but not so odd that I'm going to ask you
> to change it" :-)
yes ... I am not that happy with it either :)
I don't think I will stand adding anything else to it without
finding another solution. As U-Boot is using some extra features,
it might not survive very long in that state.
>> +
>> +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 void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + 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;
>
> Should this be a "fail the realize" instead? (I don't know the
> hardware, so genuine question.)
The FMC controller supports up to 5, the SPI only one, but there
should be more on the ast2500 for the SPI and I would like to reuse
that code.
I added the 'num_cs' property to not create uselessly slaves and fit
how a platform is used in "real". For example, one flash module for
the BMC, one for the host or two flash modules for the BMC, the extra
being a gold image and one for the host.
I don't think it should fail but I could log some message if we are
going beyond the limits.
Thanks,
C.
>> + }
>> +
>> + 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);
>> +}
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves
2016-06-23 18:48 ` Peter Maydell
@ 2016-06-24 13:42 ` Cédric Le Goater
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-24 13:42 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 06/23/2016 08:48 PM, Peter Maydell wrote:
> On 23 June 2016 at 17:33, 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 d97c077565c3..6be911ed36d0 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -98,13 +98,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)
>> @@ -217,6 +236,8 @@ static const MemoryRegionOps aspeed_smc_ops = {
>> .valid.unaligned = true,
>> };
>>
>> +static const MemoryRegionOps aspeed_smc_flash_ops;
>> +
>> static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> {
>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> @@ -254,6 +275,27 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>> 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);
>> +
>> + for (i = 0; i < s->num_cs; ++i) {
>> + Object *obj = object_new(TYPE_ASPEED_SMC_FLASH);
>> + AspeedSMCFlashState *fl = ASPEED_SMC_FLASH(obj);
>> + char name[32];
>> +
>> + 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;
>> +
>> + memory_region_init_io(&fl->mmio, obj, &aspeed_smc_flash_ops, fl, name,
>> + fl->size);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(fl), &fl->mmio);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(fl), 0, s->ctrl->segments[i].addr);
>
> Device code shouldn't call sysbus_mmio_map() -- the system memory map is
> the board's responsibility.
ok. It's here because it was easy to reuse the address segments[i].addr which
should not change, even if is reconfigurable at run time by the guest.
I will rethink the structures and find way.
Thanks,
C.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object
2016-06-23 18:56 ` Peter Maydell
@ 2016-06-24 13:47 ` Cédric Le Goater
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2016-06-24 13:47 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, QEMU Developers, qemu-arm, Kevin Wolf,
Markus Armbruster, Andrew Jeffery
On 06/23/2016 08:56 PM, Peter Maydell wrote:
> On 23 June 2016 at 17:33, Cédric Le Goater <clg@kaod.org> wrote:
>> 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 6b52123a67bb..d97c077565c3 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);
>
> You don't need these brackets.
true.
>> +}
>> +
>> +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;
>> @@ -296,3 +311,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__);
>
> This should probably be a LOG_UNIMP or LOG_GUEST_ERROR qemu_log
> message. (Similarly below.)
ok.
>> + 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),
>
> I can see where we read this, but not where we write it.
> If it's read only, it doesn't need to be migrated. If it's
> writeable, the device needs a reset function to reset it.
> ("currently r/o but will become r/w when later functionality
> is added, and don't want to make a migration break later" is
> ok, but we should be consistent about whether we treat it as
> r/w for reset and migration purposes.)
I have not given enough attention to the migration of this object.
I will check that.
Thanks,
C.
>> + 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;
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-06-24 13:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 16:33 [Qemu-devel] [PATCH v3 0/5] ast2400: SMC controllers Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 1/5] m25p80: qdev-ify drive property Cédric Le Goater
2016-06-23 18:35 ` Peter Maydell
2016-06-24 11:49 ` Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 2/5] ast2400: add SMC controllers (FMC and SPI) Cédric Le Goater
2016-06-23 18:43 ` Peter Maydell
2016-06-24 13:30 ` Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 3/5] ast2400: add SPI flash slave object Cédric Le Goater
2016-06-23 18:56 ` Peter Maydell
2016-06-24 13:47 ` Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 4/5] ast2400: create SPI flash slaves Cédric Le Goater
2016-06-23 18:48 ` Peter Maydell
2016-06-24 13:42 ` Cédric Le Goater
2016-06-23 16:33 ` [Qemu-devel] [PATCH v3 5/5] tests: add a m25p80 test Cédric Le Goater
2016-06-23 18:52 ` Peter Maydell
2016-06-23 19:09 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.