[PATCH for-7.1 00/18] hw/arm: Make exynos4210 use TYPE_SPLIT_IRQ

[PATCH for-7.1 00/18] hw/arm: Make exynos4210 use TYPE_SPLIT_IRQ

[Peter Maydell]

The primary aim of this patchset is to make the exynos4210 code use
the TYPE_SPLIT_IRQ device instead of the old qemu_split_irq() function
(which we are trying to get rid of). However, the current code is
quite complicated and so we have to do a fair amount of refactoring
in order to be able to use TYPE_SPLIT_IRQ in a clean way.

The interrupt wiring on this SoC is complicated and interrupt
lines from devices may be wired up to multiple places:
 * a GIC device
 * an internal combiner
 * an external combiner
(a combiner is a fairly simple "OR multiple IRQ sources together
in groups with enable/disable and mask logic" piece of hardware).
In some cases an interrupt is wired up to more than one input
on each combiner.

The current code has a struct Exynos4210Irq where it keeps arrays of
qemu_irqs corresponding to the inputs of these devices, and it handles
the "wire interrupt lines to multiple places" logic in functions which
are called by the SoC device model but which live in the source files
with the combiner and GIC models. This series moves the logic to the
SoC device model's source file (because it is really part of the SoC
wiring, not part of the individual combiner or GIC devices) and makes
use of the TYPE_SPLIT_IRQ ability to provide more than 2 output lines
to simplify things so that each interrupt line connects to just one
splitter, whose outputs go to all the places they need to. In the
new setup, these splitter devices clearly belong to the SoC object,
and so they are created as QOM children of it. The Exynos4210Irq
struct ends up unneeded and is deleted.

I have also done some conversion of specific child devices of this SoC
from the old-style "call qemu_new()" to the new-style "embed the child
device struct in the parent state struct". I haven't done a complete
conversion, but only touched those devices where making the conversion
was useful for the TYPE_SPLIT_IRQ changes.

I don't have a datasheet for this SoC that describes all the external
combiner and external GIC wiring, so I have mostly kept the QEMU
behaviour the same as it is currently. In a few places, however, I
have fixed what seem to me to be fairly clearly bugs in the current
handling. (Largely these bugs weren't visible to the guest because
we weren't actually connecting up devices to the affected bits of
the interrupt line wiring.)

I've tested this with a simple Linux image, which I think is basically
the same one as the 'make check-acceptance' test. If anybody has
access to other test images that would be interesting.

thanks
-- PMM

Peter Maydell (18):
  hw/arm/exynos4210: Use TYPE_OR_IRQ instead of custom OR-gate device
  hw/intc/exynos4210_gic: Remove unused TYPE_EXYNOS4210_IRQ_GATE
  hw/arm/exynos4210: Put a9mpcore device into state struct
  hw/arm/exynos4210: Drop int_gic_irq[] from Exynos4210Irq struct
  hw/arm/exynos4210: Coalesce board_irqs and irq_table
  hw/arm/exynos4210: Fix code style nit in combiner_grp_to_gic_id[]
  hw/arm/exynos4210: Move exynos4210_init_board_irqs() into exynos4210.c
  hw/arm/exynos4210: Put external GIC into state struct
  hw/arm/exynos4210: Drop ext_gic_irq[] from Exynos4210Irq struct
  hw/arm/exynos4210: Move exynos4210_combiner_get_gpioin() into
    exynos4210.c
  hw/arm/exynos4210: Delete unused macro definitions
  hw/arm/exynos4210: Use TYPE_SPLIT_IRQ in exynos4210_init_board_irqs()
  hw/arm/exynos4210: Fill in irq_table[] for internal-combiner-only IRQ
    lines
  hw/arm/exynos4210: Connect MCT_G0 and MCT_G1 to both combiners
  hw/arm/exynos4210: Don't connect multiple lines to external GIC inputs
  hw/arm/exynos4210: Fold combiner splits into
    exynos4210_init_board_irqs()
  hw/arm/exynos4210: Put combiners into state struct
  hw/arm/exynos4210: Drop Exynos4210Irq struct

 include/hw/arm/exynos4210.h           |  50 ++-
 include/hw/intc/exynos4210_combiner.h |  57 ++++
 include/hw/intc/exynos4210_gic.h      |  43 +++
 hw/arm/exynos4210.c                   | 430 +++++++++++++++++++++++---
 hw/intc/exynos4210_combiner.c         | 108 +------
 hw/intc/exynos4210_gic.c              | 344 +--------------------
 MAINTAINERS                           |   2 +-
 7 files changed, 508 insertions(+), 526 deletions(-)
 create mode 100644 include/hw/intc/exynos4210_combiner.h
 create mode 100644 include/hw/intc/exynos4210_gic.h

-- 
2.25.1

[PATCH for-7.1 01/18] hw/arm/exynos4210: Use TYPE_OR_IRQ instead of custom OR-gate device

[Peter Maydell]

The Exynos4210 SoC device currently uses a custom device
"exynos4210.irq_gate" to model the OR gate that feeds each CPU's IRQ
line.  We have a standard TYPE_OR_IRQ device for this now, so use
that instead.

(This is a migration compatibility break, but that is OK for this
machine type.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  1 +
 hw/arm/exynos4210.c         | 31 ++++++++++++++++---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 60b9e126f55..3999034053e 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -102,6 +102,7 @@ struct Exynos4210State {
     MemoryRegion bootreg_mem;
     I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
     qemu_or_irq pl330_irq_orgate[EXYNOS4210_NUM_DMA];
+    qemu_or_irq cpu_irq_orgate[EXYNOS4210_NCPUS];
 };
 
 #define TYPE_EXYNOS4210_SOC "exynos4210"
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0299e81f853..dfc0a4eec25 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -205,7 +205,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
 {
     Exynos4210State *s = EXYNOS4210_SOC(socdev);
     MemoryRegion *system_mem = get_system_memory();
-    qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
     SysBusDevice *busdev;
     DeviceState *dev, *uart[4], *pl330[3];
     int i, n;
@@ -235,18 +234,13 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
 
     /* IRQ Gate */
     for (i = 0; i < EXYNOS4210_NCPUS; i++) {
-        dev = qdev_new("exynos4210.irq_gate");
-        qdev_prop_set_uint32(dev, "n_in", EXYNOS4210_IRQ_GATE_NINPUTS);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-        /* Get IRQ Gate input in gate_irq */
-        for (n = 0; n < EXYNOS4210_IRQ_GATE_NINPUTS; n++) {
-            gate_irq[i][n] = qdev_get_gpio_in(dev, n);
-        }
-        busdev = SYS_BUS_DEVICE(dev);
-
-        /* Connect IRQ Gate output to CPU's IRQ line */
-        sysbus_connect_irq(busdev, 0,
-                           qdev_get_gpio_in(DEVICE(s->cpu[i]), ARM_CPU_IRQ));
+        DeviceState *orgate = DEVICE(&s->cpu_irq_orgate[i]);
+        object_property_set_int(OBJECT(orgate), "num-lines",
+                                EXYNOS4210_IRQ_GATE_NINPUTS,
+                                &error_abort);
+        qdev_realize(orgate, NULL, &error_abort);
+        qdev_connect_gpio_out(orgate, 0,
+                              qdev_get_gpio_in(DEVICE(s->cpu[i]), ARM_CPU_IRQ));
     }
 
     /* Private memory region and Internal GIC */
@@ -256,7 +250,8 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     sysbus_realize_and_unref(busdev, &error_fatal);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);
     for (n = 0; n < EXYNOS4210_NCPUS; n++) {
-        sysbus_connect_irq(busdev, n, gate_irq[n][0]);
+        sysbus_connect_irq(busdev, n,
+                           qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 0));
     }
     for (n = 0; n < EXYNOS4210_INT_GIC_NIRQ; n++) {
         s->irqs.int_gic_irq[n] = qdev_get_gpio_in(dev, n);
@@ -275,7 +270,8 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     /* Map Distributer interface */
     sysbus_mmio_map(busdev, 1, EXYNOS4210_EXT_GIC_DIST_BASE_ADDR);
     for (n = 0; n < EXYNOS4210_NCPUS; n++) {
-        sysbus_connect_irq(busdev, n, gate_irq[n][1]);
+        sysbus_connect_irq(busdev, n,
+                           qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 1));
     }
     for (n = 0; n < EXYNOS4210_EXT_GIC_NIRQ; n++) {
         s->irqs.ext_gic_irq[n] = qdev_get_gpio_in(dev, n);
@@ -488,6 +484,11 @@ 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->cpu_irq_orgate); i++) {
+        g_autofree char *name = g_strdup_printf("cpu-irq-orgate%d", i);
+        object_initialize_child(obj, name, &s->cpu_irq_orgate[i], TYPE_OR_IRQ);
+    }
 }
 
 static void exynos4210_class_init(ObjectClass *klass, void *data)
-- 
2.25.1

Re: [PATCH for-7.1 01/18] hw/arm/exynos4210: Use TYPE_OR_IRQ instead of custom OR-gate device

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The Exynos4210 SoC device currently uses a custom device
> "exynos4210.irq_gate" to model the OR gate that feeds each CPU's IRQ
> line.  We have a standard TYPE_OR_IRQ device for this now, so use
> that instead.
> 
> (This is a migration compatibility break, but that is OK for this
> machine type.)
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  1 +
>   hw/arm/exynos4210.c         | 31 ++++++++++++++++---------------
>   2 files changed, 17 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 02/18] hw/intc/exynos4210_gic: Remove unused TYPE_EXYNOS4210_IRQ_GATE

[Peter Maydell]

Now we have removed the only use of TYPE_EXYNOS4210_IRQ_GATE we can
delete the device entirely.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/exynos4210_gic.c | 107 ---------------------------------------
 1 file changed, 107 deletions(-)

diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index bc73d1f1152..794f6b5ac72 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -373,110 +373,3 @@ static void exynos4210_gic_register_types(void)
 }
 
 type_init(exynos4210_gic_register_types)
