ppc405: Introduce a PPC405 SoC

BALATON Zoltan Mon, 08 Aug 2022 05:47:35 -0700

On Mon, 8 Aug 2022, Cédric Le Goater wrote:

It is an initial model to start QOMification of the PPC405 board.
QOM'ified devices will be reintroduced one by one. Start with the
memory regions, which name prefix is changed to "ppc405".

Also, initialize only one RAM bank. The second bank is a dummy one
(zero size) which is here to match the hard coded number of banks in
ppc405ep_init().

Reviewed-by: Daniel Henrique Barboza <danielhb...@gmail.com>
Signed-off-by: Cédric Le Goater <c...@kaod.org>
---
hw/ppc/ppc405.h        | 16 ++++++++++++++++
hw/ppc/ppc405_boards.c | 23 ++++++++++++-----------
hw/ppc/ppc405_uc.c     | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 83f156f585c8..66dc21cdfed8 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -25,6 +25,7 @@
#ifndef PPC405_H
#define PPC405_H

+#include "qom/object.h"
#include "hw/ppc/ppc4xx.h"

#define PPC405EP_SDRAM_BASE 0x00000000
@@ -62,6 +63,21 @@ struct ppc4xx_bd_info_t {
    uint32_t bi_iic_fast[2];
};

+#define TYPE_PPC405_SOC "ppc405-soc"
+OBJECT_DECLARE_SIMPLE_TYPE(Ppc405SoCState, PPC405_SOC);
+
+struct Ppc405SoCState {
+    /* Private */
+    DeviceState parent_obj;
+
+    /* Public */
+    MemoryRegion ram_banks[2];
+    hwaddr ram_bases[2], ram_sizes[2];
+
+    MemoryRegion *dram_mr;
+    hwaddr ram_size;
+};
+
/* PowerPC 405 core */
ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size);

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index c6fa559b03d9..1dc5065fcc1d 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -57,6 +57,8 @@ struct Ppc405MachineState {
    /* Private */
    MachineState parent_obj;
    /* Public */
+
+    Ppc405SoCState soc;
};

/*****************************************************************************/
@@ -232,11 +234,10 @@ static void boot_from_kernel(MachineState *machine, 
PowerPCCPU *cpu)

static void ppc405_init(MachineState *machine)
{
+    Ppc405MachineState *ppc405 = PPC405_MACHINE(machine);
    MachineClass *mc = MACHINE_GET_CLASS(machine);
    const char *kernel_filename = machine->kernel_filename;
    PowerPCCPU *cpu;
-    MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
-    hwaddr ram_bases[2], ram_sizes[2];
    MemoryRegion *sysmem = get_system_memory();
    DeviceState *uicdev;

@@ -247,16 +248,16 @@ static void ppc405_init(MachineState *machine)
        exit(EXIT_FAILURE);
    }

-    /* XXX: fix this */
-    memory_region_init_alias(&ram_memories[0], NULL, "ef405ep.ram.alias",
-                             machine->ram, 0, machine->ram_size);
-    ram_bases[0] = 0;
-    ram_sizes[0] = machine->ram_size;
-    memory_region_init(&ram_memories[1], NULL, "ef405ep.ram1", 0);
-    ram_bases[1] = 0x00000000;
-    ram_sizes[1] = 0x00000000;
+    object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,
+                            TYPE_PPC405_SOC);
+    object_property_set_uint(OBJECT(&ppc405->soc), "ram-size",
+                             machine->ram_size, &error_fatal);
+    object_property_set_link(OBJECT(&ppc405->soc), "dram",
+                             OBJECT(machine->ram), &error_abort);
+    qdev_realize(DEVICE(&ppc405->soc), NULL, &error_abort);

-    cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
+    cpu = ppc405ep_init(sysmem, ppc405->soc.ram_banks, ppc405->soc.ram_bases,
+                        ppc405->soc.ram_sizes,
                        33333333, &uicdev, kernel_filename == NULL ? 0 : 1);

    /* allocate and load BIOS */
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index d6420c88d3a6..adadb3a0ae08 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -30,6 +30,7 @@
#include "hw/ppc/ppc.h"
#include "hw/i2c/ppc4xx_i2c.h"
#include "hw/irq.h"
+#include "hw/qdev-properties.h"
#include "ppc405.h"
#include "hw/char/serial.h"
#include "qemu/timer.h"
@@ -1530,3 +1531,42 @@ PowerPCCPU *ppc405ep_init(MemoryRegion 
*address_space_mem,

    return cpu;
}
+
+static void ppc405_soc_realize(DeviceState *dev, Error **errp)
+{
+    Ppc405SoCState *s = PPC405_SOC(dev);
+
+    /* Initialize only one bank */
+    s->ram_bases[0] = 0;
+    s->ram_sizes[0] = s->ram_size;
+    memory_region_init_alias(&s->ram_banks[0], OBJECT(s),
+                             "ppc405.sdram0", s->dram_mr,
+                             s->ram_bases[0], s->ram_sizes[0]);
+}
+
+static Property ppc405_soc_properties[] = {
+    DEFINE_PROP_LINK("dram", Ppc405SoCState, dram_mr, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT64("ram-size", Ppc405SoCState, ram_size, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};

I'm not sure why we need to duplicate these in the soc if they are alwaysthe same as machine->ram and machine->ram_size. Is it theoreticallypossible to have a soc where it uses ram that's not the whole ram? If notthen you could just uose machine which is accessible either ascurrent_machine or qdev_get_machine() (also the parent of out soc but wedon't know how to get that). If setting ram this way is still desired dowe separately need to set its size or we could use memory_region_size()instead?

+
+static void ppc405_soc_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = ppc405_soc_realize;
+    dc->user_creatable = false;

May need a comment explaining why user_creatable = false. (Also for allother similar lines in other patches, I won't repeat this there.)

Regards,
BALATON Zoltan

+    device_class_set_props(dc, ppc405_soc_properties);
+}
+
+static const TypeInfo ppc405_types[] = {
+    {
+        .name           = TYPE_PPC405_SOC,
+        .parent         = TYPE_DEVICE,
+        .instance_size  = sizeof(Ppc405SoCState),
+        .class_init     = ppc405_soc_class_init,
+    }
+};
+
+DEFINE_TYPES(ppc405_types)

Re: [PATCH v3 05/22] ppc/ppc405: Introduce a PPC405 SoC

Reply via email to