[PATCH v1 00/11] hw/arm/aspeed: Use unimp array and table for unimplemented devices

[PATCH v1 00/11] hw/arm/aspeed: Use unimp array and table for unimplemented devices

[Jamin Lin]

v1:
 1. Use unimp array and table for unimplemented devices
 2. Sort device IDs alphabetically

Jamin Lin (11):
  hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed_ast2600: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed_ast10x0: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed_ast1040: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed: Remove unused unimplemented device fields
  hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented
    devices for SSP/TSP
  hw/arm/aspeed_coprocessor: Remove unused unimplemented device fields
  hw/arm/aspeed_ast1700: Use unimp array and table for unimplemented
    devices
  hw/arm/aspeed_ast1700: Remove unused unimplemented device field
  hw/arm/aspeed: Sort device IDs alphabetically

 include/hw/arm/aspeed_ast1700.h     |   3 +-
 include/hw/arm/aspeed_coprocessor.h |   5 +-
 include/hw/arm/aspeed_soc.h         | 195 ++++++++++++++--------------
 hw/arm/aspeed_ast1040.c             |  49 ++++---
 hw/arm/aspeed_ast10x0.c             |  64 ++++-----
 hw/arm/aspeed_ast1700.c             |  42 +++---
 hw/arm/aspeed_ast2400.c             |  33 +++--
 hw/arm/aspeed_ast2600.c             |  53 ++++----
 hw/arm/aspeed_ast27x0-ssp.c         |  56 ++++----
 hw/arm/aspeed_ast27x0-tsp.c         |  56 ++++----
 hw/arm/aspeed_ast27x0.c             |  49 +++----
 11 files changed, 287 insertions(+), 318 deletions(-)

-- 
2.53.0

[PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedSoCState. This made the variable names confusing. For example,
AspeedSoCState has "udc" as an unimplemented device, but
Aspeed2600SoCState also has "udc" as the real UDC model.

Add an "unimp" array in AspeedSoCState for all unimplemented devices,
and add an AspeedUnimpDevice table to describe them. This makes the
code easier to maintain and read. Convert AST2400/AST2500 to use them.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  9 +++++++++
 hw/arm/aspeed_ast2400.c     | 33 +++++++++++++++++++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 38226d9b80..a9ef4b3550 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -64,6 +64,14 @@
 #define ASPEED_INTC_NUM  2
 #define ASPEED_IOEXP_NUM 2
 #define ASPEED_SRAM_NUM 2
+#define ASPEED_UNIMP_NUM 10
+
+typedef struct AspeedUnimpDevice {
+    const char *qom_name;
+    const char *region_name;
+    int memmap_idx;
+    uint64_t size;
+} AspeedUnimpDevice;
 
 struct AspeedSoCState {
     DeviceState parent;
@@ -119,6 +127,7 @@ struct AspeedSoCState {
     UnimplementedDeviceState espi;
     UnimplementedDeviceState udc;
     UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
+    UnimplementedDeviceState unimp[ASPEED_UNIMP_NUM];
     AspeedAPB2OPBState fsi[2];
     AspeedLTPIState ltpi_ctrl[ASPEED_IOEXP_NUM];
     AspeedAST1700SoCState ioexp[ASPEED_IOEXP_NUM];
diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c
index 79a653f65f..3bd9df4e09 100644
--- a/hw/arm/aspeed_ast2400.c
+++ b/hw/arm/aspeed_ast2400.c
@@ -23,8 +23,6 @@
 #include "system/system.h"
 #include "target/arm/cpu-qom.h"
 
-#define ASPEED_SOC_IOMEM_SIZE       0x00200000
-
 static const hwaddr aspeed_soc_ast2400_memmap[] = {
     [ASPEED_DEV_SPI_BOOT]  = 0x00000000,
     [ASPEED_DEV_IOMEM]  = 0x1E600000,
@@ -134,6 +132,12 @@ static const int aspeed_soc_ast2400_irqmap[] = {
 
 #define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
 
+static const AspeedUnimpDevice aspeed_soc_ast2400_unimp_devs[] = {
+    { "iomem", "aspeed.io",    ASPEED_DEV_IOMEM, 0x00200000 },
+    { "video", "aspeed.video", ASPEED_DEV_VIDEO, 0x1000 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast2400_unimp_devs) > ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast2400_get_irq(AspeedSoCState *s, int dev)
 {
     Aspeed2400SoCState *a = ASPEED2400_SOC(s);
@@ -240,8 +244,10 @@ static void aspeed_ast2400_soc_init(Object *obj)
     snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
     object_initialize_child(obj, "hace", &s->hace, typename);
 
-    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2400_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_soc_ast2400_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 static void aspeed_ast2400_soc_realize(DeviceState *dev, Error **errp)
@@ -251,6 +257,7 @@ static void aspeed_ast2400_soc_realize(DeviceState *dev, Error **errp)
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     g_autofree char *sram_name = NULL;
+    const AspeedUnimpDevice *unimp;
     int uart;
 
     /* Default boot region (SPI memory or ROMs) */
@@ -259,16 +266,14 @@ static void aspeed_ast2400_soc_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SPI_BOOT],
                                 &s->spi_boot_container);
 
-    /* IO space */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem),
-                                  "aspeed.io",
-                                  sc->memmap[ASPEED_DEV_IOMEM],
-                                  ASPEED_SOC_IOMEM_SIZE);
-
-    /* Video engine stub */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->video),
-                                  "aspeed.video",
-                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2400_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast2400_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
-- 
2.53.0

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Add an "unimp" array in AspeedSoCState for all unimplemented devices,
> and add an AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST2400/AST2500 to use them.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/arm/aspeed_soc.h |  9 +++++++++
>   hw/arm/aspeed_ast2400.c     | 33 +++++++++++++++++++--------------
>   2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 38226d9b80..a9ef4b3550 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -64,6 +64,14 @@
>   #define ASPEED_INTC_NUM  2
>   #define ASPEED_IOEXP_NUM 2
>   #define ASPEED_SRAM_NUM 2
> +#define ASPEED_UNIMP_NUM 10
> +
> +typedef struct AspeedUnimpDevice {
> +    const char *qom_name;
> +    const char *region_name;
> +    int memmap_idx;
> +    uint64_t size;
> +} AspeedUnimpDevice;

Note to Peter: I really think keeping duplicating the
UnimplementedDeviceConfig structure is a basic software
development antipattern. Could you reconsider your objection?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Cédric Le Goater]

> Note to Peter: I really think keeping duplicating the
> UnimplementedDeviceConfig structure is a basic software
> development antipattern. Could you reconsider your objection?
What's the background ?

Thanks,

C.

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 30/9/26 13:50, Cédric Le Goater wrote:
>> Note to Peter: I really think keeping duplicating the
>> UnimplementedDeviceConfig structure is a basic software
>> development antipattern. Could you reconsider your objection?
> What's the background ?

I think that was discussed on IRC, the only ref on the list is:
https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmcqkk-gJH+s1mHuvFp8g@mail.gmail.com/

create_unimplemented_device() calls qdev_create() and allocate on the
heap, my motivation was to have another helper for in-place init (when
the UnimplementedDeviceState state is embedded in the parent). Now we
ended with such helpers like aspeed_mmio_map_unimplemented() which
realize in place, make_unimp_dev() and create_unimp() init and realize
in place and Jamin adds yet another pattern. While all are valids,
I'd rather unify. Maybe this is now back to the "qdev alloc vs qdev
struct embedded" discussion we never figured out.

hw/arm/aspeed_ast1040.c:127:    object_initialize_child(obj, "pwm", 
&s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast1040.c:128:    object_initialize_child(obj, "espi", 
&s->espi, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast1040.c:129:    object_initialize_child(obj, "udc", 
&s->udc, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:165:    object_initialize_child(obj, "iomem", 
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:166:    object_initialize_child(obj, "pwm", 
&s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:167:    object_initialize_child(obj, "espi", 
&s->espi, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:168:    object_initialize_child(obj, "udc", 
&s->udc, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2400.c:243:    object_initialize_child(obj, "iomem", 
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2400.c:244:    object_initialize_child(obj, "video", 
&s->video, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:285:    object_initialize_child(obj, "iomem", 
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:286:    object_initialize_child(obj, "video", 
&s->video, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:287:    object_initialize_child(obj, "dpmcu", 
&s->dpmcu, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/bcm2835_peripherals.c:40:    object_initialize_child(OBJECT(ps), 
name, uds, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/mps2-tz.c:440:    object_initialize_child(OBJECT(mms), name, uds, 
TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/musca.c:146:    object_initialize_child(OBJECT(mms), name, uds, 
TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/bcm2835_peripherals.c:36:void 
create_unimp(BCMSocPeripheralBaseState *ps,
hw/arm/bcm2835_peripherals.c:521:    create_unimp(s, &s->txp, 
"bcm2835-txp", TXP_OFFSET, 0x1000);
hw/arm/bcm2835_peripherals.c:522:    create_unimp(s, &s->armtmr, 
"bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
hw/arm/bcm2835_peripherals.c:523:    create_unimp(s, &s->i2s, 
"bcm2835-i2s", I2S_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:524:    create_unimp(s, &s->smi, 
"bcm2835-smi", SMI_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:525:    create_unimp(s, &s->bscsl, 
"bcm2835-spis", BSC_SL_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:526:    create_unimp(s, &s->dbus, 
"bcm2835-dbus", DBUS_OFFSET, 0x8000);
hw/arm/bcm2835_peripherals.c:527:    create_unimp(s, &s->ave0, 
"bcm2835-ave0", AVE0_OFFSET, 0x8000);
hw/arm/bcm2835_peripherals.c:528:    create_unimp(s, &s->v3d, 
"bcm2835-v3d", V3D_OFFSET, 0x1000);
hw/arm/bcm2835_peripherals.c:529:    create_unimp(s, &s->sdramc, 
"bcm2835-sdramc", SDRAMC_OFFSET, 0x100);
hw/arm/bcm2838_peripherals.c:182:    create_unimp(s_base, &s->clkisp, 
"bcm2835-clkisp", CLOCK_ISP_OFFSET,
hw/arm/bcm2838_peripherals.c:196:    create_unimp(s_base, &s->asb, 
"bcm2838-asb", BRDG_OFFSET, 0x24);
hw/arm/mps2-tz.c:429:static MemoryRegion 
*make_unimp_dev(MPS2TZMachineState *mms,
hw/arm/mps2-tz.c:981:                { "i2s-audio", make_unimp_dev, 
&mms->i2s_audio,
hw/arm/mps2-tz.c:988:                { "gfx", make_unimp_dev, &mms->gfx, 
0x41000000, 0x140000 },
hw/arm/mps2-tz.c:989:                { "gpio0", make_unimp_dev, 
&mms->gpio[0], 0x40100000, 0x1000 },
hw/arm/mps2-tz.c:990:                { "gpio1", make_unimp_dev, 
&mms->gpio[1], 0x40101000, 0x1000 },
hw/arm/mps2-tz.c:991:                { "gpio2", make_unimp_dev, 
&mms->gpio[2], 0x40102000, 0x1000 },
hw/arm/mps2-tz.c:992:                { "gpio3", make_unimp_dev, 
&mms->gpio[3], 0x40103000, 0x1000 },
hw/arm/mps2-tz.c:1035:                { "i2s-audio", make_unimp_dev, 
&mms->i2s_audio,
hw/arm/mps2-tz.c:1045:                { "clcd", make_unimp_dev, 
&mms->cldc, 0x4130a000, 0x1000 },
hw/arm/mps2-tz.c:1051:                { "gpio0", make_unimp_dev, 
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1052:                { "gpio1", make_unimp_dev, 
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1053:                { "gpio2", make_unimp_dev, 
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1054:                { "gpio3", make_unimp_dev, 
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1102:                { "i2s-audio", make_unimp_dev, 
&mms->i2s_audio, 0x49301000, 0x1000 },
hw/arm/mps2-tz.c:1111:                { "clcd", make_unimp_dev, 
&mms->cldc, 0x4930a000, 0x1000 },
hw/arm/mps2-tz.c:1117:                { "gpio0", make_unimp_dev, 
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1118:                { "gpio1", make_unimp_dev, 
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1119:                { "gpio2", make_unimp_dev, 
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1120:                { "gpio3", make_unimp_dev, 
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1141:                { "sram-timing-adapter", 
make_unimp_dev, &mms->ta[0], 0x41700000, 0x1000 },
hw/arm/mps2-tz.c:1142:                { "qspi-timing-adapter", 
make_unimp_dev, &mms->ta[1], 0x41701000, 0x1000 },
hw/arm/mps2-tz.c:1143:                { "ddr4-timing-adapter", 
make_unimp_dev, &mms->ta[2], 0x41702000, 0x1000 },
hw/arm/mps2-tz.c:1167:                { "i2s-audio", make_unimp_dev, 
&mms->i2s_audio, 0x49301000, 0x1000 },
hw/arm/mps2-tz.c:1176:                { "clcd", make_unimp_dev, 
&mms->cldc, 0x4930a000, 0x1000 },
hw/arm/mps2-tz.c:1182:                { "gpio0", make_unimp_dev, 
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1183:                { "gpio1", make_unimp_dev, 
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1184:                { "gpio2", make_unimp_dev, 
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1185:                { "gpio3", make_unimp_dev, 
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1195:                { "qspi-controller0", 
make_unimp_dev, &mms->qspi_controller[0],
hw/arm/mps2-tz.c:1197:                { "qspi-controller1", 
make_unimp_dev, &mms->qspi_controller[1],
hw/arm/mps2-tz.c:1199:                { "qspi-controller2", 
make_unimp_dev, &mms->qspi_controller[2],
hw/arm/musca.c:137:static MemoryRegion *make_unimp_dev(MuscaMachineState 
*mms,
hw/arm/musca.c:325:        { "spi", make_unimp_dev, &mms->spi, 0x3000, 
0x1000 },
hw/arm/musca.c:326:        { "i2c0", make_unimp_dev, &mms->i2c[0], 
0x4000, 0x1000 },
hw/arm/musca.c:327:        { "i2c1", make_unimp_dev, &mms->i2c[1], 
0x5000, 0x1000 },
hw/arm/musca.c:328:        { "i2s", make_unimp_dev, &mms->i2s, 0x6000, 
0x1000 },
hw/arm/musca.c:329:        { "pwm0", make_unimp_dev, &mms->pwm[0], 
0x7000, 0x1000 },
hw/arm/musca.c:331:        { "qspi", make_unimp_dev, &mms->qspi, 0xa000, 
0x1000 },
hw/arm/musca.c:332:        { "timer", make_unimp_dev, &mms->timer, 
0xb000, 0x1000 },
hw/arm/musca.c:333:        { "scc", make_unimp_dev, &mms->scc, 0xc000, 
0x1000 },
hw/arm/musca.c:334:        { "pwm1", make_unimp_dev, &mms->pwm[1], 
0xe000, 0x1000 },
hw/arm/musca.c:335:        { "pwm2", make_unimp_dev, &mms->pwm[2], 
0xf000, 0x1000 },
hw/arm/musca.c:336:        { "gpio", make_unimp_dev, &mms->gpio, 
0x10000, 0x1000 },
hw/arm/musca.c:463:                { "eflash0", make_unimp_dev, 
&mms->eflash[0],
hw/arm/musca.c:465:                { "eflash1", make_unimp_dev, 
&mms->eflash[1],
hw/arm/musca.c:467:                { "qspi", make_unimp_dev, &mms->qspi, 
0x42800000, 0x100000 },
hw/arm/musca.c:472:                { "mhu0", make_unimp_dev, 
&mms->mhu[0], 0x42600000, 0x100000 },
hw/arm/musca.c:473:                { "mhu1", make_unimp_dev, 
&mms->mhu[1], 0x42700000, 0x100000 },
hw/arm/musca.c:484:                { "pwm0", make_unimp_dev, 
&mms->pwm[0], 0x40101000, 0x1000 },
hw/arm/musca.c:485:                { "pwm1", make_unimp_dev, 
&mms->pwm[1], 0x40102000, 0x1000 },
hw/arm/musca.c:486:                { "pwm2", make_unimp_dev, 
&mms->pwm[2], 0x40103000, 0x1000 },
hw/arm/musca.c:487:                { "i2s", make_unimp_dev, &mms->i2s, 
0x40104000, 0x1000 },
hw/arm/musca.c:490:                { "i2c0", make_unimp_dev, 
&mms->i2c[0], 0x40108000, 0x1000 },
hw/arm/musca.c:491:                { "i2c1", make_unimp_dev, 
&mms->i2c[1], 0x40109000, 0x1000 },
hw/arm/musca.c:492:                { "spi", make_unimp_dev, &mms->spi, 
0x4010a000, 0x1000 },
hw/arm/musca.c:493:                { "scc", make_unimp_dev, &mms->scc, 
0x5010b000, 0x1000 },
hw/arm/musca.c:494:                { "timer", make_unimp_dev, 
&mms->timer, 0x4010c000, 0x1000 },
hw/arm/musca.c:496:                { "pvt", make_unimp_dev, &mms->pvt, 
0x4010e000, 0x1000 },
hw/arm/musca.c:497:                { "sdio", make_unimp_dev, &mms->sdio, 
0x4010f000, 0x1000 },
hw/arm/musca.c:503:                { "gpio", make_unimp_dev, &mms->gpio, 
0x41000000, 0x1000 },

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Cédric Le Goater]

On 9/30/26 14:16, Philippe Mathieu-Daudé wrote:
> On 30/9/26 13:50, Cédric Le Goater wrote:
>>> Note to Peter: I really think keeping duplicating the
>>> UnimplementedDeviceConfig structure is a basic software
>>> development antipattern. Could you reconsider your objection?
>> What's the background ?
> 
> I think that was discussed on IRC, the only ref on the list is:
> https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmcqkk-gJH+s1mHuvFp8g@mail.gmail.com/
> 
> create_unimplemented_device() calls qdev_create() and allocate on the
> heap, my motivation was to have another helper for in-place init (when
> the UnimplementedDeviceState state is embedded in the parent). Now we
> ended with such helpers like aspeed_mmio_map_unimplemented() which
> realize in place, make_unimp_dev() and create_unimp() init and realize
> in place and Jamin adds yet another pattern. While all are valids,
> I'd rather unify.

Could an array such as  :

static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
     { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x00040000 },
     { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000 },
     { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
     { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1, 0x01000000 },
};

be defined at the class level of the SoC and handled automatically ?

I guess would need some kind on common SoC model in that case.

C.

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Jamin Lin]

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



Cédric Le Goater <clg@kaod.org> 於 2026年10月1日 下午3:41 寫道：

On 9/30/26 14:16, Philippe Mathieu-Daudé wrote:
On 30/9/26 13:50, Cédric Le Goater wrote:
Note to Peter: I really think keeping duplicating the
UnimplementedDeviceConfig structure is a basic software
development antipattern. Could you reconsider your objection?
What's the background ?
I think that was discussed on IRC, the only ref on the list is:
https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmcqkk-gJH+s1mHuvFp8g@mail.gmail.com/
create_unimplemented_device() calls qdev_create() and allocate on the
heap, my motivation was to have another helper for in-place init (when
the UnimplementedDeviceState state is embedded in the parent). Now we
ended with such helpers like aspeed_mmio_map_unimplemented() which
realize in place, make_unimp_dev() and create_unimp() init and realize
in place and Jamin adds yet another pattern. While all are valids,
I'd rather unify.

Could an array such as  :

static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
   { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x00040000 },
   { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000 },
   { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
   { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1, 0x01000000 },
};

be defined at the class level of the SoC and handled automatically ?

I guess would need some kind on common SoC model in that case.

C.


Hi Cédric,

I will resend v2 and rework the design along these lines.

Defining the unimplemented devices as a class-level array and I will look into introducing a common SoC helper/model for this.

Besides that, we found that patch 11 breaks the AST2700 I/O expander support.

The device list should include:

ASPEED_DEV_IOEXP0_I2C,
ASPEED_DEV_IOEXP1_I2C,
ASPEED_DEV_IOEXP0_I3C,
ASPEED_DEV_IOEXP1_I3C,
ASPEED_DEV_IOEXP0_INTCIO,
ASPEED_DEV_IOEXP1_INTCIO,

I will fix this in v2 as well.

Thanks,
Jamin


[-- Attachment #2: Type: text/html, Size: 9567 bytes --]

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Cédric Le Goater]

On 10/1/26 13:51, Jamin Lin wrote:
> 
> 
>> Cédric Le Goater <clg@kaod.org> 於 2026年10月1日 下午3:41 寫道：
>>
>> On 9/30/26 14:16, Philippe Mathieu-Daudé wrote:
>>> On 30/9/26 13:50, Cédric Le Goater wrote:
>>>>> Note to Peter: I really think keeping duplicating the
>>>>> UnimplementedDeviceConfig structure is a basic software
>>>>> development antipattern. Could you reconsider your objection?
>>>> What's the background ?
>>> I think that was discussed on IRC, the only ref on the list is:
>>> https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmcqkk-gJH+s1mHuvFp8g@mail.gmail.com/
>>> create_unimplemented_device() calls qdev_create() and allocate on the
>>> heap, my motivation was to have another helper for in-place init (when
>>> the UnimplementedDeviceState state is embedded in the parent). Now we
>>> ended with such helpers like aspeed_mmio_map_unimplemented() which
>>> realize in place, make_unimp_dev() and create_unimp() init and realize
>>> in place and Jamin adds yet another pattern. While all are valids,
>>> I'd rather unify.
>>
>> Could an array such as  :
>>
>> static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
>>    { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x00040000 },
>>    { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000 },
>>    { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
>>    { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1, 0x01000000 },
>> };
>>
>> be defined at the class level of the SoC and handled automatically ?
>>
>> I guess would need some kind on common SoC model in that case.
>>
>> C.
>>
> Hi Cédric,
> 
> I will resend v2 and rework the design along these lines.

Please wait a bit for Phil's feedback.

> Defining the unimplemented devices as a class-level array and I will look
> into introducing a common SoC helper/model for this.

This could be a start for a more generalized solution.

> Besides that, we found that patch 11 breaks the AST2700 I/O expander support.

Ah. Did the functional catch this regression ?

> The device list should include:
> 
> ASPEED_DEV_IOEXP0_I2C,
> ASPEED_DEV_IOEXP1_I2C,
> ASPEED_DEV_IOEXP0_I3C,
> ASPEED_DEV_IOEXP1_I3C,
> ASPEED_DEV_IOEXP0_INTCIO,
> ASPEED_DEV_IOEXP1_INTCIO,
> 
> I will fix this in v2 as well.

OK.

Thanks,

C.

RE: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Jamin Lin]

Hi Cédric

> Subject: Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and
> table for unimplemented devices
> 
> On 10/1/26 13:51, Jamin Lin wrote:
> >
> >
> >> Cédric Le Goater <clg@kaod.org> 於 2026年10月1日 下午3:41 寫道：
> >>
> >> On 9/30/26 14:16, Philippe Mathieu-Daudé wrote:
> >>> On 30/9/26 13:50, Cédric Le Goater wrote:
> >>>>> Note to Peter: I really think keeping duplicating the
> >>>>> UnimplementedDeviceConfig structure is a basic software
> >>>>> development antipattern. Could you reconsider your objection?
> >>>> What's the background ?
> >>> I think that was discussed on IRC, the only ref on the list is:
> >>>
> https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmc
> >>> qkk-gJH+s1mHuvFp8g@mail.gmail.com/
> >>> create_unimplemented_device() calls qdev_create() and allocate on
> >>> the heap, my motivation was to have another helper for in-place init
> >>> (when the UnimplementedDeviceState state is embedded in the parent).
> >>> Now we ended with such helpers like
> aspeed_mmio_map_unimplemented()
> >>> which realize in place, make_unimp_dev() and create_unimp() init and
> >>> realize in place and Jamin adds yet another pattern. While all are
> >>> valids, I'd rather unify.
> >>
> >> Could an array such as  :
> >>
> >> static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
> >>    { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x0004000
> 0 },
> >>    { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000
>  },
> >>    { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
> >>    { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1,
> 0x01000000 }, };
> >>
> >> be defined at the class level of the SoC and handled automatically ?
> >>
> >> I guess would need some kind on common SoC model in that case.
> >>
> >> C.
> >>
> > Hi Cédric,
> >
> > I will resend v2 and rework the design along these lines.
> 
> Please wait a bit for Phil's feedback.
> 
> > Defining the unimplemented devices as a class-level array and I will
> > look into introducing a common SoC helper/model for this.
> 
> This could be a start for a more generalized solution.
> 
> > Besides that, we found that patch 11 breaks the AST2700 I/O expander
> support.
> 
> Ah. Did the functional catch this regression ?
> 
No, we don't have a functional test covering IO Expander 1 I2C.
I will add a functional test to cover it and catch this kind of regression in v2.

Thanks,
Jamin

> > The device list should include:
> >
> > ASPEED_DEV_IOEXP0_I2C,
> > ASPEED_DEV_IOEXP1_I2C,
> > ASPEED_DEV_IOEXP0_I3C,
> > ASPEED_DEV_IOEXP1_I3C,
> > ASPEED_DEV_IOEXP0_INTCIO,
> > ASPEED_DEV_IOEXP1_INTCIO,
> >
> > I will fix this in v2 as well.
> 
> OK.
> 
> Thanks,
> 
> C.
> 

Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2/10/26 11:02, Jamin Lin wrote:
> Hi Cédric
> 
>> Subject: Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and
>> table for unimplemented devices
>>
>> On 10/1/26 13:51, Jamin Lin wrote:
>>>
>>>
>>>> Cédric Le Goater <clg@kaod.org> 於 2026年10月1日 下午3:41 寫道：
>>>>
>>>> On 9/30/26 14:16, Philippe Mathieu-Daudé wrote:
>>>>> On 30/9/26 13:50, Cédric Le Goater wrote:
>>>>>>> Note to Peter: I really think keeping duplicating the
>>>>>>> UnimplementedDeviceConfig structure is a basic software
>>>>>>> development antipattern. Could you reconsider your objection?
>>>>>> What's the background ?
>>>>> I think that was discussed on IRC, the only ref on the list is:
>>>>>
>> https://lore.kernel.org/qemu-devel/CAFEAcA_ob6mHqfAAM9iAMae5vfxaYhmc
>>>>> qkk-gJH+s1mHuvFp8g@mail.gmail.com/
>>>>> create_unimplemented_device() calls qdev_create() and allocate on
>>>>> the heap, my motivation was to have another helper for in-place init
>>>>> (when the UnimplementedDeviceState state is embedded in the parent).
>>>>> Now we ended with such helpers like
>> aspeed_mmio_map_unimplemented()
>>>>> which realize in place, make_unimp_dev() and create_unimp() init and
>>>>> realize in place and Jamin adds yet another pattern. While all are
>>>>> valids, I'd rather unify.
>>>>
>>>> Could an array such as  :
>>>>
>>>> static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
>>>>     { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x0004000
>> 0 },
>>>>     { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000
>>   },
>>>>     { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
>>>>     { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1,
>> 0x01000000 }, };
>>>>
>>>> be defined at the class level of the SoC and handled automatically ?
>>>>
>>>> I guess would need some kind on common SoC model in that case.
>>>>
>>>> C.
>>>>
>>> Hi Cédric,
>>>
>>> I will resend v2 and rework the design along these lines.
>>
>> Please wait a bit for Phil's feedback.
>>
>>> Defining the unimplemented devices as a class-level array and I will
>>> look into introducing a common SoC helper/model for this.
>>
>> This could be a start for a more generalized solution.

I was thinking something like (untested but you get the idea):

-- >8 --
diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h
index 77a5077511f..d12774d6cc3 100644
--- a/include/hw/misc/unimp.h
+++ b/include/hw/misc/unimp.h
@@ -25,6 +25,27 @@ struct UnimplementedDeviceState {
      uint64_t size;
  };

+typedef struct UnimplementedDeviceConfig {
+    const char *qom_name;
+    const char *region_name;
+    uint64_t offset;
+    uint64_t size;
+    int priority;
+} UnimplementedDeviceConfig;
+
+/**
+ * create_unimplemented_devices:
+ * @parentobj: the parent object
+ * @container: a #MemoryRegion within which @addr is a relative address
+ * @devconfigs: NULL terminated array of #UnimplementedDeviceConfig
+ *
+ * For each entries in @devconfigs, create a TYPE_UNIMPLEMENTED_DEVICE
+ * and mmap it in @container at UnimplementedDeviceConfig::offset.
+ * @container is expected to be a QOM child of @parentobj.
+ */
+void create_unimplemented_devices(Object *parentobj, MemoryRegion 
*container,
+                                  const UnimplementedDeviceConfig 
*const devconfigs[]);
+
  /**
   * create_unimplemented_device: create and map a dummy device
   * @name: name of the device for debug logging
---

[PATCH v1 02/11] hw/arm/aspeed_ast2600: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedSoCState. This made the variable names confusing. For example,
AspeedSoCState has "udc" as an unimplemented device, but
Aspeed2600SoCState also has "udc" as the real UDC model.

Use the "unimp" array in AspeedSoCState for all unimplemented devices,
and use the AspeedUnimpDevice table to describe them. This makes the
code easier to maintain and read. Convert AST2600 to use them.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast2600.c | 53 +++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 1a10d0fe1e..ef57bd0ba5 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -19,9 +19,6 @@
 #include "system/system.h"
 #include "target/arm/cpu-qom.h"
 
-#define ASPEED_SOC_IOMEM_SIZE       0x00200000
-#define ASPEED_SOC_DPMCU_SIZE       0x00040000
-
 static const hwaddr aspeed_soc_ast2600_memmap[] = {
     [ASPEED_DEV_SPI_BOOT]  = 0x00000000,
     [ASPEED_DEV_SRAM0]     = 0x10000000,
@@ -149,6 +146,15 @@ static const int aspeed_soc_ast2600_irqmap[] = {
     [ASPEED_DEV_ACRY]      = 160,
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast2600_unimp_devs[] = {
+    { "iomem", "aspeed.io", ASPEED_DEV_IOMEM, 0x00200000 },
+    { "video", "aspeed.video", ASPEED_DEV_VIDEO, 0x1000 },
+    { "dpmcu", "aspeed.dpmcu", ASPEED_DEV_DPMCU, 0x00040000 },
+    { "emmc-boot-controller", "aspeed.emmc-boot-controller",
+      ASPEED_DEV_EMMC_BC, 0x1000 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast2600_unimp_devs) > ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast2600_get_irq(AspeedSoCState *s, int dev)
 {
     Aspeed2600SoCState *a = ASPEED2600_SOC(s);
@@ -282,12 +288,10 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
     object_initialize_child(obj, "sbc", &s->sbc, TYPE_ASPEED_AST2600_SBC);
 
-    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "video", &s->video, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "dpmcu", &s->dpmcu, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "emmc-boot-controller",
-                            &s->emmc_boot_controller,
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2600_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_soc_ast2600_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 
     for (i = 0; i < ASPEED_FSI_NUM; i++) {
         object_initialize_child(obj, "fsi[*]", &s->fsi[i], TYPE_ASPEED_APB2OPB);
@@ -374,6 +378,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     qemu_irq irq;
     g_autofree char *sram1_name = NULL;
     g_autofree char *sram_name = NULL;
+    const AspeedUnimpDevice *unimp;
     int uart;
 
     /* Default boot region (SPI memory or ROMs) */
@@ -382,22 +387,14 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SPI_BOOT],
                                 &s->spi_boot_container);
 
-    /* IO space */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem),
-                                  "aspeed.io",
-                                  sc->memmap[ASPEED_DEV_IOMEM],
-                                  ASPEED_SOC_IOMEM_SIZE);
-
-    /* Video engine stub */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->video),
-                                  "aspeed.video",
-                                  sc->memmap[ASPEED_DEV_VIDEO], 0x1000);
-
-    /* eMMC Boot Controller stub */
-    aspeed_mmio_map_unimplemented(s->memory,
-                                  SYS_BUS_DEVICE(&s->emmc_boot_controller),
-                                  "aspeed.emmc-boot-controller",
-                                  sc->memmap[ASPEED_DEV_EMMC_BC], 0x1000);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2600_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast2600_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 
     /* CPU */
     for (i = 0; i < sc->num_cpus; i++) {
@@ -469,12 +466,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SRAM1],
                                 &s->sram_container[1]);
 
-    /* DPMCU */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->dpmcu),
-                                  "aspeed.dpmcu",
-                                  sc->memmap[ASPEED_DEV_DPMCU],
-                                  ASPEED_SOC_DPMCU_SIZE);
-
     /* SCU */
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
         return;
-- 
2.53.0

Re: [PATCH v1 02/11] hw/arm/aspeed_ast2600: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Use the "unimp" array in AspeedSoCState for all unimplemented devices,
> and use the AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST2600 to use them.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast2600.c | 53 +++++++++++++++++------------------------
>   1 file changed, 22 insertions(+), 31 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 03/11] hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedSoCState. This made the variable names confusing. For example,
AspeedSoCState has "udc" as an unimplemented device, but
Aspeed2600SoCState also has "udc" as the real UDC model.

Use the "unimp" array in AspeedSoCState for all unimplemented devices,
and use the AspeedUnimpDevice table to describe them. This makes the
code easier to maintain and read. Convert AST2700 to use them.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0.c | 49 ++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 6365dbd638..1d887fd87f 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -24,10 +24,6 @@
 #include "qobject/qlist.h"
 #include "qemu/log.h"
 
-#define AST2700_SOC_IO_SIZE          0x00FE0000
-#define AST2700_SOC_IOMEM_SIZE       0x01000000
-#define AST2700_SOC_DPMCU_SIZE       0x00040000
-
 static const hwaddr aspeed_soc_ast2700_memmap[] = {
     [ASPEED_DEV_VBOOTROM]  =  0x00000000,
     [ASPEED_DEV_IOMEM]     =  0x00020000,
@@ -263,6 +259,14 @@ static const struct gic_intc_irq_info ast2700_gic_intcmap[] = {
     {201, 3, 1, ast2700_gic201_intcmap},
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast2700_unimp_devs[] = {
+    { "dpmcu",  "aspeed.dpmcu",  ASPEED_DEV_DPMCU,  0x00040000 },
+    { "iomem",  "aspeed.io",     ASPEED_DEV_IOMEM,  0x00FE0000 },
+    { "iomem0", "aspeed.iomem0", ASPEED_DEV_IOMEM0, 0x01000000 },
+    { "iomem1", "aspeed.iomem1", ASPEED_DEV_IOMEM1, 0x01000000 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast2700_unimp_devs) > ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast2700_get_irq(AspeedSoCState *s, int dev)
 {
     Aspeed27x0SoCState *a = ASPEED27X0_SOC(s);
@@ -559,14 +563,10 @@ static void aspeed_soc_ast2700_init(Object *obj)
                              sc->silicon_rev);
     }
 
-    object_initialize_child(obj, "dpmcu", &s->dpmcu,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "iomem", &s->iomem,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "iomem0", &s->iomem0,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "iomem1", &s->iomem1,
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2700_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_soc_ast2700_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 /*
@@ -699,6 +699,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     AspeedINTCClass *ic = ASPEED_INTC_GET_CLASS(&a->intc[0]);
     AspeedINTCClass *icio = ASPEED_INTC_GET_CLASS(&a->intc[1]);
+    const AspeedUnimpDevice *unimp;
     g_autofree char *name = NULL;
     qemu_irq irq;
     int uart;
@@ -1137,22 +1138,14 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->dpmcu),
-                                  "aspeed.dpmcu",
-                                  sc->memmap[ASPEED_DEV_DPMCU],
-                                  AST2700_SOC_DPMCU_SIZE);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem),
-                                  "aspeed.io",
-                                  sc->memmap[ASPEED_DEV_IOMEM],
-                                  AST2700_SOC_IO_SIZE);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem0),
-                                  "aspeed.iomem0",
-                                  sc->memmap[ASPEED_DEV_IOMEM0],
-                                  AST2700_SOC_IOMEM_SIZE);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem1),
-                                  "aspeed.iomem1",
-                                  sc->memmap[ASPEED_DEV_IOMEM1],
-                                  AST2700_SOC_IOMEM_SIZE);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2700_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast2700_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 }
 
 static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, const void *data)
-- 
2.53.0

Re: [PATCH v1 03/11] hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Use the "unimp" array in AspeedSoCState for all unimplemented devices,
> and use the AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST2700 to use them.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0.c | 49 ++++++++++++++++++-----------------------
>   1 file changed, 21 insertions(+), 28 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 04/11] hw/arm/aspeed_ast10x0: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedSoCState. This made the variable names confusing. For example,
AspeedSoCState has "udc" as an unimplemented device, but
Aspeed2600SoCState also has "udc" as the real UDC model.

Use the "unimp" array in AspeedSoCState for all unimplemented devices,
and use the AspeedUnimpDevice table to describe them. This makes the
code easier to maintain and read. Convert AST1030/AST1060 to use them.

This also fixes a type mismatch. AST10x0 used the "pwm" field
(AspeedPWMState) and the "sgpiom" field (AspeedSGPIOState array) to
create unimplemented devices. They now use the "unimp" array instead.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast10x0.c | 64 ++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 39 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index aeb5a4423d..cc587dcc1a 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -17,8 +17,6 @@
 #include "hw/misc/unimp.h"
 #include "hw/arm/aspeed_soc.h"
 
-#define ASPEED_SOC_IOMEM_SIZE 0x00200000
-
 static const hwaddr aspeed_soc_ast1030_memmap[] = {
     [ASPEED_DEV_SRAM0]     = 0x00000000,
     [ASPEED_DEV_SRAM1]     = 0x79000000, /* SEC SRAM */
@@ -99,6 +97,17 @@ static const int aspeed_soc_ast1030_irqmap[] = {
     [ASPEED_DEV_JTAG1]     = 53,
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast10x0_unimp_devs[] = {
+    { "iomem",   "aspeed.io",     ASPEED_DEV_IOMEM,   0x00200000 },
+    { "pwm",     "aspeed.pwm",    ASPEED_DEV_PWM,     0x100 },
+    { "espi",    "aspeed.espi",   ASPEED_DEV_ESPI,    0x800 },
+    { "udc",     "aspeed.udc",    ASPEED_DEV_UDC,     0x1000 },
+    { "sgpiom",  "aspeed.sgpiom", ASPEED_DEV_SGPIOM0, 0x100 },
+    { "jtag[0]", "aspeed.jtag0",  ASPEED_DEV_JTAG0,   0x20 },
+    { "jtag[1]", "aspeed.jtag1",  ASPEED_DEV_JTAG1,   0x20 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast10x0_unimp_devs) > ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast1030_get_irq(AspeedSoCState *s, int dev)
 {
     Aspeed10x0SoCState *a = ASPEED10X0_SOC(s);
@@ -162,16 +171,10 @@ static void aspeed_soc_ast10x0_init(Object *obj, const char *socname)
     snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
     object_initialize_child(obj, "hace", &s->hace, typename);
 
-    object_initialize_child(obj, "iomem", &s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "pwm", &s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "espi", &s->espi, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "udc", &s->udc, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "sgpiom", &s->sgpiom,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "jtag[0]", &s->jtag[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "jtag[1]", &s->jtag[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast10x0_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_soc_ast10x0_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 static void aspeed_soc_ast1030_init(Object *obj)
@@ -209,17 +212,22 @@ static bool aspeed_soc_ast10x0_realize(Aspeed10x0SoCState *a, Error **errp)
     int uart;
     int i;
     g_autofree char *sram_name = NULL;
+    const AspeedUnimpDevice *unimp;
 
     if (!clock_has_source(s->sysclk)) {
         error_setg(errp, "sysclk clock must be wired up by the board code");
         return false;
     }
 
-    /* General I/O memory space to catch all unimplemented device */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->iomem),
-                                  "aspeed.io",
-                                  sc->memmap[ASPEED_DEV_IOMEM],
-                                  ASPEED_SOC_IOMEM_SIZE);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast10x0_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast10x0_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
+
     /* AST10x0 CPU Core */
     armv7m = DEVICE(&a->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 256);
@@ -394,28 +402,6 @@ static bool aspeed_soc_ast10x0_realize(Aspeed10x0SoCState *a, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
                        aspeed_soc_ast1030_get_irq(s, ASPEED_DEV_GPIO));
 
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->pwm),
-                                  "aspeed.pwm",
-                                  sc->memmap[ASPEED_DEV_PWM], 0x100);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->espi),
-                                  "aspeed.espi",
-                                  sc->memmap[ASPEED_DEV_ESPI], 0x800);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->udc),
-                                  "aspeed.udc",
-                                  sc->memmap[ASPEED_DEV_UDC], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->sgpiom),
-                                  "aspeed.sgpiom",
-                                  sc->memmap[ASPEED_DEV_SGPIOM0], 0x100);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[0]),
-                                  "aspeed.jtag",
-                                  sc->memmap[ASPEED_DEV_JTAG0], 0x20);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[1]),
-                                  "aspeed.jtag",
-                                  sc->memmap[ASPEED_DEV_JTAG1], 0x20);
-
     return true;
 }
 
-- 
2.53.0

Re: [PATCH v1 04/11] hw/arm/aspeed_ast10x0: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Use the "unimp" array in AspeedSoCState for all unimplemented devices,
> and use the AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST1030/AST1060 to use them.
> 
> This also fixes a type mismatch. AST10x0 used the "pwm" field
> (AspeedPWMState) and the "sgpiom" field (AspeedSGPIOState array) to
> create unimplemented devices. They now use the "unimp" array instead.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast10x0.c | 64 ++++++++++++++++-------------------------
>   1 file changed, 25 insertions(+), 39 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 05/11] hw/arm/aspeed_ast1040: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedSoCState. This made the variable names confusing. For example,
AspeedSoCState has "udc" as an unimplemented device, but
Aspeed2600SoCState also has "udc" as the real UDC model.

Use the "unimp" array in AspeedSoCState for all unimplemented devices,
and use the AspeedUnimpDevice table to describe them. This makes the
code easier to maintain and read. Convert AST1040 to use them.

This also fixes a type mismatch. AST1040 used the "pwm" field
(AspeedPWMState) to create an unimplemented device. It now uses the
"unimp" array instead.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast1040.c | 49 ++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/hw/arm/aspeed_ast1040.c b/hw/arm/aspeed_ast1040.c
index 57ffc0807a..f7d94bc671 100644
--- a/hw/arm/aspeed_ast1040.c
+++ b/hw/arm/aspeed_ast1040.c
@@ -81,6 +81,15 @@ static const int aspeed_soc_ast1040_irqmap[] = {
     [ASPEED_DEV_PECI]      = 164,
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast1040_unimp_devs[] = {
+    { "pwm",     "aspeed.pwm",   ASPEED_DEV_PWM,   0x10000 },
+    { "espi",    "aspeed.espi",  ASPEED_DEV_ESPI,  0x1000 },
+    { "udc",     "aspeed.udc",   ASPEED_DEV_UDC,   0x4000 },
+    { "jtag[0]", "aspeed.jtag0", ASPEED_DEV_JTAG0, 0x100 },
+    { "jtag[1]", "aspeed.jtag1", ASPEED_DEV_JTAG1, 0x100 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast1040_unimp_devs) > ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast1040_get_irq(AspeedSoCState *s, int dev)
 {
     Aspeed10x0SoCState *a = ASPEED10X0_SOC(s);
@@ -124,13 +133,10 @@ static void aspeed_soc_ast1040_init(Object *obj)
                                 "aspeed.wdt-ast2700");
     }
 
-    object_initialize_child(obj, "pwm", &s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "espi", &s->espi, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "udc", &s->udc, TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "jtag[0]", &s->jtag[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "jtag[1]", &s->jtag[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast1040_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_soc_ast1040_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 static void aspeed_soc_ast1040_realize(DeviceState *dev_soc, Error **errp)
@@ -140,6 +146,7 @@ static void aspeed_soc_ast1040_realize(DeviceState *dev_soc, Error **errp)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     g_autofree char *hyperram_name = NULL;
     g_autofree char *sram_name = NULL;
+    const AspeedUnimpDevice *unimp;
     DeviceState *armv7m;
     Error *err = NULL;
     int uart;
@@ -267,26 +274,14 @@ static void aspeed_soc_ast1040_realize(DeviceState *dev_soc, Error **errp)
         aspeed_mmio_map(s->memory, SYS_BUS_DEVICE(&s->wdt[i]), 0, wdt_offset);
     }
 
-    /* Unimplemented peripherals */
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->pwm),
-                                  "aspeed.pwm",
-                                  sc->memmap[ASPEED_DEV_PWM], 0x10000);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->espi),
-                                  "aspeed.espi",
-                                  sc->memmap[ASPEED_DEV_ESPI], 0x1000);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->udc),
-                                  "aspeed.udc",
-                                  sc->memmap[ASPEED_DEV_UDC], 0x4000);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[0]),
-                                  "aspeed.jtag0",
-                                  sc->memmap[ASPEED_DEV_JTAG0], 0x100);
-
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[1]),
-                                  "aspeed.jtag1",
-                                  sc->memmap[ASPEED_DEV_JTAG1], 0x100);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast1040_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast1040_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 }
 
 static void aspeed_soc_ast1040_class_init(ObjectClass *klass, const void *data)
-- 
2.53.0

Re: [PATCH v1 05/11] hw/arm/aspeed_ast1040: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Use the "unimp" array in AspeedSoCState for all unimplemented devices,
> and use the AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST1040 to use them.
> 
> This also fixes a type mismatch. AST1040 used the "pwm" field
> (AspeedPWMState) to create an unimplemented device. It now uses the
> "unimp" array instead.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast1040.c | 49 ++++++++++++++++++-----------------------
>   1 file changed, 22 insertions(+), 27 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 06/11] hw/arm/aspeed: Remove unused unimplemented device fields

[Jamin Lin]

All Aspeed SoCs now use the "unimp" array and the AspeedUnimpDevice
table for their unimplemented devices. Remove the old fields in
AspeedSoCState and the ASPEED_JTAG_NUM define, which are not used
anymore.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index a9ef4b3550..0ef45561e0 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -59,7 +59,6 @@
 #define ASPEED_CPUS_NUM  4
 #define ASPEED_MACS_NUM  4
 #define ASPEED_UARTS_NUM 13
-#define ASPEED_JTAG_NUM  2
 #define ASPEED_PCIE_NUM  3
 #define ASPEED_INTC_NUM  2
 #define ASPEED_IOEXP_NUM 2
@@ -118,15 +117,6 @@ struct AspeedSoCState {
     AspeedPECIState peci;
     SerialMM uart[ASPEED_UARTS_NUM];
     Clock *sysclk;
-    UnimplementedDeviceState iomem;
-    UnimplementedDeviceState iomem0;
-    UnimplementedDeviceState iomem1;
-    UnimplementedDeviceState video;
-    UnimplementedDeviceState emmc_boot_controller;
-    UnimplementedDeviceState dpmcu;
-    UnimplementedDeviceState espi;
-    UnimplementedDeviceState udc;
-    UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
     UnimplementedDeviceState unimp[ASPEED_UNIMP_NUM];
     AspeedAPB2OPBState fsi[2];
     AspeedLTPIState ltpi_ctrl[ASPEED_IOEXP_NUM];
-- 
2.53.0

[PATCH v1 07/11] hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented devices for SSP/TSP

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
the coprocessor state. This made the code hard to maintain.

Add an "unimp" array in AspeedCoprocessorState for all unimplemented
devices, and use the AspeedUnimpDevice table to describe them. This
makes the code easier to maintain and read. Convert AST2700 SSP and
TSP to use them.

This also fixes a type mismatch. SSP and TSP used the "timerctrl"
field (AspeedTimerCtrlState) to create an unimplemented device. They
now use the "unimp" array instead.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_coprocessor.h |  1 +
 hw/arm/aspeed_ast27x0-ssp.c         | 56 ++++++++++++++---------------
 hw/arm/aspeed_ast27x0-tsp.c         | 56 ++++++++++++++---------------
 3 files changed, 53 insertions(+), 60 deletions(-)

diff --git a/include/hw/arm/aspeed_coprocessor.h b/include/hw/arm/aspeed_coprocessor.h
index acb8b06457..29b66cc810 100644
--- a/include/hw/arm/aspeed_coprocessor.h
+++ b/include/hw/arm/aspeed_coprocessor.h
@@ -23,6 +23,7 @@ struct AspeedCoprocessorState {
     Clock *sysclk;
 
     AspeedTimerCtrlState timerctrl;
+    UnimplementedDeviceState unimp[ASPEED_UNIMP_NUM];
     SerialMM *uart;
     int uart_dev;
 };
diff --git a/hw/arm/aspeed_ast27x0-ssp.c b/hw/arm/aspeed_ast27x0-ssp.c
index e03653086c..b448a29665 100644
--- a/hw/arm/aspeed_ast27x0-ssp.c
+++ b/hw/arm/aspeed_ast27x0-ssp.c
@@ -101,6 +101,17 @@ static struct nvic_intc_irq_info ast2700_ssp_intcmap[] = {
     {169, 1, 9, NULL},
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast27x0ssp_unimp_devs[] = {
+    { "timerctrl", "aspeed.timerctrl", ASPEED_DEV_TIMER1, 0x200 },
+    { "ipc0",      "aspeed.ipc0",      ASPEED_DEV_IPC0,   0x1000 },
+    { "ipc1",      "aspeed.ipc1",      ASPEED_DEV_IPC1,   0x1000 },
+    { "pric0",     "aspeed.pric0",     ASPEED_DEV_PRIC0,  0x1000 },
+    { "pric1",     "aspeed.pric1",     ASPEED_DEV_PRIC1,  0x1000 },
+    { "otp",       "aspeed.otp",       ASPEED_DEV_OTP,    0x800 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast27x0ssp_unimp_devs) >
+                  ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast27x0ssp_get_irq(AspeedCoprocessorState *s,
                                               int dev)
 {
@@ -128,6 +139,7 @@ static void aspeed_soc_ast27x0ssp_init(Object *obj)
 {
     Aspeed27x0CoprocessorState *a = ASPEED27X0SSP_COPROCESSOR(obj);
     AspeedCoprocessorState *s = ASPEED_COPROCESSOR(obj);
+    int i;
 
     object_initialize_child(obj, "armv7m", &a->armv7m, TYPE_ARMV7M);
     s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
@@ -137,18 +149,11 @@ static void aspeed_soc_ast27x0ssp_init(Object *obj)
     object_initialize_child(obj, "intc1", &a->intc[1],
                             TYPE_ASPEED_2700SSP_INTCIO);
 
-    object_initialize_child(obj, "timerctrl", &s->timerctrl,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "ipc0", &a->ipc[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "ipc1", &a->ipc[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "pric0", &a->pric[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "pric1", &a->pric[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "otp", &a->otp,
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast27x0ssp_unimp_devs); i++) {
+        object_initialize_child(obj,
+                                aspeed_soc_ast27x0ssp_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 static void aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp)
@@ -159,6 +164,7 @@ static void aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp)
     DeviceState *armv7m;
     MemoryRegion *mr;
     g_autofree char *sdram_name = NULL;
+    const AspeedUnimpDevice *unimp;
     int i;
 
     if (!clock_has_source(s->sysclk)) {
@@ -283,24 +289,14 @@ static void aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp)
     memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_FMC],
                                 &a->fmc_alias);
 
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->timerctrl),
-                                  "aspeed.timerctrl",
-                                  sc->memmap[ASPEED_DEV_TIMER1], 0x200);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->ipc[0]),
-                                  "aspeed.ipc0",
-                                  sc->memmap[ASPEED_DEV_IPC0], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->ipc[1]),
-                                  "aspeed.ipc1",
-                                  sc->memmap[ASPEED_DEV_IPC1], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->pric[0]),
-                                  "aspeed.pric0",
-                                  sc->memmap[ASPEED_DEV_PRIC0], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->pric[1]),
-                                  "aspeed.pric1",
-                                  sc->memmap[ASPEED_DEV_PRIC1], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->otp),
-                                  "aspeed.otp",
-                                  sc->memmap[ASPEED_DEV_OTP], 0x800);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast27x0ssp_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast27x0ssp_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 }
 
 static const Property aspeed_27x0_coprocessor_properties[] = {
diff --git a/hw/arm/aspeed_ast27x0-tsp.c b/hw/arm/aspeed_ast27x0-tsp.c
index 39ba062a20..32b57f9363 100644
--- a/hw/arm/aspeed_ast27x0-tsp.c
+++ b/hw/arm/aspeed_ast27x0-tsp.c
@@ -101,6 +101,17 @@ static struct nvic_intc_irq_info ast2700_tsp_intcmap[] = {
     {169, 1, 9, NULL},
 };
 
+static const AspeedUnimpDevice aspeed_soc_ast27x0tsp_unimp_devs[] = {
+    { "timerctrl", "aspeed.timerctrl", ASPEED_DEV_TIMER1, 0x200 },
+    { "ipc0",      "aspeed.ipc0",      ASPEED_DEV_IPC0,   0x1000 },
+    { "ipc1",      "aspeed.ipc1",      ASPEED_DEV_IPC1,   0x1000 },
+    { "pric0",     "aspeed.pric0",     ASPEED_DEV_PRIC0,  0x1000 },
+    { "pric1",     "aspeed.pric1",     ASPEED_DEV_PRIC1,  0x1000 },
+    { "otp",       "aspeed.otp",       ASPEED_DEV_OTP,    0x800 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_soc_ast27x0tsp_unimp_devs) >
+                  ASPEED_UNIMP_NUM);
+
 static qemu_irq aspeed_soc_ast27x0tsp_get_irq(AspeedCoprocessorState *s,
                                               int dev)
 {
@@ -128,6 +139,7 @@ static void aspeed_soc_ast27x0tsp_init(Object *obj)
 {
     Aspeed27x0CoprocessorState *a = ASPEED27X0TSP_COPROCESSOR(obj);
     AspeedCoprocessorState *s = ASPEED_COPROCESSOR(obj);
+    int i;
 
     object_initialize_child(obj, "armv7m", &a->armv7m, TYPE_ARMV7M);
     s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
@@ -137,18 +149,11 @@ static void aspeed_soc_ast27x0tsp_init(Object *obj)
     object_initialize_child(obj, "intc1", &a->intc[1],
                             TYPE_ASPEED_2700TSP_INTCIO);
 
-    object_initialize_child(obj, "timerctrl", &s->timerctrl,
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "ipc0", &a->ipc[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "ipc1", &a->ipc[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "pric0", &a->pric[0],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "pric1", &a->pric[1],
-                            TYPE_UNIMPLEMENTED_DEVICE);
-    object_initialize_child(obj, "otp", &a->otp,
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast27x0tsp_unimp_devs); i++) {
+        object_initialize_child(obj,
+                                aspeed_soc_ast27x0tsp_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 }
 
 static void aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp)
@@ -159,6 +164,7 @@ static void aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp)
     DeviceState *armv7m;
     MemoryRegion *mr;
     g_autofree char *sdram_name = NULL;
+    const AspeedUnimpDevice *unimp;
     int i;
 
     if (!clock_has_source(s->sysclk)) {
@@ -283,24 +289,14 @@ static void aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp)
     memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_FMC],
                                 &a->fmc_alias);
 
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->timerctrl),
-                                  "aspeed.timerctrl",
-                                  sc->memmap[ASPEED_DEV_TIMER1], 0x200);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->ipc[0]),
-                                  "aspeed.ipc0",
-                                  sc->memmap[ASPEED_DEV_IPC0], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->ipc[1]),
-                                  "aspeed.ipc1",
-                                  sc->memmap[ASPEED_DEV_IPC1], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->pric[0]),
-                                  "aspeed.pric0",
-                                  sc->memmap[ASPEED_DEV_PRIC0], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->pric[1]),
-                                  "aspeed.pric1",
-                                  sc->memmap[ASPEED_DEV_PRIC1], 0x1000);
-    aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&a->otp),
-                                  "aspeed.otp",
-                                  sc->memmap[ASPEED_DEV_OTP], 0x800);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast27x0tsp_unimp_devs); i++) {
+        unimp = &aspeed_soc_ast27x0tsp_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->unimp[i]),
+                                      unimp->region_name,
+                                      sc->memmap[unimp->memmap_idx],
+                                      unimp->size);
+    }
 }
 
 static const Property aspeed_27x0_coprocessor_properties[] = {
-- 
2.53.0

Re: [PATCH v1 07/11] hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented devices for SSP/TSP

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> the coprocessor state. This made the code hard to maintain.
> 
> Add an "unimp" array in AspeedCoprocessorState for all unimplemented
> devices, and use the AspeedUnimpDevice table to describe them. This
> makes the code easier to maintain and read. Convert AST2700 SSP and
> TSP to use them.
> 
> This also fixes a type mismatch. SSP and TSP used the "timerctrl"
> field (AspeedTimerCtrlState) to create an unimplemented device. They
> now use the "unimp" array instead.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/arm/aspeed_coprocessor.h |  1 +
>   hw/arm/aspeed_ast27x0-ssp.c         | 56 ++++++++++++++---------------
>   hw/arm/aspeed_ast27x0-tsp.c         | 56 ++++++++++++++---------------
>   3 files changed, 53 insertions(+), 60 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 08/11] hw/arm/aspeed_coprocessor: Remove unused unimplemented device fields

