Re: [PATCH v2 3/5] hw/arm/aspeed: Init CPU defaults in a common helper

2024-01-24 Thread Philippe Mathieu-Daudé 

On 25/1/24 03:26, Gavin Shan wrote:

Hi Phil,

On 1/24/24 08:48, Philippe Mathieu-Daudé wrote:

Rework aspeed_soc_num_cpus() as a new init_cpus_defaults()
helper to reduce code duplication.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed.c | 71 +++--
  1 file changed, 28 insertions(+), 43 deletions(-)



One nit needs to be addressed:

Reviewed-by: Gavin Shan 




Failure from './scripts/checkpatch.pl --strict'

ERROR: do not use C99 // comments
#216: FILE: hw/arm/aspeed.c:1505:
+    aspeed_machine_class_init_cpus_defaults(mc); //


Oops, forgot to clean that =) Thanks!

Re: [PATCH v2 3/5] hw/arm/aspeed: Init CPU defaults in a common helper

2024-01-24 Thread Gavin Shan 

Hi Phil,

On 1/24/24 08:48, Philippe Mathieu-Daudé wrote:

Rework aspeed_soc_num_cpus() as a new init_cpus_defaults()
helper to reduce code duplication.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed.c | 71 +++--
  1 file changed, 28 insertions(+), 43 deletions(-)



One nit needs to be addressed:

Reviewed-by: Gavin Shan 


diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 5b01a4dd28..636a6269aa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1141,10 +1141,14 @@ static void aspeed_machine_class_props_init(ObjectClass 
*oc)
"Change the SPI Flash model");
  }
  
-static int aspeed_soc_num_cpus(const char *soc_name)

+static void aspeed_machine_class_init_cpus_defaults(MachineClass *mc)
  {
-   AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(soc_name));
-   return sc->num_cpus;
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
+AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
+
+mc->default_cpus = mc->min_cpus
+ = mc->max_cpus
+ = sc->num_cpus;
  }
  
  static void aspeed_machine_class_init(ObjectClass *oc, void *data)

@@ -1176,8 +1180,7 @@ static void 
aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size   = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)

@@ -1193,8 +1196,7 @@ static void 
aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = quanta_q71l_bmc_i2c_init;
  mc->default_ram_size   = 128 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,

@@ -1212,8 +1214,7 @@ static void 
aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,
  amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,

@@ -1231,8 +1232,7 @@ static void 
aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,
  amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)

@@ -1248,8 +1248,7 @@ static void 
aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = ast2500_evb_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)

@@ -1266,8 +1265,7 @@ static void 
aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = yosemitev2_bmc_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)

@@ -1283,8 +1281,7 @@ static void aspeed_machine_romulus_class_init(ObjectClass 
*oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = romulus_bmc_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)

@@ -1301,8 +1298,7 @@ static void 
aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = tiogapass_bmc_i2c_init;
  mc->default_ram_size   = 1 * GiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)

@@ -1318,8 +1314,7 @@ static void

Re: [PATCH v2 3/5] hw/arm/aspeed: Init CPU defaults in a common helper

2024-01-24 Thread Richard Henderson 

On 1/24/24 08:48, Philippe Mathieu-Daudé wrote:

Rework aspeed_soc_num_cpus() as a new init_cpus_defaults()
helper to reduce code duplication.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed.c | 71 +++--
  1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 5b01a4dd28..636a6269aa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1141,10 +1141,14 @@ static void aspeed_machine_class_props_init(ObjectClass 
*oc)
"Change the SPI Flash model");
  }
  
-static int aspeed_soc_num_cpus(const char *soc_name)

+static void aspeed_machine_class_init_cpus_defaults(MachineClass *mc)
  {
-   AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(soc_name));
-   return sc->num_cpus;
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
+AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
+
+mc->default_cpus = mc->min_cpus
+ = mc->max_cpus
+ = sc->num_cpus;


Not keen on that layout.  If you don't want to repeat sc->num_cpus, maybe just 
pull out

  int n = sc->num_cpus;

at the top.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH v2 3/5] hw/arm/aspeed: Init CPU defaults in a common helper

2024-01-23 Thread Cédric Le Goater 

On 1/23/24 23:48, Philippe Mathieu-Daudé wrote:

Rework aspeed_soc_num_cpus() as a new init_cpus_defaults()
helper to reduce code duplication.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed.c | 71 +++--
  1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 5b01a4dd28..636a6269aa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1141,10 +1141,14 @@ static void aspeed_machine_class_props_init(ObjectClass 
*oc)
"Change the SPI Flash model");
  }
  
-static int aspeed_soc_num_cpus(const char *soc_name)

