[Qemu-devel] [PATCH 0/3] hw/arm/exynos: QOM-ify the SoC

[Qemu-devel] [PATCH 0/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Hi,

Convert the Exynos4210 SoC driver into QOM model.

No external dependencies, rebased on v2.9.0-363-g0de9191deb14.

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  hw/arm/exynos: Move RAM initialization next boards
  hw/arm/exynos: Declare local variables in some order
  hw/arm/exynos: QOM-ify the SoC

 hw/arm/exynos4210.c         | 40 +++++++++++++++-------------------
 hw/arm/exynos4_boards.c     | 53 +++++++++++++++++++++++++++++++++++++++------
 include/hw/arm/exynos4210.h | 11 +++++-----
 3 files changed, 69 insertions(+), 35 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Krzysztof Kozlowski]

Before QOM-ifying the Exynos4 SoC model, move the RAM initialization
from exynos4210.c to exynos4_boards.c because RAM is board specific,
not SoC.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/arm/exynos4210.c         | 20 +-----------------
 hw/arm/exynos4_boards.c     | 50 ++++++++++++++++++++++++++++++++++++++-------
 include/hw/arm/exynos4210.h |  5 +----
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 960f27e45a36..0da877f8db0a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,13 +160,11 @@ static uint64_t exynos4210_calc_affinity(int cpu)
     return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-        unsigned long ram_size)
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
     int i, n;
     Exynos4210State *s = g_new(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-    unsigned long mem_size;
     DeviceState *dev;
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
@@ -299,22 +297,6 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
     memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
                                 &s->iram_mem);
 
