Re: [PATCH 1/2] hw/arm/aspeed: allow missing spi_model

[Cédric Le Goater]

On 3/5/22 01:06, Patrick Williams wrote:
> Generally all BMCs will use the fmc_model to hold their own flash
> and most will have a spi_model to hold the managed system's flash,
> but not all systems do.  Add a simple NULL check to allow a system
> to set the spi_model as NULL to indicate it should not be instantiated.


OK. Let's do it that way for now but we need to rework 'num_cs' in the
Aspeed SMC model and the Aspeed machines. We started with a simple
requirement (1 FMC and 1 SPI) but since, things have become more complex.

1. we should get rid of AspeedSMCState::num_cs and simply use
   AspeedSMCState::max_peripherals in the SMC model. There is no need to
   restrict the number devices of the controller to match the board layout.

2. aspeed_board_init_flashes() needs a better interface. May be, something
   like :

   static void aspeed_board_init_flashes(AspeedSMCState *s,
                                         const char **flashtype, int count,
                                         int unit0)

and change ->fmc_model and ->spi_model to be arrays. Ideas welcome.

Anyhow,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
> ---
>   hw/arm/aspeed.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 11558b327b..617a1ecbdc 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -276,7 +276,11 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
>                                         const char *flashtype,
>                                         int unit0)
>   {
> -    int i ;
> +    int i;
> +
> +    if (!flashtype) {
> +        return;
> +    }
>   
>       for (i = 0; i < s->num_cs; ++i) {
>           DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);