-
-/* IRQ OR Gate struct.
- *
- * This device models an OR gate. There are n_in input qdev gpio lines and one
- * output sysbus IRQ line. The output IRQ level is formed as OR between all
- * gpio inputs.
- */
-
-#define TYPE_EXYNOS4210_IRQ_GATE "exynos4210.irq_gate"
-OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210IRQGateState, EXYNOS4210_IRQ_GATE)
-
-struct Exynos4210IRQGateState {
-    SysBusDevice parent_obj;
-
-    uint32_t n_in;      /* inputs amount */
-    uint32_t *level;    /* input levels */
-    qemu_irq out;       /* output IRQ */
-};
-
-static Property exynos4210_irq_gate_properties[] = {
-    DEFINE_PROP_UINT32("n_in", Exynos4210IRQGateState, n_in, 1),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static const VMStateDescription vmstate_exynos4210_irq_gate = {
-    .name = "exynos4210.irq_gate",
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .fields = (VMStateField[]) {
-        VMSTATE_VBUFFER_UINT32(level, Exynos4210IRQGateState, 1, NULL, n_in),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-/* Process a change in IRQ input. */
-static void exynos4210_irq_gate_handler(void *opaque, int irq, int level)
-{
-    Exynos4210IRQGateState *s = (Exynos4210IRQGateState *)opaque;
-    uint32_t i;
-
-    assert(irq < s->n_in);
-
-    s->level[irq] = level;
-
-    for (i = 0; i < s->n_in; i++) {
-        if (s->level[i] >= 1) {
-            qemu_irq_raise(s->out);
-            return;
-        }
-    }
-
-    qemu_irq_lower(s->out);
-}
-
-static void exynos4210_irq_gate_reset(DeviceState *d)
-{
-    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(d);
-
-    memset(s->level, 0, s->n_in * sizeof(*s->level));
-}
-
-/*
- * IRQ Gate initialization.
- */
-static void exynos4210_irq_gate_init(Object *obj)
-{
-    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(obj);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
-
-    sysbus_init_irq(sbd, &s->out);
-}
-
-static void exynos4210_irq_gate_realize(DeviceState *dev, Error **errp)
-{
-    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(dev);
-
-    /* Allocate general purpose input signals and connect a handler to each of
-     * them */
-    qdev_init_gpio_in(dev, exynos4210_irq_gate_handler, s->n_in);
-
-    s->level = g_malloc0(s->n_in * sizeof(*s->level));
-}
-
-static void exynos4210_irq_gate_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    dc->reset = exynos4210_irq_gate_reset;
-    dc->vmsd = &vmstate_exynos4210_irq_gate;
-    device_class_set_props(dc, exynos4210_irq_gate_properties);
-    dc->realize = exynos4210_irq_gate_realize;
-}
-
-static const TypeInfo exynos4210_irq_gate_info = {
-    .name          = TYPE_EXYNOS4210_IRQ_GATE,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(Exynos4210IRQGateState),
-    .instance_init = exynos4210_irq_gate_init,
-    .class_init    = exynos4210_irq_gate_class_init,
-};
-
-static void exynos4210_irq_gate_register_types(void)
-{
-    type_register_static(&exynos4210_irq_gate_info);
-}
-
-type_init(exynos4210_irq_gate_register_types)
-- 
2.25.1

Re: [PATCH for-7.1 02/18] hw/intc/exynos4210_gic: Remove unused TYPE_EXYNOS4210_IRQ_GATE

[Francisco Iglesias]

On [2022 Apr 04] Mon 16:46:42, Peter Maydell wrote:
> Now we have removed the only use of TYPE_EXYNOS4210_IRQ_GATE we can
> delete the device entirely.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/intc/exynos4210_gic.c | 107 ---------------------------------------
>  1 file changed, 107 deletions(-)
> 
> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> index bc73d1f1152..794f6b5ac72 100644
> --- a/hw/intc/exynos4210_gic.c
> +++ b/hw/intc/exynos4210_gic.c
> @@ -373,110 +373,3 @@ static void exynos4210_gic_register_types(void)
>  }
>  
>  type_init(exynos4210_gic_register_types)
> -
> -/* IRQ OR Gate struct.
> - *
> - * This device models an OR gate. There are n_in input qdev gpio lines and one
> - * output sysbus IRQ line. The output IRQ level is formed as OR between all
> - * gpio inputs.
> - */
> -
> -#define TYPE_EXYNOS4210_IRQ_GATE "exynos4210.irq_gate"
> -OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210IRQGateState, EXYNOS4210_IRQ_GATE)
> -
> -struct Exynos4210IRQGateState {
> -    SysBusDevice parent_obj;
> -
> -    uint32_t n_in;      /* inputs amount */
> -    uint32_t *level;    /* input levels */
> -    qemu_irq out;       /* output IRQ */
> -};
> -
> -static Property exynos4210_irq_gate_properties[] = {
> -    DEFINE_PROP_UINT32("n_in", Exynos4210IRQGateState, n_in, 1),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static const VMStateDescription vmstate_exynos4210_irq_gate = {
> -    .name = "exynos4210.irq_gate",
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_VBUFFER_UINT32(level, Exynos4210IRQGateState, 1, NULL, n_in),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
> -/* Process a change in IRQ input. */
> -static void exynos4210_irq_gate_handler(void *opaque, int irq, int level)
> -{
> -    Exynos4210IRQGateState *s = (Exynos4210IRQGateState *)opaque;
> -    uint32_t i;
> -
> -    assert(irq < s->n_in);
> -
> -    s->level[irq] = level;
> -
> -    for (i = 0; i < s->n_in; i++) {
> -        if (s->level[i] >= 1) {
> -            qemu_irq_raise(s->out);
> -            return;
> -        }
> -    }
> -
> -    qemu_irq_lower(s->out);
> -}
> -
> -static void exynos4210_irq_gate_reset(DeviceState *d)
> -{
> -    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(d);
> -
> -    memset(s->level, 0, s->n_in * sizeof(*s->level));
> -}
> -
> -/*
> - * IRQ Gate initialization.
> - */
> -static void exynos4210_irq_gate_init(Object *obj)
> -{
> -    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(obj);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> -
> -    sysbus_init_irq(sbd, &s->out);
> -}
> -
> -static void exynos4210_irq_gate_realize(DeviceState *dev, Error **errp)
> -{
> -    Exynos4210IRQGateState *s = EXYNOS4210_IRQ_GATE(dev);
> -
> -    /* Allocate general purpose input signals and connect a handler to each of
> -     * them */
> -    qdev_init_gpio_in(dev, exynos4210_irq_gate_handler, s->n_in);
> -
> -    s->level = g_malloc0(s->n_in * sizeof(*s->level));
> -}
> -
> -static void exynos4210_irq_gate_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -
> -    dc->reset = exynos4210_irq_gate_reset;
> -    dc->vmsd = &vmstate_exynos4210_irq_gate;
> -    device_class_set_props(dc, exynos4210_irq_gate_properties);
> -    dc->realize = exynos4210_irq_gate_realize;
> -}
> -
> -static const TypeInfo exynos4210_irq_gate_info = {
> -    .name          = TYPE_EXYNOS4210_IRQ_GATE,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(Exynos4210IRQGateState),
> -    .instance_init = exynos4210_irq_gate_init,
> -    .class_init    = exynos4210_irq_gate_class_init,
> -};
> -
> -static void exynos4210_irq_gate_register_types(void)
> -{
> -    type_register_static(&exynos4210_irq_gate_info);
> -}
> -
> -type_init(exynos4210_irq_gate_register_types)
> -- 
> 2.25.1
> 
> 

[PATCH for-7.1 03/18] hw/arm/exynos4210: Put a9mpcore device into state struct

[Peter Maydell]

The exynos4210 SoC mostly creates its child devices as if it were
board code.  This includes the a9mpcore object.  Switch that to a
new-style "embedded in the state struct" creation, because in the
next commit we're going to want to refer to the object again further
down in the exynos4210_realize() function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I don't propose to try to do a full conversion of every child
device; I'm only going to do them where it makes a subsequent
commit a bit nicer, like this one.
---
 include/hw/arm/exynos4210.h |  2 ++
 hw/arm/exynos4210.c         | 11 ++++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 3999034053e..215c039b414 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -26,6 +26,7 @@
 
 #include "hw/or-irq.h"
 #include "hw/sysbus.h"
+#include "hw/cpu/a9mpcore.h"
 #include "target/arm/cpu-qom.h"
 #include "qom/object.h"
 
@@ -103,6 +104,7 @@ struct Exynos4210State {
     I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
     qemu_or_irq pl330_irq_orgate[EXYNOS4210_NUM_DMA];
     qemu_or_irq cpu_irq_orgate[EXYNOS4210_NCPUS];
+    A9MPPrivState a9mpcore;
 };
 
 #define TYPE_EXYNOS4210_SOC "exynos4210"
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index dfc0a4eec25..ef4d646eb91 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -244,17 +244,16 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     }
 
     /* Private memory region and Internal GIC */
-    dev = qdev_new(TYPE_A9MPCORE_PRIV);
-    qdev_prop_set_uint32(dev, "num-cpu", EXYNOS4210_NCPUS);
-    busdev = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(busdev, &error_fatal);
+    qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-cpu", EXYNOS4210_NCPUS);
+    busdev = SYS_BUS_DEVICE(&s->a9mpcore);
+    sysbus_realize(busdev, &error_fatal);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);
     for (n = 0; n < EXYNOS4210_NCPUS; n++) {
         sysbus_connect_irq(busdev, n,
                            qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 0));
     }
     for (n = 0; n < EXYNOS4210_INT_GIC_NIRQ; n++) {
-        s->irqs.int_gic_irq[n] = qdev_get_gpio_in(dev, n);
+        s->irqs.int_gic_irq[n] = qdev_get_gpio_in(DEVICE(&s->a9mpcore), n);
     }
 
     /* Cache controller */
@@ -489,6 +488,8 @@ static void exynos4210_init(Object *obj)
         g_autofree char *name = g_strdup_printf("cpu-irq-orgate%d", i);
         object_initialize_child(obj, name, &s->cpu_irq_orgate[i], TYPE_OR_IRQ);
     }
+
+    object_initialize_child(obj, "a9mpcore", &s->a9mpcore, TYPE_A9MPCORE_PRIV);
 }
 
 static void exynos4210_class_init(ObjectClass *klass, void *data)
-- 
2.25.1

Re: [PATCH for-7.1 03/18] hw/arm/exynos4210: Put a9mpcore device into state struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The exynos4210 SoC mostly creates its child devices as if it were
> board code.  This includes the a9mpcore object.  Switch that to a
> new-style "embedded in the state struct" creation, because in the
> next commit we're going to want to refer to the object again further
> down in the exynos4210_realize() function.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> I don't propose to try to do a full conversion of every child
> device; I'm only going to do them where it makes a subsequent
> commit a bit nicer, like this one.
> ---
>   include/hw/arm/exynos4210.h |  2 ++
>   hw/arm/exynos4210.c         | 11 ++++++-----
>   2 files changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 04/18] hw/arm/exynos4210: Drop int_gic_irq[] from Exynos4210Irq struct

[Peter Maydell]

The only time we use the int_gic_irq[] array in the Exynos4210Irq
struct is in the exynos4210_realize() function: we initialize it with
the GPIO inputs of the a9mpcore device, and then a bit later on we
connect those to the outputs of the internal combiner.  Now that the
a9mpcore object is easily accessible as s->a9mpcore we can make the
connection directly from one device to the other without going via
this array.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h | 1 -
 hw/arm/exynos4210.c         | 6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 215c039b414..923ce987627 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -82,7 +82,6 @@
 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];
-    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];
 } Exynos4210Irq;
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index ef4d646eb91..60fc5a2ffe7 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -252,9 +252,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
         sysbus_connect_irq(busdev, n,
                            qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 0));
     }
-    for (n = 0; n < EXYNOS4210_INT_GIC_NIRQ; n++) {
-        s->irqs.int_gic_irq[n] = qdev_get_gpio_in(DEVICE(&s->a9mpcore), n);
-    }
 
     /* Cache controller */
     sysbus_create_simple("l2x0", EXYNOS4210_L2X0_BASE_ADDR, NULL);
@@ -281,7 +278,8 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     busdev = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(busdev, &error_fatal);
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
-        sysbus_connect_irq(busdev, n, s->irqs.int_gic_irq[n]);
+        sysbus_connect_irq(busdev, n,
+                           qdev_get_gpio_in(DEVICE(&s->a9mpcore), n));
     }
     exynos4210_combiner_get_gpioin(&s->irqs, dev, 0);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_INT_COMBINER_BASE_ADDR);
-- 
2.25.1

Re: [PATCH for-7.1 04/18] hw/arm/exynos4210: Drop int_gic_irq[] from Exynos4210Irq struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The only time we use the int_gic_irq[] array in the Exynos4210Irq
> struct is in the exynos4210_realize() function: we initialize it with
> the GPIO inputs of the a9mpcore device, and then a bit later on we
> connect those to the outputs of the internal combiner.  Now that the
> a9mpcore object is easily accessible as s->a9mpcore we can make the
> connection directly from one device to the other without going via
> this array.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h | 1 -
>   hw/arm/exynos4210.c         | 6 ++----
>   2 files changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 05/18] hw/arm/exynos4210: Coalesce board_irqs and irq_table

[Peter Maydell]

The exynos4210 code currently has two very similar arrays of IRQs:

 * board_irqs is a field of the Exynos4210Irq struct which is filled
   in by exynos4210_init_board_irqs() with the appropriate qemu_irqs
   for each IRQ the board/SoC can assert
 * irq_table is a set of qemu_irqs pointed to from the
   Exynos4210State struct.  It's allocated in exynos4210_init_irq,
   and the only behaviour these irqs have is that they pass on the
   level to the equivalent board_irqs[] irq

The extra indirection through irq_table is unnecessary, so coalesce
these into a single irq_table[] array as a direct field in
Exynos4210State which exynos4210_init_board_irqs() fills in.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  8 ++------
 hw/arm/exynos4210.c         |  6 +-----
 hw/intc/exynos4210_gic.c    | 32 ++++++++------------------------
 3 files changed, 11 insertions(+), 35 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 923ce987627..a9f186370ee 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -83,7 +83,6 @@ 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];
     qemu_irq ext_gic_irq[EXYNOS4210_EXT_GIC_NIRQ];
