[Qemu-devel] [PATCH 0/6] aspeed: add support for the ast2500 SMC controllers

[Qemu-devel] [PATCH 0/6] aspeed: add support for the ast2500 SMC controllers

[Cédric Le Goater]

Hello,

The Aspeed AST2500 has one 'SPI' controller for the BMC firmware and
two 'SPI' for the host firmware. All controllers have now the same set
of registers compatible with the AST2400 'FMC' controller and the
legacy 'SMC' controller is fully gone.

This serie adds support for the second SPI controller and for the
segment registers which are used to configure the mapping of each
flash module in the SoC address space.

Thanks,

C. 

Cédric Le Goater (6):
  aspeed: rename smc object to fmc
  aspeed: move the flash module mapping address under the controller
    definition
  aspeed: extend the number of host SPI controllers
  aspeed: add support for the AST2500 SoC SMC controllers
  aspeed: create mapping regions for the maximum number of slaves
  aspeed: add support for the SMC segment registers

 hw/arm/aspeed.c             |   4 +-
 hw/arm/aspeed_soc.c         |  74 +++++++++++------
 hw/ssi/aspeed_smc.c         | 194 +++++++++++++++++++++++++++++++++++++++++---
 include/hw/arm/aspeed_soc.h |  10 ++-
 include/hw/ssi/aspeed_smc.h |   3 +-
 5 files changed, 241 insertions(+), 44 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/6] aspeed: rename the smc object to fmc

[Cédric Le Goater]

The Aspeed SoC has three different types of SMC (Static Memory
Controller) controllers: the SMC (legacy), the FMC (the new one) and
the SPI for the host PNOR. The FMC and the SPI models are now
converging on the AST2500 SoC and the SMC, which was still available
on the AST2400 SoC, was removed.

The Aspeed SoC does not provide support for the legacy SMC
controller. So, let's rename the 'smc' object to 'fmc' to clarify its
nature.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c             |  2 +-
 hw/arm/aspeed_soc.c         | 18 +++++++++---------
 include/hw/arm/aspeed_soc.h |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6b18c7f1727c..4bb33cbb5e70 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -128,7 +128,7 @@ static void aspeed_board_init(MachineState *machine,
     object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram),
                                    &error_abort);
 
-    aspeed_board_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
+    aspeed_board_init_flashes(&bmc->soc.fmc, "n25q256a", &error_abort);
     aspeed_board_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
 
     aspeed_board_binfo.kernel_filename = machine->kernel_filename;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index c0a310205842..479c0d2039d9 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -100,9 +100,9 @@ static void aspeed_soc_init(Object *obj)
     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->fmc, sizeof(s->fmc), "aspeed.smc.fmc");
+    object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
+    qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
 
     object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
     object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
@@ -178,17 +178,17 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     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", &local_err);
+    /* FMC */
+    object_property_set_int(OBJECT(&s->fmc), 1, "num-cs", &err);
+    object_property_set_bool(OBJECT(&s->fmc), true, "realized", &local_err);
     error_propagate(&err, local_err);
     if (err) {
         error_propagate(errp, err);
         return;
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, ASPEED_SOC_FMC_BASE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 1, ASPEED_SOC_FMC_FLASH_BASE);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1, ASPEED_SOC_FMC_FLASH_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        qdev_get_gpio_in(DEVICE(&s->vic), 19));
 
     /* SPI */
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 932704c380f2..7359e25fce49 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -31,7 +31,7 @@ typedef struct AspeedSoCState {
     AspeedTimerCtrlState timerctrl;
     AspeedI2CState i2c;
     AspeedSCUState scu;
-    AspeedSMCState smc;
+    AspeedSMCState fmc;
     AspeedSMCState spi;
     AspeedSDMCState sdmc;
 } AspeedSoCState;
-- 
2.7.4

[Qemu-devel] [PATCH 2/6] aspeed: move the flash module mapping address under the controller definition

[Cédric Le Goater]

This will ease the definition of the new controllers for the AST2500
SoC and also ease the support of the segment registers, which provide
a way to reconfigure the mapping window of each slave.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed_soc.c         |  9 ++++-----
 hw/ssi/aspeed_smc.c         | 15 +++++++++++----
 include/hw/ssi/aspeed_smc.h |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 479c0d2039d9..80ad7322bde2 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -31,9 +31,6 @@
 #define ASPEED_SOC_TIMER_BASE       0x1E782000
 #define ASPEED_SOC_I2C_BASE         0x1E78A000
 
-#define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
-#define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
-
 static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
 static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
 
@@ -187,7 +184,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         return;
     }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1, ASPEED_SOC_FMC_FLASH_BASE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
+                    s->fmc.ctrl->flash_window_base);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        qdev_get_gpio_in(DEVICE(&s->vic), 19));
 
@@ -200,7 +198,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
         return;
     }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, ASPEED_SOC_SPI_BASE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1, ASPEED_SOC_SPI_FLASH_BASE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1,
+                    s->spi.ctrl->flash_window_base);
 
     /* SDMC - SDRAM Memory Controller */
     object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index d319e04a27f0..84c18299de11 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -127,6 +127,10 @@
 #define R_SPI_MISC_CTRL   (0x10 / 4)
 #define R_SPI_TIMINGS     (0x14 / 4)
 
+#define ASPEED_SOC_SMC_FLASH_BASE   0x10000000
+#define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
+#define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
+
 /*
  * Default segments mapping addresses and size for each slave per
  * controller. These can be changed when board is initialized with the
@@ -151,11 +155,14 @@ static const AspeedSegments aspeed_segments_spi[] = {
 
 static const AspeedSMCController controllers[] = {
     { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
-      CONF_ENABLE_W0, 5, aspeed_segments_legacy, 0x6000000 },
+      CONF_ENABLE_W0, 5, aspeed_segments_legacy,
+      ASPEED_SOC_SMC_FLASH_BASE, 0x6000000 },
     { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
-      CONF_ENABLE_W0, 5, aspeed_segments_fmc, 0x10000000 },
+      CONF_ENABLE_W0, 5, aspeed_segments_fmc,
+      ASPEED_SOC_FMC_FLASH_BASE, 0x10000000 },
     { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
-      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, 0x10000000 },
+      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi,
+      ASPEED_SOC_SPI_FLASH_BASE, 0x10000000 },
 };
 
 static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
@@ -395,7 +402,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
 
     memory_region_init_io(&s->mmio_flash, OBJECT(s),
                           &aspeed_smc_flash_default_ops, s, name,
-                          s->ctrl->mapping_window_size);
+                          s->ctrl->flash_window_size);
     sysbus_init_mmio(sbd, &s->mmio_flash);
 
     s->flashes = g_new0(AspeedSMCFlash, s->num_cs);
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index def3b4507e75..bdfbcc0ffa7d 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -42,7 +42,8 @@ typedef struct AspeedSMCController {
     uint8_t conf_enable_w0;
     uint8_t max_slaves;
     const AspeedSegments *segments;
-    uint32_t mapping_window_size;
+    hwaddr flash_window_base;
+    uint32_t flash_window_size;
 } AspeedSMCController;
 
 typedef struct AspeedSMCFlash {
-- 
2.7.4

[Qemu-devel] [PATCH 3/6] aspeed: extend the number of host SPI controllers

[Cédric Le Goater]

The AST2500 SoC has two. Let's prepare ground for the next changes
which will add the required definitions for the second host SPI
controller.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c             |  2 +-
 hw/arm/aspeed_soc.c         | 44 +++++++++++++++++++++++++++++---------------
 include/hw/arm/aspeed_soc.h |  6 +++++-
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4bb33cbb5e70..c7206fda6d85 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -129,7 +129,7 @@ static void aspeed_board_init(MachineState *machine,
                                    &error_abort);
 
     aspeed_board_init_flashes(&bmc->soc.fmc, "n25q256a", &error_abort);
-    aspeed_board_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
+    aspeed_board_init_flashes(&bmc->soc.spi[0], "mx25l25635e", &error_abort);
 
     aspeed_board_binfo.kernel_filename = machine->kernel_filename;
     aspeed_board_binfo.initrd_filename = machine->initrd_filename;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 80ad7322bde2..b3103f337374 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -37,10 +37,17 @@ static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
 #define AST2400_SDRAM_BASE       0x40000000
 #define AST2500_SDRAM_BASE       0x80000000
 
+static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
+
+static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE };
+
 static const AspeedSoCInfo aspeed_socs[] = {
-    { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
-    { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
-    { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE },
+    { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
+      1, aspeed_soc_ast2400_spi_bases },
+    { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
+      1, aspeed_soc_ast2400_spi_bases },
+    { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE,
+      1, aspeed_soc_ast2500_spi_bases },
 };
 
 /*
@@ -72,6 +79,7 @@ static void aspeed_soc_init(Object *obj)
 {
     AspeedSoCState *s = ASPEED_SOC(obj);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+    int i;
 
     s->cpu = cpu_arm_init(sc->info->cpu_model);
 
@@ -101,9 +109,11 @@ static void aspeed_soc_init(Object *obj)
     object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
     qdev_set_parent_bus(DEVICE(&s->fmc), 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());
+    for (i = 0; i < sc->info->spis_num; i++) {
+        object_initialize(&s->spi[i], sizeof(s->spi[i]), "aspeed.smc.spi");
+        object_property_add_child(obj, "spi", OBJECT(&s->spi[i]), NULL);
+        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+    }
 
     object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
     object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
@@ -118,6 +128,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 {
     int i;
     AspeedSoCState *s = ASPEED_SOC(dev);
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     Error *err = NULL, *local_err = NULL;
 
     /* IO space */
