RE: [PATCH v2 2/2] aspeed: fix hardcode boot address 0
> -Original Message-
> From: Cédric Le Goater
> Sent: Friday, February 9, 2024 4:32 PM
> To: Jamin Lin ; Peter Maydell
> ; Andrew Jeffery ;
> Joel Stanley ; open list:ASPEED BMCs
> ; open list:All patches CC here
>
> Cc: Troy Lee
> 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 " for ast2700.
> > Therefore, fixed hardcode boot address 0.
> >
> > Signed-off-by: Troy Lee
> > Signed-off-by: Jamin Lin
>
> 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
>
> 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(>boot_rom, NULL,
> "aspeed.boot_rom", rom_size,
> > _abort);
> > memory_region_add_subregion_overlap(>spi_boot_container,
> 0,
> > >boot_rom, 1);
> > -write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size,
> _abort);
> > +write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
> > + rom_size, _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 0x0020
> >
> > static const hwaddr aspeed_soc_ast2400_memmap[] = {
> > -[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
> > +[ASPEED_DEV_SPI_BOOT] = 0x,
> > [ASPEED_DEV_IOMEM] = 0x1E60,
> > [ASPEED_DEV_FMC]= 0x1E62,
> > [ASPEED_DEV_SPI1] = 0x1E63,
> > @@ -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] = 0x,
> > [ASPEED_DEV_IOMEM] = 0x1E60,
> > [ASPEED_DEV_FMC]= 0x1E62,
> > [ASPEED_DEV_SPI1] = 0x1E63,
> > 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 0x0004
> >
> > static const hwaddr aspeed_soc_ast2600_memmap[] = {
> > -[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
> > +[ASPEED_DEV_SPI_BOOT] = 0x,
> > [ASPEED_DEV_SRAM] = 0x1000,
> > [ASPEED_DEV_DPMCU] = 0x1800,
> > /* 0x1600 0x17FF : 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);
RE: [PATCH v2 2/2] aspeed: fix hardcode boot address 0
> -Original Message- > From: Philippe Mathieu-Daudé > Sent: Thursday, February 8, 2024 4:29 AM > To: Jamin Lin ; Cédric Le Goater ; > Peter Maydell ; Andrew Jeffery > ; Joel Stanley ; open > list:ASPEED BMCs ; open list:All patches CC here > > Cc: Troy Lee > Subject: Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 > > Hi Jamin, > > On 7/2/24 20:52, Jamin Lin via 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 " for ast2700. > > This justification from here ... > > > Therefore, fixed hardcode boot address 0. > > ... to here is still unclear. You provided an explanation in previous patch, > maybe > worth including it in this description? > > Otherwise for the code changes: Thanks for review and sorry reply you late due to my Chinese new year holiday. Will add. Jamin > Reviewed-by: Philippe Mathieu-Daudé > > > Signed-off-by: Troy Lee > > Signed-off-by: Jamin Lin > > --- > > 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(-)
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 " for ast2700.
Therefore, fixed hardcode boot address 0.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
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.
Reviewed-by: Cédric Le Goater
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(>boot_rom, NULL, "aspeed.boot_rom", rom_size,
_abort);
memory_region_add_subregion_overlap(>spi_boot_container, 0,
>boot_rom, 1);
-write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, _abort);
+write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
+ rom_size, _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 0x0020
static const hwaddr aspeed_soc_ast2400_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
@@ -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] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
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 0x0004
static const hwaddr aspeed_soc_ast2600_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_SRAM] = 0x1000,
[ASPEED_DEV_DPMCU] = 0x1800,
/* 0x1600 0x17FF : 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);
Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0
Hi Jamin, On 7/2/24 20:52, Jamin Lin via 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 " for ast2700. This justification from here ... Therefore, fixed hardcode boot address 0. ... to here is still unclear. You provided an explanation in previous patch, maybe worth including it in this description? Otherwise for the code changes: Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Troy Lee Signed-off-by: Jamin Lin --- 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(-)
[PATCH v2 2/2] aspeed: fix hardcode boot address 0
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 " for ast2700.
Therefore, fixed hardcode boot address 0.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
---
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(>boot_rom, NULL, "aspeed.boot_rom", rom_size,
_abort);
memory_region_add_subregion_overlap(>spi_boot_container, 0,
>boot_rom, 1);
-write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, _abort);
+write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
+ rom_size, _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 0x0020
static const hwaddr aspeed_soc_ast2400_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
@@ -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] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
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 0x0004
static const hwaddr aspeed_soc_ast2600_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_SRAM] = 0x1000,
[ASPEED_DEV_DPMCU] = 0x1800,
/* 0x1600 0x17FF : 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);
--
2.25.1
[PATCH v2 2/2] aspeed: fix hardcode boot address 0
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 " for ast2700.
Therefore, fixed hardcode boot address 0.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
---
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(>boot_rom, NULL, "aspeed.boot_rom", rom_size,
_abort);
memory_region_add_subregion_overlap(>spi_boot_container, 0,
>boot_rom, 1);
-write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, _abort);
+write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
+ rom_size, _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 0x0020
static const hwaddr aspeed_soc_ast2400_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
@@ -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] = 0x,
[ASPEED_DEV_IOMEM] = 0x1E60,
[ASPEED_DEV_FMC]= 0x1E62,
[ASPEED_DEV_SPI1] = 0x1E63,
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 0x0004
static const hwaddr aspeed_soc_ast2600_memmap[] = {
-[ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT] = 0x,
[ASPEED_DEV_SRAM] = 0x1000,
[ASPEED_DEV_DPMCU] = 0x1800,
/* 0x1600 0x17FF : 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);
--
2.25.1
