Re: [PATCH v1 01/11] hw/arm/aspeed_ast2400: Use unimp array and table for unimplemented devices

["Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>]

On 2026-09-30 9:25, Jamin Lin wrote:
> Before this change, each unimplemented device needed its own field in
> AspeedSoCState. This made the variable names confusing. For example,
> AspeedSoCState has "udc" as an unimplemented device, but
> Aspeed2600SoCState also has "udc" as the real UDC model.
> 
> Add an "unimp" array in AspeedSoCState for all unimplemented devices,
> and add an AspeedUnimpDevice table to describe them. This makes the
> code easier to maintain and read. Convert AST2400/AST2500 to use them.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/arm/aspeed_soc.h |  9 +++++++++
>   hw/arm/aspeed_ast2400.c     | 33 +++++++++++++++++++--------------
>   2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 38226d9b80..a9ef4b3550 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -64,6 +64,14 @@
>   #define ASPEED_INTC_NUM  2
>   #define ASPEED_IOEXP_NUM 2
>   #define ASPEED_SRAM_NUM 2
> +#define ASPEED_UNIMP_NUM 10
> +
> +typedef struct AspeedUnimpDevice {
> +    const char *qom_name;
> +    const char *region_name;
> +    int memmap_idx;
> +    uint64_t size;
> +} AspeedUnimpDevice;

Note to Peter: I really think keeping duplicating the
UnimplementedDeviceConfig structure is a basic software
development antipattern. Could you reconsider your objection?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>