@@ -190,16 +201,19 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        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", &local_err);
-    error_propagate(&err, local_err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
+    for (i = 0; i < sc->info->spis_num; i++) {
+        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized",
+                                 &local_err);
+        error_propagate(&err, local_err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, sc->info->spi_bases[i]);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
+                        s->spi[i].ctrl->flash_window_base);
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, ASPEED_SOC_SPI_BASE);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1,
-                    s->spi.ctrl->flash_window_base);
 
     /* SDMC - SDRAM Memory Controller */
     object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 7359e25fce49..f26a9f043983 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -20,6 +20,8 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "hw/ssi/aspeed_smc.h"
 
+#define ASPEED_SPIS_NUM  2
+
 typedef struct AspeedSoCState {
     /*< private >*/
     DeviceState parent;
@@ -32,7 +34,7 @@ typedef struct AspeedSoCState {
     AspeedI2CState i2c;
     AspeedSCUState scu;
     AspeedSMCState fmc;
-    AspeedSMCState spi;
+    AspeedSMCState spi[ASPEED_SPIS_NUM];
     AspeedSDMCState sdmc;
 } AspeedSoCState;
 
@@ -44,6 +46,8 @@ typedef struct AspeedSoCInfo {
     const char *cpu_model;
     uint32_t silicon_rev;
     hwaddr sdram_base;
+    int spis_num;
+    const hwaddr *spi_bases;
 } AspeedSoCInfo;
 
 typedef struct AspeedSoCClass {
-- 
2.7.4

[Qemu-devel] [PATCH 4/6] aspeed: add support for the AST2500 SoC SMC controllers

[Cédric Le Goater]

The SMC controllers on the Aspeed AST2500 SoC are very similar to the
ones found on the AST2400. The differences are on the number of
supported flash modules and their default mappings in the SoC address
space.

The Aspeed AST2500 has one SPI controller for the BMC firmware and two
for the host firmware. All controllers have now the same set of
registers compatible with the AST2400 FMC controller and the legacy
'SMC' controller is fully gone.

We keep the FMC object to act as the BMC SPI controller and add a new
SPI controller for the host. We also have to introduce new type names
to handle the differences in the flash modules memory mappping.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed_soc.c         | 21 +++++++++++++++------
 hw/ssi/aspeed_smc.c         | 28 +++++++++++++++++++++++++++-
 include/hw/arm/aspeed_soc.h |  2 ++
 3 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index b3103f337374..e14f5c217eab 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -25,6 +25,7 @@
 #define ASPEED_SOC_IOMEM_BASE       0x1E600000
 #define ASPEED_SOC_FMC_BASE         0x1E620000
 #define ASPEED_SOC_SPI_BASE         0x1E630000
+#define ASPEED_SOC_SPI2_BASE        0x1E631000
 #define ASPEED_SOC_VIC_BASE         0x1E6C0000
 #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
 #define ASPEED_SOC_SCU_BASE         0x1E6E2000
@@ -38,16 +39,23 @@ static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
 #define AST2500_SDRAM_BASE       0x80000000
 
 static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
+static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
 
-static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE };
+static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE,
+                                                       ASPEED_SOC_SPI2_BASE};
+static const char *aspeed_soc_ast2500_typenames[] = {
+    "aspeed.smc.ast2500-spi1", "aspeed.smc.ast2500-spi2" };
 
 static const AspeedSoCInfo aspeed_socs[] = {
     { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
-      1, aspeed_soc_ast2400_spi_bases },
+      1, aspeed_soc_ast2400_spi_bases,
+      "aspeed.smc.fmc", aspeed_soc_ast2400_typenames },
     { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
-      1, aspeed_soc_ast2400_spi_bases },
+      1, aspeed_soc_ast2400_spi_bases,
+     "aspeed.smc.fmc", aspeed_soc_ast2400_typenames },
     { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE,
-      1, aspeed_soc_ast2500_spi_bases },
+      2, aspeed_soc_ast2500_spi_bases,
+      "aspeed.smc.ast2500-fmc", aspeed_soc_ast2500_typenames },
 };
 
 /*
@@ -105,12 +113,13 @@ static void aspeed_soc_init(Object *obj)
     object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
                               "hw-strap2", &error_abort);
 
-    object_initialize(&s->fmc, sizeof(s->fmc), "aspeed.smc.fmc");
+    object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
     object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
     qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
 
     for (i = 0; i < sc->info->spis_num; i++) {
-        object_initialize(&s->spi[i], sizeof(s->spi[i]), "aspeed.smc.spi");
+        object_initialize(&s->spi[i], sizeof(s->spi[i]),
+                          sc->info->spi_typename[i]);
         object_property_add_child(obj, "spi", OBJECT(&s->spi[i]), NULL);
         qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
     }
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 84c18299de11..21943f4e5dfa 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -130,6 +130,7 @@
 #define ASPEED_SOC_SMC_FLASH_BASE   0x10000000
 #define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
 #define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
+#define ASPEED_SOC_SPI2_FLASH_BASE  0x38000000
 
 /*
  * Default segments mapping addresses and size for each slave per
@@ -142,7 +143,7 @@ static const AspeedSegments aspeed_segments_legacy[] = {
 };
 
 static const AspeedSegments aspeed_segments_fmc[] = {
-    { 0x20000000, 64 * 1024 * 1024 },
+    { 0x20000000, 64 * 1024 * 1024 }, /* start address is readonly */
     { 0x24000000, 32 * 1024 * 1024 },
     { 0x26000000, 32 * 1024 * 1024 },
     { 0x28000000, 32 * 1024 * 1024 },
