Re: [PATCH v2 16/17] hw/arm/aspeed: Model AST1700 SGPIOM block as unimplemented device

["Cédric Le Goater" <clg@kaod.org>]

Hello,

On 11/11/25 02:33, Kane Chen wrote:
> Hi Cédric,
> 
> Thanks for the information. I'll investigate this model and integrate it.

Help with reviewing is welcome !

Thanks,

C.




> 
> Best Regards,
> Kane
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Tuesday, November 11, 2025 12:15 AM
>> To: Kane Chen <kane_chen@aspeedtech.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
>> Lee <leetroy@gmail.com>; Jamin Lin <jamin_lin@aspeedtech.com>; Andrew
>> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>;
>> open list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC
>> here <qemu-devel@nongnu.org>; Yubin Zou <yubinz@google.com>
>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>> Subject: Re: [PATCH v2 16/17] hw/arm/aspeed: Model AST1700 SGPIOM block
>> as unimplemented device
>>
>> Hello,
>>
>> On 11/5/25 04:58, Kane Chen wrote:
>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>
>>> AST1700 includes an SGPIOM block, but QEMU has no functional model yet.
>>
>> Does the series  "hw/gpio: Add Aspeed Serial GPIO (SGPIO) controller" [1]
>> proposed by Yubin Zou fill this gap ?
>>
>> Thanks,
>>
>> C.
>>
>> [1]
>> https://lore.kernel.org//qemu-devel/20251106-aspeed-sgpio-v1-0-b0260937
>> 16fa@google.com
>>
>>
>>> Expose it as an unimplemented device so the address space is reserved
>>> and the missing functionality is explicit to users/guests.
>>>
>>> Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>> ---
>>>    include/hw/misc/aspeed_ast1700.h |  1 +
>>>    hw/misc/aspeed_ast1700.c         | 16 ++++++++++++++++
>>>    2 files changed, 17 insertions(+)
>>>
>>> diff --git a/include/hw/misc/aspeed_ast1700.h
>>> b/include/hw/misc/aspeed_ast1700.h
>>> index 4048d31154..8ada3a7775 100644
>>> --- a/include/hw/misc/aspeed_ast1700.h
>>> +++ b/include/hw/misc/aspeed_ast1700.h
>>> @@ -44,6 +44,7 @@ struct AspeedAST1700SoCState {
>>>        AspeedWDTState wdt[AST1700_WDT_NUM];
>>>
>>>        UnimplementedDeviceState i3c;
>>> +    UnimplementedDeviceState sgpiom;
>>>    };
>>>
>>>    #endif /* ASPEED_AST1700_H */
>>> diff --git a/hw/misc/aspeed_ast1700.c b/hw/misc/aspeed_ast1700.c index
>>> 37b2946fc0..66a5f21d27 100644
>>> --- a/hw/misc/aspeed_ast1700.c
>>> +++ b/hw/misc/aspeed_ast1700.c
>>> @@ -19,6 +19,7 @@
>>>    #define AST2700_SOC_LTPI_SIZE        0x01000000
>>>    #define AST1700_SOC_SRAM_SIZE        0x00040000
>>>    #define AST1700_SOC_I3C_SIZE         0x00010000
>>> +#define AST1700_SOC_SGPIOM_SIZE      0x00002000
>>>
>>>    enum {
>>>        ASPEED_AST1700_DEV_SPI0,
>>> @@ -26,6 +27,7 @@ enum {
>>>        ASPEED_AST1700_DEV_ADC,
>>>        ASPEED_AST1700_DEV_SCU,
>>>        ASPEED_AST1700_DEV_GPIO,
>>> +    ASPEED_AST1700_DEV_SGPIOM,
>>>        ASPEED_AST1700_DEV_I2C,
>>>        ASPEED_AST1700_DEV_I3C,
>>>        ASPEED_AST1700_DEV_UART12,
>>> @@ -40,6 +42,7 @@ static const hwaddr aspeed_ast1700_io_memmap[] =
>> {
>>>        [ASPEED_AST1700_DEV_ADC]       =  0x00C00000,
>>>        [ASPEED_AST1700_DEV_SCU]       =  0x00C02000,
>>>        [ASPEED_AST1700_DEV_GPIO]      =  0x00C0B000,
>>> +    [ASPEED_AST1700_DEV_SGPIOM]    =  0x00C0C000,
>>>        [ASPEED_AST1700_DEV_I2C]       =  0x00C0F000,
>>>        [ASPEED_AST1700_DEV_I3C]       =  0x00C20000,
>>>        [ASPEED_AST1700_DEV_UART12]    =  0x00C33B00,
>>> @@ -162,6 +165,15 @@ static void aspeed_ast1700_realize(DeviceState
>> *dev, Error **errp)
>>>
>> aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_LTPI_CTRL],
>>>
>>> sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->ltpi), 0));
>>>
>>> +    /* SGPIOM */
>>> +    qdev_prop_set_string(DEVICE(&s->sgpiom), "name", "ioexp-sgpiom");
>>> +    qdev_prop_set_uint64(DEVICE(&s->sgpiom), "size",
>> AST1700_SOC_SGPIOM_SIZE);
>>> +    sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom), errp);
>>> +    memory_region_add_subregion_overlap(&s->iomem,
>>> +
>> aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SGPIOM],
>>> +
>> sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sgpiom), 0),
>>> +                        -1000);
>>> +
>>>        /* WDT */
>>>        for (i = 0; i < AST1700_WDT_NUM; i++) {
>>>            AspeedWDTClass *awc =
>> ASPEED_WDT_GET_CLASS(&s->wdt[i]); @@
>>> -225,6 +237,10 @@ static void aspeed_ast1700_instance_init(Object *obj)
>>>        object_initialize_child(obj, "ltpi-ctrl",
>>>                                &s->ltpi, TYPE_ASPEED_LTPI);
>>>
>>> +    /* SGPIOM */
>>> +    object_initialize_child(obj, "ioexp-sgpiom[*]", &s->sgpiom,
>>> +                            TYPE_UNIMPLEMENTED_DEVICE);
>>> +
>>>        /* WDT */
>>>        for (i = 0; i < AST1700_WDT_NUM; i++) {
>>>            snprintf(typename, sizeof(typename), "aspeed.wdt-%s",
>>> socname);
>