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.

Reply via email to