@@ -153,6 +154,22 @@ static const AspeedSegments aspeed_segments_spi[] = {
     { 0x30000000, 64 * 1024 * 1024 },
 };
 
+static const AspeedSegments aspeed_segments_ast2500_fmc[] = {
+    { 0x20000000, 128 * 1024 * 1024 }, /* start address is readonly */
+    { 0x28000000,  32 * 1024 * 1024 },
+    { 0x2A000000,  32 * 1024 * 1024 },
+};
+
+static const AspeedSegments aspeed_segments_ast2500_spi1[] = {
+    { 0x30000000, 32 * 1024 * 1024 }, /* start address is readonly */
+    { 0x32000000, 96 * 1024 * 1024 }, /* end address is readonly */
+};
+
+static const AspeedSegments aspeed_segments_ast2500_spi2[] = {
+    { 0x38000000, 32 * 1024 * 1024 }, /* start address is readonly */
+    { 0x3A000000, 96 * 1024 * 1024 }, /* end address is readonly */
+};
+
 static const AspeedSMCController controllers[] = {
     { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
       CONF_ENABLE_W0, 5, aspeed_segments_legacy,
@@ -163,6 +180,15 @@ static const AspeedSMCController controllers[] = {
     { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
       SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi,
       ASPEED_SOC_SPI_FLASH_BASE, 0x10000000 },
+    { "aspeed.smc.ast2500-fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+      CONF_ENABLE_W0, 3, aspeed_segments_ast2500_fmc,
+      ASPEED_SOC_FMC_FLASH_BASE, 0x10000000 },
+    { "aspeed.smc.ast2500-spi1", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+      CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi1,
+      ASPEED_SOC_SPI_FLASH_BASE, 0x8000000 },
+    { "aspeed.smc.ast2500-spi2", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
+      CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi2,
+      ASPEED_SOC_SPI2_FLASH_BASE, 0x8000000 },
 };
 
 static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index f26a9f043983..5406b498d7ef 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -48,6 +48,8 @@ typedef struct AspeedSoCInfo {
     hwaddr sdram_base;
     int spis_num;
     const hwaddr *spi_bases;
+    const char *fmc_typename;
+    const char **spi_typename;
 } AspeedSoCInfo;
 
 typedef struct AspeedSoCClass {
-- 
2.7.4

[Qemu-devel] [PATCH 5/6] aspeed: create mapping regions for the maximum number of slaves

[Cédric Le Goater]

The SMC controller on the Aspeed SoC has a set of registers to
configure the mapping of each flash module in the SoC address
space. These mapping windows are configurable even though no SPI slave
is attached to the controller.

Also rewrite a bit the comments in the code on this topic.

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

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 21943f4e5dfa..ecf39ccfde0e 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -417,12 +417,15 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
 
     aspeed_smc_reset(dev);
 
+    /* The memory region for the controller registers */
     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);
 
     /*
-     * Memory region where flash modules are remapped
+     * The container memory region representing the address space
+     * window in which the flash modules are mapped. The size and
+     * address depends on the SoC model and controller type.
      */
     snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
 
@@ -431,9 +434,16 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
                           s->ctrl->flash_window_size);
     sysbus_init_mmio(sbd, &s->mmio_flash);
 
-    s->flashes = g_new0(AspeedSMCFlash, s->num_cs);
+    s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_slaves);
 
-    for (i = 0; i < s->num_cs; ++i) {
+    /*
+     * Let's create a sub memory region for each possible slave. All
+     * have a configurable memory segment in the overall flash mapping
+     * window of the controller but, there is not necessarily a flash
+     * module behind to handle the memory accesses. This depends on
+     * the board configuration.
+     */
+    for (i = 0; i < s->ctrl->max_slaves; ++i) {
         AspeedSMCFlash *fl = &s->flashes[i];
 
         snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);
-- 
2.7.4

[Qemu-devel] [PATCH 6/6] aspeed: add support for the SMC segment registers

[Cédric Le Goater]

The SMC controller on the Aspeed SoC has a set of registers to
configure the mapping of each flash module in the SoC address
space. Writing to these registers triggers a remap of the memory
region and the spec requires a certain number of checks before doing
so.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 130 insertions(+), 5 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index ecf39ccfde0e..6e8403ebc246 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -79,10 +79,10 @@
 
 /* CEx Segment Address Register */
 #define R_SEG_ADDR0       (0x30 / 4)
-#define   SEG_SIZE_SHIFT       24   /* 8MB units */
-#define   SEG_SIZE_MASK        0x7f
+#define   SEG_END_SHIFT        24   /* 8MB units */
+#define   SEG_END_MASK         0xff
 #define   SEG_START_SHIFT      16   /* address bit [A29-A23] */
-#define   SEG_START_MASK       0x7f
+#define   SEG_START_MASK       0xff
 #define R_SEG_ADDR1       (0x34 / 4)
 #define R_SEG_ADDR2       (0x38 / 4)
 #define R_SEG_ADDR3       (0x3C / 4)
@@ -135,8 +135,7 @@
 /*
  * Default segments mapping addresses and size for each slave per
  * controller. These can be changed when board is initialized with the
- * Segment Address Registers but they don't seem do be used on the
- * field.
+ * Segment Address Registers.
  */
 static const AspeedSegments aspeed_segments_legacy[] = {
     { 0x10000000, 32 * 1024 * 1024 },
@@ -191,6 +190,118 @@ static const AspeedSMCController controllers[] = {
       ASPEED_SOC_SPI2_FLASH_BASE, 0x8000000 },
 };
 
+/*
+ * The Segment Register uses a 8MB unit to encode the start address
+ * and the end address of the mapping window of a flash SPI slave :
+ *
+ *        | byte 1 | byte 2 | byte 3 | byte 4 |
+ *        +--------+--------+--------+--------+
+ *        |  end   |  start |   0    |   0    |
+ *
+ */
+static inline uint32_t aspeed_smc_segment_to_reg(const AspeedSegments *seg)
+{
+    uint32_t reg = 0;
+    reg |= ((seg->addr >> 23) & SEG_START_MASK) << SEG_START_SHIFT;
+    reg |= (((seg->addr + seg->size) >> 23) & SEG_END_MASK) << SEG_END_SHIFT;
+    return reg;
+}
+
+static inline void aspeed_smc_reg_to_segment(uint32_t reg, AspeedSegments *seg)
+{
+    seg->addr = ((reg >> SEG_START_SHIFT) & SEG_START_MASK) << 23;
+    seg->size = (((reg >> SEG_END_SHIFT) & SEG_END_MASK) << 23) - seg->addr;
+}
+
+static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
+                                     const AspeedSegments *new,
+                                     int cs)
+{
+    AspeedSegments seg;
+    int i;
+
+    for (i = 0; i < s->ctrl->max_slaves; i++) {
+        if (i == cs) {
+            continue;
+        }
+
+        aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + i], &seg);
+
+        if (new->addr + new->size > seg.addr &&
+            new->addr < seg.addr + seg.size) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment CS%d [ 0x%"
+                          HWADDR_PRIx" - 0x%"HWADDR_PRIx" ] overlaps with "
+                          "CS%d [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
+                          s->ctrl->name, cs, new->addr, new->addr + new->size,
+                          i, seg.addr, seg.addr + seg.size);
+            return true;
+        }
+    }
+    return false;
+}
+
+static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
+                                         uint64_t new)
+{
+    AspeedSMCFlash *fl = &s->flashes[cs];
+    AspeedSegments seg;
+
+    aspeed_smc_reg_to_segment(new, &seg);
+
+    /* The start address of CS0 is read-only */
+    if (cs == 0 && seg.addr != s->ctrl->flash_window_base) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Tried to change CS0 start address to 0x%"
+                      HWADDR_PRIx "\n", s->ctrl->name, seg.addr);
+        return;
+    }
+
+    /*
+     * The end address of the AST2500 spi controllers is also
+     * read-only.
+     */
+    if ((s->ctrl->segments == aspeed_segments_ast2500_spi1 ||
+         s->ctrl->segments == aspeed_segments_ast2500_spi2) &&
+        cs == s->ctrl->max_slaves &&
+        seg.addr + seg.size != s->ctrl->segments[cs].addr +
+        s->ctrl->segments[cs].size) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Tried to change CS%d end address to 0x%"
+                      HWADDR_PRIx "\n", s->ctrl->name, cs, seg.addr);
+        return;
+    }
+
+    /* Keep the segment in the overall flash window */
+    if (seg.addr + seg.size <= s->ctrl->flash_window_base ||
+        seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : "
+                      "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
+                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
+        return;
+    }
+
+    /* Check start address vs. alignment */
+    if (seg.addr % seg.size) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is not "
+                      "aligned : [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
+                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
+    }
+
+    /* And segments should not overlap */
+    if (aspeed_smc_flash_overlap(s, &seg, cs)) {
+        return;
+    }
+
+    /* All should be fine now to move the region */
+    memory_region_transaction_begin();
+    memory_region_set_size(&fl->mmio, seg.size);
+    memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
+    memory_region_set_enabled(&fl->mmio, true);
+    memory_region_transaction_commit();
+
+    s->regs[R_SEG_ADDR0 + cs] = new;
+}
+
 static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
                                               unsigned size)
 {
@@ -314,6 +425,12 @@ static void aspeed_smc_reset(DeviceState *d)
         s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
     }
 