[Jamin Lin]

AST2700 SSP and TSP now use the "unimp" array and the AspeedUnimpDevice
table for their unimplemented devices. Remove the old "ipc", "pric" and
"otp" fields in Aspeed27x0CoprocessorState and the "timerctrl" field in
AspeedCoprocessorState, which are not used anymore.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_coprocessor.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/hw/arm/aspeed_coprocessor.h b/include/hw/arm/aspeed_coprocessor.h
index 29b66cc810..fe1f576874 100644
--- a/include/hw/arm/aspeed_coprocessor.h
+++ b/include/hw/arm/aspeed_coprocessor.h
@@ -22,7 +22,6 @@ struct AspeedCoprocessorState {
     MemoryRegion uart_alias;
     Clock *sysclk;
 
-    AspeedTimerCtrlState timerctrl;
     UnimplementedDeviceState unimp[ASPEED_UNIMP_NUM];
     SerialMM *uart;
     int uart_dev;
@@ -44,9 +43,6 @@ struct AspeedCoprocessorClass {
 struct Aspeed27x0CoprocessorState {
     AspeedCoprocessorState parent;
     AspeedINTCState intc[2];
-    UnimplementedDeviceState ipc[2];
-    UnimplementedDeviceState pric[2];
-    UnimplementedDeviceState otp;
 
     ARMv7MState armv7m;
 
-- 
2.53.0

[PATCH v1 09/11] hw/arm/aspeed_ast1700: Use unimp array and table for unimplemented devices

[Jamin Lin]

Before this change, each unimplemented device needed its own field in
AspeedAST1700SoCState. This made the code hard to maintain.

Add an "unimp" array in AspeedAST1700SoCState for all unimplemented
devices, and use the AspeedUnimpDevice table to describe them. This
makes the code easier to maintain and read. Convert AST1700 to use
them.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_ast1700.h |  2 ++
 hw/arm/aspeed_ast1700.c         | 42 ++++++++++++++++++++-------------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h
index 39c5977cf1..ed9b45e202 100644
--- a/include/hw/arm/aspeed_ast1700.h
+++ b/include/hw/arm/aspeed_ast1700.h
@@ -23,6 +23,7 @@
 
 #define AST1700_SGPIO_NUM            2
 #define AST1700_WDT_NUM              9
+#define AST1700_UNIMP_NUM            1
 
 #define TYPE_ASPEED_AST1700 "aspeed.ast1700"
 
@@ -49,6 +50,7 @@ struct AspeedAST1700SoCState {
     AspeedWDTState wdt[AST1700_WDT_NUM];
 
     UnimplementedDeviceState i3c;
+    UnimplementedDeviceState unimp[AST1700_UNIMP_NUM];
 };
 
 #endif /* ASPEED_AST1700_H */
diff --git a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c
index 5af5da5b8a..f926851166 100644
--- a/hw/arm/aspeed_ast1700.c
+++ b/hw/arm/aspeed_ast1700.c
@@ -12,10 +12,10 @@
 #include "qapi/error.h"
 #include "hw/core/qdev-properties.h"
 #include "hw/arm/aspeed_ast1700.h"
+#include "hw/arm/aspeed_soc.h"
 
 #define AST2700_SOC_LTPI_SIZE        0x01000000
 #define AST1700_SOC_SRAM_SIZE        0x00040000
-#define AST1700_SOC_I3C_SIZE         0x00010000
 
 enum {
     ASPEED_AST1700_DEV_SPI0,
@@ -51,11 +51,18 @@ static const hwaddr aspeed_ast1700_io_memmap[] = {
     [ASPEED_AST1700_DEV_SPI0_MEM]  =  0x04000000,
 };
 
+static const AspeedUnimpDevice aspeed_ast1700_unimp_devs[] = {
+    { "ioexp-i3c", "ioexp-i3c", ASPEED_AST1700_DEV_I3C, 0x00010000 },
+};
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(aspeed_ast1700_unimp_devs) > AST1700_UNIMP_NUM);
+
 static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
 {
     AspeedAST1700SoCState *s = ASPEED_AST1700(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    const AspeedUnimpDevice *unimp;
     char dev_name[32];
+    int i;
 
     if (!s->dram_mr) {
         error_setg(errp, TYPE_ASPEED_AST1700 ": 'dram' link not set");
@@ -156,7 +163,7 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
                         sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->ltpi), 0));
 
     /* SGPIOM */
-    for (int i = 0; i < AST1700_SGPIO_NUM; i++) {
+    for (i = 0; i < AST1700_SGPIO_NUM; i++) {
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) {
             return;
         }
@@ -166,7 +173,7 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
     }
 
     /* WDT */
-    for (int i = 0; i < AST1700_WDT_NUM; i++) {
+    for (i = 0; i < AST1700_WDT_NUM; i++) {
         AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
         hwaddr wdt_offset = aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_WDT] +
                             i * awc->iosize;
@@ -181,19 +188,20 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
                         sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->wdt[i]), 0));
     }
 
