diff mbox series

[v3,04/22] ppc/ppc405: Move SRAM under the ref405ep machine

Message ID 20220808102734.133084-5-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: QOM'ify 405 board | expand

Commit Message

Cédric Le Goater Aug. 8, 2022, 10:27 a.m. UTC 
It doesn't belong to the generic machine nor the SoC.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc405_boards.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

BALATON Zoltan Aug. 8, 2022, 12:25 p.m. UTC | #1 
On Mon, 8 Aug 2022, Cédric Le Goater wrote:
> It doesn't belong to the generic machine nor the SoC.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/ppc405_boards.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index f4794ba40ce6..c6fa559b03d9 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -235,7 +235,6 @@ static void ppc405_init(MachineState *machine)
>     MachineClass *mc = MACHINE_GET_CLASS(machine);
>     const char *kernel_filename = machine->kernel_filename;
>     PowerPCCPU *cpu;
> -    MemoryRegion *sram = g_new(MemoryRegion, 1);
>     MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
>     hwaddr ram_bases[2], ram_sizes[2];
>     MemoryRegion *sysmem = get_system_memory();
> @@ -260,11 +259,6 @@ static void ppc405_init(MachineState *machine)
>     cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
>                         33333333, &uicdev, kernel_filename == NULL ? 0 : 1);
>
> -    /* allocate SRAM */
> -    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
> -                           &error_fatal);
> -    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
> -
>     /* allocate and load BIOS */
>     if (machine->firmware) {
>         MemoryRegion *bios = g_new(MemoryRegion, 1);
> @@ -328,9 +322,16 @@ static void ref405ep_init(MachineState *machine)
> {
>     DeviceState *dev;
>     SysBusDevice *s;
> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
> +    MemoryRegion *sysmem = get_system_memory();

You could drop thi "system" local and just use get_system_memory() in 
add_subregion

>     ppc405_init(machine);
>
> +    /* allocate SRAM */
> +    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
> +                           &error_fatal);
> +    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
> +
>     /* Register FPGA */
>     ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);

or use it everywhere consistently.

Regards,
BALATON Zoltan

>     /* Register NVRAM */
>
Cédric Le Goater Aug. 8, 2022, 1:38 p.m. UTC | #2 
On 8/8/22 14:25, BALATON Zoltan wrote:
> On Mon, 8 Aug 2022, Cédric Le Goater wrote:
>> It doesn't belong to the generic machine nor the SoC.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> hw/ppc/ppc405_boards.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index f4794ba40ce6..c6fa559b03d9 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -235,7 +235,6 @@ static void ppc405_init(MachineState *machine)
>>     MachineClass *mc = MACHINE_GET_CLASS(machine);
>>     const char *kernel_filename = machine->kernel_filename;
>>     PowerPCCPU *cpu;
>> -    MemoryRegion *sram = g_new(MemoryRegion, 1);
>>     MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
>>     hwaddr ram_bases[2], ram_sizes[2];
>>     MemoryRegion *sysmem = get_system_memory();
>> @@ -260,11 +259,6 @@ static void ppc405_init(MachineState *machine)
>>     cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
>>                         33333333, &uicdev, kernel_filename == NULL ? 0 : 1);
>>
>> -    /* allocate SRAM */
>> -    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
>> -                           &error_fatal);
>> -    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
>> -
>>     /* allocate and load BIOS */
>>     if (machine->firmware) {
>>         MemoryRegion *bios = g_new(MemoryRegion, 1);
>> @@ -328,9 +322,16 @@ static void ref405ep_init(MachineState *machine)
>> {
>>     DeviceState *dev;
>>     SysBusDevice *s;
>> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *sysmem = get_system_memory();
> 
> You could drop thi "system" local and just use get_system_memory() in add_subregion

Yes. Let's drop it.

Thanks,

C.

> 
>>     ppc405_init(machine);
>>
>> +    /* allocate SRAM */
>> +    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
>> +                           &error_fatal);
>> +    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
>> +
>>     /* Register FPGA */
>>     ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
> 
> or use it everywhere consistently.
> 
> Regards,
> BALATON Zoltan
> 
>>     /* Register NVRAM */
>>
diff mbox series

Patch

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f4794ba40ce6..c6fa559b03d9 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -235,7 +235,6 @@  static void ppc405_init(MachineState *machine)
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     const char *kernel_filename = machine->kernel_filename;
     PowerPCCPU *cpu;
-    MemoryRegion *sram = g_new(MemoryRegion, 1);
     MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
     hwaddr ram_bases[2], ram_sizes[2];
     MemoryRegion *sysmem = get_system_memory();
@@ -260,11 +259,6 @@  static void ppc405_init(MachineState *machine)
     cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
                         33333333, &uicdev, kernel_filename == NULL ? 0 : 1);
 
-    /* allocate SRAM */
-    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
-                           &error_fatal);
-    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
-
     /* allocate and load BIOS */
     if (machine->firmware) {
         MemoryRegion *bios = g_new(MemoryRegion, 1);
@@ -328,9 +322,16 @@  static void ref405ep_init(MachineState *machine)
 {
     DeviceState *dev;
     SysBusDevice *s;
+    MemoryRegion *sram = g_new(MemoryRegion, 1);
+    MemoryRegion *sysmem = get_system_memory();
 
     ppc405_init(machine);
 
+    /* allocate SRAM */
+    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
+                           &error_fatal);
+    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
+
     /* Register FPGA */
     ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
     /* Register NVRAM */