+    /* setup default segment register values for all */
+    for (i = 0; i < s->ctrl->max_slaves; ++i) {
+        s->regs[R_SEG_ADDR0 + i] =
+            aspeed_smc_segment_to_reg(&s->ctrl->segments[i]);
+    }
+
     aspeed_smc_update_cs(s);
 }
 
@@ -334,6 +451,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
         addr == s->r_timings ||
         addr == s->r_ce_ctrl ||
         addr == R_INTR_CTRL ||
+        (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
         (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
         return s->regs[addr];
     } else {
@@ -365,6 +483,13 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
     } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
         s->regs[addr] = value;
         aspeed_smc_update_cs(s);
+    } else if (addr >= R_SEG_ADDR0 &&
+               addr < R_SEG_ADDR0 + s->ctrl->max_slaves) {
+        int cs = addr - R_SEG_ADDR0;
+
+        if (value != s->regs[R_SEG_ADDR0 + cs]) {
+            aspeed_smc_flash_set_segment(s, cs, value);
+        }
     } else {
         qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
                       __func__, addr);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/6] aspeed: rename the smc object to fmc

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> The Aspeed SoC has three different types of SMC (Static Memory
> Controller) controllers: the SMC (legacy), the FMC (the new one) and
> the SPI for the host PNOR. The FMC and the SPI models are now
> converging on the AST2500 SoC and the SMC, which was still available
> on the AST2400 SoC, was removed.
> 
> The Aspeed SoC does not provide support for the legacy SMC
> controller. So, let's rename the 'smc' object to 'fmc' to clarify its
> nature.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/arm/aspeed.c             |  2 +-
>  hw/arm/aspeed_soc.c         | 18 +++++++++---------
>  include/hw/arm/aspeed_soc.h |  2 +-
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 6b18c7f1727c..4bb33cbb5e70 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -128,7 +128,7 @@ static void aspeed_board_init(MachineState *machine,
>      object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram),
>                                     &error_abort);
>  
> -    aspeed_board_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
> +    aspeed_board_init_flashes(&bmc->soc.fmc, "n25q256a", &error_abort);
>      aspeed_board_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
>  
>      aspeed_board_binfo.kernel_filename = machine->kernel_filename;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index c0a310205842..479c0d2039d9 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -100,9 +100,9 @@ static void aspeed_soc_init(Object *obj)
>      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->fmc, sizeof(s->fmc), "aspeed.smc.fmc");
> +    object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
>  
>      object_initialize(&s->spi, sizeof(s->spi), "aspeed.smc.spi");
>      object_property_add_child(obj, "spi", OBJECT(&s->spi), NULL);
> @@ -178,17 +178,17 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>      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", &local_err);
> +    /* FMC */
> +    object_property_set_int(OBJECT(&s->fmc), 1, "num-cs", &err);
> +    object_property_set_bool(OBJECT(&s->fmc), true, "realized", &local_err);
>      error_propagate(&err, local_err);
>      if (err) {
>          error_propagate(errp, err);
>          return;
>      }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 0, ASPEED_SOC_FMC_BASE);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->smc), 1, ASPEED_SOC_FMC_FLASH_BASE);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->smc), 0,
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1, ASPEED_SOC_FMC_FLASH_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>                         qdev_get_gpio_in(DEVICE(&s->vic), 19));
>  
>      /* SPI */
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 932704c380f2..7359e25fce49 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -31,7 +31,7 @@ typedef struct AspeedSoCState {
>      AspeedTimerCtrlState timerctrl;
>      AspeedI2CState i2c;
>      AspeedSCUState scu;
> -    AspeedSMCState smc;
> +    AspeedSMCState fmc;
>      AspeedSMCState spi;
>      AspeedSDMCState sdmc;
>  } AspeedSoCState;

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

