Hi Cédric > Subject: RE: [SPAM] [PATCH v1 01/21] hw/arm/aspeed_ast27x0-fc: Support > VBootRom > > 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. >
Sorry, please ignore my previous comments. I realized that I can replace AspeedMachineState with AspeedSoCState to make the API more generic. Apologies for the inconvenience. Jamin > 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.