diff mbox series

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

Message ID 20220324181557.203805-4-zongyuan.li@smartx.com
State New
Headers show
Series Replace 'qemu_irq_split' with 'TYPE_SPLIT_IRQ' | expand

Commit Message

Zongyuan Li March 24, 2022, 6:15 p.m. UTC
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(-)

Comments

Damien Hedde April 1, 2022, 1:17 p.m. UTC | #1
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
Peter Maydell April 1, 2022, 1:35 p.m. UTC | #2
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
Zongyuan Li April 2, 2022, 2:28 a.m. UTC | #3
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
diff mbox series

Patch

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