[PATCH 0/6] aspeed/smc: 'num_cs' cleanup

[PATCH 0/6] aspeed/smc: 'num_cs' cleanup

[Cédric Le Goater]

Hi,

This series removes a redudant 'num_cs' field in the Aspeed SMC model
and does a few cleanups on the way.

Thanks,

C.

Cédric Le Goater (6):
  aspeed/smc: Use max number of CE instead of 'num_cs'
  aspeed: Rework aspeed_board_init_flashes() interface
  aspeed/smc: Remove 'num_cs' field
  aspeed/smc: Rename 'max_peripherals' to 'max_cs'
  aspeed/smc: Let the SSI core layer define the bus name
  aspeed/smc: Fix error log

 include/hw/ssi/aspeed_smc.h |  3 +--
 hw/arm/aspeed.c             | 13 ++++-----
 hw/arm/aspeed_ast2600.c     |  2 --
 hw/arm/aspeed_soc.c         |  2 --
 hw/ssi/aspeed_smc.c         | 53 ++++++++++++++++---------------------
 5 files changed, 29 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH 1/6] aspeed/smc: Use max number of CE instead of 'num_cs'

[Cédric Le Goater]

The Aspeed SMC model uses the 'num_cs' field to allocate resources
fitting the number of devices of the machine. This is a small
optimization without real need in the controller. Simplify modelling
and use the max_peripherals field instead.

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

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index d899be17fd71..a5d8bb717fc7 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -693,7 +693,7 @@ static void aspeed_smc_reset(DeviceState *d)
     }
 
     /* Unselect all peripherals */