-    /* DRAM */
-    mem_size = ram_size;
-    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);
-        vmstate_register_ram_global(&s->dram1_mem);
-        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
-                &s->dram1_mem);
-        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
-    }
-    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-                           &error_fatal);
-    vmstate_register_ram_global(&s->dram0_mem);
-    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
-            &s->dram0_mem);
-
    /* PMU.
     * The only reason of existence at the moment is that secondary CPU boot
     * loader uses PMU INFORM5 register as a holding pen.
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 4853c318023c..6240b26839cd 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -56,6 +57,12 @@ typedef enum Exynos4BoardType {
     EXYNOS4_NUM_OF_BOARDS
 } Exynos4BoardType;
 
+typedef struct Exynos4BoardState {
+    Exynos4210State *soc;
+    MemoryRegion dram0_mem;
+    MemoryRegion dram1_mem;
+} Exynos4BoardState;
+
 static int exynos4_board_id[EXYNOS4_NUM_OF_BOARDS] = {
     [EXYNOS4_BOARD_NURI]     = 0xD33,
     [EXYNOS4_BOARD_SMDKC210] = 0xB16,
@@ -96,9 +103,34 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
     }
 }
 
-static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
-                                                   Exynos4BoardType board_type)
+static void exynos4_boards_init_ram(Exynos4BoardState *s,
+                                    MemoryRegion *system_mem,
+                                    unsigned long ram_size)
+{
+    unsigned long mem_size = ram_size;
+
+    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
+        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
+                               mem_size - EXYNOS4210_DRAM_MAX_SIZE,
+                               &error_fatal);
+        vmstate_register_ram_global(&s->dram1_mem);
+        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
+                                    &s->dram1_mem);
+        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
+    }
+
+    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
+                           &error_fatal);
+    vmstate_register_ram_global(&s->dram0_mem);
+    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
+                                &s->dram0_mem);
+}
+
+static Exynos4BoardState *
+exynos4_boards_init_common(MachineState *machine,
+                           Exynos4BoardType board_type)
 {
+    Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
@@ -127,8 +159,12 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
             machine->kernel_cmdline,
             machine->initrd_filename);
 
-    return exynos4210_init(get_system_memory(),
-            exynos4_board_ram_size[board_type]);
+    exynos4_boards_init_ram(s, get_system_memory(),
+                            exynos4_board_ram_size[board_type]);
+
+    s->soc = exynos4210_init(get_system_memory());
+
+    return s;
 }
 
 static void nuri_init(MachineState *machine)
@@ -140,11 +176,11 @@ static void nuri_init(MachineState *machine)
 
 static void smdkc210_init(MachineState *machine)
 {
-    Exynos4210State *s = exynos4_boards_init_common(machine,
-                                                    EXYNOS4_BOARD_SMDKC210);
+    Exynos4BoardState *s = exynos4_boards_init_common(machine,
+                                                      EXYNOS4_BOARD_SMDKC210);
 
     lan9215_init(SMDK_LAN9118_BASE_ADDR,
-            qemu_irq_invert(s->irq_table[exynos4210_get_irq(37, 1)]));
+            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
     arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }
 
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index d9e08014d6ff..098a69ec73d3 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -93,8 +93,6 @@ typedef struct Exynos4210State {
     MemoryRegion iram_mem;
     MemoryRegion irom_mem;
     MemoryRegion irom_alias_mem;
-    MemoryRegion dram0_mem;
-    MemoryRegion dram1_mem;
     MemoryRegion boot_secondary;
     MemoryRegion bootreg_mem;
     I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
@@ -103,8 +101,7 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-        unsigned long ram_size);
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
 
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
-- 
2.9.3

[Qemu-devel] [PATCH 2/3] hw/arm/exynos: Declare local variables in some order

[Krzysztof Kozlowski]

Bring some more readability by declaring local function variables, first
initialized ones and then the rest (with reversed-christmas-tree order).

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/arm/exynos4210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0da877f8db0a..27a7bf28a5a9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,12 +162,12 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-    int i, n;
     Exynos4210State *s = g_new(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-    DeviceState *dev;
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
+    DeviceState *dev;
+    int i, n;
 
     cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
     assert(cpu_oc);
-- 
2.9.3

[Qemu-devel] [PATCH 3/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Convert the Exynos4210 SoC code into a QOM model which is a preferred
approach instead of directly initializing SoC-related devices from the
board file.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/arm/exynos4210.c         | 18 +++++++++++++++---
 hw/arm/exynos4_boards.c     |  9 ++++++---
 include/hw/arm/exynos4210.h |  8 ++++++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 27a7bf28a5a9..034fc8be9d76 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,9 +160,10 @@ static uint64_t exynos4210_calc_affinity(int cpu)
     return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
+static void exynos4210_init(Object *obj)
 {
-    Exynos4210State *s = g_new(Exynos4210State, 1);
+    MemoryRegion *system_mem = get_system_memory();
+    Exynos4210State *s = EXYNOS4210(obj);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
@@ -402,6 +403,17 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 
     sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
             s->irq_table[exynos4210_get_irq(28, 3)]);
+}
+
+static const TypeInfo exynos4210_type_info = {
+    .name = TYPE_EXYNOS4210,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(Exynos4210State),
+    .instance_init = exynos4210_init,
+};
 
-    return s;
+static void exynos4210_register_types(void)
+{
+    type_register_static(&exynos4210_type_info);
 }
+type_init(exynos4210_register_types)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 6240b26839cd..5e7c6b562ae2 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -58,7 +58,7 @@ typedef enum Exynos4BoardType {
 } Exynos4BoardType;
 
 typedef struct Exynos4BoardState {
-    Exynos4210State *soc;
+    Exynos4210State soc;
     MemoryRegion dram0_mem;
     MemoryRegion dram1_mem;
 } Exynos4BoardState;
@@ -162,7 +162,10 @@ exynos4_boards_init_common(MachineState *machine,
     exynos4_boards_init_ram(s, get_system_memory(),
                             exynos4_board_ram_size[board_type]);
 
-    s->soc = exynos4210_init(get_system_memory());
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210);
+    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
+                              &error_abort);
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
     return s;
 }
@@ -180,7 +183,7 @@ static void smdkc210_init(MachineState *machine)
                                                       EXYNOS4_BOARD_SMDKC210);
 
     lan9215_init(SMDK_LAN9118_BASE_ADDR,
-            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
+            qemu_irq_invert(s->soc.irq_table[exynos4210_get_irq(37, 1)]));
     arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }
 
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 098a69ec73d3..116eae62756b 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -29,6 +29,10 @@
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"
 
+#define TYPE_EXYNOS4210 "exynos4210"
+#define EXYNOS4210(obj) \
+    OBJECT_CHECK(Exynos4210State, (obj), TYPE_EXYNOS4210)
+
 #define EXYNOS4210_NCPUS                    2
 
 #define EXYNOS4210_DRAM0_BASE_ADDR          0x40000000
@@ -85,6 +89,8 @@ typedef struct Exynos4210Irq {
 } Exynos4210Irq;
 
 typedef struct Exynos4210State {
+    DeviceState parent_obj;
+
     ARMCPU *cpu[EXYNOS4210_NCPUS];
     Exynos4210Irq irqs;
     qemu_irq *irq_table;
@@ -101,8 +107,6 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
-
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
 
-- 
2.9.3

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Philippe Mathieu-Daudé]

Hi Krzysztof,

Please modify RAM -> DRAM in subject/description so there is no 
confusion about also moving the IRAM (which is SoC specific).

With this:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:
> Before QOM-ifying the Exynos4 SoC model, move the RAM initialization
> from exynos4210.c to exynos4_boards.c because RAM is board specific,
> not SoC.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  hw/arm/exynos4210.c         | 20 +-----------------
>  hw/arm/exynos4_boards.c     | 50 ++++++++++++++++++++++++++++++++++++++-------
>  include/hw/arm/exynos4210.h |  5 +----
>  3 files changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 960f27e45a36..0da877f8db0a 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -160,13 +160,11 @@ static uint64_t exynos4210_calc_affinity(int cpu)
>      return mp_affinity;
>  }
>
> -Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
> -        unsigned long ram_size)
> +Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
>  {
>      int i, n;
>      Exynos4210State *s = g_new(Exynos4210State, 1);
>      qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
> -    unsigned long mem_size;
>      DeviceState *dev;
>      SysBusDevice *busdev;
>      ObjectClass *cpu_oc;
> @@ -299,22 +297,6 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
>      memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
>                                  &s->iram_mem);
>
> -    /* DRAM */
> -    mem_size = ram_size;
> -    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
> -        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
> -                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);
> -        vmstate_register_ram_global(&s->dram1_mem);
> -        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
> -                &s->dram1_mem);
> -        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
> -    }
> -    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
> -                           &error_fatal);
> -    vmstate_register_ram_global(&s->dram0_mem);
> -    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
> -            &s->dram0_mem);
> -
>     /* PMU.
>      * The only reason of existence at the moment is that secondary CPU boot
>      * loader uses PMU INFORM5 register as a holding pen.
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index 4853c318023c..6240b26839cd 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -22,6 +22,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> @@ -56,6 +57,12 @@ typedef enum Exynos4BoardType {
>      EXYNOS4_NUM_OF_BOARDS
>  } Exynos4BoardType;
>
> +typedef struct Exynos4BoardState {
> +    Exynos4210State *soc;
> +    MemoryRegion dram0_mem;
> +    MemoryRegion dram1_mem;
> +} Exynos4BoardState;
> +
>  static int exynos4_board_id[EXYNOS4_NUM_OF_BOARDS] = {
>      [EXYNOS4_BOARD_NURI]     = 0xD33,
>      [EXYNOS4_BOARD_SMDKC210] = 0xB16,
> @@ -96,9 +103,34 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
>      }
>  }
>
> -static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
> -                                                   Exynos4BoardType board_type)
> +static void exynos4_boards_init_ram(Exynos4BoardState *s,
> +                                    MemoryRegion *system_mem,
> +                                    unsigned long ram_size)
> +{
> +    unsigned long mem_size = ram_size;
> +
> +    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
> +        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
> +                               mem_size - EXYNOS4210_DRAM_MAX_SIZE,
> +                               &error_fatal);
> +        vmstate_register_ram_global(&s->dram1_mem);
> +        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
> +                                    &s->dram1_mem);
> +        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
> +    }
> +
> +    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
> +                           &error_fatal);
> +    vmstate_register_ram_global(&s->dram0_mem);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
> +                                &s->dram0_mem);
> +}
> +
> +static Exynos4BoardState *
> +exynos4_boards_init_common(MachineState *machine,
> +                           Exynos4BoardType board_type)
>  {
> +    Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>
>      if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
> @@ -127,8 +159,12 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
>              machine->kernel_cmdline,
>              machine->initrd_filename);
>
> -    return exynos4210_init(get_system_memory(),
> -            exynos4_board_ram_size[board_type]);
> +    exynos4_boards_init_ram(s, get_system_memory(),
> +                            exynos4_board_ram_size[board_type]);
> +
> +    s->soc = exynos4210_init(get_system_memory());
> +
> +    return s;
>  }
>
>  static void nuri_init(MachineState *machine)
> @@ -140,11 +176,11 @@ static void nuri_init(MachineState *machine)
>
>  static void smdkc210_init(MachineState *machine)
>  {
> -    Exynos4210State *s = exynos4_boards_init_common(machine,
> -                                                    EXYNOS4_BOARD_SMDKC210);
> +    Exynos4BoardState *s = exynos4_boards_init_common(machine,
> +                                                      EXYNOS4_BOARD_SMDKC210);
>
>      lan9215_init(SMDK_LAN9118_BASE_ADDR,
> -            qemu_irq_invert(s->irq_table[exynos4210_get_irq(37, 1)]));
> +            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
>      arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
>  }
>
> diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
> index d9e08014d6ff..098a69ec73d3 100644
> --- a/include/hw/arm/exynos4210.h
> +++ b/include/hw/arm/exynos4210.h
> @@ -93,8 +93,6 @@ typedef struct Exynos4210State {
>      MemoryRegion iram_mem;
>      MemoryRegion irom_mem;
>      MemoryRegion irom_alias_mem;
> -    MemoryRegion dram0_mem;
> -    MemoryRegion dram1_mem;
>      MemoryRegion boot_secondary;
>      MemoryRegion bootreg_mem;
>      I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
> @@ -103,8 +101,7 @@ typedef struct Exynos4210State {
>  void exynos4210_write_secondary(ARMCPU *cpu,
>          const struct arm_boot_info *info);
>
> -Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
> -        unsigned long ram_size);
> +Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
>
>  /* Initialize exynos4210 IRQ subsystem stub */
>  qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
>

