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

[BALATON Zoltan]

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 */