Re: [Qemu-devel] [PATCH 2/6] aspeed: move the flash module mapping address under the controller definition

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> This will ease the definition of the new controllers for the AST2500
> SoC and also ease the support of the segment registers, which provide
> a way to reconfigure the mapping window of each slave.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/arm/aspeed_soc.c         |  9 ++++-----
>  hw/ssi/aspeed_smc.c         | 15 +++++++++++----
>  include/hw/ssi/aspeed_smc.h |  3 ++-
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 479c0d2039d9..80ad7322bde2 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -31,9 +31,6 @@
>  #define ASPEED_SOC_TIMER_BASE       0x1E782000
>  #define ASPEED_SOC_I2C_BASE         0x1E78A000
>  
> -#define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
> -#define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
> -
>  static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>  
> @@ -187,7 +184,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1, ASPEED_SOC_FMC_FLASH_BASE);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> +                    s->fmc.ctrl->flash_window_base);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>                         qdev_get_gpio_in(DEVICE(&s->vic), 19));
>  
> @@ -200,7 +198,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, ASPEED_SOC_SPI_BASE);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1, ASPEED_SOC_SPI_FLASH_BASE);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1,
> +                    s->spi.ctrl->flash_window_base);
>  
>      /* SDMC - SDRAM Memory Controller */
>      object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index d319e04a27f0..84c18299de11 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -127,6 +127,10 @@
>  #define R_SPI_MISC_CTRL   (0x10 / 4)
>  #define R_SPI_TIMINGS     (0x14 / 4)
>  
> +#define ASPEED_SOC_SMC_FLASH_BASE   0x10000000
> +#define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
> +#define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
> +
>  /*
>   * Default segments mapping addresses and size for each slave per
>   * controller. These can be changed when board is initialized with the
> @@ -151,11 +155,14 @@ static const AspeedSegments aspeed_segments_spi[] = {
>  
>  static const AspeedSMCController controllers[] = {
>      { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> -      CONF_ENABLE_W0, 5, aspeed_segments_legacy, 0x6000000 },
> +      CONF_ENABLE_W0, 5, aspeed_segments_legacy,
> +      ASPEED_SOC_SMC_FLASH_BASE, 0x6000000 },
>      { "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> -      CONF_ENABLE_W0, 5, aspeed_segments_fmc, 0x10000000 },
> +      CONF_ENABLE_W0, 5, aspeed_segments_fmc,
> +      ASPEED_SOC_FMC_FLASH_BASE, 0x10000000 },
>      { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
> -      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, 0x10000000 },
> +      SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi,
> +      ASPEED_SOC_SPI_FLASH_BASE, 0x10000000 },
>  };
>  
>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
> @@ -395,7 +402,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>  
>      memory_region_init_io(&s->mmio_flash, OBJECT(s),
>                            &aspeed_smc_flash_default_ops, s, name,
> -                          s->ctrl->mapping_window_size);
> +                          s->ctrl->flash_window_size);
>      sysbus_init_mmio(sbd, &s->mmio_flash);
>  
>      s->flashes = g_new0(AspeedSMCFlash, s->num_cs);
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index def3b4507e75..bdfbcc0ffa7d 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -42,7 +42,8 @@ typedef struct AspeedSMCController {
>      uint8_t conf_enable_w0;
>      uint8_t max_slaves;
>      const AspeedSegments *segments;
> -    uint32_t mapping_window_size;
> +    hwaddr flash_window_base;
> +    uint32_t flash_window_size;
>  } AspeedSMCController;
>  
>  typedef struct AspeedSMCFlash {

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

Re: [Qemu-devel] [PATCH 3/6] aspeed: extend the number of host SPI controllers

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> The AST2500 SoC has two. Let's prepare ground for the next changes
> which will add the required definitions for the second host SPI
> controller.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/arm/aspeed.c             |  2 +-
>  hw/arm/aspeed_soc.c         | 44 +++++++++++++++++++++++++++++---------------
>  include/hw/arm/aspeed_soc.h |  6 +++++-
>  3 files changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 4bb33cbb5e70..c7206fda6d85 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -129,7 +129,7 @@ static void aspeed_board_init(MachineState *machine,
>                                     &error_abort);
>  
>      aspeed_board_init_flashes(&bmc->soc.fmc, "n25q256a", &error_abort);
> -    aspeed_board_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
> +    aspeed_board_init_flashes(&bmc->soc.spi[0], "mx25l25635e", &error_abort);
>  
>      aspeed_board_binfo.kernel_filename = machine->kernel_filename;
>      aspeed_board_binfo.initrd_filename = machine->initrd_filename;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 80ad7322bde2..b3103f337374 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -37,10 +37,17 @@ static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>  #define AST2400_SDRAM_BASE       0x40000000
>  #define AST2500_SDRAM_BASE       0x80000000
>  
> +static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> +
> +static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> +
>  static const AspeedSoCInfo aspeed_socs[] = {
> -    { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
> -    { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
> -    { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE },
> +    { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
> +      1, aspeed_soc_ast2400_spi_bases },
> +    { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
> +      1, aspeed_soc_ast2400_spi_bases },
> +    { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE,
> +      1, aspeed_soc_ast2500_spi_bases },
>  };
>  
>  /*
> @@ -72,6 +79,7 @@ static void aspeed_soc_init(Object *obj)
>  {
>      AspeedSoCState *s = ASPEED_SOC(obj);
>      AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> +    int i;
>  
>      s->cpu = cpu_arm_init(sc->info->cpu_model);
>  
> @@ -101,9 +109,11 @@ static void aspeed_soc_init(Object *obj)
>      object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
>      qdev_set_parent_bus(DEVICE(&s->fmc), 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());
> +    for (i = 0; i < sc->info->spis_num; i++) {
> +        object_initialize(&s->spi[i], sizeof(s->spi[i]), "aspeed.smc.spi");
> +        object_property_add_child(obj, "spi", OBJECT(&s->spi[i]), NULL);
> +        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
> +    }
>  
>      object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
>      object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
> @@ -118,6 +128,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>  {
>      int i;
>      AspeedSoCState *s = ASPEED_SOC(dev);
> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>      Error *err = NULL, *local_err = NULL;
>  
>      /* IO space */
> @@ -190,16 +201,19 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>                         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", &local_err);
> -    error_propagate(&err, local_err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> +    for (i = 0; i < sc->info->spis_num; i++) {
> +        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
> +        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized",
> +                                 &local_err);
> +        error_propagate(&err, local_err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, sc->info->spi_bases[i]);
> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
> +                        s->spi[i].ctrl->flash_window_base);
>      }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 0, ASPEED_SOC_SPI_BASE);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1,
> -                    s->spi.ctrl->flash_window_base);
>  
>      /* SDMC - SDRAM Memory Controller */
>      object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 7359e25fce49..f26a9f043983 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -20,6 +20,8 @@
>  #include "hw/i2c/aspeed_i2c.h"
>  #include "hw/ssi/aspeed_smc.h"
>  
> +#define ASPEED_SPIS_NUM  2
> +
>  typedef struct AspeedSoCState {
>      /*< private >*/
>      DeviceState parent;
> @@ -32,7 +34,7 @@ typedef struct AspeedSoCState {
>      AspeedI2CState i2c;
>      AspeedSCUState scu;
>      AspeedSMCState fmc;
> -    AspeedSMCState spi;
> +    AspeedSMCState spi[ASPEED_SPIS_NUM];
>      AspeedSDMCState sdmc;
>  } AspeedSoCState;
>  
> @@ -44,6 +46,8 @@ typedef struct AspeedSoCInfo {
>      const char *cpu_model;
>      uint32_t silicon_rev;
>      hwaddr sdram_base;
> +    int spis_num;
> +    const hwaddr *spi_bases;
>  } AspeedSoCInfo;
>  
>  typedef struct AspeedSoCClass {

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

Re: [Qemu-devel] [PATCH 4/6] aspeed: add support for the AST2500 SoC SMC controllers

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
> ones found on the AST2400. The differences are on the number of
> supported flash modules and their default mappings in the SoC address
> space.
> 
> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
> for the host firmware. All controllers have now the same set of
> registers compatible with the AST2400 FMC controller and the legacy
> 'SMC' controller is fully gone.
> 
> We keep the FMC object to act as the BMC SPI controller and add a new
> SPI controller for the host. We also have to introduce new type names
> to handle the differences in the flash modules memory mappping.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/arm/aspeed_soc.c         | 21 +++++++++++++++------
>  hw/ssi/aspeed_smc.c         | 28 +++++++++++++++++++++++++++-
>  include/hw/arm/aspeed_soc.h |  2 ++
>  3 files changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index b3103f337374..e14f5c217eab 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -25,6 +25,7 @@
>  #define ASPEED_SOC_IOMEM_BASE       0x1E600000
>  #define ASPEED_SOC_FMC_BASE         0x1E620000
>  #define ASPEED_SOC_SPI_BASE         0x1E630000
> +#define ASPEED_SOC_SPI2_BASE        0x1E631000
>  #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>  #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>  #define ASPEED_SOC_SCU_BASE         0x1E6E2000
> @@ -38,16 +39,23 @@ static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>  #define AST2500_SDRAM_BASE       0x80000000
>  
>  static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> +static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
>  
> -static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> +static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE,
> +                                                       ASPEED_SOC_SPI2_BASE};
> +static const char *aspeed_soc_ast2500_typenames[] = {
> +    "aspeed.smc.ast2500-spi1", "aspeed.smc.ast2500-spi2" };
>  
>  static const AspeedSoCInfo aspeed_socs[] = {
>      { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
> -      1, aspeed_soc_ast2400_spi_bases },
> +      1, aspeed_soc_ast2400_spi_bases,
> +      "aspeed.smc.fmc", aspeed_soc_ast2400_typenames },
>      { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE,
> -      1, aspeed_soc_ast2400_spi_bases },
> +      1, aspeed_soc_ast2400_spi_bases,
> +     "aspeed.smc.fmc", aspeed_soc_ast2400_typenames },
>      { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE,
> -      1, aspeed_soc_ast2500_spi_bases },
> +      2, aspeed_soc_ast2500_spi_bases,
> +      "aspeed.smc.ast2500-fmc", aspeed_soc_ast2500_typenames },
>  };
>  
>  /*
> @@ -105,12 +113,13 @@ static void aspeed_soc_init(Object *obj)
>      object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
>                                "hw-strap2", &error_abort);
>  
> -    object_initialize(&s->fmc, sizeof(s->fmc), "aspeed.smc.fmc");
> +    object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
>      object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
>      qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
>  
>      for (i = 0; i < sc->info->spis_num; i++) {
> -        object_initialize(&s->spi[i], sizeof(s->spi[i]), "aspeed.smc.spi");
> +        object_initialize(&s->spi[i], sizeof(s->spi[i]),
> +                          sc->info->spi_typename[i]);
>          object_property_add_child(obj, "spi", OBJECT(&s->spi[i]), NULL);
>          qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
>      }
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 84c18299de11..21943f4e5dfa 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -130,6 +130,7 @@
>  #define ASPEED_SOC_SMC_FLASH_BASE   0x10000000
>  #define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
>  #define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
> +#define ASPEED_SOC_SPI2_FLASH_BASE  0x38000000
>  
>  /*
>   * Default segments mapping addresses and size for each slave per
> @@ -142,7 +143,7 @@ static const AspeedSegments aspeed_segments_legacy[] = {
>  };
>  
>  static const AspeedSegments aspeed_segments_fmc[] = {
> -    { 0x20000000, 64 * 1024 * 1024 },
> +    { 0x20000000, 64 * 1024 * 1024 }, /* start address is readonly */
>      { 0x24000000, 32 * 1024 * 1024 },
>      { 0x26000000, 32 * 1024 * 1024 },
>      { 0x28000000, 32 * 1024 * 1024 },
> @@ -153,6 +154,22 @@ static const AspeedSegments aspeed_segments_spi[] = {
>      { 0x30000000, 64 * 1024 * 1024 },
>  };
>  
> +static const AspeedSegments aspeed_segments_ast2500_fmc[] = {
> +    { 0x20000000, 128 * 1024 * 1024 }, /* start address is readonly */
> +    { 0x28000000,  32 * 1024 * 1024 },
> +    { 0x2A000000,  32 * 1024 * 1024 },
> +};
> +
> +static const AspeedSegments aspeed_segments_ast2500_spi1[] = {
> +    { 0x30000000, 32 * 1024 * 1024 }, /* start address is readonly */
> +    { 0x32000000, 96 * 1024 * 1024 }, /* end address is readonly */
> +};
> +
> +static const AspeedSegments aspeed_segments_ast2500_spi2[] = {
> +    { 0x38000000, 32 * 1024 * 1024 }, /* start address is readonly */
> +    { 0x3A000000, 96 * 1024 * 1024 }, /* end address is readonly */
> +};
> +
>  static const AspeedSMCController controllers[] = {
>      { "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
>        CONF_ENABLE_W0, 5, aspeed_segments_legacy,
> @@ -163,6 +180,15 @@ static const AspeedSMCController controllers[] = {
>      { "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS,
>        SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi,
>        ASPEED_SOC_SPI_FLASH_BASE, 0x10000000 },
> +    { "aspeed.smc.ast2500-fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> +      CONF_ENABLE_W0, 3, aspeed_segments_ast2500_fmc,
> +      ASPEED_SOC_FMC_FLASH_BASE, 0x10000000 },
> +    { "aspeed.smc.ast2500-spi1", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> +      CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi1,
> +      ASPEED_SOC_SPI_FLASH_BASE, 0x8000000 },
> +    { "aspeed.smc.ast2500-spi2", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS,
> +      CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi2,
> +      ASPEED_SOC_SPI2_FLASH_BASE, 0x8000000 },
>  };
>  
>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index f26a9f043983..5406b498d7ef 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -48,6 +48,8 @@ typedef struct AspeedSoCInfo {
>      hwaddr sdram_base;
>      int spis_num;
>      const hwaddr *spi_bases;
> +    const char *fmc_typename;
> +    const char **spi_typename;
>  } AspeedSoCInfo;
>  
>  typedef struct AspeedSoCClass {

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

Re: [Qemu-devel] [PATCH 5/6] aspeed: create mapping regions for the maximum number of slaves

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> The SMC controller on the Aspeed SoC has a set of registers to
> configure the mapping of each flash module in the SoC address
> space. These mapping windows are configurable even though no SPI slave
> is attached to the controller.
> 
> Also rewrite a bit the comments in the code on this topic.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/ssi/aspeed_smc.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 21943f4e5dfa..ecf39ccfde0e 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -417,12 +417,15 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>  
>      aspeed_smc_reset(dev);
>  
> +    /* The memory region for the controller registers */
>      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);
>  
>      /*
> -     * Memory region where flash modules are remapped
> +     * The container memory region representing the address space
> +     * window in which the flash modules are mapped. The size and
> +     * address depends on the SoC model and controller type.
>       */
>      snprintf(name, sizeof(name), "%s.flash", s->ctrl->name);
>  
> @@ -431,9 +434,16 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>                            s->ctrl->flash_window_size);
>      sysbus_init_mmio(sbd, &s->mmio_flash);
>  
> -    s->flashes = g_new0(AspeedSMCFlash, s->num_cs);
> +    s->flashes = g_new0(AspeedSMCFlash, s->ctrl->max_slaves);
>  
> -    for (i = 0; i < s->num_cs; ++i) {
> +    /*
> +     * Let's create a sub memory region for each possible slave. All
> +     * have a configurable memory segment in the overall flash mapping
> +     * window of the controller but, there is not necessarily a flash
> +     * module behind to handle the memory accesses. This depends on
> +     * the board configuration.
> +     */
> +    for (i = 0; i < s->ctrl->max_slaves; ++i) {
>          AspeedSMCFlash *fl = &s->flashes[i];
>  
>          snprintf(name, sizeof(name), "%s.%d", s->ctrl->name, i);

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

Re: [Qemu-devel] [PATCH 6/6] aspeed: add support for the SMC segment registers

[Andrew Jeffery]

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

On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
> The SMC controller on the Aspeed SoC has a set of registers to
> configure the mapping of each flash module in the SoC address
> space. Writing to these registers triggers a remap of the memory
> region and the spec requires a certain number of checks before doing
> so.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  hw/ssi/aspeed_smc.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 130 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index ecf39ccfde0e..6e8403ebc246 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -79,10 +79,10 @@
>  
>  /* CEx Segment Address Register */
>  #define R_SEG_ADDR0       (0x30 / 4)
> -#define   SEG_SIZE_SHIFT       24   /* 8MB units */
> -#define   SEG_SIZE_MASK        0x7f
> +#define   SEG_END_SHIFT        24   /* 8MB units */
> +#define   SEG_END_MASK         0xff

Was this a bug? The top bit is reserved as 0 in the 2500 datasheet, but
valid as your change suggests in the 2400.

>  #define   SEG_START_SHIFT      16   /* address bit [A29-A23] */
> -#define   SEG_START_MASK       0x7f
> +#define   SEG_START_MASK       0xff

As above.

>  #define R_SEG_ADDR1       (0x34 / 4)
>  #define R_SEG_ADDR2       (0x38 / 4)
>  #define R_SEG_ADDR3       (0x3C / 4)
> @@ -135,8 +135,7 @@
>  /*
>   * Default segments mapping addresses and size for each slave per
>   * controller. These can be changed when board is initialized with the
> - * Segment Address Registers but they don't seem do be used on the
> - * field.
> + * Segment Address Registers.
>   */
>  static const AspeedSegments aspeed_segments_legacy[] = {
>      { 0x10000000, 32 * 1024 * 1024 },
> @@ -191,6 +190,118 @@ static const AspeedSMCController controllers[] = {
>        ASPEED_SOC_SPI2_FLASH_BASE, 0x8000000 },
>  };
>  
> +/*
> + * The Segment Register uses a 8MB unit to encode the start address
> + * and the end address of the mapping window of a flash SPI slave :
> + *
> + *        | byte 1 | byte 2 | byte 3 | byte 4 |
> + *        +--------+--------+--------+--------+
> + *        |  end   |  start |   0    |   0    |
> + *
> + */
> +static inline uint32_t aspeed_smc_segment_to_reg(const AspeedSegments *seg)
> +{
> +    uint32_t reg = 0;
> +    reg |= ((seg->addr >> 23) & SEG_START_MASK) << SEG_START_SHIFT;
> +    reg |= (((seg->addr + seg->size) >> 23) & SEG_END_MASK) << SEG_END_SHIFT;
> +    return reg;
> +}
> +
> +static inline void aspeed_smc_reg_to_segment(uint32_t reg, AspeedSegments *seg)
> +{
> +    seg->addr = ((reg >> SEG_START_SHIFT) & SEG_START_MASK) << 23;
> +    seg->size = (((reg >> SEG_END_SHIFT) & SEG_END_MASK) << 23) - seg->addr;
> +}
> +
> +static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
> +                                     const AspeedSegments *new,
> +                                     int cs)
> +{
> +    AspeedSegments seg;
> +    int i;
> +
> +    for (i = 0; i < s->ctrl->max_slaves; i++) {
> +        if (i == cs) {
> +            continue;
> +        }
> +
> +        aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + i], &seg);
> +
> +        if (new->addr + new->size > seg.addr &&
> +            new->addr < seg.addr + seg.size) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment CS%d [ 0x%"
> +                          HWADDR_PRIx" - 0x%"HWADDR_PRIx" ] overlaps with "
> +                          "CS%d [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
> +                          s->ctrl->name, cs, new->addr, new->addr + new->size,
> +                          i, seg.addr, seg.addr + seg.size);
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
> +                                         uint64_t new)
> +{
> +    AspeedSMCFlash *fl = &s->flashes[cs];
> +    AspeedSegments seg;
> +
> +    aspeed_smc_reg_to_segment(new, &seg);
> +
> +    /* The start address of CS0 is read-only */
> +    if (cs == 0 && seg.addr != s->ctrl->flash_window_base) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Tried to change CS0 start address to 0x%"
> +                      HWADDR_PRIx "\n", s->ctrl->name, seg.addr);
> +        return;
> +    }
> +
> +    /*
> +     * The end address of the AST2500 spi controllers is also
> +     * read-only.
> +     */
> +    if ((s->ctrl->segments == aspeed_segments_ast2500_spi1 ||
> +         s->ctrl->segments == aspeed_segments_ast2500_spi2) &&
> +        cs == s->ctrl->max_slaves &&
> +        seg.addr + seg.size != s->ctrl->segments[cs].addr +
> +        s->ctrl->segments[cs].size) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Tried to change CS%d end address to 0x%"
> +                      HWADDR_PRIx "\n", s->ctrl->name, cs, seg.addr);
> +        return;
> +    }
> +
> +    /* Keep the segment in the overall flash window */
> +    if (seg.addr + seg.size <= s->ctrl->flash_window_base ||
> +        seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : "
> +                      "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
> +                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
> +        return;
> +    }
> +
> +    /* Check start address vs. alignment */
> +    if (seg.addr % seg.size) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is not "
> +                      "aligned : [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
> +                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
> +    }
> +
> +    /* And segments should not overlap */
> +    if (aspeed_smc_flash_overlap(s, &seg, cs)) {
> +        return;
> +    }
> +
> +    /* All should be fine now to move the region */
> +    memory_region_transaction_begin();
> +    memory_region_set_size(&fl->mmio, seg.size);
> +    memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
> +    memory_region_set_enabled(&fl->mmio, true);
> +    memory_region_transaction_commit();
> +
> +    s->regs[R_SEG_ADDR0 + cs] = new;
> +}
> +
>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
>                                                unsigned size)
>  {
> @@ -314,6 +425,12 @@ static void aspeed_smc_reset(DeviceState *d)
>          s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
>      }
>  
> +    /* setup default segment register values for all */
> +    for (i = 0; i < s->ctrl->max_slaves; ++i) {
> +        s->regs[R_SEG_ADDR0 + i] =
> +            aspeed_smc_segment_to_reg(&s->ctrl->segments[i]);
> +    }
> +
>      aspeed_smc_update_cs(s);
>  }
>  
> @@ -334,6 +451,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>          addr == s->r_timings ||
>          addr == s->r_ce_ctrl ||
>          addr == R_INTR_CTRL ||
> +        (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
>          (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
>          return s->regs[addr];
>      } else {
> @@ -365,6 +483,13 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>      } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
>          s->regs[addr] = value;
>          aspeed_smc_update_cs(s);
> +    } else if (addr >= R_SEG_ADDR0 &&
> +               addr < R_SEG_ADDR0 + s->ctrl->max_slaves) {
> +        int cs = addr - R_SEG_ADDR0;
> +
> +        if (value != s->regs[R_SEG_ADDR0 + cs]) {
> +            aspeed_smc_flash_set_segment(s, cs, value);
> +        }
>      } else {
>          qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>                        __func__, addr);

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

