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

[Mark Cave-Ayland]

On 16/12/2023 20:20, Thomas Huth wrote:

> 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.

No problem, I can drop this patch from the series.


ATB,

Mark.