-    qemu_irq board_irqs[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
 } Exynos4210Irq;
 
 struct Exynos4210State {
@@ -92,7 +91,7 @@ struct Exynos4210State {
     /*< public >*/
     ARMCPU *cpu[EXYNOS4210_NCPUS];
     Exynos4210Irq irqs;
-    qemu_irq *irq_table;
+    qemu_irq irq_table[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
 
     MemoryRegion chipid_mem;
     MemoryRegion iram_mem;
@@ -112,12 +111,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210State, EXYNOS4210_SOC)
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-/* Initialize exynos4210 IRQ subsystem stub */
-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(Exynos4210State *s);
 
 /* Get IRQ number from exynos4210 IRQ subsystem stub.
  * To identify IRQ source use internal combiner group and bit number
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 60fc5a2ffe7..11e321d7830 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -228,10 +228,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
         qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
     }
 
-    /*** IRQs ***/
-
-    s->irq_table = exynos4210_init_irq(&s->irqs);
-
     /* IRQ Gate */
     for (i = 0; i < EXYNOS4210_NCPUS; i++) {
         DeviceState *orgate = DEVICE(&s->cpu_irq_orgate[i]);
@@ -296,7 +292,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_COMBINER_BASE_ADDR);
 
     /* Initialize board IRQs. */
-    exynos4210_init_board_irqs(&s->irqs);
+    exynos4210_init_board_irqs(s);
 
     /*** Memory ***/
 
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index 794f6b5ac72..ec79b96f6d1 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -192,30 +192,14 @@ combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
 #define EXYNOS4210_GIC_CPU_REGION_SIZE  0x100
 #define EXYNOS4210_GIC_DIST_REGION_SIZE 0x1000
 
-static void exynos4210_irq_handler(void *opaque, int irq, int level)
-{
-    Exynos4210Irq *s = (Exynos4210Irq *)opaque;
-
-    /* Bypass */
-    qemu_set_irq(s->board_irqs[irq], level);
-}
-
-/*
- * Initialize exynos4210 IRQ subsystem stub.
- */
-qemu_irq *exynos4210_init_irq(Exynos4210Irq *s)
-{
-    return qemu_allocate_irqs(exynos4210_irq_handler, s,
-            EXYNOS4210_MAX_INT_COMBINER_IN_IRQ);
-}
-
 /*
  * 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(Exynos4210State *s)
 {
     uint32_t grp, bit, irq_id, n;
+    Exynos4210Irq *is = &s->irqs;
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
         irq_id = 0;
@@ -230,11 +214,11 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s)
             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]);
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_gic_irq[irq_id - 32]);
         } else {
-            s->board_irqs[n] = qemu_irq_split(s->int_combiner_irq[n],
-                    s->ext_combiner_irq[n]);
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_combiner_irq[n]);
         }
     }
     for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
@@ -245,8 +229,8 @@ 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]);
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_gic_irq[irq_id - 32]);
         }
     }
 }
-- 
2.25.1

Re: [PATCH for-7.1 05/18] hw/arm/exynos4210: Coalesce board_irqs and irq_table

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The exynos4210 code currently has two very similar arrays of IRQs:
> 
>   * board_irqs is a field of the Exynos4210Irq struct which is filled
>     in by exynos4210_init_board_irqs() with the appropriate qemu_irqs
>     for each IRQ the board/SoC can assert
>   * irq_table is a set of qemu_irqs pointed to from the
>     Exynos4210State struct.  It's allocated in exynos4210_init_irq,
>     and the only behaviour these irqs have is that they pass on the
>     level to the equivalent board_irqs[] irq
> 
> The extra indirection through irq_table is unnecessary, so coalesce
> these into a single irq_table[] array as a direct field in
> Exynos4210State which exynos4210_init_board_irqs() fills in.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  8 ++------
>   hw/arm/exynos4210.c         |  6 +-----
>   hw/intc/exynos4210_gic.c    | 32 ++++++++------------------------
>   3 files changed, 11 insertions(+), 35 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 06/18] hw/arm/exynos4210: Fix code style nit in combiner_grp_to_gic_id[]

[Peter Maydell]

Fix a missing set of spaces around '-' in the definition of
combiner_grp_to_gic_id[]. We're about to move this code, so
fix the style issue first to keep checkpatch happy with the
code-motion patch.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/exynos4210_gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index ec79b96f6d1..3b77a485780 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -121,7 +121,7 @@ enum ExtInt {
  */
 
 static const uint32_t
-combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
+combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
     /* int combiner groups 16-19 */
     { }, { }, { }, { },
     /* int combiner group 20 */
-- 
2.25.1

Re: [PATCH for-7.1 06/18] hw/arm/exynos4210: Fix code style nit in combiner_grp_to_gic_id[]

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> Fix a missing set of spaces around '-' in the definition of
> combiner_grp_to_gic_id[]. We're about to move this code, so
> fix the style issue first to keep checkpatch happy with the
> code-motion patch.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/intc/exynos4210_gic.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 07/18] hw/arm/exynos4210: Move exynos4210_init_board_irqs() into exynos4210.c

[Peter Maydell]

The function exynos4210_init_board_irqs() currently lives in
exynos4210_gic.c, but it isn't really part of the exynos4210.gic
device -- it is a function that implements (some of) the wiring up of
interrupts between the SoC's GIC and combiner components.  This means
it fits better in exynos4210.c, which is the SoC-level code.  Move it
there. Similarly, exynos4210_git_irq() is used almost only in the
SoC-level code, so move it too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |   4 -
 hw/arm/exynos4210.c         | 202 +++++++++++++++++++++++++++++++++++
 hw/intc/exynos4210_gic.c    | 204 ------------------------------------
 3 files changed, 202 insertions(+), 208 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index a9f186370ee..d83e96a091e 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -111,10 +111,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210State, EXYNOS4210_SOC)
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-/* Initialize board IRQs.
- * These IRQs contain splitted Int/External Combiner and External Gic IRQs */
-void exynos4210_init_board_irqs(Exynos4210State *s);
-
 /* Get IRQ number from exynos4210 IRQ subsystem stub.
  * To identify IRQ source use internal combiner group and bit number
  *  grp - group number
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 11e321d7830..742666ba779 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -101,6 +101,208 @@
 #define EXYNOS4210_PL330_BASE1_ADDR         0x12690000
 #define EXYNOS4210_PL330_BASE2_ADDR         0x12850000
 
+enum ExtGicId {
+    EXT_GIC_ID_MDMA_LCD0 = 66,
+    EXT_GIC_ID_PDMA0,
+    EXT_GIC_ID_PDMA1,
+    EXT_GIC_ID_TIMER0,
+    EXT_GIC_ID_TIMER1,
+    EXT_GIC_ID_TIMER2,
+    EXT_GIC_ID_TIMER3,
+    EXT_GIC_ID_TIMER4,
+    EXT_GIC_ID_MCT_L0,
+    EXT_GIC_ID_WDT,
+    EXT_GIC_ID_RTC_ALARM,
+    EXT_GIC_ID_RTC_TIC,
+    EXT_GIC_ID_GPIO_XB,
+    EXT_GIC_ID_GPIO_XA,
+    EXT_GIC_ID_MCT_L1,
+    EXT_GIC_ID_IEM_APC,
+    EXT_GIC_ID_IEM_IEC,
+    EXT_GIC_ID_NFC,
+    EXT_GIC_ID_UART0,
+    EXT_GIC_ID_UART1,
+    EXT_GIC_ID_UART2,
+    EXT_GIC_ID_UART3,
+    EXT_GIC_ID_UART4,
+    EXT_GIC_ID_MCT_G0,
+    EXT_GIC_ID_I2C0,
+    EXT_GIC_ID_I2C1,
+    EXT_GIC_ID_I2C2,
+    EXT_GIC_ID_I2C3,
+    EXT_GIC_ID_I2C4,
+    EXT_GIC_ID_I2C5,
+    EXT_GIC_ID_I2C6,
+    EXT_GIC_ID_I2C7,
+    EXT_GIC_ID_SPI0,
+    EXT_GIC_ID_SPI1,
+    EXT_GIC_ID_SPI2,
+    EXT_GIC_ID_MCT_G1,
+    EXT_GIC_ID_USB_HOST,
+    EXT_GIC_ID_USB_DEVICE,
+    EXT_GIC_ID_MODEMIF,
+    EXT_GIC_ID_HSMMC0,
+    EXT_GIC_ID_HSMMC1,
+    EXT_GIC_ID_HSMMC2,
+    EXT_GIC_ID_HSMMC3,
+    EXT_GIC_ID_SDMMC,
+    EXT_GIC_ID_MIPI_CSI_4LANE,
+    EXT_GIC_ID_MIPI_DSI_4LANE,
+    EXT_GIC_ID_MIPI_CSI_2LANE,
+    EXT_GIC_ID_MIPI_DSI_2LANE,
+    EXT_GIC_ID_ONENAND_AUDI,
+    EXT_GIC_ID_ROTATOR,
+    EXT_GIC_ID_FIMC0,
+    EXT_GIC_ID_FIMC1,
+    EXT_GIC_ID_FIMC2,
+    EXT_GIC_ID_FIMC3,
+    EXT_GIC_ID_JPEG,
+    EXT_GIC_ID_2D,
+    EXT_GIC_ID_PCIe,
+    EXT_GIC_ID_MIXER,
+    EXT_GIC_ID_HDMI,
+    EXT_GIC_ID_HDMI_I2C,
+    EXT_GIC_ID_MFC,
+    EXT_GIC_ID_TVENC,
+};
+
+enum ExtInt {
+    EXT_GIC_ID_EXTINT0 = 48,
+    EXT_GIC_ID_EXTINT1,
+    EXT_GIC_ID_EXTINT2,
+    EXT_GIC_ID_EXTINT3,
+    EXT_GIC_ID_EXTINT4,
+    EXT_GIC_ID_EXTINT5,
+    EXT_GIC_ID_EXTINT6,
+    EXT_GIC_ID_EXTINT7,
+    EXT_GIC_ID_EXTINT8,
+    EXT_GIC_ID_EXTINT9,
+    EXT_GIC_ID_EXTINT10,
+    EXT_GIC_ID_EXTINT11,
+    EXT_GIC_ID_EXTINT12,
+    EXT_GIC_ID_EXTINT13,
+    EXT_GIC_ID_EXTINT14,
+    EXT_GIC_ID_EXTINT15
+};
+
+/*
+ * External GIC sources which are not from External Interrupt Combiner or
+ * External Interrupts are starting from EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ,
+ * which is INTG16 in Internal Interrupt Combiner.
+ */
+
+static const uint32_t
+combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
+    /* int combiner groups 16-19 */
+    { }, { }, { }, { },
+    /* int combiner group 20 */
+    { 0, EXT_GIC_ID_MDMA_LCD0 },
+    /* int combiner group 21 */
+    { EXT_GIC_ID_PDMA0, EXT_GIC_ID_PDMA1 },
+    /* int combiner group 22 */
+    { EXT_GIC_ID_TIMER0, EXT_GIC_ID_TIMER1, EXT_GIC_ID_TIMER2,
+            EXT_GIC_ID_TIMER3, EXT_GIC_ID_TIMER4 },
+    /* int combiner group 23 */
+    { EXT_GIC_ID_RTC_ALARM, EXT_GIC_ID_RTC_TIC },
+    /* int combiner group 24 */
+    { EXT_GIC_ID_GPIO_XB, EXT_GIC_ID_GPIO_XA },
+    /* int combiner group 25 */
+    { EXT_GIC_ID_IEM_APC, EXT_GIC_ID_IEM_IEC },
+    /* int combiner group 26 */
+    { EXT_GIC_ID_UART0, EXT_GIC_ID_UART1, EXT_GIC_ID_UART2, EXT_GIC_ID_UART3,
+            EXT_GIC_ID_UART4 },
+    /* int combiner group 27 */
+    { EXT_GIC_ID_I2C0, EXT_GIC_ID_I2C1, EXT_GIC_ID_I2C2, EXT_GIC_ID_I2C3,
+            EXT_GIC_ID_I2C4, EXT_GIC_ID_I2C5, EXT_GIC_ID_I2C6,
+            EXT_GIC_ID_I2C7 },
+    /* int combiner group 28 */
+    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 , EXT_GIC_ID_USB_HOST},
+    /* int combiner group 29 */
+    { EXT_GIC_ID_HSMMC0, EXT_GIC_ID_HSMMC1, EXT_GIC_ID_HSMMC2,
+     EXT_GIC_ID_HSMMC3, EXT_GIC_ID_SDMMC },
+    /* int combiner group 30 */
+    { EXT_GIC_ID_MIPI_CSI_4LANE, EXT_GIC_ID_MIPI_CSI_2LANE },
+    /* int combiner group 31 */
+    { EXT_GIC_ID_MIPI_DSI_4LANE, EXT_GIC_ID_MIPI_DSI_2LANE },
+    /* int combiner group 32 */
+    { EXT_GIC_ID_FIMC0, EXT_GIC_ID_FIMC1 },
+    /* int combiner group 33 */
+    { EXT_GIC_ID_FIMC2, EXT_GIC_ID_FIMC3 },
+    /* int combiner group 34 */
+    { EXT_GIC_ID_ONENAND_AUDI, EXT_GIC_ID_NFC },
+    /* int combiner group 35 */
+    { 0, 0, 0, EXT_GIC_ID_MCT_L1, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    /* int combiner group 36 */
+    { EXT_GIC_ID_MIXER },
+    /* int combiner group 37 */
+    { EXT_GIC_ID_EXTINT4, EXT_GIC_ID_EXTINT5, EXT_GIC_ID_EXTINT6,
+     EXT_GIC_ID_EXTINT7 },
+    /* groups 38-50 */
+    { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { },
+    /* int combiner group 51 */
+    { EXT_GIC_ID_MCT_L0, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    /* group 52 */
+    { },
+    /* int combiner group 53 */
+    { EXT_GIC_ID_WDT, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    /* groups 54-63 */
+    { }, { }, { }, { }, { }, { }, { }, { }, { }, { }
+};
+
+/*
+ * Initialize board IRQs.
+ * These IRQs contain splitted Int/External Combiner and External Gic IRQs.
+ */
+static void exynos4210_init_board_irqs(Exynos4210State *s)
+{
+    uint32_t grp, bit, irq_id, n;
+    Exynos4210Irq *is = &s->irqs;
+
+    for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
+        irq_id = 0;
+        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4) ||
+                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4)) {
+            /* MCT_G0 is passed to External GIC */
+            irq_id = EXT_GIC_ID_MCT_G0;
+        }
+        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 5) ||
+                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 5)) {
+            /* MCT_G1 is passed to External and GIC */
+            irq_id = EXT_GIC_ID_MCT_G1;
+        }
+        if (irq_id) {
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_gic_irq[irq_id - 32]);
+        } else {
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_combiner_irq[n]);
+        }
+    }
+    for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
+        /* these IDs are passed to Internal Combiner and External GIC */
+        grp = EXYNOS4210_COMBINER_GET_GRP_NUM(n);
+        bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
+        irq_id = combiner_grp_to_gic_id[grp -
+                     EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][bit];
+
+        if (irq_id) {
+            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
+                    is->ext_gic_irq[irq_id - 32]);
+        }
+    }
+}
+
+/*
+ * Get IRQ number from exynos4210 IRQ subsystem stub.
+ * To identify IRQ source use internal combiner group and bit number
+ *  grp - group number
+ *  bit - bit number inside group
+ */
+uint32_t exynos4210_get_irq(uint32_t grp, uint32_t bit)
+{
+    return EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit);
+}
+
 static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
                                     0x09, 0x00, 0x00, 0x00 };
 
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index 3b77a485780..d8cad537fbf 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -30,154 +30,6 @@
 #include "hw/arm/exynos4210.h"
 #include "qom/object.h"
 