+static void aspeed_machine_class_init_cpus_defaults(MachineClass *mc)

  {
-   AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(soc_name));
-   return sc->num_cpus;
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
+AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
+
+mc->default_cpus = mc->min_cpus
+ = mc->max_cpus
+ = sc->num_cpus;
  }
  
  static void aspeed_machine_class_init(ObjectClass *oc, void *data)

@@ -1176,8 +1180,7 @@ static void 
aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size   = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)

@@ -1193,8 +1196,7 @@ static void 
aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = quanta_q71l_bmc_i2c_init;
  mc->default_ram_size   = 128 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,

@@ -1212,8 +1214,7 @@ static void 
aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,
  amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,

@@ -1231,8 +1232,7 @@ static void 
aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,
  amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
  amc->i2c_init  = palmetto_bmc_i2c_init;
  mc->default_ram_size = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  }
  
  static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)

@@ -1248,8 +1248,7 @@ static void 
aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 1;
  amc->i2c_init  = ast2500_evb_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)

@@ -1266,8 +1265,7 @@ static void 
aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = yosemitev2_bmc_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)

@@ -1283,8 +1281,7 @@ static void aspeed_machine_romulus_class_init(ObjectClass 
*oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = romulus_bmc_i2c_init;
  mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)

@@ -1301,8 +1298,7 @@ static void 
aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 2;
  amc->i2c_init  = tiogapass_bmc_i2c_init;
  mc->default_ram_size   = 1 * GiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
  };
  
  static void aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)

@@ -1318,8 +1314,7 @@ static void 
aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)
  amc->num_cs= 2;

[PATCH v2 3/5] hw/arm/aspeed: Init CPU defaults in a common helper

2024-01-23 Thread Philippe Mathieu-Daudé 
Rework aspeed_soc_num_cpus() as a new init_cpus_defaults()
helper to reduce code duplication.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/aspeed.c | 71 +++--
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 5b01a4dd28..636a6269aa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1141,10 +1141,14 @@ static void aspeed_machine_class_props_init(ObjectClass 
*oc)
   "Change the SPI Flash model");
 }
 
-static int aspeed_soc_num_cpus(const char *soc_name)
+static void aspeed_machine_class_init_cpus_defaults(MachineClass *mc)
 {
-   AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(soc_name));
-   return sc->num_cpus;
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(mc);
+AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
+
+mc->default_cpus = mc->min_cpus
+ = mc->max_cpus
+ = sc->num_cpus;
 }
 
 static void aspeed_machine_class_init(ObjectClass *oc, void *data)
@@ -1176,8 +1180,7 @@ static void 
aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 1;
 amc->i2c_init  = palmetto_bmc_i2c_init;
 mc->default_ram_size   = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 };
 
 static void aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)
@@ -1193,8 +1196,7 @@ static void 
aspeed_machine_quanta_q71l_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 1;
 amc->i2c_init  = quanta_q71l_bmc_i2c_init;
 mc->default_ram_size   = 128 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 }
 
 static void aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,
@@ -1212,8 +1214,7 @@ static void 
aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc,
 amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
 amc->i2c_init  = palmetto_bmc_i2c_init;
 mc->default_ram_size = 256 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 }
 
 static void aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,
@@ -1231,8 +1232,7 @@ static void 
aspeed_machine_supermicro_x11spi_bmc_class_init(ObjectClass *oc,
 amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
 amc->i2c_init  = palmetto_bmc_i2c_init;
 mc->default_ram_size = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 }
 
 static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
@@ -1248,8 +1248,7 @@ static void 
aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 1;
 amc->i2c_init  = ast2500_evb_i2c_init;
 mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 };
 
 static void aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)
@@ -1266,8 +1265,7 @@ static void 
aspeed_machine_yosemitev2_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 2;
 amc->i2c_init  = yosemitev2_bmc_i2c_init;
 mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 };
 
 static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)
@@ -1283,8 +1281,7 @@ static void aspeed_machine_romulus_class_init(ObjectClass 
*oc, void *data)
 amc->num_cs= 2;
 amc->i2c_init  = romulus_bmc_i2c_init;
 mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 };
 
 static void aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)
@@ -1301,8 +1298,7 @@ static void 
aspeed_machine_tiogapass_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 2;
 amc->i2c_init  = tiogapass_bmc_i2c_init;
 mc->default_ram_size   = 1 * GiB;
-mc->default_cpus = mc->min_cpus = mc->max_cpus =
-aspeed_soc_num_cpus(amc->soc_name);
+aspeed_machine_class_init_cpus_defaults(mc);
 };
 
 static void aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)
@@ -1318,8 +1314,7 @@ static void 
aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)
 amc->num_cs= 2;
 amc->i2c_init  = sonorapass_bmc_i2c_init;
 mc->default_ram_size   = 512 * MiB;
-mc->default_cpus = mc->min_cpus =