[PATCH 6/8] hw/arm/aspeed/2600: Check for CPU types in machine_run_board_init()

Philippe Mathieu-Daudé posted 8 patches 10 months, 1 week ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Rob Herring <robh@kernel.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>
There is a newer version of this series
[PATCH 6/8] hw/arm/aspeed/2600: Check for CPU types in machine_run_board_init()
Posted by Philippe Mathieu-Daudé 10 months, 1 week ago
Restrict MachineClass::valid_cpu_types[] to the single
valid CPU type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/aspeed.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index df627096d2..393c97d55e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1157,6 +1157,11 @@ static const char * const ast2500_a1_valid_cpu_types[] = {
     NULL
 };
 
+static const char * const ast2600_a3_valid_cpu_types[] = {
+    ARM_CPU_TYPE_NAME("cortex-a9"),
+    NULL
+};
+
 static void aspeed_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1373,6 +1378,7 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
     amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON |
                      ASPEED_MAC3_ON;
     amc->i2c_init  = ast2600_evb_i2c_init;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1392,6 +1398,7 @@ static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
     amc->num_cs    = 2;
     amc->macs_mask  = ASPEED_MAC2_ON;
     amc->i2c_init  = witherspoon_bmc_i2c_init; /* Same board layout */
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1449,6 +1456,7 @@ static void aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
     amc->num_cs    = 2;
     amc->macs_mask  = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
     amc->i2c_init  = rainier_bmc_i2c_init;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1471,6 +1479,7 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
     amc->macs_mask = ASPEED_MAC3_ON;
     amc->i2c_init = fuji_bmc_i2c_init;
     amc->uart_default = ASPEED_DEV_UART1;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = FUJI_BMC_RAM_SIZE;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1492,6 +1501,7 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
     amc->num_cs    = 2;
     amc->macs_mask = ASPEED_MAC2_ON;
     amc->i2c_init  = bletchley_bmc_i2c_init;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = BLETCHLEY_BMC_RAM_SIZE;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1631,6 +1641,7 @@ static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
     amc->num_cs    = 2;
     amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
     amc->i2c_init  = qcom_dc_scm_bmc_i2c_init;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
@@ -1651,6 +1662,7 @@ static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
     amc->num_cs    = 2;
     amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
     amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
+    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
     mc->default_ram_size = 1 * GiB;
     mc->default_cpus = mc->min_cpus = mc->max_cpus =
         aspeed_soc_num_cpus(amc->soc_name);
-- 
2.41.0


Re: [PATCH 6/8] hw/arm/aspeed/2600: Check for CPU types in machine_run_board_init()
Posted by Cédric Le Goater 10 months, 1 week ago
On 1/23/24 07:38, Philippe Mathieu-Daudé wrote:
> Restrict MachineClass::valid_cpu_types[] to the single
> valid CPU type.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index df627096d2..393c97d55e 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1157,6 +1157,11 @@ static const char * const ast2500_a1_valid_cpu_types[] = {
>       NULL
>   };
>   
> +static const char * const ast2600_a3_valid_cpu_types[] = {
> +    ARM_CPU_TYPE_NAME("cortex-a9"),

This should be "cortex-a7"

Looking closer, the CPU information is under AspeedSoCClass. Why not build the
valid_cpu_types array with something like :

     struct AspeedMachineClass {
	...
	const char *valid_cpu_types[2];
     };

     static void aspeed_machine_set_valid_cpu_types(MachineClass *mc)
     {
         AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
         AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
     
         amc->valid_cpu_types[0] = sc->cpu_type;
         amc->valid_cpu_types[1] = NULL;
         mc->valid_cpu_types = amc->valid_cpu_types;
     };


Thanks,

C.



> +    NULL
> +};
> +
>   static void aspeed_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1373,6 +1378,7 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
>       amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON |
>                        ASPEED_MAC3_ON;
>       amc->i2c_init  = ast2600_evb_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1392,6 +1398,7 @@ static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask  = ASPEED_MAC2_ON;
>       amc->i2c_init  = witherspoon_bmc_i2c_init; /* Same board layout */
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1449,6 +1456,7 @@ static void aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask  = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = rainier_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1471,6 +1479,7 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
>       amc->macs_mask = ASPEED_MAC3_ON;
>       amc->i2c_init = fuji_bmc_i2c_init;
>       amc->uart_default = ASPEED_DEV_UART1;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = FUJI_BMC_RAM_SIZE;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1492,6 +1501,7 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON;
>       amc->i2c_init  = bletchley_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = BLETCHLEY_BMC_RAM_SIZE;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1631,6 +1641,7 @@ static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = qcom_dc_scm_bmc_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);
> @@ -1651,6 +1662,7 @@ static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
>       amc->num_cs    = 2;
>       amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>       amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
> +    mc->valid_cpu_types = ast2600_a3_valid_cpu_types;
>       mc->default_ram_size = 1 * GiB;
>       mc->default_cpus = mc->min_cpus = mc->max_cpus =
>           aspeed_soc_num_cpus(amc->soc_name);


Re: [PATCH 6/8] hw/arm/aspeed/2600: Check for CPU types in machine_run_board_init()
Posted by Cédric Le Goater 10 months, 1 week ago
On 1/23/24 10:27, Cédric Le Goater wrote:
> On 1/23/24 07:38, Philippe Mathieu-Daudé wrote:
>> Restrict MachineClass::valid_cpu_types[] to the single
>> valid CPU type.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/aspeed.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index df627096d2..393c97d55e 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -1157,6 +1157,11 @@ static const char * const ast2500_a1_valid_cpu_types[] = {
>>       NULL
>>   };
>> +static const char * const ast2600_a3_valid_cpu_types[] = {
>> +    ARM_CPU_TYPE_NAME("cortex-a9"),
> 
> This should be "cortex-a7"
> 
> Looking closer, the CPU information is under AspeedSoCClass. Why not build the
> valid_cpu_types array with something like :
> 
>      struct AspeedMachineClass {
>      ...
>      const char *valid_cpu_types[2];
>      };
> 
>      static void aspeed_machine_set_valid_cpu_types(MachineClass *mc)
>      {
>          AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
>          AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
>          amc->valid_cpu_types[0] = sc->cpu_type;
>          amc->valid_cpu_types[1] = NULL;
>          mc->valid_cpu_types = amc->valid_cpu_types;
>      };

or better, change AspeedSoCClass::cpu_type to an array.

	mc->valid_cpu_types =
		ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name))->cpu_types;

C.