| Message ID | 20231227210212.245106-1-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | q800: move dp8393x_prom memory region to Q800MachineState | expand |
Am Wed, 27 Dec 2023 21:02:12 +0000 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: > There is no need to dynamically allocate the memory region from the heap. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/q800.c | 7 +++---- > include/hw/m68k/q800.h | 1 + > 2 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Thomas Huth <huth@tuxfamily.org>
On 27/12/23 22:02, Mark Cave-Ayland wrote: > There is no need to dynamically allocate the memory region from the heap. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/q800.c | 7 +++---- > include/hw/m68k/q800.h | 1 + > 2 files changed, 4 insertions(+), 4 deletions(-) > @@ -406,13 +405,13 @@ static void q800_machine_init(MachineState *machine) > sysbus_connect_irq(sysbus, 0, > qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC)); > > - memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom", > + memory_region_init_rom(&m->dp8393x_prom, NULL, "dp8393x-q800.prom", > SONIC_PROM_SIZE, &error_fatal); > memory_region_add_subregion(get_system_memory(), SONIC_PROM_BASE, > - dp8393x_prom); > + &m->dp8393x_prom); > > /* Add MAC address with valid checksum to PROM */ > - prom = memory_region_get_ram_ptr(dp8393x_prom); > + prom = memory_region_get_ram_ptr(&m->dp8393x_prom); > checksum = 0; > for (i = 0; i < 6; i++) { > prom[i] = revbit8(nd_table[0].macaddr.a[i]); Similar pattern in mips_jazz_init(). I wonder if we can extract the PROM checksums in a common helper declared in "hw/net/dp8393x.h". Anyhow for this patch: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Le 27/12/2023 à 22:02, Mark Cave-Ayland a écrit : > There is no need to dynamically allocate the memory region from the heap. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/q800.c | 7 +++---- > include/hw/m68k/q800.h | 1 + > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c > index 83d1571d02..b80a3b6d5f 100644 > --- a/hw/m68k/q800.c > +++ b/hw/m68k/q800.c > @@ -253,7 +253,6 @@ static void q800_machine_init(MachineState *machine) > int bios_size; > ram_addr_t initrd_base; > int32_t initrd_size; > - MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1); > uint8_t *prom; > int i, checksum; > MacFbMode *macfb_mode; > @@ -406,13 +405,13 @@ static void q800_machine_init(MachineState *machine) > sysbus_connect_irq(sysbus, 0, > qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC)); > > - memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom", > + memory_region_init_rom(&m->dp8393x_prom, NULL, "dp8393x-q800.prom", > SONIC_PROM_SIZE, &error_fatal); > memory_region_add_subregion(get_system_memory(), SONIC_PROM_BASE, > - dp8393x_prom); > + &m->dp8393x_prom); > > /* Add MAC address with valid checksum to PROM */ > - prom = memory_region_get_ram_ptr(dp8393x_prom); > + prom = memory_region_get_ram_ptr(&m->dp8393x_prom); > checksum = 0; > for (i = 0; i < 6; i++) { > prom[i] = revbit8(nd_table[0].macaddr.a[i]); > diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h > index a9661f65f6..34365c9860 100644 > --- a/include/hw/m68k/q800.h > +++ b/include/hw/m68k/q800.h > @@ -55,6 +55,7 @@ struct Q800MachineState { > MOS6522Q800VIA1State via1; > MOS6522Q800VIA2State via2; > dp8393xState dp8393x; > + MemoryRegion dp8393x_prom; > ESCCState escc; > OrIRQState escc_orgate; > SysBusESPState esp; Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On 28/12/2023 09:46, Philippe Mathieu-Daudé wrote: > On 27/12/23 22:02, Mark Cave-Ayland wrote: >> There is no need to dynamically allocate the memory region from the heap. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/m68k/q800.c | 7 +++---- >> include/hw/m68k/q800.h | 1 + >> 2 files changed, 4 insertions(+), 4 deletions(-) > > >> @@ -406,13 +405,13 @@ static void q800_machine_init(MachineState *machine) >> sysbus_connect_irq(sysbus, 0, >> qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC)); >> - memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom", >> + memory_region_init_rom(&m->dp8393x_prom, NULL, "dp8393x-q800.prom", >> SONIC_PROM_SIZE, &error_fatal); >> memory_region_add_subregion(get_system_memory(), SONIC_PROM_BASE, >> - dp8393x_prom); >> + &m->dp8393x_prom); >> /* Add MAC address with valid checksum to PROM */ >> - prom = memory_region_get_ram_ptr(dp8393x_prom); >> + prom = memory_region_get_ram_ptr(&m->dp8393x_prom); >> checksum = 0; >> for (i = 0; i < 6; i++) { >> prom[i] = revbit8(nd_table[0].macaddr.a[i]); > > Similar pattern in mips_jazz_init(). I wonder if we can extract the > PROM checksums in a common helper declared in "hw/net/dp8393x.h". That used to be the case before I added the support for the q800 machine, however the encoding of the MAC address and checksum are completely different between the jazz and q800 machines. The misleading part is the memory regions have been created with a _prom suffix and AFAICT the SONIC doesn't have any on-board NVRAM at all, so my guess is that the MAC address is stored in per-machine non-volatile memory. > Anyhow for this patch: > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Thanks! ATB, Mark.
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c index 83d1571d02..b80a3b6d5f 100644 --- a/hw/m68k/q800.c +++ b/hw/m68k/q800.c @@ -253,7 +253,6 @@ static void q800_machine_init(MachineState *machine) int bios_size; ram_addr_t initrd_base; int32_t initrd_size; - MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1); uint8_t *prom; int i, checksum; MacFbMode *macfb_mode; @@ -406,13 +405,13 @@ static void q800_machine_init(MachineState *machine) sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC)); - memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom", + memory_region_init_rom(&m->dp8393x_prom, NULL, "dp8393x-q800.prom", SONIC_PROM_SIZE, &error_fatal); memory_region_add_subregion(get_system_memory(), SONIC_PROM_BASE, - dp8393x_prom); + &m->dp8393x_prom); /* Add MAC address with valid checksum to PROM */ - prom = memory_region_get_ram_ptr(dp8393x_prom); + prom = memory_region_get_ram_ptr(&m->dp8393x_prom); checksum = 0; for (i = 0; i < 6; i++) { prom[i] = revbit8(nd_table[0].macaddr.a[i]); diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h index a9661f65f6..34365c9860 100644 --- a/include/hw/m68k/q800.h +++ b/include/hw/m68k/q800.h @@ -55,6 +55,7 @@ struct Q800MachineState { MOS6522Q800VIA1State via1; MOS6522Q800VIA2State via2; dp8393xState dp8393x; + MemoryRegion dp8393x_prom; ESCCState escc; OrIRQState escc_orgate; SysBusESPState esp;
There is no need to dynamically allocate the memory region from the heap. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/m68k/q800.c | 7 +++---- include/hw/m68k/q800.h | 1 + 2 files changed, 4 insertions(+), 4 deletions(-)