Re: [Qemu-devel] [PATCH 6/6] aspeed: add support for the SMC segment registers

[Cédric Le Goater]

On 10/05/2016 01:53 AM, Andrew Jeffery wrote:
> On Tue, 2016-09-27 at 13:57 +0200, Cédric Le Goater wrote:
>> The SMC controller on the Aspeed SoC has a set of registers to
>> configure the mapping of each flash module in the SoC address
>> space. Writing to these registers triggers a remap of the memory
>> region and the spec requires a certain number of checks before doing
>> so.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> 
>> ---
>>  hw/ssi/aspeed_smc.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 130 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index ecf39ccfde0e..6e8403ebc246 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -79,10 +79,10 @@
>>  
>>  /* CEx Segment Address Register */
>>  #define R_SEG_ADDR0       (0x30 / 4)
>> -#define   SEG_SIZE_SHIFT       24   /* 8MB units */
>> -#define   SEG_SIZE_MASK        0x7f
>> +#define   SEG_END_SHIFT        24   /* 8MB units */
>> +#define   SEG_END_MASK         0xff
> 
> Was this a bug? The top bit is reserved as 0 in the 2500 datasheet, but
> valid as your change suggests in the 2400.

This true that the allowed bits in the mask are smaller but they are 
also different on all SPI controllers. There is at least three 
different ranges, on the AST2400 FMC, the AST2500 FMC and on the 
AST2500 SPI. 

