Re: [PATCH v3 04/17] hw/intc/loongson_ipi: Extract loongson_ipi_common_realize()

[maobibo]

On 2024/7/18 上午5:46, Philippe Mathieu-Daudé wrote:
From: Bibo Mao <maob...@loongson.cn>

In preparation to extract common IPI code in few commits,
extract loongson_ipi_common_realize().

Signed-off-by: Bibo Mao <maob...@loongson.cn>
[PMD: Extracted from bigger commit, added commit description]
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  hw/intc/loongson_ipi.c | 25 ++++++++++++++++++-------
  1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c
index 3b3481c43e..40ac769aad 100644
--- a/hw/intc/loongson_ipi.c
+++ b/hw/intc/loongson_ipi.c
@@ -275,7 +275,7 @@ static const MemoryRegionOps loongson_ipi64_ops = {
      .endianness = DEVICE_LITTLE_ENDIAN,
  };
  
-static void loongson_ipi_realize(DeviceState *dev, Error **errp)
+static void loongson_ipi_common_realize(DeviceState *dev, Error **errp)
  {
      LoongsonIPIState *s = LOONGSON_IPI(dev);
      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
@@ -301,20 +301,31 @@ static void loongson_ipi_realize(DeviceState *dev, Error 
**errp)
      sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
  
      s->cpu = g_new0(IPICore, s->num_cpu);
-    if (s->cpu == NULL) {
-        error_setg(errp, "Memory allocation for IPICore faile");
Philippe,

Thanks for the whole series, it looks to me. It is split into small 
patches and adds new option CONFIG_LOONGSON_IPI_COMMON, it is easier to 
review and compile for multiple targets.

One small nit, do we need keep checking sentence for if (s->cpu == NULL)?

Overall, for the whole series it is ok for me and works well on 
LoongArch machine.

Reviewed-by: Bibo Mao <maob...@loongson.cn>
Tested-by: Bibo Mao <maob...@loongson.cn>

+    for (i = 0; i < s->num_cpu; i++) {
+        s->cpu[i].ipi = s;
+
+        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
+    }
+}
+
+static void loongson_ipi_realize(DeviceState *dev, Error **errp)
+{
+    LoongsonIPIState *s = LOONGSON_IPI(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Error *local_err = NULL;
+
+    loongson_ipi_common_realize(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
          return;
      }
  
-    for (i = 0; i < s->num_cpu; i++) {
-        s->cpu[i].ipi = s;
+    for (unsigned i = 0; i < s->num_cpu; i++) {
          s->cpu[i].ipi_mmio_mem = g_new0(MemoryRegion, 1);
          g_autofree char *name = g_strdup_printf("loongson_ipi_cpu%d_mmio", i);
          memory_region_init_io(s->cpu[i].ipi_mmio_mem, OBJECT(dev),
                                &loongson_ipi_core_ops, &s->cpu[i], name, 0x48);
          sysbus_init_mmio(sbd, s->cpu[i].ipi_mmio_mem);
-
-        qdev_init_gpio_out(dev, &s->cpu[i].irq, 1);
      }
  }