Re: [PATCH v2 08/17] hw/arm/aspeed: Attach SRAM device to AST1700 model

[Cédric Le Goater]

On 11/5/25 04:58, Kane Chen wrote:

From: Kane-Chen-AS <kane_c...@aspeedtech.com>

Map the SRAM device to AST1700 model

Signed-off-by: Kane-Chen-AS <kane_c...@aspeedtech.com>
---
  include/hw/misc/aspeed_ast1700.h |  1 +
  hw/misc/aspeed_ast1700.c         | 25 +++++++++++++++++++++++++
  2 files changed, 26 insertions(+)

diff --git a/include/hw/misc/aspeed_ast1700.h b/include/hw/misc/aspeed_ast1700.h
index e105ceb027..391c8687f5 100644
--- a/include/hw/misc/aspeed_ast1700.h
+++ b/include/hw/misc/aspeed_ast1700.h
@@ -32,6 +32,7 @@ struct AspeedAST1700SoCState {

AspeedLTPIState ltpi;

      SerialMM uart;
+    MemoryRegion sram;
  };

#endif /* ASPEED_AST1700_H */

diff --git a/hw/misc/aspeed_ast1700.c b/hw/misc/aspeed_ast1700.c
index 1c2d367cdb..6f7ff625b5 100644
--- a/hw/misc/aspeed_ast1700.c
+++ b/hw/misc/aspeed_ast1700.c
@@ -15,14 +15,18 @@
  #include "migration/vmstate.h"
  #include "hw/misc/aspeed_ast1700.h"

+#define AST1700_BOARD1_MEM_ADDR 0x30000000

  #define AST2700_SOC_LTPI_SIZE        0x01000000
+#define AST1700_SOC_SRAM_SIZE        0x00040000

enum {

+    ASPEED_AST1700_DEV_SRAM,
      ASPEED_AST1700_DEV_UART12,
      ASPEED_AST1700_DEV_LTPI_CTRL,
  };

static const hwaddr aspeed_ast1700_io_memmap[] = {

+    [ASPEED_AST1700_DEV_SRAM]      =  0x00BC0000,
      [ASPEED_AST1700_DEV_UART12]    =  0x00C33B00,
      [ASPEED_AST1700_DEV_LTPI_CTRL] =  0x00C34000,
  };
@@ -31,12 +35,33 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error 
**errp)
      AspeedAST1700SoCState *s = ASPEED_AST1700(dev);
      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
      hwaddr uart_base;
+    Error *err = NULL;

This variable could be avoided.

+    int board_idx;
+    char sram_name[32];
+
+    if (s->mapped_base == AST1700_BOARD1_MEM_ADDR) {
+        board_idx = 0;
+    } else {
+        board_idx = 1;
+    }

That's a bit hacky.

An "index" property set at the SoC level would avoid this weak heuristic.

/* Occupy memory space for all controllers in AST1700 */

      memory_region_init(&s->iomem, OBJECT(s), TYPE_ASPEED_AST1700,
                         AST2700_SOC_LTPI_SIZE);
      sysbus_init_mmio(sbd, &s->iomem);

+ /* SRAM */

+    snprintf(sram_name, sizeof(sram_name), "aspeed.ioexp-sram.%d", board_idx);
+    memory_region_init_ram(&s->sram, OBJECT(s), sram_name,
+                           AST1700_SOC_SRAM_SIZE, &err);

Just pass 'errp' and test the return value of memory_region_init_ram()


Thanks,

C.

+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+    memory_region_add_subregion(&s->iomem,
+                                
aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SRAM],
+                                &s->sram);
+
      /* UART */
      uart_base = s->mapped_base +
                 aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_UART12];