-enum ExtGicId {
-    EXT_GIC_ID_MDMA_LCD0 = 66,
-    EXT_GIC_ID_PDMA0,
-    EXT_GIC_ID_PDMA1,
-    EXT_GIC_ID_TIMER0,
-    EXT_GIC_ID_TIMER1,
-    EXT_GIC_ID_TIMER2,
-    EXT_GIC_ID_TIMER3,
-    EXT_GIC_ID_TIMER4,
-    EXT_GIC_ID_MCT_L0,
-    EXT_GIC_ID_WDT,
-    EXT_GIC_ID_RTC_ALARM,
-    EXT_GIC_ID_RTC_TIC,
-    EXT_GIC_ID_GPIO_XB,
-    EXT_GIC_ID_GPIO_XA,
-    EXT_GIC_ID_MCT_L1,
-    EXT_GIC_ID_IEM_APC,
-    EXT_GIC_ID_IEM_IEC,
-    EXT_GIC_ID_NFC,
-    EXT_GIC_ID_UART0,
-    EXT_GIC_ID_UART1,
-    EXT_GIC_ID_UART2,
-    EXT_GIC_ID_UART3,
-    EXT_GIC_ID_UART4,
-    EXT_GIC_ID_MCT_G0,
-    EXT_GIC_ID_I2C0,
-    EXT_GIC_ID_I2C1,
-    EXT_GIC_ID_I2C2,
-    EXT_GIC_ID_I2C3,
-    EXT_GIC_ID_I2C4,
-    EXT_GIC_ID_I2C5,
-    EXT_GIC_ID_I2C6,
-    EXT_GIC_ID_I2C7,
-    EXT_GIC_ID_SPI0,
-    EXT_GIC_ID_SPI1,
-    EXT_GIC_ID_SPI2,
-    EXT_GIC_ID_MCT_G1,
-    EXT_GIC_ID_USB_HOST,
-    EXT_GIC_ID_USB_DEVICE,
-    EXT_GIC_ID_MODEMIF,
-    EXT_GIC_ID_HSMMC0,
-    EXT_GIC_ID_HSMMC1,
-    EXT_GIC_ID_HSMMC2,
-    EXT_GIC_ID_HSMMC3,
-    EXT_GIC_ID_SDMMC,
-    EXT_GIC_ID_MIPI_CSI_4LANE,
-    EXT_GIC_ID_MIPI_DSI_4LANE,
-    EXT_GIC_ID_MIPI_CSI_2LANE,
-    EXT_GIC_ID_MIPI_DSI_2LANE,
-    EXT_GIC_ID_ONENAND_AUDI,
-    EXT_GIC_ID_ROTATOR,
-    EXT_GIC_ID_FIMC0,
-    EXT_GIC_ID_FIMC1,
-    EXT_GIC_ID_FIMC2,
-    EXT_GIC_ID_FIMC3,
-    EXT_GIC_ID_JPEG,
-    EXT_GIC_ID_2D,
-    EXT_GIC_ID_PCIe,
-    EXT_GIC_ID_MIXER,
-    EXT_GIC_ID_HDMI,
-    EXT_GIC_ID_HDMI_I2C,
-    EXT_GIC_ID_MFC,
-    EXT_GIC_ID_TVENC,
-};
-
-enum ExtInt {
-    EXT_GIC_ID_EXTINT0 = 48,
-    EXT_GIC_ID_EXTINT1,
-    EXT_GIC_ID_EXTINT2,
-    EXT_GIC_ID_EXTINT3,
-    EXT_GIC_ID_EXTINT4,
-    EXT_GIC_ID_EXTINT5,
-    EXT_GIC_ID_EXTINT6,
-    EXT_GIC_ID_EXTINT7,
-    EXT_GIC_ID_EXTINT8,
-    EXT_GIC_ID_EXTINT9,
-    EXT_GIC_ID_EXTINT10,
-    EXT_GIC_ID_EXTINT11,
-    EXT_GIC_ID_EXTINT12,
-    EXT_GIC_ID_EXTINT13,
-    EXT_GIC_ID_EXTINT14,
-    EXT_GIC_ID_EXTINT15
-};
-
-/*
- * External GIC sources which are not from External Interrupt Combiner or
- * External Interrupts are starting from EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ,
- * which is INTG16 in Internal Interrupt Combiner.
- */
-
-static const uint32_t
-combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
-    /* int combiner groups 16-19 */
-    { }, { }, { }, { },
-    /* int combiner group 20 */
-    { 0, EXT_GIC_ID_MDMA_LCD0 },
-    /* int combiner group 21 */
-    { EXT_GIC_ID_PDMA0, EXT_GIC_ID_PDMA1 },
-    /* int combiner group 22 */
-    { EXT_GIC_ID_TIMER0, EXT_GIC_ID_TIMER1, EXT_GIC_ID_TIMER2,
-            EXT_GIC_ID_TIMER3, EXT_GIC_ID_TIMER4 },
-    /* int combiner group 23 */
-    { EXT_GIC_ID_RTC_ALARM, EXT_GIC_ID_RTC_TIC },
-    /* int combiner group 24 */
-    { EXT_GIC_ID_GPIO_XB, EXT_GIC_ID_GPIO_XA },
-    /* int combiner group 25 */
-    { EXT_GIC_ID_IEM_APC, EXT_GIC_ID_IEM_IEC },
-    /* int combiner group 26 */
-    { EXT_GIC_ID_UART0, EXT_GIC_ID_UART1, EXT_GIC_ID_UART2, EXT_GIC_ID_UART3,
-            EXT_GIC_ID_UART4 },
-    /* int combiner group 27 */
-    { EXT_GIC_ID_I2C0, EXT_GIC_ID_I2C1, EXT_GIC_ID_I2C2, EXT_GIC_ID_I2C3,
-            EXT_GIC_ID_I2C4, EXT_GIC_ID_I2C5, EXT_GIC_ID_I2C6,
-            EXT_GIC_ID_I2C7 },
-    /* int combiner group 28 */
-    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 , EXT_GIC_ID_USB_HOST},
-    /* int combiner group 29 */
-    { EXT_GIC_ID_HSMMC0, EXT_GIC_ID_HSMMC1, EXT_GIC_ID_HSMMC2,
-     EXT_GIC_ID_HSMMC3, EXT_GIC_ID_SDMMC },
-    /* int combiner group 30 */
-    { EXT_GIC_ID_MIPI_CSI_4LANE, EXT_GIC_ID_MIPI_CSI_2LANE },
-    /* int combiner group 31 */
-    { EXT_GIC_ID_MIPI_DSI_4LANE, EXT_GIC_ID_MIPI_DSI_2LANE },
-    /* int combiner group 32 */
-    { EXT_GIC_ID_FIMC0, EXT_GIC_ID_FIMC1 },
-    /* int combiner group 33 */
-    { EXT_GIC_ID_FIMC2, EXT_GIC_ID_FIMC3 },
-    /* int combiner group 34 */
-    { EXT_GIC_ID_ONENAND_AUDI, EXT_GIC_ID_NFC },
-    /* int combiner group 35 */
-    { 0, 0, 0, EXT_GIC_ID_MCT_L1, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
-    /* int combiner group 36 */
-    { EXT_GIC_ID_MIXER },
-    /* int combiner group 37 */
-    { EXT_GIC_ID_EXTINT4, EXT_GIC_ID_EXTINT5, EXT_GIC_ID_EXTINT6,
-     EXT_GIC_ID_EXTINT7 },
-    /* groups 38-50 */
-    { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { },
-    /* int combiner group 51 */
-    { EXT_GIC_ID_MCT_L0, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
-    /* group 52 */
-    { },
-    /* int combiner group 53 */
-    { EXT_GIC_ID_WDT, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
-    /* groups 54-63 */
-    { }, { }, { }, { }, { }, { }, { }, { }, { }, { }
-};
-
 #define EXYNOS4210_GIC_NIRQ 160
 
 #define EXYNOS4210_EXT_GIC_CPU_REGION_SIZE     0x10000
@@ -192,62 +44,6 @@ combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
 #define EXYNOS4210_GIC_CPU_REGION_SIZE  0x100
 #define EXYNOS4210_GIC_DIST_REGION_SIZE 0x1000
 
-/*
- * Initialize board IRQs.
- * These IRQs contain splitted Int/External Combiner and External Gic IRQs.
- */
-void exynos4210_init_board_irqs(Exynos4210State *s)
-{
-    uint32_t grp, bit, irq_id, n;
-    Exynos4210Irq *is = &s->irqs;
-
-    for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
-        irq_id = 0;
-        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4) ||
-                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4)) {
-            /* MCT_G0 is passed to External GIC */
-            irq_id = EXT_GIC_ID_MCT_G0;
-        }
-        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 5) ||
-                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 5)) {
-            /* MCT_G1 is passed to External and GIC */
-            irq_id = EXT_GIC_ID_MCT_G1;
-        }
-        if (irq_id) {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_gic_irq[irq_id - 32]);
-        } else {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_combiner_irq[n]);
-        }
-    }
-    for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
-        /* these IDs are passed to Internal Combiner and External GIC */
-        grp = EXYNOS4210_COMBINER_GET_GRP_NUM(n);
-        bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
-        irq_id = combiner_grp_to_gic_id[grp -
-                     EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][bit];
-
-        if (irq_id) {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_gic_irq[irq_id - 32]);
-        }
-    }
-}
-
-/*
- * Get IRQ number from exynos4210 IRQ subsystem stub.
- * To identify IRQ source use internal combiner group and bit number
- *  grp - group number
- *  bit - bit number inside group
- */
-uint32_t exynos4210_get_irq(uint32_t grp, uint32_t bit)
-{
-    return EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit);
-}
-
-/********* GIC part *********/
-
 #define TYPE_EXYNOS4210_GIC "exynos4210.gic"
 OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210GicState, EXYNOS4210_GIC)
 
-- 
2.25.1

Re: [PATCH for-7.1 07/18] hw/arm/exynos4210: Move exynos4210_init_board_irqs() into exynos4210.c

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The function exynos4210_init_board_irqs() currently lives in
> exynos4210_gic.c, but it isn't really part of the exynos4210.gic
> device -- it is a function that implements (some of) the wiring up of
> interrupts between the SoC's GIC and combiner components.  This means
> it fits better in exynos4210.c, which is the SoC-level code.  Move it
> there. Similarly, exynos4210_git_irq() is used almost only in the
> SoC-level code, so move it too.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |   4 -
>   hw/arm/exynos4210.c         | 202 +++++++++++++++++++++++++++++++++++
>   hw/intc/exynos4210_gic.c    | 204 ------------------------------------
>   3 files changed, 202 insertions(+), 208 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 08/18] hw/arm/exynos4210: Put external GIC into state struct

[Peter Maydell]

Switch the creation of the external GIC to the new-style "embedded in
state struct" approach, so we can easily refer to the object
elsewhere during realize.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h      |  2 ++
 include/hw/intc/exynos4210_gic.h | 43 ++++++++++++++++++++++++++++++++
 hw/arm/exynos4210.c              | 10 ++++----
 hw/intc/exynos4210_gic.c         | 17 ++-----------
 MAINTAINERS                      |  2 +-
 5 files changed, 53 insertions(+), 21 deletions(-)
 create mode 100644 include/hw/intc/exynos4210_gic.h

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index d83e96a091e..f35ae90000f 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -27,6 +27,7 @@
 #include "hw/or-irq.h"
 #include "hw/sysbus.h"
 #include "hw/cpu/a9mpcore.h"
+#include "hw/intc/exynos4210_gic.h"
 #include "target/arm/cpu-qom.h"
 #include "qom/object.h"
 
@@ -103,6 +104,7 @@ struct Exynos4210State {
     qemu_or_irq pl330_irq_orgate[EXYNOS4210_NUM_DMA];
     qemu_or_irq cpu_irq_orgate[EXYNOS4210_NCPUS];
     A9MPPrivState a9mpcore;
+    Exynos4210GicState ext_gic;
 };
 
 #define TYPE_EXYNOS4210_SOC "exynos4210"
diff --git a/include/hw/intc/exynos4210_gic.h b/include/hw/intc/exynos4210_gic.h
new file mode 100644
index 00000000000..f64c4069c6d
--- /dev/null
+++ b/include/hw/intc/exynos4210_gic.h
@@ -0,0 +1,43 @@
+/*
+ * Samsung exynos4210 GIC implementation. Based on hw/arm_gic.c
+ *
+ * Copyright (c) 2000 - 2011 Samsung Electronics Co., Ltd.
+ * All rights reserved.
+ *
+ * Evgeny Voevodin <e.voevodin@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef HW_INTC_EXYNOS4210_GIC_H
+#define HW_INTC_EXYNOS4210_GIC_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_EXYNOS4210_GIC "exynos4210.gic"
+OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210GicState, EXYNOS4210_GIC)
+
+#define EXYNOS4210_GIC_NCPUS 2
+
+struct Exynos4210GicState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion cpu_container;
+    MemoryRegion dist_container;
+    MemoryRegion cpu_alias[EXYNOS4210_GIC_NCPUS];
+    MemoryRegion dist_alias[EXYNOS4210_GIC_NCPUS];
+    uint32_t num_cpu;
+    DeviceState *gic;
+};
+
+#endif
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 742666ba779..2058df9aecf 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -455,10 +455,9 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     sysbus_create_simple("l2x0", EXYNOS4210_L2X0_BASE_ADDR, NULL);
 
     /* External GIC */
