diff mbox series

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

Message ID 20230214171830.681594-8-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series aspeed: I2C fixes, -drive removal (first step) | expand

Commit Message

Cédric Le Goater Feb. 14, 2023, 5:18 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Feb. 15, 2023, 11:02 a.m. UTC | #1
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 ...

>       [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);
Cédric Le Goater March 1, 2023, 1:27 p.m. UTC | #2
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);
>
diff mbox series

Patch

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index bd1e03e78a..db55505d14 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -58,6 +58,8 @@  struct AspeedSoCState {
     MemoryRegion *dram_mr;
     MemoryRegion dram_container;
     MemoryRegion sram;
+    MemoryRegion spi_boot_container;
+    MemoryRegion spi_boot;
     AspeedVICState vic;
     AspeedRtcState rtc;
     AspeedTimerCtrlState timerctrl;
@@ -120,6 +122,7 @@  struct AspeedSoCClass {
 
 
 enum {
+    ASPEED_DEV_SPI_BOOT,
     ASPEED_DEV_IOMEM,
     ASPEED_DEV_UART1,
     ASPEED_DEV_UART2,
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 21184f3ad4..998dc57969 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -384,18 +384,7 @@  static void aspeed_machine_init(MachineState *machine)
         MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
         uint64_t size = memory_region_size(&fl->mmio);
 
-        /*
-         * create a ROM region using the default mapping window size of
-         * the flash module. The window size is 64MB for the AST2400
-         * SoC and 128MB for the AST2500 SoC, which is twice as big as
-         * needed by the flash modules of the Aspeed machines.
-         */
-        if (ASPEED_MACHINE(machine)->mmio_exec) {
-            memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
-                                     &fl->mmio, 0, size);
-            memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
-                                        boot_rom);
-        } else {
+        if (!ASPEED_MACHINE(machine)->mmio_exec) {
             memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
                                    size, &error_abort);
             memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index bb2769df04..20f0b772d6 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -21,6 +21,7 @@ 
 #define ASPEED_SOC_DPMCU_SIZE       0x00040000
 
 static const hwaddr aspeed_soc_ast2600_memmap[] = {
+    [ASPEED_DEV_SPI_BOOT]  = 0x0,
     [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],
+                                &s->spi_boot_container);
+
     /* IO space */
     aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
                                   sc->memmap[ASPEED_DEV_IOMEM],
@@ -431,6 +438,12 @@  static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
 
+    /* Set up an alias on the FMC CE0 region (boot default) */
+    MemoryRegion *fmc0_mmio = &s->fmc.flashes[0].mmio;
+    memory_region_init_alias(&s->spi_boot, OBJECT(s), "aspeed.spi_boot",
+                             fmc0_mmio, 0, memory_region_size(fmc0_mmio));
+    memory_region_add_subregion(&s->spi_boot_container, 0x0, &s->spi_boot);
+
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
         object_property_set_link(OBJECT(&s->spi[i]), "dram",
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index e884d6badc..3507ea5818 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -25,6 +25,7 @@ 
 #define ASPEED_SOC_IOMEM_SIZE       0x00200000
 
 static const hwaddr aspeed_soc_ast2400_memmap[] = {
+    [ASPEED_DEV_SPI_BOOT]  = 0x0,
     [ASPEED_DEV_IOMEM]  = 0x1E600000,
     [ASPEED_DEV_FMC]    = 0x1E620000,
     [ASPEED_DEV_SPI1]   = 0x1E630000,
@@ -59,6 +60,7 @@  static const hwaddr aspeed_soc_ast2400_memmap[] = {
 };
 
 static const hwaddr aspeed_soc_ast2500_memmap[] = {
+    [ASPEED_DEV_SPI_BOOT]  = 0x0,
     [ASPEED_DEV_IOMEM]  = 0x1E600000,
     [ASPEED_DEV_FMC]    = 0x1E620000,
     [ASPEED_DEV_SPI1]   = 0x1E630000,
@@ -245,6 +247,12 @@  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     Error *err = NULL;
     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],
+                                &s->spi_boot_container);
+
     /* IO space */
     aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
                                   sc->memmap[ASPEED_DEV_IOMEM],
@@ -354,6 +362,12 @@  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_FMC));
 
+    /* Set up an alias on the FMC CE0 region (boot default) */
+    MemoryRegion *fmc0_mmio = &s->fmc.flashes[0].mmio;
+    memory_region_init_alias(&s->spi_boot, OBJECT(s), "aspeed.spi_boot",
+                             fmc0_mmio, 0, memory_region_size(fmc0_mmio));
+    memory_region_add_subregion(&s->spi_boot_container, 0x0, &s->spi_boot);
+
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 90c04bbc33..f4600c290b 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -100,13 +100,7 @@  static void fby35_bmc_init(Fby35State *s)
         MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
         uint64_t size = memory_region_size(&fl->mmio);
 
-        if (s->mmio_exec) {
-            memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
-                                     &fl->mmio, 0, size);
-            memory_region_add_subregion(&s->bmc_memory, FBY35_BMC_FIRMWARE_ADDR,
-                                        boot_rom);
-        } else {
-
+        if (!s->mmio_exec) {
             memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
                                    size, &error_abort);
             memory_region_add_subregion(&s->bmc_memory, FBY35_BMC_FIRMWARE_ADDR,