Re: [PATCH 11/12] next-cube.c: replace sysmem with get_system_memory() in next_cube_init()

[Thomas Huth <huth@tuxfamily.org>]

Am Fri, 15 Dec 2023 20:00:08 +0000
schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:

> Removing the intermediate variable helps simplify the code in next_cube_init().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/m68k/next-cube.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index d9a1f234ec..73deef25ca 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -974,7 +974,6 @@ static void next_cube_init(MachineState *machine)
>      MemoryRegion *dmamem = g_new(MemoryRegion, 1);
>      MemoryRegion *bmapm1 = g_new(MemoryRegion, 1);
>      MemoryRegion *bmapm2 = g_new(MemoryRegion, 1);
> -    MemoryRegion *sysmem = get_system_memory();
>      const char *bios_name = machine->firmware ?: ROM_FILE;
>      DeviceState *pcdev;
>  
> @@ -996,7 +995,8 @@ static void next_cube_init(MachineState *machine)
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(pcdev), &error_fatal);
>  
>      /* 64MB RAM starting at 0x04000000  */
> -    memory_region_add_subregion(sysmem, 0x04000000, machine->ram);
> +    memory_region_add_subregion(get_system_memory(), 0x04000000,
> +                                machine->ram);
>  
>      /* Framebuffer */
>      sysbus_create_simple(TYPE_NEXTFB, 0x0B000000, NULL);
> @@ -1010,19 +1010,19 @@ static void next_cube_init(MachineState *machine)
>      /* BMAP memory */
>      memory_region_init_ram_flags_nomigrate(bmapm1, NULL, "next.bmapmem", 64,
>                                             RAM_SHARED, &error_fatal);
> -    memory_region_add_subregion(sysmem, 0x020c0000, bmapm1);
> +    memory_region_add_subregion(get_system_memory(), 0x020c0000, bmapm1);
>      /* The Rev_2.5_v66.bin firmware accesses it at 0x820c0020, too */
>      memory_region_init_alias(bmapm2, NULL, "next.bmapmem2", bmapm1, 0x0, 64);
> -    memory_region_add_subregion(sysmem, 0x820c0000, bmapm2);
> +    memory_region_add_subregion(get_system_memory(), 0x820c0000, bmapm2);
>  
>      /* KBD */
>      sysbus_create_simple(TYPE_NEXTKBD, 0x0200e000, NULL);
>  
>      /* Load ROM here */
>      memory_region_init_rom(rom, NULL, "next.rom", 0x20000, &error_fatal);
> -    memory_region_add_subregion(sysmem, 0x01000000, rom);
> +    memory_region_add_subregion(get_system_memory(), 0x01000000, rom);
>      memory_region_init_alias(rom2, NULL, "next.rom2", rom, 0x0, 0x20000);
> -    memory_region_add_subregion(sysmem, 0x0, rom2);
> +    memory_region_add_subregion(get_system_memory(), 0x0, rom2);
>      if (load_image_targphys(bios_name, 0x01000000, 0x20000) < 8) {
>          if (!qtest_enabled()) {
>              error_report("Failed to load firmware '%s'.", bios_name);
> @@ -1051,7 +1051,7 @@ static void next_cube_init(MachineState *machine)
>      /* DMA */
>      memory_region_init_io(dmamem, NULL, &next_dma_ops, machine, "next.dma",
>                            0x5000);
> -    memory_region_add_subregion(sysmem, 0x02000000, dmamem);
> +    memory_region_add_subregion(get_system_memory(), 0x02000000, dmamem);
>  }

Mostly a matter of taste, but I'd prefer to keep it like it was before - I
dislike calling functions multiple times if one time is sufficient.

 Thomas