-    dev = qdev_new("exynos4210.gic");
-    qdev_prop_set_uint32(dev, "num-cpu", EXYNOS4210_NCPUS);
-    busdev = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(busdev, &error_fatal);
+    qdev_prop_set_uint32(DEVICE(&s->ext_gic), "num-cpu", EXYNOS4210_NCPUS);
+    busdev = SYS_BUS_DEVICE(&s->ext_gic);
+    sysbus_realize(busdev, &error_fatal);
     /* Map CPU interface */
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_GIC_CPU_BASE_ADDR);
     /* Map Distributer interface */
@@ -468,7 +467,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
                            qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 1));
     }
     for (n = 0; n < EXYNOS4210_EXT_GIC_NIRQ; n++) {
-        s->irqs.ext_gic_irq[n] = qdev_get_gpio_in(dev, n);
+        s->irqs.ext_gic_irq[n] = qdev_get_gpio_in(DEVICE(&s->ext_gic), n);
     }
 
     /* Internal Interrupt Combiner */
@@ -686,6 +685,7 @@ static void exynos4210_init(Object *obj)
     }
 
     object_initialize_child(obj, "a9mpcore", &s->a9mpcore, TYPE_A9MPCORE_PRIV);
+    object_initialize_child(obj, "ext-gic", &s->ext_gic, TYPE_EXYNOS4210_GIC);
 }
 
 static void exynos4210_class_init(ObjectClass *klass, void *data)
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index d8cad537fbf..71a88c86bc1 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -27,6 +27,7 @@
 #include "qemu/module.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "hw/intc/exynos4210_gic.h"
 #include "hw/arm/exynos4210.h"
 #include "qom/object.h"
 
@@ -44,20 +45,6 @@
 #define EXYNOS4210_GIC_CPU_REGION_SIZE  0x100
 #define EXYNOS4210_GIC_DIST_REGION_SIZE 0x1000
 
-#define TYPE_EXYNOS4210_GIC "exynos4210.gic"
-OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210GicState, EXYNOS4210_GIC)
-
-struct Exynos4210GicState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion cpu_container;
-    MemoryRegion dist_container;
-    MemoryRegion cpu_alias[EXYNOS4210_NCPUS];
-    MemoryRegion dist_alias[EXYNOS4210_NCPUS];
-    uint32_t num_cpu;
-    DeviceState *gic;
-};
-
 static void exynos4210_gic_set_irq(void *opaque, int irq, int level)
 {
     Exynos4210GicState *s = (Exynos4210GicState *)opaque;
@@ -100,7 +87,7 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
      * enough room for the cpu numbers.  gcc 9.2.1 on 32-bit x86
      * doesn't figure this out, otherwise and gives spurious warnings.
      */
-    assert(n <= EXYNOS4210_NCPUS);
+    assert(n <= EXYNOS4210_GIC_NCPUS);
     for (i = 0; i < n; i++) {
         /* Map CPU interface per SMP Core */
         sprintf(cpu_alias_name, "%s%x", cpu_prefix, i);
diff --git a/MAINTAINERS b/MAINTAINERS
index cc364afef73..5ac10b88977 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -648,7 +648,7 @@ M: Peter Maydell <peter.maydell@linaro.org>
 L: qemu-arm@nongnu.org
 S: Odd Fixes
 F: hw/*/exynos*
-F: include/hw/arm/exynos4210.h
+F: include/hw/*/exynos*
 
 Calxeda Highbank
 M: Rob Herring <robh@kernel.org>
-- 
2.25.1

Re: [PATCH for-7.1 08/18] hw/arm/exynos4210: Put external GIC into state struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> Switch the creation of the external GIC to the new-style "embedded in
> state struct" approach, so we can easily refer to the object
> elsewhere during realize.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h      |  2 ++
>   include/hw/intc/exynos4210_gic.h | 43 ++++++++++++++++++++++++++++++++
>   hw/arm/exynos4210.c              | 10 ++++----
>   hw/intc/exynos4210_gic.c         | 17 ++-----------
>   MAINTAINERS                      |  2 +-
>   5 files changed, 53 insertions(+), 21 deletions(-)
>   create mode 100644 include/hw/intc/exynos4210_gic.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 09/18] hw/arm/exynos4210: Drop ext_gic_irq[] from Exynos4210Irq struct

[Peter Maydell]

The only time we use the ext_gic_irq[] array in the Exynos4210Irq
struct is during realize of the SoC -- we initialize it with the
input IRQs of the external GIC device, and then connect those to
outputs of other devices further on in realize (including in the
exynos4210_init_board_irqs() function).  Now that the ext_gic object
is easily accessible as s->ext_gic we can make the connections
directly from one device to the other without going via this array.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  1 -
 hw/arm/exynos4210.c         | 12 ++++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index f35ae90000f..08f52c511ff 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -83,7 +83,6 @@
 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];
-    qemu_irq ext_gic_irq[EXYNOS4210_EXT_GIC_NIRQ];
 } Exynos4210Irq;
 
 struct Exynos4210State {
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 2058df9aecf..5a41af089f9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -257,6 +257,7 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
 {
     uint32_t grp, bit, irq_id, n;
     Exynos4210Irq *is = &s->irqs;
+    DeviceState *extgicdev = DEVICE(&s->ext_gic);
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
         irq_id = 0;
@@ -272,7 +273,8 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
         }
         if (irq_id) {
             s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_gic_irq[irq_id - 32]);
+                                             qdev_get_gpio_in(extgicdev,
+                                                              irq_id - 32));
         } else {
             s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
                     is->ext_combiner_irq[n]);
@@ -287,7 +289,8 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
 
         if (irq_id) {
             s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_gic_irq[irq_id - 32]);
+                                             qdev_get_gpio_in(extgicdev,
+                                                              irq_id - 32));
         }
     }
 }
@@ -466,9 +469,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
         sysbus_connect_irq(busdev, n,
                            qdev_get_gpio_in(DEVICE(&s->cpu_irq_orgate[n]), 1));
     }
-    for (n = 0; n < EXYNOS4210_EXT_GIC_NIRQ; n++) {
-        s->irqs.ext_gic_irq[n] = qdev_get_gpio_in(DEVICE(&s->ext_gic), n);
-    }
 
     /* Internal Interrupt Combiner */
     dev = qdev_new("exynos4210.combiner");
@@ -487,7 +487,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     busdev = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(busdev, &error_fatal);
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
-        sysbus_connect_irq(busdev, n, s->irqs.ext_gic_irq[n]);
+        sysbus_connect_irq(busdev, n, qdev_get_gpio_in(DEVICE(&s->ext_gic), n));
     }
     exynos4210_combiner_get_gpioin(&s->irqs, dev, 1);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_COMBINER_BASE_ADDR);
-- 
2.25.1

Re: [PATCH for-7.1 09/18] hw/arm/exynos4210: Drop ext_gic_irq[] from Exynos4210Irq struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The only time we use the ext_gic_irq[] array in the Exynos4210Irq
> struct is during realize of the SoC -- we initialize it with the
> input IRQs of the external GIC device, and then connect those to
> outputs of other devices further on in realize (including in the
> exynos4210_init_board_irqs() function).  Now that the ext_gic object
> is easily accessible as s->ext_gic we can make the connections
> directly from one device to the other without going via this array.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  1 -
>   hw/arm/exynos4210.c         | 12 ++++++------
>   2 files changed, 6 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 10/18] hw/arm/exynos4210: Move exynos4210_combiner_get_gpioin() into exynos4210.c

[Peter Maydell]

The function exynos4210_combiner_get_gpioin() currently lives in
exynos4210_combiner.c, but it isn't really part of the combiner
device itself -- it is a function that implements the wiring up of
some interrupt sources to multiple combiner inputs.  Move it to live
with the other SoC-level code in exynos4210.c, along with a few
macros previously defined in exynos4210.h which are now used only
in exynos4210.c.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h   | 11 -----
 hw/arm/exynos4210.c           | 82 +++++++++++++++++++++++++++++++++++
 hw/intc/exynos4210_combiner.c | 77 --------------------------------
 3 files changed, 82 insertions(+), 88 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 08f52c511ff..b564e3582bb 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -67,11 +67,6 @@
 #define EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ   \
     (EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ * 8)
 
-#define EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit)  ((grp)*8 + (bit))
-#define EXYNOS4210_COMBINER_GET_GRP_NUM(irq)       ((irq) / 8)
-#define EXYNOS4210_COMBINER_GET_BIT_NUM(irq) \
-    ((irq) - 8 * EXYNOS4210_COMBINER_GET_GRP_NUM(irq))
-
 /* IRQs number for external and internal GIC */
 #define EXYNOS4210_EXT_GIC_NIRQ     (160-32)
 #define EXYNOS4210_INT_GIC_NIRQ     64
@@ -118,12 +113,6 @@ void exynos4210_write_secondary(ARMCPU *cpu,
  *  bit - bit number inside group */
 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,
-        int ext);
-
 /*
  * exynos4210 UART
  */
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 5a41af089f9..86a9a0dae12 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -249,6 +249,11 @@ combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
     { }, { }, { }, { }, { }, { }, { }, { }, { }, { }
 };
 
+#define EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit)  ((grp) * 8 + (bit))
+#define EXYNOS4210_COMBINER_GET_GRP_NUM(irq)       ((irq) / 8)
+#define EXYNOS4210_COMBINER_GET_BIT_NUM(irq) \
+    ((irq) - 8 * EXYNOS4210_COMBINER_GET_GRP_NUM(irq))
+
 /*
  * Initialize board IRQs.
  * These IRQs contain splitted Int/External Combiner and External Gic IRQs.
@@ -306,6 +311,83 @@ uint32_t exynos4210_get_irq(uint32_t grp, uint32_t bit)
     return EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit);
 }
 
+/*
+ * Get Combiner input GPIO into irqs structure
+ */
+static void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs,
+                                           DeviceState *dev, int ext)
+{
+    int n;
+    int bit;
+    int max;
+    qemu_irq *irq;
+
+    max = ext ? EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ :
+        EXYNOS4210_MAX_INT_COMBINER_IN_IRQ;
+    irq = ext ? irqs->ext_combiner_irq : irqs->int_combiner_irq;
+
+    /*
+     * Some IRQs of Int/External Combiner are going to two Combiners groups,
+     * so let split them.
+     */
+    for (n = 0; n < max; n++) {
+
+        bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
+
+        switch (n) {
+        /* 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)]);
+            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)]);
+            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)]);
+            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)]);
+            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)]);
+            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)]);
+            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)]);
+            continue;
+        }
+
+        irq[n] = qdev_get_gpio_in(dev, n);
+    }
+}
+
 static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
                                     0x09, 0x00, 0x00, 0x00 };
 
diff --git a/hw/intc/exynos4210_combiner.c b/hw/intc/exynos4210_combiner.c
index 4534ee248db..83b42b9bce1 100644
--- a/hw/intc/exynos4210_combiner.c
+++ b/hw/intc/exynos4210_combiner.c
@@ -105,83 +105,6 @@ static const VMStateDescription vmstate_exynos4210_combiner = {
     }
 };
 
-/*
- * Get Combiner input GPIO into irqs structure
- */
-void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *dev,
-        int ext)
-{
-    int n;
-    int bit;
-    int max;
-    qemu_irq *irq;
-
-    max = ext ? EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ :
-        EXYNOS4210_MAX_INT_COMBINER_IN_IRQ;
-    irq = ext ? irqs->ext_combiner_irq : irqs->int_combiner_irq;
-
-    /*
-     * Some IRQs of Int/External Combiner are going to two Combiners groups,
-     * so let split them.
-     */
-    for (n = 0; n < max; n++) {
-
-        bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
-
-        switch (n) {
-        /* 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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            continue;
-        }
-
-        irq[n] = qdev_get_gpio_in(dev, n);
-    }
-}
-
 static uint64_t
 exynos4210_combiner_read(void *opaque, hwaddr offset, unsigned size)
 {
-- 
2.25.1

Re: [PATCH for-7.1 10/18] hw/arm/exynos4210: Move exynos4210_combiner_get_gpioin() into exynos4210.c

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The function exynos4210_combiner_get_gpioin() currently lives in
> exynos4210_combiner.c, but it isn't really part of the combiner
> device itself -- it is a function that implements the wiring up of
> some interrupt sources to multiple combiner inputs.  Move it to live
> with the other SoC-level code in exynos4210.c, along with a few
> macros previously defined in exynos4210.h which are now used only
> in exynos4210.c.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h   | 11 -----
>   hw/arm/exynos4210.c           | 82 +++++++++++++++++++++++++++++++++++
>   hw/intc/exynos4210_combiner.c | 77 --------------------------------
>   3 files changed, 82 insertions(+), 88 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 11/18] hw/arm/exynos4210: Delete unused macro definitions

[Peter Maydell]

Delete a couple of #defines which are never used.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index b564e3582bb..f0769a4045b 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -67,10 +67,6 @@
 #define EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ   \
     (EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ * 8)
 
-/* IRQs number for external and internal GIC */
-#define EXYNOS4210_EXT_GIC_NIRQ     (160-32)
-#define EXYNOS4210_INT_GIC_NIRQ     64
-
 #define EXYNOS4210_I2C_NUMBER               9
 
 #define EXYNOS4210_NUM_DMA      3
-- 
2.25.1

Re: [PATCH for-7.1 11/18] hw/arm/exynos4210: Delete unused macro definitions

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> Delete a couple of #defines which are never used.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h | 4 ----
>   1 file changed, 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 12/18] hw/arm/exynos4210: Use TYPE_SPLIT_IRQ in exynos4210_init_board_irqs()

[Peter Maydell]

In exynos4210_init_board_irqs(), use the TYPE_SPLIT_IRQ device
instead of qemu_irq_split().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  9 ++++++++
 hw/arm/exynos4210.c         | 41 +++++++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index f0769a4045b..f58ee0f2686 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -28,6 +28,7 @@
 #include "hw/sysbus.h"
 #include "hw/cpu/a9mpcore.h"
 #include "hw/intc/exynos4210_gic.h"
+#include "hw/core/split-irq.h"
 #include "target/arm/cpu-qom.h"
 #include "qom/object.h"
 
@@ -71,6 +72,13 @@
 
 #define EXYNOS4210_NUM_DMA      3
 
+/*
+ * We need one splitter for every external combiner input, plus
+ * one for every non-zero entry in combiner_grp_to_gic_id[].
+ * We'll assert in exynos4210_init_board_irqs() if this is wrong.
+ */
+#define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 60)
+
 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];
@@ -95,6 +103,7 @@ struct Exynos4210State {
     qemu_or_irq cpu_irq_orgate[EXYNOS4210_NCPUS];
     A9MPPrivState a9mpcore;
     Exynos4210GicState ext_gic;
+    SplitIRQ splitter[EXYNOS4210_NUM_SPLITTERS];
 };
 
 #define TYPE_EXYNOS4210_SOC "exynos4210"
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 86a9a0dae12..919821833b5 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -263,6 +263,8 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
     uint32_t grp, bit, irq_id, n;
     Exynos4210Irq *is = &s->irqs;
     DeviceState *extgicdev = DEVICE(&s->ext_gic);
+    int splitcount = 0;
+    DeviceState *splitter;
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
         irq_id = 0;
@@ -276,13 +278,19 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
             /* MCT_G1 is passed to External and GIC */
             irq_id = EXT_GIC_ID_MCT_G1;
         }
+
+        assert(splitcount < EXYNOS4210_NUM_SPLITTERS);
+        splitter = DEVICE(&s->splitter[splitcount]);
+        qdev_prop_set_uint16(splitter, "num-lines", 2);
+        qdev_realize(splitter, NULL, &error_abort);
+        splitcount++;
+        s->irq_table[n] = qdev_get_gpio_in(splitter, 0);
+        qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
         if (irq_id) {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                                             qdev_get_gpio_in(extgicdev,
-                                                              irq_id - 32));
+            qdev_connect_gpio_out(splitter, 1,
+                                  qdev_get_gpio_in(extgicdev, irq_id - 32));
         } else {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                    is->ext_combiner_irq[n]);
+            qdev_connect_gpio_out(splitter, 1, is->ext_combiner_irq[n]);
         }
     }
     for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
