> -----Original Message----- > From: Cédric Le Goater <[email protected]> > Sent: Monday, January 26, 2026 4:40 PM > To: Kane Chen <[email protected]>; Peter Maydell > <[email protected]>; Steven Lee <[email protected]>; Troy > Lee <[email protected]>; Jamin Lin <[email protected]>; Andrew > Jeffery <[email protected]>; Joel Stanley <[email protected]>; > open list:ASPEED BMCs <[email protected]>; open list:All patches CC > here <[email protected]> > Cc: Troy Lee <[email protected]>; Cédric Le Goater > <[email protected]>; Nabih Estefan <[email protected]> > Subject: Re: [PATCH v5 08/22] hw/arm/aspeed: Attach UART device to > AST1700 model > > On 1/20/26 06:18, Kane Chen via qemu development wrote: > > From: Kane-Chen-AS <[email protected]> > > > > Connect the UART controller to the AST1700 model by mapping its MMIO > > region. > > > > Signed-off-by: Kane-Chen-AS <[email protected]> > > Reviewed-by: Cédric Le Goater <[email protected]> > > Reviewed-by: Nabih Estefan <[email protected]> > > Tested-by: Nabih Estefan <[email protected]> > > --- > > include/hw/arm/aspeed_ast1700.h | 2 ++ > > hw/arm/aspeed_ast1700.c | 18 ++++++++++++++++++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/include/hw/arm/aspeed_ast1700.h > > b/include/hw/arm/aspeed_ast1700.h index addea3ab1f..b15b13aedd > 100644 > > --- a/include/hw/arm/aspeed_ast1700.h > > +++ b/include/hw/arm/aspeed_ast1700.h > > @@ -10,6 +10,7 @@ > > > > #include "hw/core/sysbus.h" > > #include "hw/misc/aspeed_ltpi.h" > > +#include "hw/char/serial-mm.h" > > > > #define TYPE_ASPEED_AST1700 "aspeed.ast1700" > > > > @@ -21,6 +22,7 @@ struct AspeedAST1700SoCState { > > MemoryRegion iomem; > > > > AspeedLTPIState ltpi; > > + SerialMM uart; > > }; > > > > #endif /* ASPEED_AST1700_H */ > > diff --git a/hw/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c index > > e4c8565d3f..450ca6f5c7 100644 > > --- a/hw/arm/aspeed_ast1700.c > > +++ b/hw/arm/aspeed_ast1700.c > > @@ -9,15 +9,18 @@ > > #include "qemu/osdep.h" > > #include "hw/core/boards.h" > > #include "qom/object.h" > > +#include "hw/core/qdev-properties.h" > > #include "hw/arm/aspeed_ast1700.h" > > > > #define AST2700_SOC_LTPI_SIZE 0x01000000 > > > > enum { > > + ASPEED_AST1700_DEV_UART12, > > ASPEED_AST1700_DEV_LTPI_CTRL, > > }; > > > > static const hwaddr aspeed_ast1700_io_memmap[] = { > > + [ASPEED_AST1700_DEV_UART12] = 0x00C33B00, > > [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000, > > }; > > > > @@ -31,6 +34,17 @@ static void aspeed_ast1700_realize(DeviceState *dev, > Error **errp) > > AST2700_SOC_LTPI_SIZE); > > sysbus_init_mmio(sbd, &s->iomem); > > > > + /* UART */ > > + qdev_prop_set_uint8(DEVICE(&s->uart), "regshift", 2); > > + qdev_prop_set_uint32(DEVICE(&s->uart), "baudbase", 38400); > > + qdev_prop_set_uint8(DEVICE(&s->uart), "endianness", > DEVICE_LITTLE_ENDIAN); > > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->uart), errp)) { > > + return; > > + } > > + memory_region_add_subregion(&s->iomem, > > + > aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_UART12], > > + > > + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart), 0)); > > + > > /* LTPI controller */ > > if (!sysbus_realize(SYS_BUS_DEVICE(&s->ltpi), errp)) { > > return; > > @@ -44,6 +58,10 @@ static void aspeed_ast1700_instance_init(Object > *obj) > > { > > AspeedAST1700SoCState *s = ASPEED_AST1700(obj); > > > > + /* UART */ > > + object_initialize_child(obj, "uart[*]", &s->uart, > > + TYPE_SERIAL_MM); > > There is only one uart device per AspeedAST1700SoCState; so you don't need > the "[*]" array suffix. > > This is true for several other devices added to AspeedAST1700SoCState. > Please remove. > > Thanks, > > C.
Hi Cédric, Thanks for your review and comments. I will remove the "[*]" array suffix in the next patch series. Best Regards, Kane > > > > + > > /* LTPI controller */ > > object_initialize_child(obj, "ltpi-ctrl", > > &s->ltpi, TYPE_ASPEED_LTPI);