-    /* I3C */
-    qdev_prop_set_string(DEVICE(&s->i3c), "name", "ioexp-i3c");
-    qdev_prop_set_uint64(DEVICE(&s->i3c), "size", AST1700_SOC_I3C_SIZE);
-    sysbus_realize(SYS_BUS_DEVICE(&s->i3c), errp);
-    memory_region_add_subregion_overlap(&s->iomem,
-                        aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_I3C],
-                        sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i3c), 0),
-                        -1000);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_ast1700_unimp_devs); i++) {
+        unimp = &aspeed_ast1700_unimp_devs[i];
+        aspeed_mmio_map_unimplemented(&s->iomem, SYS_BUS_DEVICE(&s->unimp[i]),
+                        unimp->region_name,
+                        aspeed_ast1700_io_memmap[unimp->memmap_idx],
+                        unimp->size);
+    }
 }
 
 static void aspeed_ast1700_instance_init(Object *obj)
 {
     AspeedAST1700SoCState *s = ASPEED_AST1700(obj);
+    int i;
 
     /* UART */
     object_initialize_child(obj, "uart", &s->uart,
@@ -227,20 +235,22 @@ static void aspeed_ast1700_instance_init(Object *obj)
                             &s->ltpi, TYPE_ASPEED_LTPI);
 
     /* SGPIOM */
-    for (int i = 0; i < AST1700_SGPIO_NUM; i++) {
+    for (i = 0; i < AST1700_SGPIO_NUM; i++) {
         object_initialize_child(obj, "ioexp-sgpiom[*]", &s->sgpiom[i],
                                 "aspeed.sgpio-ast2700");
     }
 
     /* WDT */
-    for (int i = 0; i < AST1700_WDT_NUM; i++) {
+    for (i = 0; i < AST1700_WDT_NUM; i++) {
         object_initialize_child(obj, "ioexp-wdt[*]",
                                 &s->wdt[i], "aspeed.wdt-ast2700");
     }
 
-    /* I3C */
-    object_initialize_child(obj, "ioexp-i3c", &s->i3c,
-                            TYPE_UNIMPLEMENTED_DEVICE);
+    /* Unimplemented devices */
+    for (i = 0; i < ARRAY_SIZE(aspeed_ast1700_unimp_devs); i++) {
+        object_initialize_child(obj, aspeed_ast1700_unimp_devs[i].qom_name,
+                                &s->unimp[i], TYPE_UNIMPLEMENTED_DEVICE);
+    }
 
     return;
 }
-- 
2.53.0

Re: [PATCH v1 09/11] hw/arm/aspeed_ast1700: Use unimp array and table for unimplemented devices

[Philippe Mathieu-Daudé]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedAST1700SoCState. This made the code hard to maintain.
> 
> Add an "unimp" array in AspeedAST1700SoCState for all unimplemented
> devices, and use the AspeedUnimpDevice table to describe them. This
> makes the code easier to maintain and read. Convert AST1700 to use
> them.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/arm/aspeed_ast1700.h |  2 ++
>   hw/arm/aspeed_ast1700.c         | 42 ++++++++++++++++++++-------------
>   2 files changed, 28 insertions(+), 16 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

[PATCH v1 10/11] hw/arm/aspeed_ast1700: Remove unused unimplemented device field

[Jamin Lin]

AST1700 now uses the "unimp" array and the AspeedUnimpDevice table for
its unimplemented devices. Remove the old "i3c" field in
AspeedAST1700SoCState, which is not used anymore.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_ast1700.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h
index ed9b45e202..015d1c51db 100644
--- a/include/hw/arm/aspeed_ast1700.h
+++ b/include/hw/arm/aspeed_ast1700.h
@@ -49,7 +49,6 @@ struct AspeedAST1700SoCState {
     AspeedPWMState pwm;
     AspeedWDTState wdt[AST1700_WDT_NUM];
 
-    UnimplementedDeviceState i3c;
     UnimplementedDeviceState unimp[AST1700_UNIMP_NUM];
 };
 
-- 
2.53.0

[PATCH v1 11/11] hw/arm/aspeed: Sort device IDs alphabetically

[Jamin Lin]

The ASPEED device ID enum has grown over time without a consistent
ordering rule. This makes it difficult to determine where new device
IDs should be added and reduces readability and maintainability as
more devices are introduced.

Sort the device IDs alphabetically to provide a consistent ordering
rule. Keep related numbered devices together, such as SPI0, SPI1 and
SPI2.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h | 176 ++++++++++++++++++------------------
 1 file changed, 88 insertions(+), 88 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 0ef45561e0..c201263937 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -194,64 +194,56 @@ struct AspeedSoCClass {
 };
 
 enum {
-    ASPEED_DEV_VBOOTROM,
-    ASPEED_DEV_SPI_BOOT,
-    ASPEED_DEV_IOMEM,
-    ASPEED_DEV_IOMEM0,
-    ASPEED_DEV_IOMEM1,
-    ASPEED_DEV_UART0,
-    ASPEED_DEV_UART1,
-    ASPEED_DEV_UART2,
-    ASPEED_DEV_UART3,
-    ASPEED_DEV_UART4,
-    ASPEED_DEV_UART5,
-    ASPEED_DEV_UART6,
-    ASPEED_DEV_UART7,
-    ASPEED_DEV_UART8,
-    ASPEED_DEV_UART9,
-    ASPEED_DEV_UART10,
-    ASPEED_DEV_UART11,
-    ASPEED_DEV_UART12,
-    ASPEED_DEV_UART13,
-    ASPEED_DEV_VUART,
-    ASPEED_DEV_FMC,
-    ASPEED_DEV_SPI0,
-    ASPEED_DEV_SPI1,
-    ASPEED_DEV_SPI2,
+    ASPEED_DEV_ACRY,
+    ASPEED_DEV_ADC,
+    ASPEED_DEV_DP,
+    ASPEED_DEV_DPMCU,
     ASPEED_DEV_EHCI1,
     ASPEED_DEV_EHCI2,
     ASPEED_DEV_EHCI3,
     ASPEED_DEV_EHCI4,
-    ASPEED_DEV_VIC,
-    ASPEED_DEV_INTC,
-    ASPEED_DEV_INTCIO,
-    ASPEED_DEV_SDMC,
-    ASPEED_DEV_SCU,
-    ASPEED_DEV_ADC,
-    ASPEED_DEV_SBC,
+    ASPEED_DEV_EMMC,
     ASPEED_DEV_EMMC_BC,
-    ASPEED_DEV_VIDEO,
-    ASPEED_DEV_SRAM0,
-    ASPEED_DEV_SRAM1,
-    ASPEED_DEV_SDHCI,
+    ASPEED_DEV_ESPI,
+    ASPEED_DEV_ETH1,
+    ASPEED_DEV_ETH2,
+    ASPEED_DEV_ETH3,
+    ASPEED_DEV_ETH4,
+    ASPEED_DEV_FMC,
+    ASPEED_DEV_FSI1,
+    ASPEED_DEV_FSI2,
     ASPEED_DEV_GPIO,
     ASPEED_DEV_GPIO_1_8V,
-    ASPEED_DEV_SGPIOM0,
-    ASPEED_DEV_SGPIOM1,
-    ASPEED_DEV_RTC,
-    ASPEED_DEV_TIMER1,
-    ASPEED_DEV_TIMER2,
-    ASPEED_DEV_TIMER3,
-    ASPEED_DEV_TIMER4,
-    ASPEED_DEV_TIMER5,
-    ASPEED_DEV_TIMER6,
-    ASPEED_DEV_TIMER7,
-    ASPEED_DEV_TIMER8,
-    ASPEED_DEV_WDT,
-    ASPEED_DEV_PWM,
-    ASPEED_DEV_LPC,
-    ASPEED_DEV_IBT,
+    ASPEED_DEV_HACE,
     ASPEED_DEV_I2C,
+    ASPEED_DEV_I3C,
+    ASPEED_DEV_IBT,
+    ASPEED_DEV_INTC,
+    ASPEED_DEV_INTCIO,
+    ASPEED_DEV_IOMEM,
+    ASPEED_DEV_IOMEM0,
+    ASPEED_DEV_IOMEM1,
+    ASPEED_DEV_IOEXP0_I2C,
+    ASPEED_DEV_IOEXP0_I3C,
+    ASPEED_DEV_IOEXP0_INTCIO,
+    ASPEED_DEV_IOEXP1_I2C,
+    ASPEED_DEV_IOEXP1_I3C,
+    ASPEED_DEV_IOEXP1_INTCIO,
+    ASPEED_DEV_IPC0,
+    ASPEED_DEV_IPC1,
+    ASPEED_DEV_JTAG0,
+    ASPEED_DEV_JTAG1,
+    ASPEED_DEV_KCS,
+    ASPEED_DEV_LPC,
+    ASPEED_DEV_LTPI_CTRL1,
+    ASPEED_DEV_LTPI_CTRL2,
+    ASPEED_DEV_LTPI_IO0,
+    ASPEED_DEV_LTPI_IO1,
+    ASPEED_DEV_MII1,
+    ASPEED_DEV_MII2,
+    ASPEED_DEV_MII3,
+    ASPEED_DEV_MII4,
+    ASPEED_DEV_OTP,
     ASPEED_DEV_PCIE0,
     ASPEED_DEV_PCIE1,
     ASPEED_DEV_PCIE2,
@@ -262,49 +254,57 @@ enum {
     ASPEED_DEV_PCIE_MMIO1,
     ASPEED_DEV_PCIE_MMIO2,
     ASPEED_DEV_PECI,
-    ASPEED_DEV_ETH1,
-    ASPEED_DEV_ETH2,
-    ASPEED_DEV_ETH3,
-    ASPEED_DEV_ETH4,
-    ASPEED_DEV_MII1,
-    ASPEED_DEV_MII2,
-    ASPEED_DEV_MII3,
-    ASPEED_DEV_MII4,
-    ASPEED_DEV_SDRAM,
-    ASPEED_DEV_XDMA,
-    ASPEED_DEV_EMMC,
-    ASPEED_DEV_KCS,
-    ASPEED_DEV_HACE,
-    ASPEED_DEV_ACRY,
-    ASPEED_DEV_DPMCU,
-    ASPEED_DEV_DP,
-    ASPEED_DEV_I3C,
-    ASPEED_DEV_ESPI,
-    ASPEED_DEV_UDC,
-    ASPEED_DEV_JTAG0,
-    ASPEED_DEV_JTAG1,
-    ASPEED_DEV_FSI1,
-    ASPEED_DEV_FSI2,
+    ASPEED_DEV_PRIC0,
+    ASPEED_DEV_PRIC1,
+    ASPEED_DEV_PWM,
+    ASPEED_DEV_RTC,
+    ASPEED_DEV_SBC,
+    ASPEED_DEV_SCU,
     ASPEED_DEV_SCUIO,
+    ASPEED_DEV_SDHCI,
+    ASPEED_DEV_SDMC,
+    ASPEED_DEV_SDRAM,
+    ASPEED_DEV_SGPIOM0,
+    ASPEED_DEV_SGPIOM1,
     ASPEED_DEV_SLI,
     ASPEED_DEV_SLIIO,
+    ASPEED_DEV_SPI_BOOT,
+    ASPEED_DEV_SPI0,
+    ASPEED_DEV_SPI1,
+    ASPEED_DEV_SPI2,
+    ASPEED_DEV_SRAM0,
+    ASPEED_DEV_SRAM1,
+    ASPEED_DEV_TIMER1,
+    ASPEED_DEV_TIMER2,
+    ASPEED_DEV_TIMER3,
+    ASPEED_DEV_TIMER4,
+    ASPEED_DEV_TIMER5,
+    ASPEED_DEV_TIMER6,
+    ASPEED_DEV_TIMER7,
+    ASPEED_DEV_TIMER8,
+    ASPEED_DEV_UART0,
+    ASPEED_DEV_UART1,
+    ASPEED_DEV_UART2,
+    ASPEED_DEV_UART3,
+    ASPEED_DEV_UART4,
+    ASPEED_DEV_UART5,
+    ASPEED_DEV_UART6,
+    ASPEED_DEV_UART7,
+    ASPEED_DEV_UART8,
+    ASPEED_DEV_UART9,
+    ASPEED_DEV_UART10,
+    ASPEED_DEV_UART11,
+    ASPEED_DEV_UART12,
+    ASPEED_DEV_UART13,
+    ASPEED_DEV_UDC,
+    ASPEED_DEV_VBOOTROM,
+    ASPEED_DEV_VIC,
+    ASPEED_DEV_VIDEO,
+    ASPEED_DEV_VUART,
+    ASPEED_DEV_WDT,
+    ASPEED_DEV_XDMA,
     ASPEED_GIC_DIST,
     ASPEED_GIC_REDIST,
-    ASPEED_DEV_IPC0,
-    ASPEED_DEV_IPC1,
-    ASPEED_DEV_LTPI_CTRL1,
-    ASPEED_DEV_LTPI_CTRL2,
-    ASPEED_DEV_LTPI_IO0,
-    ASPEED_DEV_LTPI_IO1,
-    ASPEED_DEV_IOEXP0_I2C,
-    ASPEED_DEV_IOEXP1_I2C,
-    ASPEED_DEV_IOEXP0_INTCIO,
-    ASPEED_DEV_IOEXP1_INTCIO,
-    ASPEED_DEV_IOEXP0_I3C,
-    ASPEED_DEV_IOEXP1_I3C,
-    ASPEED_DEV_PRIC0,
-    ASPEED_DEV_PRIC1,
-    ASPEED_DEV_OTP,
 };
 
 const char *aspeed_soc_cpu_type(const char * const *valid_cpu_types);
-- 
2.53.0

Re: [PATCH v1 00/11] hw/arm/aspeed: Use unimp array and table for unimplemented devices

[Cédric Le Goater]

On 9/30/26 09:25, Jamin Lin wrote:
> v1:
>   1. Use unimp array and table for unimplemented devices
>   2. Sort device IDs alphabetically
> 
> Jamin Lin (11):
>    hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed_ast2600: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed_ast10x0: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed_ast1040: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed: Remove unused unimplemented device fields
>    hw/arm/aspeed_ast27x0: Use unimp array and table for unimplemented
>      devices for SSP/TSP
>    hw/arm/aspeed_coprocessor: Remove unused unimplemented device fields
>    hw/arm/aspeed_ast1700: Use unimp array and table for unimplemented
>      devices
>    hw/arm/aspeed_ast1700: Remove unused unimplemented device field
>    hw/arm/aspeed: Sort device IDs alphabetically
> 
>   include/hw/arm/aspeed_ast1700.h     |   3 +-
>   include/hw/arm/aspeed_coprocessor.h |   5 +-
>   include/hw/arm/aspeed_soc.h         | 195 ++++++++++++++--------------
>   hw/arm/aspeed_ast1040.c             |  49 ++++---
>   hw/arm/aspeed_ast10x0.c             |  64 ++++-----
>   hw/arm/aspeed_ast1700.c             |  42 +++---
>   hw/arm/aspeed_ast2400.c             |  33 +++--
>   hw/arm/aspeed_ast2600.c             |  53 ++++----
>   hw/arm/aspeed_ast27x0-ssp.c         |  56 ++++----
>   hw/arm/aspeed_ast27x0-tsp.c         |  56 ++++----
>   hw/arm/aspeed_ast27x0.c             |  49 +++----
>   11 files changed, 287 insertions(+), 318 deletions(-)
> 

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.