@@ -293,11 +301,23 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
                      EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][bit];
 
         if (irq_id) {
-            s->irq_table[n] = qemu_irq_split(is->int_combiner_irq[n],
-                                             qdev_get_gpio_in(extgicdev,
-                                                              irq_id - 32));
+            assert(splitcount < EXYNOS4210_NUM_SPLITTERS);
+            splitter = DEVICE(&s->splitter[splitcount]);
+            qdev_prop_set_uint16(splitter, "num-lines", 2);
+            qdev_realize(splitter, NULL, &error_abort);
+            splitcount++;
+            s->irq_table[n] = qdev_get_gpio_in(splitter, 0);
+            qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
+            qdev_connect_gpio_out(splitter, 1,
+                                  qdev_get_gpio_in(extgicdev, irq_id - 32));
         }
     }
+    /*
+     * We check this here to avoid a more obscure assert later when
+     * qdev_assert_realized_properly() checks that we realized every
+     * child object we initialized.
+     */
+    assert(splitcount == EXYNOS4210_NUM_SPLITTERS);
 }
 
 /*
@@ -766,6 +786,11 @@ static void exynos4210_init(Object *obj)
         object_initialize_child(obj, name, &s->cpu_irq_orgate[i], TYPE_OR_IRQ);
     }
 
+    for (i = 0; i < ARRAY_SIZE(s->splitter); i++) {
+        g_autofree char *name = g_strdup_printf("irq-splitter%d", i);
+        object_initialize_child(obj, name, &s->splitter[i], TYPE_SPLIT_IRQ);
+    }
+
     object_initialize_child(obj, "a9mpcore", &s->a9mpcore, TYPE_A9MPCORE_PRIV);
     object_initialize_child(obj, "ext-gic", &s->ext_gic, TYPE_EXYNOS4210_GIC);
 }
-- 
2.25.1

Re: [PATCH for-7.1 12/18] hw/arm/exynos4210: Use TYPE_SPLIT_IRQ in exynos4210_init_board_irqs()

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> In exynos4210_init_board_irqs(), use the TYPE_SPLIT_IRQ device
> instead of qemu_irq_split().
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  9 ++++++++
>   hw/arm/exynos4210.c         | 41 +++++++++++++++++++++++++++++--------
>   2 files changed, 42 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 13/18] hw/arm/exynos4210: Fill in irq_table[] for internal-combiner-only IRQ lines

[Peter Maydell]

In exynos4210_init_board_irqs(), the loop that handles IRQ lines that
are in a range that applies to the internal combiner only creates a
splitter for those interrupts which go to both the internal combiner
and to the external GIC, but it does nothing at all for the
interrupts which don't go to the external GIC, leaving the
irq_table[] array element empty for those.  (This will result in
those interrupts simply being lost, not in a QEMU crash.)

I don't have a reliable datasheet for this SoC, but since we do wire
up one interrupt line in this category (the HDMI I2C device on
interrupt 16,1), this seems like it must be a bug in the existing
QEMU code.  Fill in the irq_table[] entries where we're not splitting
the IRQ to both the internal combiner and the external GIC with the
IRQ line of the internal combiner.  (That is, these IRQ lines go to
just one device, not multiple.)

This bug didn't have any visible guest effects because the only
implemented device that was affected was the HDMI I2C controller,
and we never connect any I2C devices to that bus.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/exynos4210.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 919821833b5..a4527f819ef 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -310,6 +310,8 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
             qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
             qdev_connect_gpio_out(splitter, 1,
                                   qdev_get_gpio_in(extgicdev, irq_id - 32));
+        } else {
+            s->irq_table[n] = is->int_combiner_irq[n];
         }
     }
     /*
-- 
2.25.1

Re: [PATCH for-7.1 13/18] hw/arm/exynos4210: Fill in irq_table[] for internal-combiner-only IRQ lines

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> In exynos4210_init_board_irqs(), the loop that handles IRQ lines that
> are in a range that applies to the internal combiner only creates a
> splitter for those interrupts which go to both the internal combiner
> and to the external GIC, but it does nothing at all for the
> interrupts which don't go to the external GIC, leaving the
> irq_table[] array element empty for those.  (This will result in
> those interrupts simply being lost, not in a QEMU crash.)
> 
> I don't have a reliable datasheet for this SoC, but since we do wire
> up one interrupt line in this category (the HDMI I2C device on
> interrupt 16,1), this seems like it must be a bug in the existing
> QEMU code.  Fill in the irq_table[] entries where we're not splitting
> the IRQ to both the internal combiner and the external GIC with the
> IRQ line of the internal combiner.  (That is, these IRQ lines go to
> just one device, not multiple.)
> 
> This bug didn't have any visible guest effects because the only
> implemented device that was affected was the HDMI I2C controller,
> and we never connect any I2C devices to that bus.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/arm/exynos4210.c | 2 ++
>   1 file changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 14/18] hw/arm/exynos4210: Connect MCT_G0 and MCT_G1 to both combiners

[Peter Maydell]

Currently for the interrupts MCT_G0 and MCT_G1 which are
the only ones in the input range of the external combiner
and which are also wired to the external GIC, we connect
them only to the internal combiner and the external GIC.
This seems likely to be a bug, as all other interrupts
which are in the input range of both combiners are
connected to both combiners. (The fact that the code in
exynos4210_combiner_get_gpioin() is also trying to wire
up these inputs on both combiners also suggests this.)

Wire these interrupts up to both combiners, like the rest.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/exynos4210.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index a4527f819ef..962d6d0ac2a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -281,16 +281,15 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
 
         assert(splitcount < EXYNOS4210_NUM_SPLITTERS);
         splitter = DEVICE(&s->splitter[splitcount]);
-        qdev_prop_set_uint16(splitter, "num-lines", 2);
+        qdev_prop_set_uint16(splitter, "num-lines", irq_id ? 3 : 2);
         qdev_realize(splitter, NULL, &error_abort);
         splitcount++;
         s->irq_table[n] = qdev_get_gpio_in(splitter, 0);
         qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
+        qdev_connect_gpio_out(splitter, 1, is->ext_combiner_irq[n]);
         if (irq_id) {
-            qdev_connect_gpio_out(splitter, 1,
+            qdev_connect_gpio_out(splitter, 2,
                                   qdev_get_gpio_in(extgicdev, irq_id - 32));
-        } else {
-            qdev_connect_gpio_out(splitter, 1, is->ext_combiner_irq[n]);
         }
     }
     for (; n < EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) {
-- 
2.25.1

Re: [PATCH for-7.1 14/18] hw/arm/exynos4210: Connect MCT_G0 and MCT_G1 to both combiners

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> Currently for the interrupts MCT_G0 and MCT_G1 which are
> the only ones in the input range of the external combiner
> and which are also wired to the external GIC, we connect
> them only to the internal combiner and the external GIC.
> This seems likely to be a bug, as all other interrupts
> which are in the input range of both combiners are
> connected to both combiners. (The fact that the code in
> exynos4210_combiner_get_gpioin() is also trying to wire
> up these inputs on both combiners also suggests this.)
> 
> Wire these interrupts up to both combiners, like the rest.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/arm/exynos4210.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 15/18] hw/arm/exynos4210: Don't connect multiple lines to external GIC inputs

[Peter Maydell]

The combiner_grp_to_gic_id[] array includes the EXT_GIC_ID_MCT_G0
and EXT_GIC_ID_MCT_G1 multiple times. This means that we will
connect multiple IRQs up to the same external GIC input, which
is not permitted. We do the same thing in the code in
exynos4210_init_board_irqs() because the conditionals selecting
an irq_id in the first loop match multiple interrupt IDs.

Overall we do this for interrupt IDs
(1, 4), (12, 4), (35, 4), (51, 4), (53, 4) for EXT_GIC_ID_MCT_G0
and
(1, 5), (12, 5), (35, 5), (51, 5), (53, 5) for EXT_GIC_ID_MCT_G1

These correspond to the cases for the multi-core timer that we are
wiring up to multiple inputs on the combiner in
exynos4210_combiner_get_gpioin().  That code already deals with all
these interrupt IDs being the same input source, so we don't need to
connect the external GIC interrupt for any of them except the first
(1, 4) and (1, 5). Remove the array entries and conditionals which
were incorrectly causing us to wire up extra lines.

This bug didn't cause any visible effects, because we only connect
up a device to the "primary" ID values (1, 4) and (1, 5), so the
extra lines would never be set to a level.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  2 +-
 hw/arm/exynos4210.c         | 12 +++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index f58ee0f2686..7da3eddea5f 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -77,7 +77,7 @@
  * one for every non-zero entry in combiner_grp_to_gic_id[].
  * We'll assert in exynos4210_init_board_irqs() if this is wrong.
  */
-#define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 60)
+#define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 54)
 
 typedef struct Exynos4210Irq {
     qemu_irq int_combiner_irq[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 962d6d0ac2a..39e334e0773 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -231,7 +231,7 @@ combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
     /* int combiner group 34 */
     { EXT_GIC_ID_ONENAND_AUDI, EXT_GIC_ID_NFC },
     /* int combiner group 35 */
-    { 0, 0, 0, EXT_GIC_ID_MCT_L1, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    { 0, 0, 0, EXT_GIC_ID_MCT_L1 },
     /* int combiner group 36 */
     { EXT_GIC_ID_MIXER },
     /* int combiner group 37 */
@@ -240,11 +240,11 @@ combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
     /* groups 38-50 */
     { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { }, { },
     /* int combiner group 51 */
-    { EXT_GIC_ID_MCT_L0, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    { EXT_GIC_ID_MCT_L0 },
     /* group 52 */
     { },
     /* int combiner group 53 */
-    { EXT_GIC_ID_WDT, 0, 0, 0, EXT_GIC_ID_MCT_G0, EXT_GIC_ID_MCT_G1 },
+    { EXT_GIC_ID_WDT },
     /* groups 54-63 */
     { }, { }, { }, { }, { }, { }, { }, { }, { }, { }
 };
@@ -268,13 +268,11 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
         irq_id = 0;
-        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4) ||
-                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4)) {
+        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4)) {
             /* MCT_G0 is passed to External GIC */
             irq_id = EXT_GIC_ID_MCT_G0;
         }
-        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 5) ||
-                n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 5)) {
+        if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 5)) {
             /* MCT_G1 is passed to External and GIC */
             irq_id = EXT_GIC_ID_MCT_G1;
         }
-- 
2.25.1