-    for (i = 0; i < s->num_cs; ++i) {
+    for (i = 0; i < asc->max_peripherals; ++i) {
         s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
         qemu_set_irq(s->cs_lines[i], true);
     }
@@ -1042,7 +1042,7 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
          addr < s->r_timings + asc->nregs_timings) ||
         addr == s->r_ce_ctrl) {
         s->regs[addr] = value;
-    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
+    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) {
         int cs = addr - s->r_ctrl0;
         aspeed_smc_flash_update_ctrl(&s->flashes[cs], value);
     } else if (addr >= R_SEG_ADDR0 &&
@@ -1139,9 +1139,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
     s->spi = ssi_create_bus(dev, "spi");
 
     /* Setup cs_lines for peripherals */
-    s->cs_lines = g_new0(qemu_irq, s->num_cs);
+    s->cs_lines = g_new0(qemu_irq, asc->max_peripherals);
 
-    for (i = 0; i < s->num_cs; ++i) {
+    for (i = 0; i < asc->max_peripherals; ++i) {
         sysbus_init_irq(sbd, &s->cs_lines[i]);
     }
 
-- 
2.34.1

[PATCH 2/6] aspeed: Rework aspeed_board_init_flashes() interface

[Cédric Le Goater]

Currently, the allocation of the flash devices uses the number of
slave selects configured in the SoC realize routine. It is simpler to
use directly the number of FMC devices defined in the machine class
and 1 for spi devices (which is what the SoC does in the back of the
machine).

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 02918a594d24..be24508a9854 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -276,9 +276,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
-static void aspeed_board_init_flashes(AspeedSMCState *s,
-                                      const char *flashtype,
-                                      int unit0)
+static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+                                      int count, int unit0)
 {
     int i;
 
@@ -286,7 +285,7 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
         return;
     }
 
-    for (i = 0; i < s->num_cs; ++i) {
+    for (i = 0; i < count; ++i) {
         DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
         qemu_irq cs_line;
         DeviceState *dev;
@@ -382,10 +381,10 @@ static void aspeed_machine_init(MachineState *machine)
 
     aspeed_board_init_flashes(&bmc->soc.fmc,
                               bmc->fmc_model ? bmc->fmc_model : amc->fmc_model,
-                              0);
+                              amc->num_cs, 0);
     aspeed_board_init_flashes(&bmc->soc.spi[0],
                               bmc->spi_model ? bmc->spi_model : amc->spi_model,
-                              bmc->soc.fmc.num_cs);
+                              1, amc->num_cs);
 
     /* Install first FMC flash content as a boot rom. */
     if (drive0) {
-- 
2.34.1

[PATCH 3/6] aspeed/smc: Remove 'num_cs' field

[Cédric Le Goater]

It is not used anymore.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ssi/aspeed_smc.h | 1 -
 hw/arm/aspeed.c             | 2 --
 hw/arm/aspeed_ast2600.c     | 2 --
 hw/arm/aspeed_soc.c         | 2 --
 hw/ssi/aspeed_smc.c         | 7 -------
 5 files changed, 14 deletions(-)

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index cad73ddc13f2..4a9354e13c7f 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -57,7 +57,6 @@ struct AspeedSMCState {
 
     qemu_irq irq;
 
-    uint32_t num_cs;
     qemu_irq *cs_lines;
     bool inject_failure;
 
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index be24508a9854..8f14738676f6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -352,8 +352,6 @@ static void aspeed_machine_init(MachineState *machine)
                             &error_abort);
     object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2,
                             &error_abort);
-    object_property_set_int(OBJECT(&bmc->soc), "num-cs", amc->num_cs,
-                            &error_abort);
     object_property_set_link(OBJECT(&bmc->soc), "dram",
                              OBJECT(machine->ram), &error_abort);
     if (machine->kernel_filename) {
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 21cd3342c578..c1e15e37739c 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -163,7 +163,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname);
     object_initialize_child(obj, "fmc", &s->fmc, typename);
-    object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs");
 
     for (i = 0; i < sc->spis_num; i++) {
         snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
@@ -383,7 +382,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->spis_num; i++) {
         object_property_set_link(OBJECT(&s->spi[i]), "dram",
                                  OBJECT(s->dram_mr), &error_abort);
-        object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 7d53cf2f5133..58714cb2a01d 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -170,7 +170,6 @@ static void aspeed_soc_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname);
     object_initialize_child(obj, "fmc", &s->fmc, typename);
-    object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs");
 
     for (i = 0; i < sc->spis_num; i++) {
         snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
@@ -327,7 +326,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
-        object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index a5d8bb717fc7..6859f061c8be 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1127,12 +1127,6 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
     s->r_timings = asc->r_timings;
     s->conf_enable_w0 = asc->conf_enable_w0;
 
-    /* Enforce some real HW limits */
-    if (s->num_cs > asc->max_peripherals) {
-        aspeed_smc_error("num_cs cannot exceed: %d", asc->max_peripherals);
-        s->num_cs = asc->max_peripherals;
-    }
-
     /* DMA irq. Keep it first for the initialization in the SoC */
     sysbus_init_irq(sbd, &s->irq);
 
@@ -1211,7 +1205,6 @@ static const VMStateDescription vmstate_aspeed_smc = {
 };
 
 static Property aspeed_smc_properties[] = {
-    DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
     DEFINE_PROP_BOOL("inject-failure", AspeedSMCState, inject_failure, false),
     DEFINE_PROP_LINK("dram", AspeedSMCState, dram_mr,
                      TYPE_MEMORY_REGION, MemoryRegion *),
-- 
2.34.1

[PATCH 4/6] aspeed/smc: Rename 'max_peripherals' to 'max_cs'

[Cédric Le Goater]

The naming makes more sense in a SPI controller model.

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

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 4a9354e13c7f..6501a04b8f3e 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -95,7 +95,7 @@ struct AspeedSMCClass {
     uint8_t r_timings;
     uint8_t nregs_timings;
     uint8_t conf_enable_w0;
-    uint8_t max_peripherals;
+    uint8_t max_cs;
     const uint32_t *resets;
     const AspeedSegments *segments;
     uint32_t segment_addr_mask;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 6859f061c8be..f194182beacf 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -224,7 +224,7 @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
     AspeedSegments seg;
     int i;
 
-    for (i = 0; i < asc->max_peripherals; i++) {
+    for (i = 0; i < asc->max_cs; i++) {
         if (i == cs) {
             continue;
         }
@@ -290,7 +290,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
      */
     if ((asc->segments == aspeed_2500_spi1_segments ||
          asc->segments == aspeed_2500_spi2_segments) &&
-        cs == asc->max_peripherals &&
+        cs == asc->max_cs &&
         seg.addr + seg.size != asc->segments[cs].addr +
         asc->segments[cs].size) {
         aspeed_smc_error("Tried to change CS%d end address to 0x%"
@@ -693,13 +693,13 @@ static void aspeed_smc_reset(DeviceState *d)
     }
 
     /* Unselect all peripherals */
-    for (i = 0; i < asc->max_peripherals; ++i) {
+    for (i = 0; i < asc->max_cs; ++i) {
         s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
         qemu_set_irq(s->cs_lines[i], true);
     }
 
     /* setup the default segment register values and regions for all */
-    for (i = 0; i < asc->max_peripherals; ++i) {
+    for (i = 0; i < asc->max_cs; ++i) {
         aspeed_smc_flash_set_segment_region(s, i,
                     asc->segment_to_reg(s, &asc->segments[i]));
     }
@@ -729,8 +729,8 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
         (aspeed_smc_has_dma(asc) && addr == R_DMA_LEN) ||
         (aspeed_smc_has_dma(asc) && addr == R_DMA_CHECKSUM) ||
         (addr >= R_SEG_ADDR0 &&
-         addr < R_SEG_ADDR0 + asc->max_peripherals) ||
-        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals)) {
+         addr < R_SEG_ADDR0 + asc->max_cs) ||
+        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_cs)) {
 
         trace_aspeed_smc_read(addr << 2, size, s->regs[addr]);
 
@@ -1042,11 +1042,11 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
          addr < s->r_timings + asc->nregs_timings) ||
         addr == s->r_ce_ctrl) {
         s->regs[addr] = value;
-    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) {
+    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_cs) {
         int cs = addr - s->r_ctrl0;
         aspeed_smc_flash_update_ctrl(&s->flashes[cs], value);
     } else if (addr >= R_SEG_ADDR0 &&
-               addr < R_SEG_ADDR0 + asc->max_peripherals) {
+               addr < R_SEG_ADDR0 + asc->max_cs) {
         int cs = addr - R_SEG_ADDR0;
 
         if (value != s->regs[R_SEG_ADDR0 + cs]) {
@@ -1090,7 +1090,7 @@ static void aspeed_smc_instance_init(Object *obj)
     AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
     int i;
 
-    for (i = 0; i < asc->max_peripherals; i++) {
+    for (i = 0; i < asc->max_cs; i++) {
         object_initialize_child(obj, "flash[*]", &s->flashes[i],
                                 TYPE_ASPEED_SMC_FLASH);
     }
@@ -1133,9 +1133,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
     s->spi = ssi_create_bus(dev, "spi");
 
     /* Setup cs_lines for peripherals */
-    s->cs_lines = g_new0(qemu_irq, asc->max_peripherals);
+    s->cs_lines = g_new0(qemu_irq, asc->max_cs);
 
-    for (i = 0; i < asc->max_peripherals; ++i) {
+    for (i = 0; i < asc->max_cs; ++i) {
         sysbus_init_irq(sbd, &s->cs_lines[i]);
     }
 
@@ -1168,7 +1168,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
      * module behind to handle the memory accesses. This depends on
      * the board configuration.
      */
-    for (i = 0; i < asc->max_peripherals; ++i) {
+    for (i = 0; i < asc->max_cs; ++i) {
         AspeedSMCFlash *fl = &s->flashes[i];
 
         if (!object_property_set_link(OBJECT(fl), "controller", OBJECT(s),
@@ -1314,7 +1314,7 @@ static void aspeed_2400_smc_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 1;
+    asc->max_cs            = 1;
     asc->segments          = aspeed_2400_smc_segments;
     asc->flash_window_base = 0x10000000;
     asc->flash_window_size = 0x6000000;
@@ -1359,7 +1359,7 @@ static void aspeed_2400_fmc_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 5;
+    asc->max_cs            = 5;
     asc->segments          = aspeed_2400_fmc_segments;
     asc->segment_addr_mask = 0xffff0000;
     asc->resets            = aspeed_2400_fmc_resets;
@@ -1401,7 +1401,7 @@ static void aspeed_2400_spi1_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_SPI_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = SPI_CONF_ENABLE_W0;
-    asc->max_peripherals   = 1;
+    asc->max_cs            = 1;
     asc->segments          = aspeed_2400_spi1_segments;
     asc->flash_window_base = 0x30000000;
     asc->flash_window_size = 0x10000000;
@@ -1442,7 +1442,7 @@ static void aspeed_2500_fmc_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 3;
+    asc->max_cs            = 3;
     asc->segments          = aspeed_2500_fmc_segments;
     asc->segment_addr_mask = 0xffff0000;
     asc->resets            = aspeed_2500_fmc_resets;
@@ -1480,7 +1480,7 @@ static void aspeed_2500_spi1_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 2;
+    asc->max_cs            = 2;
     asc->segments          = aspeed_2500_spi1_segments;
     asc->segment_addr_mask = 0xffff0000;
     asc->flash_window_base = 0x30000000;
@@ -1515,7 +1515,7 @@ static void aspeed_2500_spi2_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 2;
+    asc->max_cs            = 2;
     asc->segments          = aspeed_2500_spi2_segments;
     asc->segment_addr_mask = 0xffff0000;
     asc->flash_window_base = 0x38000000;
@@ -1597,7 +1597,7 @@ static void aspeed_2600_fmc_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 1;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 3;
+    asc->max_cs            = 3;
     asc->segments          = aspeed_2600_fmc_segments;
     asc->segment_addr_mask = 0x0ff00ff0;
     asc->resets            = aspeed_2600_fmc_resets;
@@ -1636,7 +1636,7 @@ static void aspeed_2600_spi1_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 2;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 2;
+    asc->max_cs            = 2;
     asc->segments          = aspeed_2600_spi1_segments;
     asc->segment_addr_mask = 0x0ff00ff0;
     asc->flash_window_base = 0x30000000;
@@ -1675,7 +1675,7 @@ static void aspeed_2600_spi2_class_init(ObjectClass *klass, void *data)
     asc->r_timings         = R_TIMINGS;
     asc->nregs_timings     = 3;
     asc->conf_enable_w0    = CONF_ENABLE_W0;
-    asc->max_peripherals   = 3;
+    asc->max_cs            = 3;
     asc->segments          = aspeed_2600_spi2_segments;
     asc->segment_addr_mask = 0x0ff00ff0;
     asc->flash_window_base = 0x50000000;
-- 
2.34.1

[PATCH 5/6] aspeed/smc: Let the SSI core layer define the bus name

[Cédric Le Goater]

If no id is provided, qdev automatically assigns a unique ename with
the following pattern "<type>.<index>".

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index f194182beacf..113f31899a6b 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -1130,7 +1130,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
     /* DMA irq. Keep it first for the initialization in the SoC */
     sysbus_init_irq(sbd, &s->irq);
 
-    s->spi = ssi_create_bus(dev, "spi");
+    s->spi = ssi_create_bus(dev, NULL);
 
     /* Setup cs_lines for peripherals */
     s->cs_lines = g_new0(qemu_irq, asc->max_cs);
-- 
2.34.1

[PATCH 6/6] aspeed/smc: Fix error log

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 113f31899a6b..f410248938c9 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -327,7 +327,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
 static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
                                               unsigned size)
 {
-    aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u" PRIx64, addr, size);
+    aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u", addr, size);
     return 0;
 }
 
-- 
2.34.1

Re: [PATCH 1/6] aspeed/smc: Use max number of CE instead of 'num_cs'

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> The Aspeed SMC model uses the 'num_cs' field to allocate resources
> fitting the number of devices of the machine. This is a small
> optimization without real need in the controller. Simplify modelling
> and use the max_peripherals field instead.

"(which by the way is not very descriptive, but we are going to
rename it to 'cs_num_max' in a pair of commits)."

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

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 4/6] aspeed/smc: Rename 'max_peripherals' to 'max_cs'

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> The naming makes more sense in a SPI controller model.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   include/hw/ssi/aspeed_smc.h |  2 +-
>   hw/ssi/aspeed_smc.c         | 42 ++++++++++++++++++-------------------
>   2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 4a9354e13c7f..6501a04b8f3e 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -95,7 +95,7 @@ struct AspeedSMCClass {
>       uint8_t r_timings;
>       uint8_t nregs_timings;
>       uint8_t conf_enable_w0;
> -    uint8_t max_peripherals;
> +    uint8_t max_cs;

Can we use 'cs_num_max' or 'cs_count_max' instead?

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 2/6] aspeed: Rework aspeed_board_init_flashes() interface

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> Currently, the allocation of the flash devices uses the number of
> slave selects configured in the SoC realize routine. It is simpler to
> use directly the number of FMC devices defined in the machine class
> and 1 for spi devices (which is what the SoC does in the back of the
> machine).
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/arm/aspeed.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 02918a594d24..be24508a9854 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -276,9 +276,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
>       rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
>   }
>   
> -static void aspeed_board_init_flashes(AspeedSMCState *s,
> -                                      const char *flashtype,
> -                                      int unit0)
> +static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
> +                                      int count, int unit0)

'unsigned count', otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 3/6] aspeed/smc: Remove 'num_cs' field

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> It is not used anymore.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   include/hw/ssi/aspeed_smc.h | 1 -
>   hw/arm/aspeed.c             | 2 --
>   hw/arm/aspeed_ast2600.c     | 2 --
>   hw/arm/aspeed_soc.c         | 2 --
>   hw/ssi/aspeed_smc.c         | 7 -------
>   5 files changed, 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 6/6] aspeed/smc: Fix error log

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ssi/aspeed_smc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 5/6] aspeed/smc: Let the SSI core layer define the bus name

[Philippe Mathieu-Daudé]

On 7/3/22 08:18, Cédric Le Goater wrote:
> If no id is provided, qdev automatically assigns a unique ename with

"an unique name"?

> the following pattern "<type>.<index>".

Maybe complete with smth like:

"which avoid bus name collision when using multiple buses."

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ssi/aspeed_smc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Re: [PATCH 5/6] aspeed/smc: Let the SSI core layer define the bus name

[Cédric Le Goater]

On 3/7/22 11:25, Philippe Mathieu-Daudé wrote:
> On 7/3/22 08:18, Cédric Le Goater wrote:
>> If no id is provided, qdev automatically assigns a unique ename with
> 
> "an unique name"?
> 
>> the following pattern "<type>.<index>".
> 
> Maybe complete with smth like:
> 
> "which avoid bus name collision when using multiple buses."

sure. will do.

The goal behind this patch is to start experimenting with user created
"m25p80" devices such :
  
   -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0

BMC machines are starting to duplicate and I think, at least for Aspeed,
that we should offer a bare machine per SoC and let the user define
the I2C topology on the command line. The other aspect to cover is
the definition of the flash devices (possibly 3 controllers * 2-3 CS).

Adding a cs= or and addr= is the next step.

Thanks,

C.

Re: [PATCH 6/6] aspeed/smc: Fix error log

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:32 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ssi/aspeed_smc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 113f31899a6b..f410248938c9 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -327,7 +327,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
>  static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr,
>                                                unsigned size)
>  {
> -    aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u" PRIx64, addr, size);
> +    aspeed_smc_error("To 0x%" HWADDR_PRIx " of size %u", addr, size);
>      return 0;
>  }
>
> --
> 2.34.1
>
>

Re: [PATCH 1/6] aspeed/smc: Use max number of CE instead of 'num_cs'

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:43 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> The Aspeed SMC model uses the 'num_cs' field to allocate resources
> fitting the number of devices of the machine. This is a small
> optimization without real need in the controller. Simplify modelling
> and use the max_peripherals field instead.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ssi/aspeed_smc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index d899be17fd71..a5d8bb717fc7 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -693,7 +693,7 @@ static void aspeed_smc_reset(DeviceState *d)
>      }
>
>      /* Unselect all peripherals */
> -    for (i = 0; i < s->num_cs; ++i) {
> +    for (i = 0; i < asc->max_peripherals; ++i) {
>          s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
>          qemu_set_irq(s->cs_lines[i], true);
>      }
> @@ -1042,7 +1042,7 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>           addr < s->r_timings + asc->nregs_timings) ||
>          addr == s->r_ce_ctrl) {
>          s->regs[addr] = value;
> -    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) {
> +    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) {
>          int cs = addr - s->r_ctrl0;
>          aspeed_smc_flash_update_ctrl(&s->flashes[cs], value);
>      } else if (addr >= R_SEG_ADDR0 &&
> @@ -1139,9 +1139,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>      s->spi = ssi_create_bus(dev, "spi");
>
>      /* Setup cs_lines for peripherals */
> -    s->cs_lines = g_new0(qemu_irq, s->num_cs);
> +    s->cs_lines = g_new0(qemu_irq, asc->max_peripherals);
>
> -    for (i = 0; i < s->num_cs; ++i) {
> +    for (i = 0; i < asc->max_peripherals; ++i) {
>          sysbus_init_irq(sbd, &s->cs_lines[i]);
>      }
>
> --
> 2.34.1
>
>

Re: [PATCH 3/6] aspeed/smc: Remove 'num_cs' field

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:19 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> It is not used anymore.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/ssi/aspeed_smc.h | 1 -
>  hw/arm/aspeed.c             | 2 --
>  hw/arm/aspeed_ast2600.c     | 2 --
>  hw/arm/aspeed_soc.c         | 2 --
>  hw/ssi/aspeed_smc.c         | 7 -------
>  5 files changed, 14 deletions(-)
>
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index cad73ddc13f2..4a9354e13c7f 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -57,7 +57,6 @@ struct AspeedSMCState {
>
>      qemu_irq irq;
>
> -    uint32_t num_cs;
>      qemu_irq *cs_lines;
>      bool inject_failure;
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index be24508a9854..8f14738676f6 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -352,8 +352,6 @@ static void aspeed_machine_init(MachineState *machine)
>                              &error_abort);
>      object_property_set_int(OBJECT(&bmc->soc), "hw-strap2", amc->hw_strap2,
>                              &error_abort);
> -    object_property_set_int(OBJECT(&bmc->soc), "num-cs", amc->num_cs,
> -                            &error_abort);
>      object_property_set_link(OBJECT(&bmc->soc), "dram",
>                               OBJECT(machine->ram), &error_abort);
>      if (machine->kernel_filename) {
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 21cd3342c578..c1e15e37739c 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -163,7 +163,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
>
>      snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname);
>      object_initialize_child(obj, "fmc", &s->fmc, typename);
> -    object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs");
>
>      for (i = 0; i < sc->spis_num; i++) {
>          snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
> @@ -383,7 +382,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>      for (i = 0; i < sc->spis_num; i++) {
>          object_property_set_link(OBJECT(&s->spi[i]), "dram",
>                                   OBJECT(s->dram_mr), &error_abort);
> -        object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort);
>          if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
>              return;
>          }
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 7d53cf2f5133..58714cb2a01d 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -170,7 +170,6 @@ static void aspeed_soc_init(Object *obj)
>
>      snprintf(typename, sizeof(typename), "aspeed.fmc-%s", socname);
>      object_initialize_child(obj, "fmc", &s->fmc, typename);
> -    object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs");
>
>      for (i = 0; i < sc->spis_num; i++) {
>          snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname);
> @@ -327,7 +326,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>
>      /* SPI */
>      for (i = 0; i < sc->spis_num; i++) {
> -        object_property_set_int(OBJECT(&s->spi[i]), "num-cs", 1, &error_abort);
>          if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
>              return;
>          }
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index a5d8bb717fc7..6859f061c8be 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -1127,12 +1127,6 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>      s->r_timings = asc->r_timings;
>      s->conf_enable_w0 = asc->conf_enable_w0;
>
> -    /* Enforce some real HW limits */
> -    if (s->num_cs > asc->max_peripherals) {
> -        aspeed_smc_error("num_cs cannot exceed: %d", asc->max_peripherals);
> -        s->num_cs = asc->max_peripherals;
> -    }
> -
>      /* DMA irq. Keep it first for the initialization in the SoC */
>      sysbus_init_irq(sbd, &s->irq);
>
> @@ -1211,7 +1205,6 @@ static const VMStateDescription vmstate_aspeed_smc = {
>  };
>
>  static Property aspeed_smc_properties[] = {
> -    DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
>      DEFINE_PROP_BOOL("inject-failure", AspeedSMCState, inject_failure, false),
>      DEFINE_PROP_LINK("dram", AspeedSMCState, dram_mr,
>                       TYPE_MEMORY_REGION, MemoryRegion *),
> --
> 2.34.1
>
>

Re: [PATCH 4/6] aspeed/smc: Rename 'max_peripherals' to 'max_cs'

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:37 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> The naming makes more sense in a SPI controller model.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/ssi/aspeed_smc.h |  2 +-
>  hw/ssi/aspeed_smc.c         | 42 ++++++++++++++++++-------------------
>  2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 4a9354e13c7f..6501a04b8f3e 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -95,7 +95,7 @@ struct AspeedSMCClass {
>      uint8_t r_timings;
>      uint8_t nregs_timings;
>      uint8_t conf_enable_w0;
> -    uint8_t max_peripherals;
> +    uint8_t max_cs;
>      const uint32_t *resets;
>      const AspeedSegments *segments;
>      uint32_t segment_addr_mask;
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 6859f061c8be..f194182beacf 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -224,7 +224,7 @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s,
>      AspeedSegments seg;
>      int i;
>
> -    for (i = 0; i < asc->max_peripherals; i++) {
> +    for (i = 0; i < asc->max_cs; i++) {
>          if (i == cs) {
>              continue;
>          }
> @@ -290,7 +290,7 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs,
>       */
>      if ((asc->segments == aspeed_2500_spi1_segments ||
>           asc->segments == aspeed_2500_spi2_segments) &&
> -        cs == asc->max_peripherals &&
> +        cs == asc->max_cs &&
>          seg.addr + seg.size != asc->segments[cs].addr +
>          asc->segments[cs].size) {
>          aspeed_smc_error("Tried to change CS%d end address to 0x%"
> @@ -693,13 +693,13 @@ static void aspeed_smc_reset(DeviceState *d)
>      }
>
>      /* Unselect all peripherals */
> -    for (i = 0; i < asc->max_peripherals; ++i) {
> +    for (i = 0; i < asc->max_cs; ++i) {
>          s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE;
>          qemu_set_irq(s->cs_lines[i], true);
>      }
>
>      /* setup the default segment register values and regions for all */
> -    for (i = 0; i < asc->max_peripherals; ++i) {
> +    for (i = 0; i < asc->max_cs; ++i) {
>          aspeed_smc_flash_set_segment_region(s, i,
>                      asc->segment_to_reg(s, &asc->segments[i]));
>      }
> @@ -729,8 +729,8 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>          (aspeed_smc_has_dma(asc) && addr == R_DMA_LEN) ||
>          (aspeed_smc_has_dma(asc) && addr == R_DMA_CHECKSUM) ||
>          (addr >= R_SEG_ADDR0 &&
> -         addr < R_SEG_ADDR0 + asc->max_peripherals) ||
> -        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals)) {
> +         addr < R_SEG_ADDR0 + asc->max_cs) ||
> +        (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_cs)) {
>
>          trace_aspeed_smc_read(addr << 2, size, s->regs[addr]);
>
> @@ -1042,11 +1042,11 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>           addr < s->r_timings + asc->nregs_timings) ||
>          addr == s->r_ce_ctrl) {
>          s->regs[addr] = value;
> -    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_peripherals) {
> +    } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + asc->max_cs) {
>          int cs = addr - s->r_ctrl0;
>          aspeed_smc_flash_update_ctrl(&s->flashes[cs], value);
>      } else if (addr >= R_SEG_ADDR0 &&
> -               addr < R_SEG_ADDR0 + asc->max_peripherals) {
> +               addr < R_SEG_ADDR0 + asc->max_cs) {
>          int cs = addr - R_SEG_ADDR0;
>
>          if (value != s->regs[R_SEG_ADDR0 + cs]) {
> @@ -1090,7 +1090,7 @@ static void aspeed_smc_instance_init(Object *obj)
>      AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
>      int i;
>
> -    for (i = 0; i < asc->max_peripherals; i++) {
> +    for (i = 0; i < asc->max_cs; i++) {
>          object_initialize_child(obj, "flash[*]", &s->flashes[i],
>                                  TYPE_ASPEED_SMC_FLASH);
>      }
> @@ -1133,9 +1133,9 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>      s->spi = ssi_create_bus(dev, "spi");
>
>      /* Setup cs_lines for peripherals */
> -    s->cs_lines = g_new0(qemu_irq, asc->max_peripherals);
> +    s->cs_lines = g_new0(qemu_irq, asc->max_cs);
>
> -    for (i = 0; i < asc->max_peripherals; ++i) {
> +    for (i = 0; i < asc->max_cs; ++i) {
>          sysbus_init_irq(sbd, &s->cs_lines[i]);
>      }
>
> @@ -1168,7 +1168,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>       * module behind to handle the memory accesses. This depends on
>       * the board configuration.
>       */
> -    for (i = 0; i < asc->max_peripherals; ++i) {
> +    for (i = 0; i < asc->max_cs; ++i) {
>          AspeedSMCFlash *fl = &s->flashes[i];
>
>          if (!object_property_set_link(OBJECT(fl), "controller", OBJECT(s),
> @@ -1314,7 +1314,7 @@ static void aspeed_2400_smc_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 1;
> +    asc->max_cs            = 1;
>      asc->segments          = aspeed_2400_smc_segments;
>      asc->flash_window_base = 0x10000000;
>      asc->flash_window_size = 0x6000000;
> @@ -1359,7 +1359,7 @@ static void aspeed_2400_fmc_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 5;
> +    asc->max_cs            = 5;
>      asc->segments          = aspeed_2400_fmc_segments;
>      asc->segment_addr_mask = 0xffff0000;
>      asc->resets            = aspeed_2400_fmc_resets;
> @@ -1401,7 +1401,7 @@ static void aspeed_2400_spi1_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_SPI_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = SPI_CONF_ENABLE_W0;
> -    asc->max_peripherals   = 1;
> +    asc->max_cs            = 1;
>      asc->segments          = aspeed_2400_spi1_segments;
>      asc->flash_window_base = 0x30000000;
>      asc->flash_window_size = 0x10000000;
> @@ -1442,7 +1442,7 @@ static void aspeed_2500_fmc_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 3;
> +    asc->max_cs            = 3;
>      asc->segments          = aspeed_2500_fmc_segments;
>      asc->segment_addr_mask = 0xffff0000;
>      asc->resets            = aspeed_2500_fmc_resets;
> @@ -1480,7 +1480,7 @@ static void aspeed_2500_spi1_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 2;
> +    asc->max_cs            = 2;
>      asc->segments          = aspeed_2500_spi1_segments;
>      asc->segment_addr_mask = 0xffff0000;
>      asc->flash_window_base = 0x30000000;
> @@ -1515,7 +1515,7 @@ static void aspeed_2500_spi2_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 2;
> +    asc->max_cs            = 2;
>      asc->segments          = aspeed_2500_spi2_segments;
>      asc->segment_addr_mask = 0xffff0000;
>      asc->flash_window_base = 0x38000000;
> @@ -1597,7 +1597,7 @@ static void aspeed_2600_fmc_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 1;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 3;
> +    asc->max_cs            = 3;
>      asc->segments          = aspeed_2600_fmc_segments;
>      asc->segment_addr_mask = 0x0ff00ff0;
>      asc->resets            = aspeed_2600_fmc_resets;
> @@ -1636,7 +1636,7 @@ static void aspeed_2600_spi1_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 2;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 2;
> +    asc->max_cs            = 2;
>      asc->segments          = aspeed_2600_spi1_segments;
>      asc->segment_addr_mask = 0x0ff00ff0;
>      asc->flash_window_base = 0x30000000;
> @@ -1675,7 +1675,7 @@ static void aspeed_2600_spi2_class_init(ObjectClass *klass, void *data)
>      asc->r_timings         = R_TIMINGS;
>      asc->nregs_timings     = 3;
>      asc->conf_enable_w0    = CONF_ENABLE_W0;
> -    asc->max_peripherals   = 3;
> +    asc->max_cs            = 3;
>      asc->segments          = aspeed_2600_spi2_segments;
>      asc->segment_addr_mask = 0x0ff00ff0;
>      asc->flash_window_base = 0x50000000;
> --
> 2.34.1
>
>

Re: [PATCH 2/6] aspeed: Rework aspeed_board_init_flashes() interface

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:24 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> Currently, the allocation of the flash devices uses the number of
> slave selects configured in the SoC realize routine. It is simpler to
> use directly the number of FMC devices defined in the machine class
> and 1 for spi devices (which is what the SoC does in the back of the
> machine).
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/aspeed.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 02918a594d24..be24508a9854 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -276,9 +276,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
>      rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
>  }
>
> -static void aspeed_board_init_flashes(AspeedSMCState *s,
> -                                      const char *flashtype,
> -                                      int unit0)
> +static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
> +                                      int count, int unit0)
>  {
>      int i;
>
> @@ -286,7 +285,7 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
>          return;
>      }
>
> -    for (i = 0; i < s->num_cs; ++i) {
> +    for (i = 0; i < count; ++i) {
>          DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
>          qemu_irq cs_line;
>          DeviceState *dev;
> @@ -382,10 +381,10 @@ static void aspeed_machine_init(MachineState *machine)
>
>      aspeed_board_init_flashes(&bmc->soc.fmc,
>                                bmc->fmc_model ? bmc->fmc_model : amc->fmc_model,
> -                              0);
> +                              amc->num_cs, 0);
>      aspeed_board_init_flashes(&bmc->soc.spi[0],
>                                bmc->spi_model ? bmc->spi_model : amc->spi_model,
> -                              bmc->soc.fmc.num_cs);
> +                              1, amc->num_cs);
>
>      /* Install first FMC flash content as a boot rom. */
>      if (drive0) {
> --
> 2.34.1
>
>

Re: [PATCH 5/6] aspeed/smc: Let the SSI core layer define the bus name

[Alistair Francis]

On Mon, Mar 7, 2022 at 5:34 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> If no id is provided, qdev automatically assigns a unique ename with
> the following pattern "<type>.<index>".
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ssi/aspeed_smc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index f194182beacf..113f31899a6b 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -1130,7 +1130,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>      /* DMA irq. Keep it first for the initialization in the SoC */
>      sysbus_init_irq(sbd, &s->irq);
>
> -    s->spi = ssi_create_bus(dev, "spi");
> +    s->spi = ssi_create_bus(dev, NULL);
>
>      /* Setup cs_lines for peripherals */
>      s->cs_lines = g_new0(qemu_irq, asc->max_cs);
> --
> 2.34.1
>
>