So I tried to find a common ground, that is a larger definition,
to avoid complexity in this first step and support all SoC.

A Next step could be to modify : 

	uint32_t aspeed_smc_segment_to_reg(const AspeedSegments *seg)
	void aspeed_smc_reg_to_segment(uint32_t reg, AspeedSegments *seg)

to add the extra complexity and handle the range differences. Or add
a new routine for this purpose with clear values for a human. The
register definitions in the spec are really foggy.

Thanks

C. 

>>  #define   SEG_START_SHIFT      16   /* address bit [A29-A23] */
>>
>>  #define   SEG_START_SHIFT      16   /* address bit [A29-A23] */
>> -#define   SEG_START_MASK       0x7f
>> +#define   SEG_START_MASK       0xff
> 
> As above.
> 
>>  #define R_SEG_ADDR1       (0x34 / 4)
>>  #define R_SEG_ADDR2       (0x38 / 4)
>>  #define R_SEG_ADDR3       (0x3C / 4)
>> @@ -135,8 +135,7 @@
>>  /*
>>   * Default segments mapping addresses and size for each slave per
>>   * controller. These can be changed when board is initialized with the
>> - * Segment Address Registers but they don't seem do be used on the
>> - * field.
>> + * Segment Address Registers.
>>   */
>>  static const AspeedSegments aspeed_segments_legacy[] = {
>>      { 0x10000000, 32 * 1024 * 1024 },
>> @@ -191,6 +190,118 @@ static const AspeedSMCController controllers[] = {
>>        ASPEED_SOC_SPI2_FLASH_BASE, 0x8000000 },
>>  };
>>  
>> +/*
>> + * The Segment Register uses a 8MB unit to encode the start address
>> + * and the end address of the mapping window of a flash SPI slave :
>> + *
>> + *        | byte 1 | byte 2 | byte 3 | byte 4 |
>> + *        +--------+--------+--------+--------+
>> + *        |  end   |  start |   0    |   0    |
>> + *
>> + */
>> +static inline uint32_t aspeed_smc_segment_to_reg(const AspeedSegments *seg)
>> +{
>> +    uint32_t reg = 0;
>> +    reg |= ((seg->addr >> 23) & SEG_START_MASK) << SEG_START_SHIFT;
>> +    reg |= (((seg->addr + seg->size) >> 23) & SEG_END_MASK) << SEG_END_SHIFT;
>> +    return reg;
>> +}
>> +
>> +static inline void aspeed_smc_reg_to_segment(uint32_t reg, AspeedSegments *seg)
>> +{
>> +    seg->addr = ((reg >> SEG_START_SHIFT) & SEG_START_MASK) << 23;
>> +    seg->size = (((reg >> SEG_END_SHIFT) & SEG_END_MASK) << 23) - seg->addr;
>> +}
>> +
>> +static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
>> +                                     const AspeedSegments *new,
>> +                                     int cs)
>> +{
>> +    AspeedSegments seg;
>> +    int i;
>> +
>> +    for (i = 0; i < s->ctrl->max_slaves; i++) {
>> +        if (i == cs) {
>> +            continue;
>> +        }
>> +
>> +        aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + i], &seg);
>> +
>> +        if (new->addr + new->size > seg.addr &&
>> +            new->addr < seg.addr + seg.size) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment CS%d [ 0x%"
>> +                          HWADDR_PRIx" - 0x%"HWADDR_PRIx" ] overlaps with "
>> +                          "CS%d [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
>> +                          s->ctrl->name, cs, new->addr, new->addr + new->size,
>> +                          i, seg.addr, seg.addr + seg.size);
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>> +static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
>> +                                         uint64_t new)
>> +{
>> +    AspeedSMCFlash *fl = &s->flashes[cs];
>> +    AspeedSegments seg;
>> +
>> +    aspeed_smc_reg_to_segment(new, &seg);
>> +
>> +    /* The start address of CS0 is read-only */
>> +    if (cs == 0 && seg.addr != s->ctrl->flash_window_base) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: Tried to change CS0 start address to 0x%"
>> +                      HWADDR_PRIx "\n", s->ctrl->name, seg.addr);
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * The end address of the AST2500 spi controllers is also
>> +     * read-only.
>> +     */
>> +    if ((s->ctrl->segments == aspeed_segments_ast2500_spi1 ||
>> +         s->ctrl->segments == aspeed_segments_ast2500_spi2) &&
>> +        cs == s->ctrl->max_slaves &&
>> +        seg.addr + seg.size != s->ctrl->segments[cs].addr +
>> +        s->ctrl->segments[cs].size) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: Tried to change CS%d end address to 0x%"
>> +                      HWADDR_PRIx "\n", s->ctrl->name, cs, seg.addr);
>> +        return;
>> +    }
>> +
>> +    /* Keep the segment in the overall flash window */
>> +    if (seg.addr + seg.size <= s->ctrl->flash_window_base ||
>> +        seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : "
>> +                      "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
>> +                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
>> +        return;
>> +    }
>> +
>> +    /* Check start address vs. alignment */
>> +    if (seg.addr % seg.size) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is not "
>> +                      "aligned : [ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n",
>> +                      s->ctrl->name, cs, seg.addr, seg.addr + seg.size);
>> +    }
>> +
>> +    /* And segments should not overlap */
>> +    if (aspeed_smc_flash_overlap(s, &seg, cs)) {
>> +        return;
>> +    }
>> +
>> +    /* All should be fine now to move the region */
>> +    memory_region_transaction_begin();
>> +    memory_region_set_size(&fl->mmio, seg.size);
>> +    memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base);
>> +    memory_region_set_enabled(&fl->mmio, true);
>> +    memory_region_transaction_commit();
>> +
>> +    s->regs[R_SEG_ADDR0 + cs] = new;
>> +}
>> +
>>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
>>                                                unsigned size)
>>  {
>> @@ -314,6 +425,12 @@ static void aspeed_smc_reset(DeviceState *d)
>>          s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
>>      }
>>  
>> +    /* setup default segment register values for all */
>> +    for (i = 0; i < s->ctrl->max_slaves; ++i) {
>> +        s->regs[R_SEG_ADDR0 + i] =
>> +            aspeed_smc_segment_to_reg(&s->ctrl->segments[i]);
>> +    }
>> +
>>      aspeed_smc_update_cs(s);
>>  }
>>  
>> @@ -334,6 +451,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>>          addr == s->r_timings ||
>>          addr == s->r_ce_ctrl ||
>>          addr == R_INTR_CTRL ||
>> +        (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
>>          (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs)) {
>>          return s->regs[addr];
>>      } else {
>> @@ -365,6 +483,13 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>>      } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
>>          s->regs[addr] = value;
>>          aspeed_smc_update_cs(s);
>> +    } else if (addr >= R_SEG_ADDR0 &&
>> +               addr < R_SEG_ADDR0 + s->ctrl->max_slaves) {
>> +        int cs = addr - R_SEG_ADDR0;
>> +
>> +        if (value != s->regs[R_SEG_ADDR0 + cs]) {
>> +            aspeed_smc_flash_set_segment(s, cs, value);
>> +        }
>>      } else {
>>          qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
>>                        __func__, addr);

Re: [Qemu-devel] [PATCH 0/6] aspeed: add support for the ast2500 SMC controllers

[Peter Maydell]

On 27 September 2016 at 12:57, Cédric Le Goater <clg@kaod.org> wrote:
> Hello,
>
> The Aspeed AST2500 has one 'SPI' controller for the BMC firmware and
> two 'SPI' for the host firmware. All controllers have now the same set
> of registers compatible with the AST2400 'FMC' controller and the
> legacy 'SMC' controller is fully gone.
>
> This serie adds support for the second SPI controller and for the
> segment registers which are used to configure the mapping of each
> flash module in the SoC address space.
>



Applied to target-arm.next, thanks.

-- PMM