> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, January 26, 2026 4:40 PM
> 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>
> Cc: Troy Lee <troy_lee@aspeedtech.com>; Cédric Le Goater
> <clg@redhat.com>; Nabih Estefan <nabihestefan@google.com>
> 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 <kane_chen@aspeedtech.com>
> >
> > Connect the UART controller to the AST1700 model by mapping its MMIO
> > region.
> >
> > Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
> > Reviewed-by: Cédric Le Goater <clg@redhat.com>
> > Reviewed-by: Nabih Estefan <nabihestefan@google.com>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> > 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);