Re: [PATCH 7/8] aspeed: Introduce a spi_boot region under the SoC

[Cédric Le Goater]

On 2/15/23 12:02, Philippe Mathieu-Daudé wrote:
> On 14/2/23 18:18, Cédric Le Goater wrote:
> > The default boot of the Aspeed SoCs is address 0x0. For this reason,
> > the FMC flash device contents are remapped by HW on the first 256MB of
> > the address space. In QEMU, this is currently done in the machine init
> > with the setup of a region alias.
> >
> > Move this code to the SoC and introduce an extra container to prepare
> > ground for the boot ROM region which will overlap the FMC flash
> > remapping.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >   include/hw/arm/aspeed_soc.h |  3 +++
> >   hw/arm/aspeed.c             | 13 +------------
> >   hw/arm/aspeed_ast2600.c     | 13 +++++++++++++
> >   hw/arm/aspeed_soc.c         | 14 ++++++++++++++
> >   hw/arm/fby35.c              |  8 +-------
> >   5 files changed, 32 insertions(+), 19 deletions(-)
>
> >   enum {
> > +    ASPEED_DEV_SPI_BOOT,
> >       ASPEED_DEV_IOMEM,
> >       ASPEED_DEV_UART1,
> >       ASPEED_DEV_UART2,
>
>
> >   #define ASPEED_SOC_DPMCU_SIZE       0x00040000
> >   static const hwaddr aspeed_soc_ast2600_memmap[] = {
> > +    [ASPEED_DEV_SPI_BOOT]  = 0x0,
>
> Isn't this a constant address for this Soc family?
> If so, we can define ASPEED_SOC_RESET_ADDR once ...

I will introduce :

  #define ASPEED_SPI_BOOT_ADDR 0x0

and use it every where it makes sense. This should replace FIRMWARE_ADDR
in aspeed.c also.

Thanks,

C.

> >       [ASPEED_DEV_SRAM]      = 0x10000000,
> >       [ASPEED_DEV_DPMCU]     = 0x18000000,
> >       /* 0x16000000     0x17FFFFFF : AHB BUS do LPC Bus bridge */
> > @@ -282,6 +283,12 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> > Error **errp)
> >       qemu_irq irq;
> >       g_autofree char *sram_name = NULL;
> > +    /* Default boot region (SPI memory or ROMs) */
> > +    memory_region_init(&s->spi_boot_container, OBJECT(s),
> > +                       "aspeed.spi_boot_container", 0x10000000);
> > +    memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SPI_BOOT],
>
> ... and use it here.
>
> > +                                &s->spi_boot_container);