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_...@aspeedtech.com>
> > Reviewed-by: Cédric Le Goater <c...@redhat.com>
> > Tested-by: Nabih Estefan <nabiheste...@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.
>
