Hi Cédric,
> Subject: Re: [SPAM] [PATCH v1 01/21] hw/arm/aspeed_ast27x0-fc: Support
> VBootRom
>
> On 9/2/25 10:28, Jamin Lin wrote:
> > Hi Cédric
> >
> >> Subject: Re: [SPAM] [PATCH v1 01/21] hw/arm/aspeed_ast27x0-fc:
> >> Support VBootRom
> >>
> >
> > Thanks for your review and suggestion.
> >
> > Per our earlier discussion, we plan to refactor hw/arm/aspeed.c. As a
> > first step, I can move the vbootrom helpers into a common source file
> > so they can be reused by other boards.
> >
> > Do you have a preference for the filename?
> > hw/arm/aspeed_utils.c (with a small header in
> > include/hw/arm/aspeed_utils.h),
>
>
> There is a aspeed_soc_common.c file for such helpers.
>
Thanks for the suggestions.
It seems that aspeed_soc_common.c is meant for code shared across all ASPEED
SoCs rather than the board-specific code.
I am planning to move the following APIs into a common file.
static void aspeed_load_vbootrom(AspeedMachineState *bmc, const char *bios_name,
Error **errp)
static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk,
uint64_t rom_size)
static void write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size,
Error **errp)
To build successfully, the AspeedMachineState struct also needs to be moved
into aspeed.h.
struct AspeedMachineState {
/* Private */
MachineState parent_obj;
/* Public */
AspeedSoCState *soc;
MemoryRegion boot_rom;
bool mmio_exec;
uint32_t uart_chosen;
char *fmc_model;
char *spi_model;
uint32_t hw_strap1;
};
If I place the above APIs in aspeed_soc_common.h that header would also need to
include aspeed.h.
To avoid mixing SOC-level and board-level code, I propose creating a new c/h
file to place them such
as aspeed_board_common.c and aspeed_board_common.h
Do you have any concerns or could you please give me any suggestion?
Thanks-Jamin
>
> Thanks,
>
> C.
