Hi Cédric > Subject: Re: [PATCH v4 18/21] hw/arm/ast27x0: Share single SCUIO instance > across PSP, SSP, and TSP > > On 4/17/26 05:29, Jamin Lin wrote: > > AST2700 has a single SCUIO hardware block, memory-mapped at > > 0x14C02000–0x14C03FFF from the perspective of the main CA35 processor > (PSP). > > The SSP and TSP coprocessors access this same SCUIO block at different > > addresses: 0x74C02000–0x74C03FFF. > > > > Previously, each subsystem (PSP, SSP, and TSP) instantiated its own > > SCUIO device, resulting in three independent SCUIO instances in the QEMU > model. > > In real hardware, however, only a single SCUIO exists and is shared > > among all processors. > > > > This commit reworks the SCUIO model to correctly reflect the hardware > > behavior by allowing SSP and TSP to reference the PSP’s SCUIO instance. > > The following changes are introduced: > > > > - Add a scuio property to Aspeed27x0CoprocessorState for linking the > > coprocessor to the PSP’s SCUIO instance. > > - Replace per-coprocessor SCUIO instantiation with a shared SCUIO link. > > - Add "MemoryRegion scuio_alias" to model address remapping for SSP and > TSP. > > - Create SCUIO alias regions in both SSP and TSP coprocessors and map > > them at 0x74C02000 to mirror the PSP’s SCUIO registers. > > - Ensure the SCUIO device in PSP is realized before SSP/TSP alias setup. > > > > With this change, PSP, SSP, and TSP now share a consistent SCUIO > > state, matching the single-SCUIO hardware design of AST2700. > > > > Signed-off-by: Jamin Lin <[email protected]> > > --- > > include/hw/arm/aspeed_coprocessor.h | 4 ++-- > > hw/arm/aspeed_ast27x0-fc.c | 4 ++++ > > hw/arm/aspeed_ast27x0-ssp.c | 14 +++++++++----- > > hw/arm/aspeed_ast27x0-tsp.c | 14 +++++++++----- > > 4 files changed, 24 insertions(+), 12 deletions(-) > > > > diff --git a/include/hw/arm/aspeed_coprocessor.h > > b/include/hw/arm/aspeed_coprocessor.h > > index 7750569eed..23c3b97f06 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; > > > > - AspeedSCUState scuio; > > AspeedTimerCtrlState timerctrl; > > SerialMM *uart; > > int uart_dev; > > @@ -45,15 +44,16 @@ struct Aspeed27x0CoprocessorState { > > AspeedCoprocessorState parent; > > AspeedINTCState intc[2]; > > UnimplementedDeviceState ipc[2]; > > - UnimplementedDeviceState scuio; > > UnimplementedDeviceState pric[2]; > > UnimplementedDeviceState otp; > > > > ARMv7MState armv7m; > > > > MemoryRegion scu_alias; > > + MemoryRegion scuio_alias; > > MemoryRegion fmc_alias; > > Aspeed2700SCUState *scu; > > + AspeedSCUState *scuio; > > AspeedSMCState *fmc; > > }; > > > > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c > > index 56dd86e2c2..d1ff6fbd4d 100644 > > --- a/hw/arm/aspeed_ast27x0-fc.c > > +++ b/hw/arm/aspeed_ast27x0-fc.c > > @@ -161,6 +161,8 @@ static bool ast2700fc_ssp_init(MachineState > *machine, Error **errp) > > OBJECT(&psp->sram), &error_abort); > > object_property_set_link(OBJECT(&s->ssp), "scu", > > OBJECT(&s->ca35.scu), > &error_abort); > > + object_property_set_link(OBJECT(&s->ssp), "scuio", > > + OBJECT(&psp->scuio), &error_abort); > > object_property_set_link(OBJECT(&s->ssp), "fmc", > > OBJECT(&psp->fmc), &error_abort); > > if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) { @@ -195,6 > > +197,8 @@ static bool ast2700fc_tsp_init(MachineState *machine, Error > **errp) > > OBJECT(&psp->sram), &error_abort); > > object_property_set_link(OBJECT(&s->tsp), "scu", > > OBJECT(&s->ca35.scu), > &error_abort); > > + object_property_set_link(OBJECT(&s->tsp), "scuio", > > + OBJECT(&psp->scuio), &error_abort); > > object_property_set_link(OBJECT(&s->tsp), "fmc", > > OBJECT(&psp->fmc), &error_abort); > > if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) { diff --git > > a/hw/arm/aspeed_ast27x0-ssp.c b/hw/arm/aspeed_ast27x0-ssp.c index > > 78bd6f342c..6c8945ce6c 100644 > > --- a/hw/arm/aspeed_ast27x0-ssp.c > > +++ b/hw/arm/aspeed_ast27x0-ssp.c > > @@ -143,8 +143,6 @@ static void aspeed_soc_ast27x0ssp_init(Object *obj) > > TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "ipc1", &a->ipc[1], > > TYPE_UNIMPLEMENTED_DEVICE); > > - object_initialize_child(obj, "scuio", &a->scuio, > > - TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "pric0", &a->pric[0], > > TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "pric1", &a->pric[1], @@ -215,6 > > +213,13 @@ static void aspeed_soc_ast27x0ssp_realize(DeviceState > *dev_soc, Error **errp) > > memory_region_size(&a->scu->dram_remap_alias[1]), > > &a->scu->dram_remap_alias[0]); > > > > + /* SCUIO */ > > + memory_region_init_alias(&a->scuio_alias, OBJECT(a), "scuio.alias", > > + &a->scuio->iomem, 0, > > + > memory_region_size(&a->scuio->iomem)); > > + memory_region_add_subregion(s->memory, > sc->memmap[ASPEED_DEV_SCUIO], > > + &a->scuio_alias); > > + > > /* INTC */ > > if (!sysbus_realize(SYS_BUS_DEVICE(&a->intc[0]), errp)) { > > return; > > @@ -282,9 +287,6 @@ static void > aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp) > > 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->scuio), > > - "aspeed.scuio", > > - > sc->memmap[ASPEED_DEV_SCUIO], 0x1000); > > aspeed_mmio_map_unimplemented(s->memory, > SYS_BUS_DEVICE(&a->pric[0]), > > "aspeed.pric0", > > > sc->memmap[ASPEED_DEV_PRIC0], > > 0x1000); @@ -299,6 +301,8 @@ static void > aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp) > > static const Property aspeed_27x0_coprocessor_properties[] = { > > DEFINE_PROP_LINK("scu", Aspeed27x0CoprocessorState, scu, > > TYPE_ASPEED_2700_SCU, Aspeed2700SCUState > *), > > + DEFINE_PROP_LINK("scuio", Aspeed27x0CoprocessorState, scuio, > > + TYPE_ASPEED_SCU, AspeedSCUState *), > > Check that the link is set in the realize routines. > Thanks for the review and suggestion. Will fix it. Jamin > Thanks, > > C. > > > > DEFINE_PROP_LINK("fmc", Aspeed27x0CoprocessorState, fmc, > TYPE_ASPEED_SMC, > > AspeedSMCState *), > > }; > > diff --git a/hw/arm/aspeed_ast27x0-tsp.c b/hw/arm/aspeed_ast27x0-tsp.c > > index d6448d82f5..cab7f47ac8 100644 > > --- a/hw/arm/aspeed_ast27x0-tsp.c > > +++ b/hw/arm/aspeed_ast27x0-tsp.c > > @@ -143,8 +143,6 @@ static void aspeed_soc_ast27x0tsp_init(Object *obj) > > TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "ipc1", &a->ipc[1], > > TYPE_UNIMPLEMENTED_DEVICE); > > - object_initialize_child(obj, "scuio", &a->scuio, > > - TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "pric0", &a->pric[0], > > TYPE_UNIMPLEMENTED_DEVICE); > > object_initialize_child(obj, "pric1", &a->pric[1], @@ -212,6 > > +210,13 @@ static void aspeed_soc_ast27x0tsp_realize(DeviceState > *dev_soc, Error **errp) > > /* SDRAM remap alias used by PSP to access TSP SDRAM */ > > memory_region_add_subregion(&s->sdram, 0, > > &a->scu->dram_remap_alias[2]); > > > > + /* SCUIO */ > > + memory_region_init_alias(&a->scuio_alias, OBJECT(a), "scuio.alias", > > + &a->scuio->iomem, 0, > > + > memory_region_size(&a->scuio->iomem)); > > + memory_region_add_subregion(s->memory, > sc->memmap[ASPEED_DEV_SCUIO], > > + &a->scuio_alias); > > + > > /* INTC */ > > if (!sysbus_realize(SYS_BUS_DEVICE(&a->intc[0]), errp)) { > > return; > > @@ -279,9 +284,6 @@ static void > aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp) > > 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->scuio), > > - "aspeed.scuio", > > - > sc->memmap[ASPEED_DEV_SCUIO], 0x1000); > > aspeed_mmio_map_unimplemented(s->memory, > SYS_BUS_DEVICE(&a->pric[0]), > > "aspeed.pric0", > > > sc->memmap[ASPEED_DEV_PRIC0], > > 0x1000); @@ -296,6 +298,8 @@ static void > aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp) > > static const Property aspeed_27x0_coprocessor_properties[] = { > > DEFINE_PROP_LINK("scu", Aspeed27x0CoprocessorState, scu, > > TYPE_ASPEED_2700_SCU, Aspeed2700SCUState > *), > > + DEFINE_PROP_LINK("scuio", Aspeed27x0CoprocessorState, scuio, > > + TYPE_ASPEED_SCU, AspeedSCUState *), > > DEFINE_PROP_LINK("fmc", Aspeed27x0CoprocessorState, fmc, > TYPE_ASPEED_SMC, > > AspeedSMCState *), > > };
