RE: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to AST1700 model
Hi Cédric > Subject: Re: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to > AST1700 model > > On 12/8/25 19:21, Nabih Estefan wrote: > > On Sun, Dec 7, 2025 at 11:48 PM Kane Chen via > wrote: > >> > >> From: Kane-Chen-AS > >> > >> Connect the SGPIOM device to AST1700 model. > >> > >> Signed-off-by: Kane-Chen-AS > >> --- > >> include/hw/arm/aspeed_ast1700.h | 3 +++ > >> hw/arm/aspeed_ast1700.c | 20 > >> 2 files changed, 23 insertions(+) > >> > >> diff --git a/include/hw/arm/aspeed_ast1700.h > >> b/include/hw/arm/aspeed_ast1700.h index 7292719dc2..490f2a3b05 > 100644 > >> --- a/include/hw/arm/aspeed_ast1700.h > >> +++ b/include/hw/arm/aspeed_ast1700.h > >> @@ -12,6 +12,7 @@ > >> #include "hw/misc/aspeed_scu.h" > >> #include "hw/adc/aspeed_adc.h" > >> #include "hw/gpio/aspeed_gpio.h" > >> +#include "hw/gpio/aspeed_sgpio.h" > > > > As far as I can tell this depends on Yubin Zou's SGPIO series (link below)? > > Does that mean the series looks good? Can you reply to the series > > itself if it is? > > > > > > https://lore.kernel.org//qemu-devel/20251106-aspeed-sgpio-v1-0-b026093 > > [email protected] > > > Nabih, Kane, Jamin, > > Could you please help review Yubin's series ? > Cédric, Nabih Sorry, I’m currently stuck with AST2700 A2 SDK development. I’ve asked Kane to help test the Yubin's SGPIO model. Once I have time, I will try to test Joe’s I3C model. Jamin > Thanks, > > C. > > > >> #include "hw/i2c/aspeed_i2c.h" > >> #include "hw/misc/aspeed_ltpi.h" > >> #include "hw/misc/aspeed_pwm.h" > >> @@ -19,6 +20,7 @@ > >> #include "hw/watchdog/wdt_aspeed.h" > >> #include "hw/char/serial-mm.h" > >> > >> +#define AST1700_SGPIO_NUM2 > >> #define AST1700_WDT_NUM 9 > >> > >> #define TYPE_ASPEED_AST1700 "aspeed.ast1700" > >> @@ -39,6 +41,7 @@ struct AspeedAST1700SoCState { > >> AspeedADCState adc; > >> AspeedSCUState scu; > >> AspeedGPIOState gpio; > >> +AspeedSGPIOState sgpiom[AST1700_SGPIO_NUM]; > >> AspeedI2CState i2c; > >> AspeedPWMState pwm; > >> AspeedWDTState wdt[AST1700_WDT_NUM]; diff --git > >> a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c index > >> c9d7a97a80..e027ae02ad 100644 > >> --- a/hw/arm/aspeed_ast1700.c > >> +++ b/hw/arm/aspeed_ast1700.c > >> @@ -23,6 +23,8 @@ enum { > >> ASPEED_AST1700_DEV_ADC, > >> ASPEED_AST1700_DEV_SCU, > >> ASPEED_AST1700_DEV_GPIO, > >> +ASPEED_AST1700_DEV_SGPIOM0, > >> +ASPEED_AST1700_DEV_SGPIOM1, > >> ASPEED_AST1700_DEV_I2C, > >> ASPEED_AST1700_DEV_UART12, > >> ASPEED_AST1700_DEV_LTPI_CTRL, > >> @@ -37,6 +39,8 @@ static const hwaddr aspeed_ast1700_io_memmap[] = > { > >> [ASPEED_AST1700_DEV_ADC] = 0x00C0, > >> [ASPEED_AST1700_DEV_SCU] = 0x00C02000, > >> [ASPEED_AST1700_DEV_GPIO] = 0x00C0B000, > >> +[ASPEED_AST1700_DEV_SGPIOM0] = 0x00C0C000, > >> +[ASPEED_AST1700_DEV_SGPIOM1] = 0x00C0D000, > >> [ASPEED_AST1700_DEV_I2C] = 0x00C0F000, > >> [ASPEED_AST1700_DEV_UART12]= 0x00C33B00, > >> [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000, @@ -142,6 > +146,16 > >> @@ 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 */ > >> +for (int i = 0; i < AST1700_SGPIO_NUM; i++) { > >> +if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) { > >> +return; > >> +} > >> +memory_region_add_subregion(&s->iomem, > >> + > aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SGPIOM0 + i], > >> + > sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sgpiom[i]), 0)); > >> +} > >> + > >> /* WDT */ > >> for (int i = 0; i < AST1700_WDT_NUM; i++) { > >> AspeedWDTClass *awc = > ASPEED_WDT_GET_CLASS(&s->wdt[i]); @@ > >> -194,6 +208,12 @@ static void aspeed_ast1700_instance_init(Object *obj) > >> object_initialize_child(obj, "ltpi-ctrl", > >> &s->ltpi, TYPE_ASPEED_LTPI); > >> > >> +/* SGPIOM */ > >> +for (int 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++) { > >> object_initialize_child(obj, "ioexp-wdt[*]", > >> -- > >> 2.43.0 > >> > >>
RE: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to AST1700 model
> -Original Message- > From: Cédric Le Goater > Sent: Tuesday, December 9, 2025 4:49 PM > To: Nabih Estefan ; Kane Chen > > Cc: Peter Maydell ; Steven Lee > ; Troy Lee ; Jamin Lin > ; Andrew Jeffery > ; Joel Stanley ; open > list:ASPEED BMCs ; open list:All patches CC here > ; Troy Lee > Subject: Re: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to > AST1700 model > > On 12/8/25 19:21, Nabih Estefan wrote: > > On Sun, Dec 7, 2025 at 11:48 PM Kane Chen via > wrote: > >> > >> From: Kane-Chen-AS > >> > >> Connect the SGPIOM device to AST1700 model. > >> > >> Signed-off-by: Kane-Chen-AS > >> --- > >> include/hw/arm/aspeed_ast1700.h | 3 +++ > >> hw/arm/aspeed_ast1700.c | 20 > >> 2 files changed, 23 insertions(+) > >> > >> diff --git a/include/hw/arm/aspeed_ast1700.h > >> b/include/hw/arm/aspeed_ast1700.h index 7292719dc2..490f2a3b05 > 100644 > >> --- a/include/hw/arm/aspeed_ast1700.h > >> +++ b/include/hw/arm/aspeed_ast1700.h > >> @@ -12,6 +12,7 @@ > >> #include "hw/misc/aspeed_scu.h" > >> #include "hw/adc/aspeed_adc.h" > >> #include "hw/gpio/aspeed_gpio.h" > >> +#include "hw/gpio/aspeed_sgpio.h" > > > > As far as I can tell this depends on Yubin Zou's SGPIO series (link below)? > > Does that mean the series looks good? Can you reply to the series > > itself if it is? > > > > > > https://lore.kernel.org//qemu-devel/20251106-aspeed-sgpio-v1-0-b026093 > > [email protected] > > > Nabih, Kane, Jamin, > > Could you please help review Yubin's series ? > > Thanks, > > C. I will test Yubin’s series, and I will let you know if I encounter anything unexpected. Best Regards, Kane > > > >> #include "hw/i2c/aspeed_i2c.h" > >> #include "hw/misc/aspeed_ltpi.h" > >> #include "hw/misc/aspeed_pwm.h" > >> @@ -19,6 +20,7 @@ > >> #include "hw/watchdog/wdt_aspeed.h" > >> #include "hw/char/serial-mm.h" > >> > >> +#define AST1700_SGPIO_NUM2 > >> #define AST1700_WDT_NUM 9 > >> > >> #define TYPE_ASPEED_AST1700 "aspeed.ast1700" > >> @@ -39,6 +41,7 @@ struct AspeedAST1700SoCState { > >> AspeedADCState adc; > >> AspeedSCUState scu; > >> AspeedGPIOState gpio; > >> +AspeedSGPIOState sgpiom[AST1700_SGPIO_NUM]; > >> AspeedI2CState i2c; > >> AspeedPWMState pwm; > >> AspeedWDTState wdt[AST1700_WDT_NUM]; diff --git > >> a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c index > >> c9d7a97a80..e027ae02ad 100644 > >> --- a/hw/arm/aspeed_ast1700.c > >> +++ b/hw/arm/aspeed_ast1700.c > >> @@ -23,6 +23,8 @@ enum { > >> ASPEED_AST1700_DEV_ADC, > >> ASPEED_AST1700_DEV_SCU, > >> ASPEED_AST1700_DEV_GPIO, > >> +ASPEED_AST1700_DEV_SGPIOM0, > >> +ASPEED_AST1700_DEV_SGPIOM1, > >> ASPEED_AST1700_DEV_I2C, > >> ASPEED_AST1700_DEV_UART12, > >> ASPEED_AST1700_DEV_LTPI_CTRL, > >> @@ -37,6 +39,8 @@ static const hwaddr aspeed_ast1700_io_memmap[] > = { > >> [ASPEED_AST1700_DEV_ADC] = 0x00C0, > >> [ASPEED_AST1700_DEV_SCU] = 0x00C02000, > >> [ASPEED_AST1700_DEV_GPIO] = 0x00C0B000, > >> +[ASPEED_AST1700_DEV_SGPIOM0] = 0x00C0C000, > >> +[ASPEED_AST1700_DEV_SGPIOM1] = 0x00C0D000, > >> [ASPEED_AST1700_DEV_I2C] = 0x00C0F000, > >> [ASPEED_AST1700_DEV_UART12]= 0x00C33B00, > >> [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000, @@ -142,6 > +146,16 > >> @@ 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 */ > >> +for (int i = 0; i < AST1700_SGPIO_NUM; i++) { > >> +if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) { > >> +return; > >> +} > >> +memory_region_add_subregion(&s->iomem, > >> + > aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SGPIOM0 + i], > >> + > sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sgpiom[i]), 0)); > >> +} > >> + > >> /* WDT */ > >> for (int i = 0; i < AST1700_WDT_NUM; i++) { > >> AspeedWDTClass *awc = > ASPEED_WDT_GET_CLASS(&s->wdt[i]); @@ > >> -194,6 +208,12 @@ static void aspeed_ast1700_instance_init(Object *obj) > >> object_initialize_child(obj, "ltpi-ctrl", > >> &s->ltpi, TYPE_ASPEED_LTPI); > >> > >> +/* SGPIOM */ > >> +for (int 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++) { > >> object_initialize_child(obj, "ioexp-wdt[*]", > >> -- > >> 2.43.0 > >> > >>
Re: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to AST1700 model
On 12/8/25 19:21, Nabih Estefan wrote: On Sun, Dec 7, 2025 at 11:48 PM Kane Chen via wrote: From: Kane-Chen-AS Connect the SGPIOM device to AST1700 model. Signed-off-by: Kane-Chen-AS --- include/hw/arm/aspeed_ast1700.h | 3 +++ hw/arm/aspeed_ast1700.c | 20 2 files changed, 23 insertions(+) diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h index 7292719dc2..490f2a3b05 100644 --- a/include/hw/arm/aspeed_ast1700.h +++ b/include/hw/arm/aspeed_ast1700.h @@ -12,6 +12,7 @@ #include "hw/misc/aspeed_scu.h" #include "hw/adc/aspeed_adc.h" #include "hw/gpio/aspeed_gpio.h" +#include "hw/gpio/aspeed_sgpio.h" As far as I can tell this depends on Yubin Zou's SGPIO series (link below)? Does that mean the series looks good? Can you reply to the series itself if it is? https://lore.kernel.org//qemu-devel/[email protected] Nabih, Kane, Jamin, Could you please help review Yubin's series ? Thanks, C. #include "hw/i2c/aspeed_i2c.h" #include "hw/misc/aspeed_ltpi.h" #include "hw/misc/aspeed_pwm.h" @@ -19,6 +20,7 @@ #include "hw/watchdog/wdt_aspeed.h" #include "hw/char/serial-mm.h" +#define AST1700_SGPIO_NUM2 #define AST1700_WDT_NUM 9 #define TYPE_ASPEED_AST1700 "aspeed.ast1700" @@ -39,6 +41,7 @@ struct AspeedAST1700SoCState { AspeedADCState adc; AspeedSCUState scu; AspeedGPIOState gpio; +AspeedSGPIOState sgpiom[AST1700_SGPIO_NUM]; AspeedI2CState i2c; AspeedPWMState pwm; AspeedWDTState wdt[AST1700_WDT_NUM]; diff --git a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c index c9d7a97a80..e027ae02ad 100644 --- a/hw/arm/aspeed_ast1700.c +++ b/hw/arm/aspeed_ast1700.c @@ -23,6 +23,8 @@ enum { ASPEED_AST1700_DEV_ADC, ASPEED_AST1700_DEV_SCU, ASPEED_AST1700_DEV_GPIO, +ASPEED_AST1700_DEV_SGPIOM0, +ASPEED_AST1700_DEV_SGPIOM1, ASPEED_AST1700_DEV_I2C, ASPEED_AST1700_DEV_UART12, ASPEED_AST1700_DEV_LTPI_CTRL, @@ -37,6 +39,8 @@ static const hwaddr aspeed_ast1700_io_memmap[] = { [ASPEED_AST1700_DEV_ADC] = 0x00C0, [ASPEED_AST1700_DEV_SCU] = 0x00C02000, [ASPEED_AST1700_DEV_GPIO] = 0x00C0B000, +[ASPEED_AST1700_DEV_SGPIOM0] = 0x00C0C000, +[ASPEED_AST1700_DEV_SGPIOM1] = 0x00C0D000, [ASPEED_AST1700_DEV_I2C] = 0x00C0F000, [ASPEED_AST1700_DEV_UART12]= 0x00C33B00, [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000, @@ -142,6 +146,16 @@ 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 */ +for (int i = 0; i < AST1700_SGPIO_NUM; i++) { +if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) { +return; +} +memory_region_add_subregion(&s->iomem, +aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SGPIOM0 + i], +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sgpiom[i]), 0)); +} + /* WDT */ for (int i = 0; i < AST1700_WDT_NUM; i++) { AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); @@ -194,6 +208,12 @@ static void aspeed_ast1700_instance_init(Object *obj) object_initialize_child(obj, "ltpi-ctrl", &s->ltpi, TYPE_ASPEED_LTPI); +/* SGPIOM */ +for (int 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++) { object_initialize_child(obj, "ioexp-wdt[*]", -- 2.43.0
Re: [PATCH v3 17/18] hw/arm/aspeed: Attach SGPIOM device to AST1700 model
On Sun, Dec 7, 2025 at 11:48 PM Kane Chen via wrote: > > From: Kane-Chen-AS > > Connect the SGPIOM device to AST1700 model. > > Signed-off-by: Kane-Chen-AS > --- > include/hw/arm/aspeed_ast1700.h | 3 +++ > hw/arm/aspeed_ast1700.c | 20 > 2 files changed, 23 insertions(+) > > diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h > index 7292719dc2..490f2a3b05 100644 > --- a/include/hw/arm/aspeed_ast1700.h > +++ b/include/hw/arm/aspeed_ast1700.h > @@ -12,6 +12,7 @@ > #include "hw/misc/aspeed_scu.h" > #include "hw/adc/aspeed_adc.h" > #include "hw/gpio/aspeed_gpio.h" > +#include "hw/gpio/aspeed_sgpio.h" As far as I can tell this depends on Yubin Zou's SGPIO series (link below)? Does that mean the series looks good? Can you reply to the series itself if it is? https://lore.kernel.org//qemu-devel/[email protected] > #include "hw/i2c/aspeed_i2c.h" > #include "hw/misc/aspeed_ltpi.h" > #include "hw/misc/aspeed_pwm.h" > @@ -19,6 +20,7 @@ > #include "hw/watchdog/wdt_aspeed.h" > #include "hw/char/serial-mm.h" > > +#define AST1700_SGPIO_NUM2 > #define AST1700_WDT_NUM 9 > > #define TYPE_ASPEED_AST1700 "aspeed.ast1700" > @@ -39,6 +41,7 @@ struct AspeedAST1700SoCState { > AspeedADCState adc; > AspeedSCUState scu; > AspeedGPIOState gpio; > +AspeedSGPIOState sgpiom[AST1700_SGPIO_NUM]; > AspeedI2CState i2c; > AspeedPWMState pwm; > AspeedWDTState wdt[AST1700_WDT_NUM]; > diff --git a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c > index c9d7a97a80..e027ae02ad 100644 > --- a/hw/arm/aspeed_ast1700.c > +++ b/hw/arm/aspeed_ast1700.c > @@ -23,6 +23,8 @@ enum { > ASPEED_AST1700_DEV_ADC, > ASPEED_AST1700_DEV_SCU, > ASPEED_AST1700_DEV_GPIO, > +ASPEED_AST1700_DEV_SGPIOM0, > +ASPEED_AST1700_DEV_SGPIOM1, > ASPEED_AST1700_DEV_I2C, > ASPEED_AST1700_DEV_UART12, > ASPEED_AST1700_DEV_LTPI_CTRL, > @@ -37,6 +39,8 @@ static const hwaddr aspeed_ast1700_io_memmap[] = { > [ASPEED_AST1700_DEV_ADC] = 0x00C0, > [ASPEED_AST1700_DEV_SCU] = 0x00C02000, > [ASPEED_AST1700_DEV_GPIO] = 0x00C0B000, > +[ASPEED_AST1700_DEV_SGPIOM0] = 0x00C0C000, > +[ASPEED_AST1700_DEV_SGPIOM1] = 0x00C0D000, > [ASPEED_AST1700_DEV_I2C] = 0x00C0F000, > [ASPEED_AST1700_DEV_UART12]= 0x00C33B00, > [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000, > @@ -142,6 +146,16 @@ 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 */ > +for (int i = 0; i < AST1700_SGPIO_NUM; i++) { > +if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) { > +return; > +} > +memory_region_add_subregion(&s->iomem, > +aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_SGPIOM0 + i], > +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->sgpiom[i]), > 0)); > +} > + > /* WDT */ > for (int i = 0; i < AST1700_WDT_NUM; i++) { > AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); > @@ -194,6 +208,12 @@ static void aspeed_ast1700_instance_init(Object *obj) > object_initialize_child(obj, "ltpi-ctrl", > &s->ltpi, TYPE_ASPEED_LTPI); > > +/* SGPIOM */ > +for (int 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++) { > object_initialize_child(obj, "ioexp-wdt[*]", > -- > 2.43.0 > >