Re: [PATCH for-7.1 15/18] hw/arm/exynos4210: Don't connect multiple lines to external GIC inputs

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The combiner_grp_to_gic_id[] array includes the EXT_GIC_ID_MCT_G0
> and EXT_GIC_ID_MCT_G1 multiple times. This means that we will
> connect multiple IRQs up to the same external GIC input, which
> is not permitted. We do the same thing in the code in
> exynos4210_init_board_irqs() because the conditionals selecting
> an irq_id in the first loop match multiple interrupt IDs.
> 
> Overall we do this for interrupt IDs
> (1, 4), (12, 4), (35, 4), (51, 4), (53, 4) for EXT_GIC_ID_MCT_G0
> and
> (1, 5), (12, 5), (35, 5), (51, 5), (53, 5) for EXT_GIC_ID_MCT_G1
> 
> These correspond to the cases for the multi-core timer that we are
> wiring up to multiple inputs on the combiner in
> exynos4210_combiner_get_gpioin().  That code already deals with all
> these interrupt IDs being the same input source, so we don't need to
> connect the external GIC interrupt for any of them except the first
> (1, 4) and (1, 5). Remove the array entries and conditionals which
> were incorrectly causing us to wire up extra lines.
> 
> This bug didn't cause any visible effects, because we only connect
> up a device to the "primary" ID values (1, 4) and (1, 5), so the
> extra lines would never be set to a level.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  2 +-
>   hw/arm/exynos4210.c         | 12 +++++-------
>   2 files changed, 6 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 16/18] hw/arm/exynos4210: Fold combiner splits into exynos4210_init_board_irqs()

[Peter Maydell]

At this point, the function exynos4210_init_board_irqs() splits input
IRQ lines to connect them to the input combiner, output combiner and
external GIC.  The function exynos4210_combiner_get_gpioin() splits
some of the combiner input lines further to connect them to multiple
different inputs on the combiner.

Because (unlike qemu_irq_split()) the TYPE_SPLIT_IRQ device has a
configurable number of outputs, we can do all this in one place, by
making exynos4210_init_board_irqs() add extra outputs to the splitter
device when it must be connected to more than one input on each
combiner.

We do this with a new data structure, the combinermap, which is an
array each of whose elements is a list of the interrupt IDs on the
combiner which must be tied together.  As we loop through each
interrupt ID, if we find that it is the first one in one of these
lists, we configure the splitter device with eonugh extra outputs and
wire them up to the other interrupt IDs in the list.

Conveniently, for all the cases where this is necessary, the
lowest-numbered interrupt ID in each group is in the range of the
external combiner, so we only need to code for this in the first of
the two loops in exynos4210_init_board_irqs().

The old code in exynos4210_combiner_get_gpioin() which is being
deleted here had several problems which don't exist in the new code
in its handling of the multi-core timer interrupts:
 (1) the case labels specified bits 4 ... 8, but bit '8' doesn't
     exist; these should have been 4 ... 7
 (2) it used the input irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]
     multiple times as the input of several different splitters,
     which isn't allowed
 (3) in an apparent cut-and-paste error, the cases for all the
     multi-core timer inputs used "bit + 4" even though the
     bit range for the case was (intended to be) 4 ... 7, which
     meant it was looking at non-existent bits 8 ... 11.
None of these exist in the new code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |   6 +-
 hw/arm/exynos4210.c         | 178 +++++++++++++++++++++++-------------
 2 files changed, 119 insertions(+), 65 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 7da3eddea5f..f24617f681d 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -74,10 +74,12 @@
 
 /*
  * We need one splitter for every external combiner input, plus
- * one for every non-zero entry in combiner_grp_to_gic_id[].
+ * one for every non-zero entry in combiner_grp_to_gic_id[],
+ * minus one for every external combiner ID in second or later
+ * places in a combinermap[] line.
  * We'll assert in exynos4210_init_board_irqs() if this is wrong.
  */
-#define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 54)
+#define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 38)
 
 typedef struct Exynos4210Irq {
     qemu_irq int_combiner_irq[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 39e334e0773..05b28cf5905 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -254,6 +254,76 @@ combiner_grp_to_gic_id[64 - EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
 #define EXYNOS4210_COMBINER_GET_BIT_NUM(irq) \
     ((irq) - 8 * EXYNOS4210_COMBINER_GET_GRP_NUM(irq))
 
+/*
+ * Some interrupt lines go to multiple combiner inputs.
+ * This data structure defines those: each array element is
+ * a list of combiner inputs which are connected together;
+ * the one with the smallest interrupt ID value must be first.
+ * As with combiner_grp_to_gic_id[], we rely on (0, 0) not being
+ * wired to anything so we can use 0 as a terminator.
+ */
+#define IRQNO(G, B) EXYNOS4210_COMBINER_GET_IRQ_NUM(G, B)
+#define IRQNONE 0
+
+#define COMBINERMAP_SIZE 16
+
+static const int combinermap[COMBINERMAP_SIZE][6] = {
+    /* MDNIE_LCD1 */
+    { IRQNO(0, 4), IRQNO(1, 0), IRQNONE },
+    { IRQNO(0, 5), IRQNO(1, 1), IRQNONE },
+    { IRQNO(0, 6), IRQNO(1, 2), IRQNONE },
+    { IRQNO(0, 7), IRQNO(1, 3), IRQNONE },
+    /* TMU */
+    { IRQNO(2, 4), IRQNO(3, 4), IRQNONE },
+    { IRQNO(2, 5), IRQNO(3, 5), IRQNONE },
+    { IRQNO(2, 6), IRQNO(3, 6), IRQNONE },
+    { IRQNO(2, 7), IRQNO(3, 7), IRQNONE },
+    /* LCD1 */
+    { IRQNO(11, 4), IRQNO(12, 0), IRQNONE },
+    { IRQNO(11, 5), IRQNO(12, 1), IRQNONE },
+    { IRQNO(11, 6), IRQNO(12, 2), IRQNONE },
+    { IRQNO(11, 7), IRQNO(12, 3), IRQNONE },
+    /* Multi-core timer */
+    { IRQNO(1, 4), IRQNO(12, 4), IRQNO(35, 4), IRQNO(51, 4), IRQNO(53, 4), IRQNONE },
+    { IRQNO(1, 5), IRQNO(12, 5), IRQNO(35, 5), IRQNO(51, 5), IRQNO(53, 5), IRQNONE },
+    { IRQNO(1, 6), IRQNO(12, 6), IRQNO(35, 6), IRQNO(51, 6), IRQNO(53, 6), IRQNONE },
+    { IRQNO(1, 7), IRQNO(12, 7), IRQNO(35, 7), IRQNO(51, 7), IRQNO(53, 7), IRQNONE },
+};
+
+#undef IRQNO
+
+static const int *combinermap_entry(int irq)
+{
+    /*
+     * If the interrupt number passed in is the first entry in some
+     * line of the combinermap, return a pointer to that line;
+     * otherwise return NULL.
+     */
+    int i;
+    for (i = 0; i < COMBINERMAP_SIZE; i++) {
+        if (combinermap[i][0] == irq) {
+            return combinermap[i];
+        }
+    }
+    return NULL;
+}
+
+static int mapline_size(const int *mapline)
+{
+    /* Return number of entries in this mapline in total */
+    int i = 0;
+
+    if (!mapline) {
+        /* Not in the map? IRQ goes to exactly one combiner input */
+        return 1;
+    }
+    while (*mapline != IRQNONE) {
+        mapline++;
+        i++;
+    }
+    return i;
+}
+
 /*
  * Initialize board IRQs.
  * These IRQs contain splitted Int/External Combiner and External Gic IRQs.
@@ -265,6 +335,8 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
     DeviceState *extgicdev = DEVICE(&s->ext_gic);
     int splitcount = 0;
     DeviceState *splitter;
+    const int *mapline;
+    int numlines, splitin, in;
 
     for (n = 0; n < EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) {
         irq_id = 0;
@@ -277,16 +349,46 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
             irq_id = EXT_GIC_ID_MCT_G1;
         }
 
+        if (s->irq_table[n]) {
+            /*
+             * This must be some non-first entry in a combinermap line,
+             * and we've already filled it in.
+             */
+            continue;
+        }
+        mapline = combinermap_entry(n);
+        /*
+         * We need to connect the IRQ to multiple inputs on both combiners
+         * and possibly also to the external GIC.
+         */
+        numlines = 2 * mapline_size(mapline);
+        if (irq_id) {
+            numlines++;
+        }
         assert(splitcount < EXYNOS4210_NUM_SPLITTERS);
         splitter = DEVICE(&s->splitter[splitcount]);
-        qdev_prop_set_uint16(splitter, "num-lines", irq_id ? 3 : 2);
+        qdev_prop_set_uint16(splitter, "num-lines", numlines);
         qdev_realize(splitter, NULL, &error_abort);
         splitcount++;
-        s->irq_table[n] = qdev_get_gpio_in(splitter, 0);
-        qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
-        qdev_connect_gpio_out(splitter, 1, is->ext_combiner_irq[n]);
+
+        in = n;
+        splitin = 0;
+        for (;;) {
+            s->irq_table[in] = qdev_get_gpio_in(splitter, 0);
+            qdev_connect_gpio_out(splitter, splitin, is->int_combiner_irq[in]);
+            qdev_connect_gpio_out(splitter, splitin + 1, is->ext_combiner_irq[in]);
+            splitin += 2;
+            if (!mapline) {
+                break;
+            }
+            mapline++;
+            in = *mapline;
+            if (in == IRQNONE) {
+                break;
+            }
+        }
         if (irq_id) {
-            qdev_connect_gpio_out(splitter, 2,
+            qdev_connect_gpio_out(splitter, splitin,
                                   qdev_get_gpio_in(extgicdev, irq_id - 32));
         }
     }
@@ -297,6 +399,14 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
         irq_id = combiner_grp_to_gic_id[grp -
                      EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][bit];
 
+        if (s->irq_table[n]) {
+            /*
+             * This must be some non-first entry in a combinermap line,
+             * and we've already filled it in.
+             */
+            continue;
+        }
+
         if (irq_id) {
             assert(splitcount < EXYNOS4210_NUM_SPLITTERS);
             splitter = DEVICE(&s->splitter[splitcount]);
@@ -337,7 +447,6 @@ static void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs,
                                            DeviceState *dev, int ext)
 {
     int n;
-    int bit;
     int max;
     qemu_irq *irq;
 
@@ -345,64 +454,7 @@ static void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs,
         EXYNOS4210_MAX_INT_COMBINER_IN_IRQ;
     irq = ext ? irqs->ext_combiner_irq : irqs->int_combiner_irq;
 
-    /*
-     * Some IRQs of Int/External Combiner are going to two Combiners groups,
-     * so let split them.
-     */
     for (n = 0; n < max; n++) {
-
-        bit = EXYNOS4210_COMBINER_GET_BIT_NUM(n);
-
-        switch (n) {
-        /* 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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            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)]);
-            continue;
-        }
-
         irq[n] = qdev_get_gpio_in(dev, n);
     }
 }
-- 
2.25.1

Re: [PATCH for-7.1 16/18] hw/arm/exynos4210: Fold combiner splits into exynos4210_init_board_irqs()

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> At this point, the function exynos4210_init_board_irqs() splits input
> IRQ lines to connect them to the input combiner, output combiner and
> external GIC.  The function exynos4210_combiner_get_gpioin() splits
> some of the combiner input lines further to connect them to multiple
> different inputs on the combiner.
> 
> Because (unlike qemu_irq_split()) the TYPE_SPLIT_IRQ device has a
> configurable number of outputs, we can do all this in one place, by
> making exynos4210_init_board_irqs() add extra outputs to the splitter
> device when it must be connected to more than one input on each
> combiner.
> 
> We do this with a new data structure, the combinermap, which is an
> array each of whose elements is a list of the interrupt IDs on the
> combiner which must be tied together.  As we loop through each
> interrupt ID, if we find that it is the first one in one of these
> lists, we configure the splitter device with eonugh extra outputs and
> wire them up to the other interrupt IDs in the list.
> 
> Conveniently, for all the cases where this is necessary, the
> lowest-numbered interrupt ID in each group is in the range of the
> external combiner, so we only need to code for this in the first of
> the two loops in exynos4210_init_board_irqs().
> 
> The old code in exynos4210_combiner_get_gpioin() which is being
> deleted here had several problems which don't exist in the new code
> in its handling of the multi-core timer interrupts:
>   (1) the case labels specified bits 4 ... 8, but bit '8' doesn't
>       exist; these should have been 4 ... 7
>   (2) it used the input irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]
>       multiple times as the input of several different splitters,
>       which isn't allowed
>   (3) in an apparent cut-and-paste error, the cases for all the
>       multi-core timer inputs used "bit + 4" even though the
>       bit range for the case was (intended to be) 4 ... 7, which
>       meant it was looking at non-existent bits 8 ... 11.
> None of these exist in the new code.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |   6 +-
>   hw/arm/exynos4210.c         | 178 +++++++++++++++++++++++-------------
>   2 files changed, 119 insertions(+), 65 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 17/18] hw/arm/exynos4210: Put combiners into state struct

[Peter Maydell]

Switch the creation of the combiner devices to the new-style
"embedded in state struct" approach, so we can easily refer
to the object elsewhere during realize.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h           |  3 ++
 include/hw/intc/exynos4210_combiner.h | 57 +++++++++++++++++++++++++++
 hw/arm/exynos4210.c                   | 20 +++++-----
 hw/intc/exynos4210_combiner.c         | 31 +--------------
 4 files changed, 72 insertions(+), 39 deletions(-)
 create mode 100644 include/hw/intc/exynos4210_combiner.h

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index f24617f681d..d38be8767b3 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -28,6 +28,7 @@
 #include "hw/sysbus.h"
 #include "hw/cpu/a9mpcore.h"
 #include "hw/intc/exynos4210_gic.h"
+#include "hw/intc/exynos4210_combiner.h"
 #include "hw/core/split-irq.h"
 #include "target/arm/cpu-qom.h"
 #include "qom/object.h"
@@ -105,6 +106,8 @@ struct Exynos4210State {
     qemu_or_irq cpu_irq_orgate[EXYNOS4210_NCPUS];
     A9MPPrivState a9mpcore;
     Exynos4210GicState ext_gic;
+    Exynos4210CombinerState int_combiner;
+    Exynos4210CombinerState ext_combiner;
     SplitIRQ splitter[EXYNOS4210_NUM_SPLITTERS];
 };
 
diff --git a/include/hw/intc/exynos4210_combiner.h b/include/hw/intc/exynos4210_combiner.h
new file mode 100644
index 00000000000..429844fed41
--- /dev/null
+++ b/include/hw/intc/exynos4210_combiner.h
@@ -0,0 +1,57 @@
+/*
+ * Samsung exynos4210 Interrupt Combiner
+ *
+ * Copyright (c) 2000 - 2011 Samsung Electronics Co., Ltd.
+ * All rights reserved.
+ *
+ * Evgeny Voevodin <e.voevodin@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_INTC_EXYNOS4210_COMBINER
+#define HW_INTC_EXYNOS4210_COMBINER
+
+#include "hw/sysbus.h"
+
+/*
+ * State for each output signal of internal combiner
+ */
+typedef struct CombinerGroupState {
+    uint8_t src_mask;            /* 1 - source enabled, 0 - disabled */
+    uint8_t src_pending;        /* Pending source interrupts before masking */
+} CombinerGroupState;
+
+#define TYPE_EXYNOS4210_COMBINER "exynos4210.combiner"
+OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210CombinerState, EXYNOS4210_COMBINER)
+
+/* Number of groups and total number of interrupts for the internal combiner */
+#define IIC_NGRP 64
+#define IIC_NIRQ (IIC_NGRP * 8)
+#define IIC_REGSET_SIZE 0x41
+
+struct Exynos4210CombinerState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+
+    struct CombinerGroupState group[IIC_NGRP];
+    uint32_t reg_set[IIC_REGSET_SIZE];
+    uint32_t icipsr[2];
+    uint32_t external;          /* 1 means that this combiner is external */
+
+    qemu_irq output_irq[IIC_NGRP];
+};
+
+#endif
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 05b28cf5905..27c6ab27123 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -624,25 +624,23 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     }
 
     /* Internal Interrupt Combiner */
-    dev = qdev_new("exynos4210.combiner");
-    busdev = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(busdev, &error_fatal);
+    busdev = SYS_BUS_DEVICE(&s->int_combiner);
+    sysbus_realize(busdev, &error_fatal);
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
         sysbus_connect_irq(busdev, n,
                            qdev_get_gpio_in(DEVICE(&s->a9mpcore), n));
     }
-    exynos4210_combiner_get_gpioin(&s->irqs, dev, 0);
+    exynos4210_combiner_get_gpioin(&s->irqs, DEVICE(&s->int_combiner), 0);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_INT_COMBINER_BASE_ADDR);
 
     /* External Interrupt Combiner */
