[PATCH v5 0/4] Replace 'qemu_irq_split' with 'TYPE_SPLIT_IRQ'

[PATCH v5 0/4] Replace 'qemu_irq_split' with 'TYPE_SPLIT_IRQ'

[Zongyuan Li]

This patch set tries to replace 'qemu_irq_split' function with QOM
object 'TYPE_SPLIT_IRQ' and totally remove this call.

If this patch set is applied, issue:

https://gitlab.com/qemu-project/qemu/-/issues/811

can be closed.

Changes since v3:

1. Squash Patch 3 & 4 into one, since they would affect each other.
2. Use `object_initialize_with_child` and `qdev_realize` for device code.
3. Code style fixes.
4. Remove unnecessary `if` statement used with `qdev_realize_and_unref.
5. Narrow scope of some variables.

Changes since v4

1. Code style fixes.

Zongyuan Li (4):
  hw/arm/realview: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'
  hw/arm/stellaris: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'
  hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic
  hw/core/irq: remove unused 'qemu_irq_split' function

 hw/arm/exynos4210.c           | 26 +++++++++++
 hw/arm/realview.c             | 33 ++++++++++----
 hw/arm/stellaris.c            | 15 ++++++-
 hw/core/irq.c                 | 15 -------
 hw/intc/exynos4210_combiner.c | 81 +++++++++++++++++++++++++++--------
 hw/intc/exynos4210_gic.c      | 36 +++++++++++++---
 include/hw/arm/exynos4210.h   | 11 ++---
 include/hw/core/split-irq.h   |  5 +--
 include/hw/irq.h              |  5 ---
 9 files changed, 163 insertions(+), 64 deletions(-)

-- 
2.34.0

[PATCH v5 1/4] hw/arm/realview: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'

[Zongyuan Li]

Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
---
 hw/arm/realview.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 7b424e94a5..d2dc8a8952 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -13,9 +13,11 @@
 #include "hw/sysbus.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/primecell.h"
+#include "hw/core/split-irq.h"
 #include "hw/net/lan9118.h"
 #include "hw/net/smc91c111.h"
 #include "hw/pci/pci.h"
+#include "hw/qdev-core.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
@@ -53,6 +55,20 @@ static const int realview_board_id[] = {
     0x76d
 };
 
+static void split_irq_from_named(DeviceState *src, const char* outname,
+                                 qemu_irq out1, qemu_irq out2) {
+    DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
+
+    qdev_prop_set_uint32(splitter, "num-lines", 2);
+
+    qdev_realize_and_unref(splitter, NULL, &error_fatal);
+
+    qdev_connect_gpio_out(splitter, 0, out1);
+    qdev_connect_gpio_out(splitter, 1, out2);
+    qdev_connect_gpio_out_named(src, outname, 0,
+                                qdev_get_gpio_in(splitter, 0));
+}
+
 static void realview_init(MachineState *machine,
                           enum realview_board_type board_type)
 {
@@ -66,7 +82,6 @@ static void realview_init(MachineState *machine,
     DeviceState *dev, *sysctl, *gpio2, *pl041;
     SysBusDevice *busdev;
     qemu_irq pic[64];
-    qemu_irq mmc_irq[2];
     PCIBus *pci_bus = NULL;
     NICInfo *nd;
     DriveInfo *dinfo;
@@ -229,14 +244,14 @@ static void realview_init(MachineState *machine,
      * and the PL061 has them the other way about. Also the card
      * detect line is inverted.
      */
-    mmc_irq[0] = qemu_irq_split(
-        qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT),
-        qdev_get_gpio_in(gpio2, 1));
-    mmc_irq[1] = qemu_irq_split(
-        qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN),
-        qemu_irq_invert(qdev_get_gpio_in(gpio2, 0)));
-    qdev_connect_gpio_out_named(dev, "card-read-only", 0, mmc_irq[0]);
-    qdev_connect_gpio_out_named(dev, "card-inserted", 0, mmc_irq[1]);
+    split_irq_from_named(dev, "card-read-only",
+                   qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT),
+                   qdev_get_gpio_in(gpio2, 1));
+
+    split_irq_from_named(dev, "card-inserted",
+                   qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN),
+                   qemu_irq_invert(qdev_get_gpio_in(gpio2, 0)));
+
     dinfo = drive_get(IF_SD, 0, 0);
     if (dinfo) {
         DeviceState *card;
-- 
2.34.0

Re: [PATCH v5 1/4] hw/arm/realview: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'

[Peter Maydell]

On Thu, 24 Mar 2022 at 18:16, Zongyuan Li <zongyuan.li@smartx.com> wrote:
>
> Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> ---
>  hw/arm/realview.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v5 2/4] hw/arm/stellaris: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'

[Zongyuan Li]

Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
---
 hw/arm/stellaris.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index b6c8a5d609..12c673c917 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -9,6 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "hw/core/split-irq.h"
 #include "hw/sysbus.h"
 #include "hw/sd/sd.h"
 #include "hw/ssi/ssi.h"
@@ -1160,6 +1161,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
             DeviceState *ssddev;
             DriveInfo *dinfo;
             DeviceState *carddev;
+            DeviceState *gpio_d_splitter;
             BlockBackend *blk;
 
             /*
@@ -1237,9 +1239,18 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
                                    &error_fatal);
 
             ssddev = ssi_create_peripheral(bus, "ssd0323");
-            gpio_out[GPIO_D][0] = qemu_irq_split(
-                    qdev_get_gpio_in_named(sddev, SSI_GPIO_CS, 0),
+
+            gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
+            qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
+            qdev_realize_and_unref(gpio_d_splitter, NULL, &error_fatal);
+            qdev_connect_gpio_out(
+                    gpio_d_splitter, 0,
+                    qdev_get_gpio_in_named(sddev, SSI_GPIO_CS, 0));
+            qdev_connect_gpio_out(
+                    gpio_d_splitter, 1,
                     qdev_get_gpio_in_named(ssddev, SSI_GPIO_CS, 0));
+            gpio_out[GPIO_D][0] = qdev_get_gpio_in(gpio_d_splitter, 0);
+
             gpio_out[GPIO_C][7] = qdev_get_gpio_in(ssddev, 0);
 
             /* Make sure the select pin is high.  */
-- 
2.34.0

Re: [PATCH v5 2/4] hw/arm/stellaris: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'

[Peter Maydell]

On Thu, 24 Mar 2022 at 18:16, Zongyuan Li <zongyuan.li@smartx.com> wrote:
>
> Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> ---
>  hw/arm/stellaris.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v5 3/4] hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic

[Zongyuan Li]

Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
---
 hw/arm/exynos4210.c           | 26 +++++++++++
 hw/intc/exynos4210_combiner.c | 81 +++++++++++++++++++++++++++--------
 hw/intc/exynos4210_gic.c      | 36 +++++++++++++---
 include/hw/arm/exynos4210.h   | 11 ++---
 include/hw/core/split-irq.h   |  5 +--
 5 files changed, 126 insertions(+), 33 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0299e81f85..28bcf97af9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -288,6 +288,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
         sysbus_connect_irq(busdev, n, s->irqs.int_gic_irq[n]);
     }
+    /* SplitIRQ for internal irq realized here */
     exynos4210_combiner_get_gpioin(&s->irqs, dev, 0);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_INT_COMBINER_BASE_ADDR);
 
@@ -299,6 +300,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
         sysbus_connect_irq(busdev, n, s->irqs.ext_gic_irq[n]);
     }
+    /* SplitIRQ for external irq realized here */
     exynos4210_combiner_get_gpioin(&s->irqs, dev, 1);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_COMBINER_BASE_ADDR);
 
@@ -488,6 +490,30 @@ static void exynos4210_init(Object *obj)
         object_initialize_child(obj, name, orgate, TYPE_OR_IRQ);
         g_free(name);
     }
+
+    for (i = 0; i < ARRAY_SIZE(s->irqs.int_combiner_irq); i++) {
+        char *name = g_strdup_printf("internal-combiner-irq-%d", i);
+
+        object_initialize_child(obj, name, &s->irqs.int_combiner_irq[i],
+                                TYPE_SPLIT_IRQ);
+        g_free(name);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(s->irqs.ext_combiner_irq); i++) {
+        char *name = g_strdup_printf("external-combiner-irq-%d", i);
+
+        object_initialize_child(obj, name, &s->irqs.ext_combiner_irq[i],
+                                TYPE_SPLIT_IRQ);
+        g_free(name);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(s->irqs.board_irqs); i++) {
+        char *name = g_strdup_printf("board-irq-%d", i);
+
+        object_initialize_child(obj, name, &s->irqs.board_irqs[i],
+                                TYPE_SPLIT_IRQ);
+        g_free(name);
+    }
 }
 
 static void exynos4210_class_init(ObjectClass *klass, void *data)
diff --git a/hw/intc/exynos4210_combiner.c b/hw/intc/exynos4210_combiner.c
index 4534ee248d..fafcfc484d 100644
--- a/hw/intc/exynos4210_combiner.c
+++ b/hw/intc/exynos4210_combiner.c
@@ -31,6 +31,7 @@
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
 
 #include "hw/arm/exynos4210.h"
 #include "hw/hw.h"
@@ -105,16 +106,39 @@ static const VMStateDescription vmstate_exynos4210_combiner = {
     }
 };
 
+static void split_irq(SplitIRQ *splitter, qemu_irq out1, qemu_irq out2)
+{
+    DeviceState *dev = DEVICE(splitter);
+
+    qdev_prop_set_uint32(dev, "num-lines", 2);
+
+    qdev_realize(dev, NULL, &error_fatal);
+
+    qdev_connect_gpio_out(dev, 0, out1);
+    qdev_connect_gpio_out(dev, 1, out2);
+}
+
+static void passthrough_irq(SplitIRQ *splitter, qemu_irq out)
+{
+    DeviceState *dev = DEVICE(splitter);
+
+    qdev_prop_set_uint32(dev, "num-lines", 1);
+
+    qdev_realize(dev, NULL, &error_fatal);
+
+    qdev_connect_gpio_out(dev, 0, out);
+}
+
 /*
  * Get Combiner input GPIO into irqs structure
  */
-void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *dev,
-        int ext)
+void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *combiner,
+                                    int ext)
 {
     int n;
     int bit;
     int max;
-    qemu_irq *irq;
+    SplitIRQ *irq;
 
     max = ext ? EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ :
         EXYNOS4210_MAX_INT_COMBINER_IN_IRQ;
@@ -132,53 +156,74 @@ void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *dev,
         /* MDNIE_LCD1 INTG1 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 0) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 3):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(0, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(0, bit + 4)]),
+                        0));
             continue;
 
         /* TMU INTG3 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(3, 4):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(2, bit)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(2, bit)]),
+                        0));
             continue;
 
         /* LCD1 INTG12 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 0) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 3):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(11, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(11, bit + 4)]),
+                        0));
             continue;
 
         /* Multi-Core Timer INTG12 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 8):
-               irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                       irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]),
+                        0));
             continue;
 
         /* Multi-Core Timer INTG35 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(35, 4) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(35, 8):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]),
+                        0));
             continue;
 
         /* Multi-Core Timer INTG51 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(51, 4) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(51, 8):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]),
+                        0));
             continue;
 
         /* Multi-Core Timer INTG53 */
         case EXYNOS4210_COMBINER_GET_IRQ_NUM(53, 4) ...
              EXYNOS4210_COMBINER_GET_IRQ_NUM(53, 8):
-            irq[n] = qemu_irq_split(qdev_get_gpio_in(dev, n),
-                    irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]);
+            split_irq(&irq[n], qdev_get_gpio_in(combiner, n),
+                    qdev_get_gpio_in(
+                        DEVICE(
+                            &irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]),
+                        0));
             continue;
         }
 
-        irq[n] = qdev_get_gpio_in(dev, n);
+        passthrough_irq(&irq[n], qdev_get_gpio_in(combiner, n));
     }
 }
 
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index bc73d1f115..888233c681 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -29,6 +29,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/arm/exynos4210.h"
 #include "qom/object.h"
+#include "hw/hw.h"
 
 enum ExtGicId {
     EXT_GIC_ID_MDMA_LCD0 = 66,
@@ -197,7 +198,7 @@ static void exynos4210_irq_handler(void *opaque, int irq, int level)
     Exynos4210Irq *s = (Exynos4210Irq *)opaque;
 
     /* Bypass */
-    qemu_set_irq(s->board_irqs[irq], level);
+    qemu_set_irq(qdev_get_gpio_in(DEVICE(&s->board_irqs[irq]), 0), level);
 }
 
 /*
@@ -218,6 +219,12 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s)
     uint32_t grp, bit, irq_id, n;
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
+        SplitIRQ *splitter = &s->board_irqs[n];
+        DeviceState *dev = DEVICE(splitter);
+
+        qdev_prop_set_uint32(dev, "num-lines", 2);
+        qdev_realize(dev, NULL, &error_fatal);
+
         irq_id = 0;
         if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4) ||
                 n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4)) {
@@ -229,15 +236,28 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s)
             /* MCT_G1 is passed to External and GIC */
             irq_id = EXT_GIC_ID_MCT_G1;
         }
+
         if (irq_id) {
-            s->board_irqs[n] = qemu_irq_split(s->int_combiner_irq[n],
-                    s->ext_gic_irq[irq_id-32]);
+            qdev_connect_gpio_out(dev, 0,
+                                  qdev_get_gpio_in(
+                                      DEVICE(&s->int_combiner_irq[n]), 0));
+            qdev_connect_gpio_out(dev, 1, s->ext_gic_irq[irq_id - 32]);
         } else {
-            s->board_irqs[n] = qemu_irq_split(s->int_combiner_irq[n],
-                    s->ext_combiner_irq[n]);
+            qdev_connect_gpio_out(dev, 0,
+                                  qdev_get_gpio_in(
+                                      DEVICE(&s->int_combiner_irq[n]), 0));
+            qdev_connect_gpio_out(dev, 1,
+                                  qdev_get_gpio_in(
+                                      DEVICE(&s->ext_combiner_irq[n]), 0));
         }
     }
     for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
+        SplitIRQ *splitter = &s->board_irqs[n];
+        DeviceState *dev = DEVICE(splitter);
+
+        qdev_prop_set_uint32(dev, "num-lines", 2);
+        qdev_realize(dev, NULL, &error_fatal);
+
         /* these IDs are passed to Internal Combiner and External GIC */
         grp = EXYNOS4210_COMBINER_GET_GRP_NUM(n);
         bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
@@ -245,8 +265,10 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s)
                      EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][bit];
 
         if (irq_id) {
-            s->board_irqs[n] = qemu_irq_split(s->int_combiner_irq[n],
-                    s->ext_gic_irq[irq_id-32]);
+            qdev_connect_gpio_out(dev, 0,
+                                  qdev_get_gpio_in(
+                                      DEVICE(&s->int_combiner_irq[n]), 0));
+            qdev_connect_gpio_out(dev, 1, s->ext_gic_irq[irq_id - 32]);
         }
     }
 }
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 60b9e126f5..be36665e04 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -28,6 +28,7 @@
 #include "hw/sysbus.h"
 #include "target/arm/cpu-qom.h"
 #include "qom/object.h"
+#include "hw/core/split-irq.h"
 
 #define EXYNOS4210_NCPUS                    2
 
@@ -79,11 +80,11 @@
 #define EXYNOS4210_NUM_DMA      3
 
 typedef struct Exynos4210Irq {
-    qemu_irq int_combiner_irq[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
-    qemu_irq ext_combiner_irq[EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ];
+    SplitIRQ int_combiner_irq[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
+    SplitIRQ ext_combiner_irq[EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ];
     qemu_irq int_gic_irq[EXYNOS4210_INT_GIC_NIRQ];
     qemu_irq ext_gic_irq[EXYNOS4210_EXT_GIC_NIRQ];
-    qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
+    SplitIRQ board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
 } Exynos4210Irq;
 
 struct Exynos4210State {
@@ -115,7 +116,7 @@ qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
 
 /* Initialize board IRQs.
  * These IRQs contain splitted Int/External Combiner and External Gic IRQs */
-void exynos4210_init_board_irqs(Exynos4210Irq *s);
+void exynos4210_init_board_irqs(Exynos4210Irq *irqs);
 
 /* Get IRQ number from exynos4210 IRQ subsystem stub.
  * To identify IRQ source use internal combiner group and bit number
@@ -126,7 +127,7 @@ uint32_t exynos4210_get_irq(uint32_t grp, uint32_t bit);
 /*
  * Get Combiner input GPIO into irqs structure
  */
-void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *dev,
+void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *combiner,
         int ext);
 
 /*
diff --git a/include/hw/core/split-irq.h b/include/hw/core/split-irq.h
index ff8852f407..eb485dd7a6 100644
--- a/include/hw/core/split-irq.h
+++ b/include/hw/core/split-irq.h
@@ -42,9 +42,6 @@
 
 #define MAX_SPLIT_LINES 16
 
-
-OBJECT_DECLARE_SIMPLE_TYPE(SplitIRQ, SPLIT_IRQ)
-
 struct SplitIRQ {
     DeviceState parent_obj;
 
@@ -52,4 +49,6 @@ struct SplitIRQ {
     uint16_t num_lines;
 };
 
+OBJECT_DECLARE_SIMPLE_TYPE(SplitIRQ, SPLIT_IRQ)
+
 #endif
-- 
2.34.0

Re: [PATCH v5 3/4] hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic

[Damien Hedde]

On 3/24/22 19:15, Zongyuan Li wrote:
> Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> ---
>   hw/arm/exynos4210.c           | 26 +++++++++++
>   hw/intc/exynos4210_combiner.c | 81 +++++++++++++++++++++++++++--------
>   hw/intc/exynos4210_gic.c      | 36 +++++++++++++---
>   include/hw/arm/exynos4210.h   | 11 ++---
>   include/hw/core/split-irq.h   |  5 +--
>   5 files changed, 126 insertions(+), 33 deletions(-)
> 
> diff --git a/include/hw/core/split-irq.h b/include/hw/core/split-irq.h
> index ff8852f407..eb485dd7a6 100644
> --- a/include/hw/core/split-irq.h
> +++ b/include/hw/core/split-irq.h
> @@ -42,9 +42,6 @@
>   
>   #define MAX_SPLIT_LINES 16
>   
> -
> -OBJECT_DECLARE_SIMPLE_TYPE(SplitIRQ, SPLIT_IRQ)
> -
>   struct SplitIRQ {
>       DeviceState parent_obj;
>   
> @@ -52,4 +49,6 @@ struct SplitIRQ {
>       uint16_t num_lines;
>   };
>   
> +OBJECT_DECLARE_SIMPLE_TYPE(SplitIRQ, SPLIT_IRQ)
> +
>   #endif

Is there a reason to move the OBJECT_DECLARE_SIMPLE_TYPE line ?

--
Damien

Re: [PATCH v5 3/4] hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic

[Peter Maydell]

On Thu, 24 Mar 2022 at 18:16, Zongyuan Li <zongyuan.li@smartx.com> wrote:
>
> Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> ---
>  hw/arm/exynos4210.c           | 26 +++++++++++
>  hw/intc/exynos4210_combiner.c | 81 +++++++++++++++++++++++++++--------
>  hw/intc/exynos4210_gic.c      | 36 +++++++++++++---
>  include/hw/arm/exynos4210.h   | 11 ++---
>  include/hw/core/split-irq.h   |  5 +--
>  5 files changed, 126 insertions(+), 33 deletions(-)

Looking at this patch, I think it's ended up quite complicated
because the exynos4210 code as it stands today is doing
some rather odd things with interrupts. I'm going to have a
go at some refactoring patches which try to clean some of that
oddness up into code more like what we'd write today, which
should make getting rid of the use of qemu_split_irq() a bit easier.

thanks
-- PMM

Re: [PATCH v5 3/4] hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic

[Zongyuan Li]

On Fri, Apr 1, 2022 at 9:35 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 24 Mar 2022 at 18:16, Zongyuan Li <zongyuan.li@smartx.com> wrote:
> >
> > Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> > ---
> >  hw/arm/exynos4210.c           | 26 +++++++++++
> >  hw/intc/exynos4210_combiner.c | 81 +++++++++++++++++++++++++++--------
> >  hw/intc/exynos4210_gic.c      | 36 +++++++++++++---
> >  include/hw/arm/exynos4210.h   | 11 ++---
> >  include/hw/core/split-irq.h   |  5 +--
> >  5 files changed, 126 insertions(+), 33 deletions(-)
>
> Looking at this patch, I think it's ended up quite complicated
> because the exynos4210 code as it stands today is doing
> some rather odd things with interrupts. I'm going to have a
> go at some refactoring patches which try to clean some of that
> oddness up into code more like what we'd write today, which
> should make getting rid of the use of qemu_split_irq() a bit easier.

Should we wait for the refactoring to be done and rebase this patch set on it,
or just remove exynos4210 related code? I'm glad to see if there's anything I
can help with the refactoring.

Thanks for your review.
Li

[PATCH v5 4/4] hw/core/irq: remove unused 'qemu_irq_split' function

[Zongyuan Li]

Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/811
---
 hw/core/irq.c    | 15 ---------------
 include/hw/irq.h |  5 -----
 2 files changed, 20 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 741219277b..3623f711fe 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -106,21 +106,6 @@ qemu_irq qemu_irq_invert(qemu_irq irq)
     return qemu_allocate_irq(qemu_notirq, irq, 0);
 }
 
-static void qemu_splitirq(void *opaque, int line, int level)
-{
-    struct IRQState **irq = opaque;
-    irq[0]->handler(irq[0]->opaque, irq[0]->n, level);
-    irq[1]->handler(irq[1]->opaque, irq[1]->n, level);
-}
-
-qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2)
-{
-    qemu_irq *s = g_new0(qemu_irq, 2);
-    s[0] = irq1;
-    s[1] = irq2;
-    return qemu_allocate_irq(qemu_splitirq, s, 0);
-}
-
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n)
 {
     int i;
diff --git a/include/hw/irq.h b/include/hw/irq.h
index dc7abf199e..645b73d251 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -46,11 +46,6 @@ void qemu_free_irq(qemu_irq irq);
 /* Returns a new IRQ with opposite polarity.  */
 qemu_irq qemu_irq_invert(qemu_irq irq);
 
-/* Returns a new IRQ which feeds into both the passed IRQs.
- * It's probably better to use the TYPE_SPLIT_IRQ device instead.
- */
-qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2);
-
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
    on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-- 
2.34.0

Re: [PATCH v5 0/4] Replace 'qemu_irq_split' with 'TYPE_SPLIT_IRQ'

[Peter Maydell]

On Thu, 24 Mar 2022 at 18:22, Zongyuan Li <zongyuan.li@smartx.com> wrote:
>
> This patch set tries to replace 'qemu_irq_split' function with QOM
> object 'TYPE_SPLIT_IRQ' and totally remove this call.
>
> If this patch set is applied, issue:
>
> https://gitlab.com/qemu-project/qemu/-/issues/811
>
> can be closed.
>
> Changes since v3:
>
> 1. Squash Patch 3 & 4 into one, since they would affect each other.
> 2. Use `object_initialize_with_child` and `qdev_realize` for device code.
> 3. Code style fixes.
> 4. Remove unnecessary `if` statement used with `qdev_realize_and_unref.
> 5. Narrow scope of some variables.
>
> Changes since v4
>
> 1. Code style fixes.
>
> Zongyuan Li (4):
>   hw/arm/realview: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'
>   hw/arm/stellaris: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'
>   hw/intc/exynos4210: replace 'qemu_split_irq' in combiner and gic
>   hw/core/irq: remove unused 'qemu_irq_split' function

I've applied patches 1, 2 and 4 to target-arm.next, thanks
(after my exynos4210 cleanup series).

-- PMM