Re: [Qemu-devel] [PATCH 2/3] hw/arm/exynos: Declare local variables in some order

[Philippe Mathieu-Daudé]

Aesthetic.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:
> Bring some more readability by declaring local function variables, first
> initialized ones and then the rest (with reversed-christmas-tree order).
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  hw/arm/exynos4210.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 0da877f8db0a..27a7bf28a5a9 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -162,12 +162,12 @@ static uint64_t exynos4210_calc_affinity(int cpu)
>
>  Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
>  {
> -    int i, n;
>      Exynos4210State *s = g_new(Exynos4210State, 1);
>      qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
> -    DeviceState *dev;
>      SysBusDevice *busdev;
>      ObjectClass *cpu_oc;
> +    DeviceState *dev;
> +    int i, n;
>
>      cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
>      assert(cpu_oc);
>

Re: [Qemu-devel] [PATCH 3/3] hw/arm/exynos: QOM-ify the SoC

[Philippe Mathieu-Daudé]

On 05/06/2017 12:24 PM, Krzysztof Kozlowski wrote:
> Convert the Exynos4210 SoC code into a QOM model which is a preferred
> approach instead of directly initializing SoC-related devices from the
> board file.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/exynos4210.c         | 18 +++++++++++++++---
>  hw/arm/exynos4_boards.c     |  9 ++++++---
>  include/hw/arm/exynos4210.h |  8 ++++++--
>  3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 27a7bf28a5a9..034fc8be9d76 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -160,9 +160,10 @@ static uint64_t exynos4210_calc_affinity(int cpu)
>      return mp_affinity;
>  }
>
> -Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
> +static void exynos4210_init(Object *obj)
>  {
> -    Exynos4210State *s = g_new(Exynos4210State, 1);
> +    MemoryRegion *system_mem = get_system_memory();
> +    Exynos4210State *s = EXYNOS4210(obj);
>      qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
>      SysBusDevice *busdev;
>      ObjectClass *cpu_oc;
> @@ -402,6 +403,17 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
>
>      sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
>              s->irq_table[exynos4210_get_irq(28, 3)]);
> +}
> +
> +static const TypeInfo exynos4210_type_info = {
> +    .name = TYPE_EXYNOS4210,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(Exynos4210State),
> +    .instance_init = exynos4210_init,
> +};
>
> -    return s;
> +static void exynos4210_register_types(void)
> +{
> +    type_register_static(&exynos4210_type_info);
>  }
> +type_init(exynos4210_register_types)
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index 6240b26839cd..5e7c6b562ae2 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -58,7 +58,7 @@ typedef enum Exynos4BoardType {
>  } Exynos4BoardType;
>
>  typedef struct Exynos4BoardState {
> -    Exynos4210State *soc;
> +    Exynos4210State soc;
>      MemoryRegion dram0_mem;
>      MemoryRegion dram1_mem;
>  } Exynos4BoardState;
> @@ -162,7 +162,10 @@ exynos4_boards_init_common(MachineState *machine,
>      exynos4_boards_init_ram(s, get_system_memory(),
>                              exynos4_board_ram_size[board_type]);
>
> -    s->soc = exynos4210_init(get_system_memory());
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210);
> +    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
> +                              &error_abort);
> +    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
>
>      return s;
>  }
> @@ -180,7 +183,7 @@ static void smdkc210_init(MachineState *machine)
>                                                        EXYNOS4_BOARD_SMDKC210);
>
>      lan9215_init(SMDK_LAN9118_BASE_ADDR,
> -            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
> +            qemu_irq_invert(s->soc.irq_table[exynos4210_get_irq(37, 1)]));
>      arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
>  }
>
> diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
> index 098a69ec73d3..116eae62756b 100644
> --- a/include/hw/arm/exynos4210.h
> +++ b/include/hw/arm/exynos4210.h
> @@ -29,6 +29,10 @@
>  #include "exec/memory.h"
>  #include "target/arm/cpu-qom.h"
>
> +#define TYPE_EXYNOS4210 "exynos4210"
> +#define EXYNOS4210(obj) \
> +    OBJECT_CHECK(Exynos4210State, (obj), TYPE_EXYNOS4210)
> +
>  #define EXYNOS4210_NCPUS                    2
>
>  #define EXYNOS4210_DRAM0_BASE_ADDR          0x40000000
> @@ -85,6 +89,8 @@ typedef struct Exynos4210Irq {
>  } Exynos4210Irq;
>
>  typedef struct Exynos4210State {
> +    DeviceState parent_obj;
> +
>      ARMCPU *cpu[EXYNOS4210_NCPUS];
>      Exynos4210Irq irqs;
>      qemu_irq *irq_table;
> @@ -101,8 +107,6 @@ typedef struct Exynos4210State {
>  void exynos4210_write_secondary(ARMCPU *cpu,
>          const struct arm_boot_info *info);
>
> -Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
> -
>  /* Initialize exynos4210 IRQ subsystem stub */
>  qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
>
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/arm/exynos: Move RAM initialization next boards

[Krzysztof Kozlowski]

On Sat, May 06, 2017 at 01:05:19PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Krzysztof,
> 
> Please modify RAM -> DRAM in subject/description so there is no confusion
> about also moving the IRAM (which is SoC specific).
> 
> With this:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Sure, thanks for review.

Best regards,
Krzysztof