Hi Cédric ************* Email Confidentiality Notice ******************** 免責聲明: 本信件(或其附件)可能包含機密資訊，並受法律保護。如 台端非指定之收件者，請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作!
DISCLAIMER: This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you. > -----Original Message----- > From: Jamin Lin > Sent: Tuesday, July 7, 2026 11:38 AM > To: 'Cédric Le Goater' <[email protected]>; Peter Maydell > <[email protected]>; Steven Lee <[email protected]>; Troy > Lee <[email protected]>; Kane Chen <[email protected]>; > Andrew Jeffery <[email protected]>; Joel Stanley > <[email protected]>; Pierrick Bouvier <[email protected]>; Alistair > Francis <[email protected]>; Kevin Wolf <[email protected]>; Hanna > Reitz <[email protected]>; open list:ASPEED BMCs <[email protected]>; > open list:All patches CC here <[email protected]>; open list:Block layer > core <[email protected]> > Cc: Troy Lee <[email protected]> > Subject: RE: [PATCH v4 11/21] hw/arm/ast27x0: Share FMC controller with SSP > and TSP > > Hi Cédric, > > > Subject: Re: [PATCH v4 11/21] hw/arm/ast27x0: Share FMC controller > > with SSP and TSP > > > > On 4/17/26 05:28, Jamin Lin wrote: > > > AST2700 provides a single FMC controller shared by the main CA35 > > > processor > > > (PSP) and the SSP/TSP coprocessors. > > > > > > From the PSP perspective, the FMC controller is memory-mapped at > > > 0x14000000–0x140000FF. The SSP and TSP access the same controller > > > through a different address window at 0x74000000–0x740000FF. > > > > > > This change allows the SSP and TSP SoC models to reference the > > > existing PSP FMC instance instead of creating independent controllers. > > > An MMIO alias is added in the SSP and TSP address spaces to map > > > their FMC access window to the shared FMC device. > > > > > > This ensures consistent FMC state across PSP, SSP, and TSP and > > > matches the AST2700 hardware design. > > > > > > Signed-off-by: Jamin Lin <[email protected]> > > > --- > > > include/hw/arm/aspeed_coprocessor.h | 2 ++ > > > hw/arm/aspeed_ast27x0-fc.c | 4 ++++ > > > hw/arm/aspeed_ast27x0-ssp.c | 10 ++++++++++ > > > hw/arm/aspeed_ast27x0-tsp.c | 10 ++++++++++ > > > 4 files changed, 26 insertions(+) > > > > > > diff --git a/include/hw/arm/aspeed_coprocessor.h > > > b/include/hw/arm/aspeed_coprocessor.h > > > index 4db995d251..2460a11401 100644 > > > --- a/include/hw/arm/aspeed_coprocessor.h > > > +++ b/include/hw/arm/aspeed_coprocessor.h > > > @@ -50,7 +50,9 @@ struct Aspeed27x0CoprocessorState { > > > ARMv7MState armv7m; > > > > > > MemoryRegion scu_alias; > > > + MemoryRegion fmc_alias; > > > Aspeed2700SCUState *scu; > > > + AspeedSMCState *fmc; > > > }; > > > > > > #define TYPE_ASPEED27X0SSP_COPROCESSOR > > "aspeed27x0ssp-coprocessor" > > > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c > > > index 62bd6a0568..56dd86e2c2 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); > > > > I would change ast2700fc_ssp_init() and ast2700fc_tsp_init() to use a > > AspeedSoCState argument and not a MachineState. This to reflect that > > the AspeedSoCState *needs* to be realized before the coprocessors. > > A comment is worth adding. > > > Will do > > > > > + object_property_set_link(OBJECT(&s->ssp), "fmc", > > > + OBJECT(&psp->fmc), &error_abort); > > > if (!qdev_realize(DEVICE(&s->ssp), NULL, errp)) { > > > return false; > > > } > > > @@ -193,6 +195,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), "fmc", > > > + OBJECT(&psp->fmc), &error_abort); > > > if (!qdev_realize(DEVICE(&s->tsp), NULL, errp)) { > > > return false; > > > } > > > diff --git a/hw/arm/aspeed_ast27x0-ssp.c > > > b/hw/arm/aspeed_ast27x0-ssp.c index 0500c02755..bfd6fe4485 100644 > > > --- a/hw/arm/aspeed_ast27x0-ssp.c > > > +++ b/hw/arm/aspeed_ast27x0-ssp.c > > > @@ -26,6 +26,7 @@ static const hwaddr > aspeed_soc_ast27x0ssp_memmap[] > > = { > > > [ASPEED_DEV_TIMER1] = 0x72C10000, > > > [ASPEED_DEV_UART4] = 0x72C1A000, > > > [ASPEED_DEV_IPC0] = 0x72C1C000, > > > + [ASPEED_DEV_FMC] = 0x74000000, > > > [ASPEED_DEV_SCUIO] = 0x74C02000, > > > [ASPEED_DEV_INTCIO] = 0x74C18000, > > > [ASPEED_DEV_UART0] = 0x74C33000, > > > @@ -256,6 +257,13 @@ static void > > aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp) > > > sysbus_connect_irq(SYS_BUS_DEVICE(s->uart), 0, > > > aspeed_soc_ast27x0ssp_get_irq(s, > > > s->uart_dev)); > > > > Please check that a->fmc is set : > > > > > > if (!a->fmc) { > > error_setg(errp, TYPE_ASPEED27X0SSP_COPROCESSOR ": 'fmc' link > > not set"); > > return; > > } > > > > > Will add > > > > > > > > + /* FMC */ > > > + memory_region_init_alias(&a->fmc_alias, OBJECT(a), "fmc.alias", > > > + &a->fmc->mmio, 0, > > > + > > memory_region_size(&a->fmc->mmio)); > > > > Isn't that too big for 0x14000000–0x140000FF ? > Isn't that too big for 0x14000000–0x140000FF? I noticed that the MMIO region size became `0x8bf`, which is incorrect. This was introduced by patch 15: hw/ssi/aspeed_smc: Add Data FIFO-based flash access support for AST2700 https://patchwork.kernel.org/project/qemu-devel/patch/[email protected]/ The issue has been fixed in the latest v2 series: hw/ssi/aspeed_smc: Add Data FIFO-based flash access support for AST2700 https://patchew.org/QEMU/[email protected]/[email protected]/ The root cause was that `asc->nregs` is measured in 32-bit registers, but I forgot to convert the size from bytes to registers by dividing by 4. The corrected code is: ```c asc->nregs = (0x200 + (asc->cs_num_max * 0x10)) >> 2; ``` Previously, I had mistakenly omitted the `>> 2`, which caused the MMIO region size to be four times larger than intended. Thanks, Jamin > > What about the flash contents window ? Is there one in the coprocessor > socs ? > > Is is unimplemented ? > > > > > AST2700 has only one FMC controller, and it always operates in the PSP address > space. > > The FMC controller can only access the PSP view of the system, including: > > DRAM base address: 0x400000000 > FMC controller registers: 0x14000000 - 0x140000FF (PSP view) > > For the coprocessors (CM4), the FMC controller registers are visible at > 0x74000000 - 0x740000FF. However, this is simply a hardware address remap. > The hardware remaps 0x74000000 to the PSP FMC register space at > 0x14000000, so accesses from either the PSP or a coprocessor reach the same > FMC controller registers. > > Regarding DMA, the FMC controller also only understands the PSP DRAM > address space. The DMA engine reads the SCU DRAM remap configuration to > determine which CM4 address range is being accessed, then translates it into > the corresponding PSP DRAM address (starting at 0x400000000) before issuing > the DMA transaction. > > Please see the Zephyr implementation here: > https://github.com/AspeedTech-BMC/zephyr/blob/aspeed-main-v3.7.0/soc/asp > eed/ast27xx/soc.c > > Regarding the flash contents window, the PSP maps the FMC flash memory at > 0x100000000, since the FMC controller only exposes the PSP address space. > The coprocessors do not have a direct flash memory window. They can access > the flash only through the FMC Data FIFO interface. The Zephyr driver > implements this FIFO-based access mechanism: > https://github.com/AspeedTech-BMC/zephyr/blob/aspeed-main-v3.7.0/drivers/ > spi/spi_aspeed_ast2700.c > > This is also the reason why I added Patch 15 ("hw/ssi/aspeed_smc: Add Data > FIFO-based flash access support for AST2700") in this patch series: > https://patchwork.kernel.org/project/qemu-devel/patch/20260417032837.266 > [email protected]/ > > Thanks, > Jamin > > > > + memory_region_add_subregion(s->memory, > > sc->memmap[ASPEED_DEV_FMC], > > > + &a->fmc_alias); > > > + > > > aspeed_mmio_map_unimplemented(s->memory, > > SYS_BUS_DEVICE(&s->timerctrl), > > > "aspeed.timerctrl", > > > > > sc->memmap[ASPEED_DEV_TIMER1], > > > 0x200); @@ -273,6 +281,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("fmc", Aspeed27x0CoprocessorState, fmc, > > TYPE_ASPEED_SMC, > > > + AspeedSMCState *), > > > }; > > > > > > static void aspeed_soc_ast27x0ssp_class_init(ObjectClass *klass, > > > diff --git a/hw/arm/aspeed_ast27x0-tsp.c > > > b/hw/arm/aspeed_ast27x0-tsp.c index 4da4741d8a..166dcb14ed 100644 > > > --- a/hw/arm/aspeed_ast27x0-tsp.c > > > +++ b/hw/arm/aspeed_ast27x0-tsp.c > > > @@ -26,6 +26,7 @@ static const hwaddr > aspeed_soc_ast27x0tsp_memmap[] > > = { > > > [ASPEED_DEV_TIMER1] = 0x72C10000, > > > [ASPEED_DEV_UART4] = 0x72C1A000, > > > [ASPEED_DEV_IPC0] = 0x72C1C000, > > > + [ASPEED_DEV_FMC] = 0x74000000, > > > [ASPEED_DEV_SCUIO] = 0x74C02000, > > > [ASPEED_DEV_INTCIO] = 0x74C18000, > > > [ASPEED_DEV_UART0] = 0x74C33000, > > > @@ -253,6 +254,13 @@ static void > > aspeed_soc_ast27x0tsp_realize(DeviceState *dev_soc, Error **errp) > > > sysbus_connect_irq(SYS_BUS_DEVICE(s->uart), 0, > > > aspeed_soc_ast27x0tsp_get_irq(s, > > > s->uart_dev)); > > > > > > + /* FMC */ > > > + memory_region_init_alias(&a->fmc_alias, OBJECT(a), "fmc.alias", > > > + &a->fmc->mmio, 0, > > > + > > memory_region_size(&a->fmc->mmio)); > > > + memory_region_add_subregion(s->memory, > > sc->memmap[ASPEED_DEV_FMC], > > > + &a->fmc_alias); > > > + > > > aspeed_mmio_map_unimplemented(s->memory, > > SYS_BUS_DEVICE(&s->timerctrl), > > > "aspeed.timerctrl", > > > > > sc->memmap[ASPEED_DEV_TIMER1], > > > 0x200); @@ -270,6 +278,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("fmc", Aspeed27x0CoprocessorState, fmc, > > TYPE_ASPEED_SMC, > > > + AspeedSMCState *), > > > }; > > > > > > static void aspeed_soc_ast27x0tsp_class_init(ObjectClass *klass,
