> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Friday, February 9, 2024 4:32 PM
> To: Jamin Lin <jamin_...@aspeedtech.com>; Peter Maydell
> <peter.mayd...@linaro.org>; Andrew Jeffery <and...@codeconstruct.com.au>;
> Joel Stanley <j...@jms.id.au>; open list:ASPEED BMCs
> <qemu-...@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_...@aspeedtech.com>
> Subject: Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0
>
> On 2/7/24 20:52, Jamin Lin wrote:
> > In the previous design of ASPEED SOCs QEMU model, it set the boot
> > address at "0" which was the hardcode setting for ast10x0, ast2600,
> > ast2500 and ast2400.
> >
> > According to the design of ast2700, it has bootmcu which is used for
> > executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute
> > u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) parts
> > and the boot address is "0x4 00000000" for ast2700.
> > Therefore, fixed hardcode boot address 0.
> >
> > Signed-off-by: Troy Lee <troy_...@aspeedtech.com>
> > Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
>
> I agree with Philippe that the justification could be simpler. This change is
> just
> a cleanup preparing ground for future models using a different mapping
> address.
>
Will fix
Jamin
> Reviewed-by: Cédric Le Goater <c...@kaod.org>
>
> Thanks,
>
> C.
>
>
> > ---
> > hw/arm/aspeed.c | 4 +++-
> > hw/arm/aspeed_ast2400.c | 4 ++--
> > hw/arm/aspeed_ast2600.c | 2 +-
> > include/hw/arm/aspeed_soc.h | 2 --
> > 4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > 06d863958b..39758557be 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -289,12 +289,14 @@ static void
> aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
> > uint64_t rom_size)
> > {
> > AspeedSoCState *soc = bmc->soc;
> > + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
> >
> > memory_region_init_rom(&bmc->boot_rom, NULL,
> "aspeed.boot_rom", rom_size,
> > &error_abort);
> > memory_region_add_subregion_overlap(&soc->spi_boot_container,
> 0,
> > &bmc->boot_rom, 1);
> > - write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size,
> &error_abort);
> > + write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> > + rom_size, &error_abort);
> > }
> >
> > void aspeed_board_init_flashes(AspeedSMCState *s, const char
> > *flashtype, diff --git a/hw/arm/aspeed_ast2400.c
> > b/hw/arm/aspeed_ast2400.c index 95da85fee0..d125886207 100644
> > --- a/hw/arm/aspeed_ast2400.c
> > +++ b/hw/arm/aspeed_ast2400.c
> > @@ -26,7 +26,7 @@
> > #define ASPEED_SOC_IOMEM_SIZE 0x00200000
> >
> > static const hwaddr aspeed_soc_ast2400_memmap[] = {
> > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
> > + [ASPEED_DEV_SPI_BOOT] = 0x00000000,
> > [ASPEED_DEV_IOMEM] = 0x1E600000,
> > [ASPEED_DEV_FMC] = 0x1E620000,
> > [ASPEED_DEV_SPI1] = 0x1E630000,
> > @@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] =
> {
> > };
> >
> > static const hwaddr aspeed_soc_ast2500_memmap[] = {
> > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
> > + [ASPEED_DEV_SPI_BOOT] = 0x00000000,
> > [ASPEED_DEV_IOMEM] = 0x1E600000,
> > [ASPEED_DEV_FMC] = 0x1E620000,
> > [ASPEED_DEV_SPI1] = 0x1E630000,
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index
> > f74561ecdc..174be53770 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -22,7 +22,7 @@
> > #define ASPEED_SOC_DPMCU_SIZE 0x00040000
> >
> > static const hwaddr aspeed_soc_ast2600_memmap[] = {
> > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
> > + [ASPEED_DEV_SPI_BOOT] = 0x00000000,
> > [ASPEED_DEV_SRAM] = 0x10000000,
> > [ASPEED_DEV_DPMCU] = 0x18000000,
> > /* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 5ab0902da0..bf43ad8351 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -224,8 +224,6 @@ enum {
> > ASPEED_DEV_FSI2,
> > };
> >
> > -#define ASPEED_SOC_SPI_BOOT_ADDR 0x0
> > -
> > qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
> > bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp);
> > void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev
> > *chr);