-    dev = qdev_new("exynos4210.combiner");
-    qdev_prop_set_uint32(dev, "external", 1);
-    busdev = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(busdev, &error_fatal);
+    qdev_prop_set_uint32(DEVICE(&s->ext_combiner), "external", 1);
+    busdev = SYS_BUS_DEVICE(&s->ext_combiner);
+    sysbus_realize(busdev, &error_fatal);
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
         sysbus_connect_irq(busdev, n, qdev_get_gpio_in(DEVICE(&s->ext_gic), n));
     }
-    exynos4210_combiner_get_gpioin(&s->irqs, dev, 1);
+    exynos4210_combiner_get_gpioin(&s->irqs, DEVICE(&s->ext_combiner), 1);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_COMBINER_BASE_ADDR);
 
     /* Initialize board IRQs. */
@@ -844,6 +842,10 @@ static void exynos4210_init(Object *obj)
 
     object_initialize_child(obj, "a9mpcore", &s->a9mpcore, TYPE_A9MPCORE_PRIV);
     object_initialize_child(obj, "ext-gic", &s->ext_gic, TYPE_EXYNOS4210_GIC);
+    object_initialize_child(obj, "int-combiner", &s->int_combiner,
+                            TYPE_EXYNOS4210_COMBINER);
+    object_initialize_child(obj, "ext-combiner", &s->ext_combiner,
+                            TYPE_EXYNOS4210_COMBINER);
 }
 
 static void exynos4210_class_init(ObjectClass *klass, void *data)
diff --git a/hw/intc/exynos4210_combiner.c b/hw/intc/exynos4210_combiner.c
index 83b42b9bce1..a289510bdb8 100644
--- a/hw/intc/exynos4210_combiner.c
+++ b/hw/intc/exynos4210_combiner.c
@@ -31,7 +31,7 @@
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-
+#include "hw/intc/exynos4210_combiner.h"
 #include "hw/arm/exynos4210.h"
 #include "hw/hw.h"
 #include "hw/irq.h"
@@ -48,36 +48,7 @@
 #define DPRINTF(fmt, ...) do {} while (0)
 #endif
 
-#define    IIC_NGRP        64            /* Internal Interrupt Combiner
-                                            Groups number */
-#define    IIC_NIRQ        (IIC_NGRP * 8)/* Internal Interrupt Combiner
-                                            Interrupts number */
 #define IIC_REGION_SIZE    0x108         /* Size of memory mapped region */
-#define IIC_REGSET_SIZE    0x41
-
-/*
- * State for each output signal of internal combiner
- */
-typedef struct CombinerGroupState {
-    uint8_t src_mask;            /* 1 - source enabled, 0 - disabled */
-    uint8_t src_pending;        /* Pending source interrupts before masking */
-} CombinerGroupState;
-
-#define TYPE_EXYNOS4210_COMBINER "exynos4210.combiner"
-OBJECT_DECLARE_SIMPLE_TYPE(Exynos4210CombinerState, EXYNOS4210_COMBINER)
-
-struct Exynos4210CombinerState {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-
-    struct CombinerGroupState group[IIC_NGRP];
-    uint32_t reg_set[IIC_REGSET_SIZE];
-    uint32_t icipsr[2];
-    uint32_t external;          /* 1 means that this combiner is external */
-
-    qemu_irq output_irq[IIC_NGRP];
-};
 
 static const VMStateDescription vmstate_exynos4210_combiner_group_state = {
     .name = "exynos4210.combiner.groupstate",
-- 
2.25.1

Re: [PATCH for-7.1 17/18] hw/arm/exynos4210: Put combiners into state struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> Switch the creation of the combiner devices to the new-style
> "embedded in state struct" approach, so we can easily refer
> to the object elsewhere during realize.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h           |  3 ++
>   include/hw/intc/exynos4210_combiner.h | 57 +++++++++++++++++++++++++++
>   hw/arm/exynos4210.c                   | 20 +++++-----
>   hw/intc/exynos4210_combiner.c         | 31 +--------------
>   4 files changed, 72 insertions(+), 39 deletions(-)
>   create mode 100644 include/hw/intc/exynos4210_combiner.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH for-7.1 18/18] hw/arm/exynos4210: Drop Exynos4210Irq struct

[Peter Maydell]

The only time we use the int_combiner_irq[] and ext_combiner_irq[]
arrays in the Exynos4210Irq struct is during realize of the SoC -- we
initialize them with the input IRQs of the combiner devices, and then
connect those to outputs of other devices in
exynos4210_init_board_irqs().  Now that the combiner objects are
easily accessible as s->int_combiner and s->ext_combiner we can make
the connections directly from one device to the other without going
via these arrays.

Since these are the only two remaining elements of Exynos4210Irq,
we can remove that struct entirely.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/exynos4210.h |  6 ------
 hw/arm/exynos4210.c         | 34 ++++++++--------------------------
 2 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index d38be8767b3..97353f1c02f 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -82,17 +82,11 @@
  */
 #define EXYNOS4210_NUM_SPLITTERS (EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ + 38)
 
-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];
-} Exynos4210Irq;
-
 struct Exynos4210State {
     /*< private >*/
     SysBusDevice parent_obj;
     /*< public >*/
     ARMCPU *cpu[EXYNOS4210_NCPUS];
-    Exynos4210Irq irqs;
     qemu_irq irq_table[EXYNOS4210_MAX_INT_COMBINER_IN_IRQ];
 
     MemoryRegion chipid_mem;
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 27c6ab27123..8dafa2215b6 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -331,8 +331,9 @@ static int mapline_size(const int *mapline)
 static void exynos4210_init_board_irqs(Exynos4210State *s)
 {
     uint32_t grp, bit, irq_id, n;
-    Exynos4210Irq *is = &s->irqs;
     DeviceState *extgicdev = DEVICE(&s->ext_gic);
+    DeviceState *intcdev = DEVICE(&s->int_combiner);
+    DeviceState *extcdev = DEVICE(&s->ext_combiner);
     int splitcount = 0;
     DeviceState *splitter;
     const int *mapline;
@@ -375,8 +376,10 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
         splitin = 0;
         for (;;) {
             s->irq_table[in] = qdev_get_gpio_in(splitter, 0);
-            qdev_connect_gpio_out(splitter, splitin, is->int_combiner_irq[in]);
-            qdev_connect_gpio_out(splitter, splitin + 1, is->ext_combiner_irq[in]);
+            qdev_connect_gpio_out(splitter, splitin,
+                                  qdev_get_gpio_in(intcdev, in));
+            qdev_connect_gpio_out(splitter, splitin + 1,
+                                  qdev_get_gpio_in(extcdev, in));
             splitin += 2;
             if (!mapline) {
                 break;
@@ -414,11 +417,11 @@ static void exynos4210_init_board_irqs(Exynos4210State *s)
             qdev_realize(splitter, NULL, &error_abort);
             splitcount++;
             s->irq_table[n] = qdev_get_gpio_in(splitter, 0);
-            qdev_connect_gpio_out(splitter, 0, is->int_combiner_irq[n]);
+            qdev_connect_gpio_out(splitter, 0, qdev_get_gpio_in(intcdev, n));
             qdev_connect_gpio_out(splitter, 1,
                                   qdev_get_gpio_in(extgicdev, irq_id - 32));
         } else {
-            s->irq_table[n] = is->int_combiner_irq[n];
+            s->irq_table[n] = qdev_get_gpio_in(intcdev, n);
         }
     }
     /*
@@ -440,25 +443,6 @@ uint32_t exynos4210_get_irq(uint32_t grp, uint32_t bit)
     return EXYNOS4210_COMBINER_GET_IRQ_NUM(grp, bit);
 }
 
-/*
- * Get Combiner input GPIO into irqs structure
- */
-static void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs,
-                                           DeviceState *dev, int ext)
-{
-    int n;
-    int max;
-    qemu_irq *irq;
-
-    max = ext ? EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ :
-        EXYNOS4210_MAX_INT_COMBINER_IN_IRQ;
-    irq = ext ? irqs->ext_combiner_irq : irqs->int_combiner_irq;
-
-    for (n = 0; n < max; n++) {
-        irq[n] = qdev_get_gpio_in(dev, n);
-    }
-}
-
 static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
                                     0x09, 0x00, 0x00, 0x00 };
 
@@ -630,7 +614,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
         sysbus_connect_irq(busdev, n,
                            qdev_get_gpio_in(DEVICE(&s->a9mpcore), n));
     }
-    exynos4210_combiner_get_gpioin(&s->irqs, DEVICE(&s->int_combiner), 0);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_INT_COMBINER_BASE_ADDR);
 
     /* External Interrupt Combiner */
@@ -640,7 +623,6 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
     for (n = 0; n < EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ; n++) {
         sysbus_connect_irq(busdev, n, qdev_get_gpio_in(DEVICE(&s->ext_gic), n));
     }
-    exynos4210_combiner_get_gpioin(&s->irqs, DEVICE(&s->ext_combiner), 1);
     sysbus_mmio_map(busdev, 0, EXYNOS4210_EXT_COMBINER_BASE_ADDR);
 
     /* Initialize board IRQs. */
-- 
2.25.1

Re: [PATCH for-7.1 18/18] hw/arm/exynos4210: Drop Exynos4210Irq struct

[Richard Henderson]

On 4/4/22 10:46, Peter Maydell wrote:
> The only time we use the int_combiner_irq[] and ext_combiner_irq[]
> arrays in the Exynos4210Irq struct is during realize of the SoC -- we
> initialize them with the input IRQs of the combiner devices, and then
> connect those to outputs of other devices in
> exynos4210_init_board_irqs().  Now that the combiner objects are
> easily accessible as s->int_combiner and s->ext_combiner we can make
> the connections directly from one device to the other without going
> via these arrays.
> 
> Since these are the only two remaining elements of Exynos4210Irq,
> we can remove that struct entirely.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |  6 ------
>   hw/arm/exynos4210.c         | 34 ++++++++--------------------------
>   2 files changed, 8 insertions(+), 32 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~