[v5] Support vbootrom for AST2700

[PATCH v5 04/11] hw/arm/aspeed: Reuse rom_size variable for vbootrom setup

Posted by Jamin Lin via 6 months, 3 weeks ago

Move the declaration of "rom_size" to an outer scope in aspeed_machine_init()
so it can be reused for setting up the vbootrom region as well.

This avoids introducing a redundant local variable and ensures consistent
ROM sizing logic when both SPI boot and vbootrom are used.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/arm/aspeed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index e852bbc4cb..b70a120e62 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -381,6 +381,7 @@ static void aspeed_machine_init(MachineState *machine)
     AspeedSoCClass *sc;
     int i;
     DriveInfo *emmc0 = NULL;
+    uint64_t rom_size;
     bool boot_emmc;
 
     bmc->soc = ASPEED_SOC(object_new(amc->soc_name));
@@ -475,7 +476,7 @@ static void aspeed_machine_init(MachineState *machine)
         BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
 
         if (fmc0 && !boot_emmc) {
-            uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
+            rom_size = memory_region_size(&bmc->soc->spi_boot);
             aspeed_install_boot_rom(bmc, fmc0, rom_size);
         } else if (emmc0) {
             aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
-- 
2.43.0

Re: [PATCH v5 04/11] hw/arm/aspeed: Reuse rom_size variable for vbootrom setup

Posted by Cédric Le Goater 6 months, 3 weeks ago

On 4/23/25 09:23, Jamin Lin wrote:
> Move the declaration of "rom_size" to an outer scope in aspeed_machine_init()
> so it can be reused for setting up the vbootrom region as well.
> 
> This avoids introducing a redundant local variable and ensures consistent
> ROM sizing logic when both SPI boot and vbootrom are used.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> Tested-by: Nabih Estefan <nabihestefan@google.com>
> ---
>   hw/arm/aspeed.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index e852bbc4cb..b70a120e62 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -381,6 +381,7 @@ static void aspeed_machine_init(MachineState *machine)
>       AspeedSoCClass *sc;
>       int i;
>       DriveInfo *emmc0 = NULL;
> +    uint64_t rom_size;
>       bool boot_emmc;
>   
>       bmc->soc = ASPEED_SOC(object_new(amc->soc_name));
> @@ -475,7 +476,7 @@ static void aspeed_machine_init(MachineState *machine)
>           BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
>   
>           if (fmc0 && !boot_emmc) {
> -            uint64_t rom_size = memory_region_size(&bmc->soc->spi_boot);
> +            rom_size = memory_region_size(&bmc->soc->spi_boot);
>               aspeed_install_boot_rom(bmc, fmc0, rom_size);
>           } else if (emmc0) {
>               aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);

I don't think this patch is useful. See comment on patch 6.


Thanks,

C.

RE: [PATCH v5 04/11] hw/arm/aspeed: Reuse rom_size variable for vbootrom setup

Posted by Jamin Lin 6 months, 3 weeks ago

Hi Cedric, 

> Subject: Re: [PATCH v5 04/11] hw/arm/aspeed: Reuse rom_size variable for
> vbootrom setup
> 
> On 4/23/25 09:23, Jamin Lin wrote:
> > Move the declaration of "rom_size" to an outer scope in
> > aspeed_machine_init() so it can be reused for setting up the vbootrom region
> as well.
> >
> > This avoids introducing a redundant local variable and ensures
> > consistent ROM sizing logic when both SPI boot and vbootrom are used.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Cédric Le Goater <clg@redhat.com>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >   hw/arm/aspeed.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > e852bbc4cb..b70a120e62 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -381,6 +381,7 @@ static void aspeed_machine_init(MachineState
> *machine)
> >       AspeedSoCClass *sc;
> >       int i;
> >       DriveInfo *emmc0 = NULL;
> > +    uint64_t rom_size;
> >       bool boot_emmc;
> >
> >       bmc->soc = ASPEED_SOC(object_new(amc->soc_name));
> > @@ -475,7 +476,7 @@ static void aspeed_machine_init(MachineState
> *machine)
> >           BlockBackend *fmc0 = dev ? m25p80_get_blk(dev) : NULL;
> >
> >           if (fmc0 && !boot_emmc) {
> > -            uint64_t rom_size =
> memory_region_size(&bmc->soc->spi_boot);
> > +            rom_size = memory_region_size(&bmc->soc->spi_boot);
> >               aspeed_install_boot_rom(bmc, fmc0, rom_size);
> >           } else if (emmc0) {
> >               aspeed_install_boot_rom(bmc,
> blk_by_legacy_dinfo(emmc0),
> > 64 * KiB);
> 
> I don't think this patch is useful. See comment on patch 6.
> 

Thanks for suggestion and review.
Will drop this patch
Jamin
> 
> Thanks,
> 
> C.
>