On Thu, 2 May 2019 at 06:41, Alistair Francis <alist...@alistair23.me> wrote: > > Signed-off-by: Alistair Francis <alist...@alistair23.me> > --- > MAINTAINERS | 8 + > default-configs/arm-softmmu.mak | 1 + > hw/arm/Kconfig | 3 + > hw/arm/Makefile.objs | 1 + > hw/arm/stm32f405_soc.c | 306 ++++++++++++++++++++++++++++++++ > include/hw/arm/stm32f405_soc.h | 74 ++++++++ > 6 files changed, 393 insertions(+) > create mode 100644 hw/arm/stm32f405_soc.c > create mode 100644 include/hw/arm/stm32f405_soc.h >
> +static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp) > +{ > + STM32F405State *s = STM32F405_SOC(dev_soc); > + DeviceState *dev, *armv7m; > + SysBusDevice *busdev; > + Error *err = NULL; > + int i; > + > + s->system_memory = get_system_memory(); > + s->sram = g_new(MemoryRegion, 1); > + s->flash = g_new(MemoryRegion, 1); > + s->flash_alias = g_new(MemoryRegion, 1); What I meant by my comment on v1 was that rather than doing g_new() here you can just have the STM32F405State struct have MemoryRegion sram; MemoryRegion flash; etc and then instead of memory_region_init_ram(s->flash, ...) you use memory_region_init_ram(&s->flash, ...) etc which avoids doing separate memory allocations (which would need to be freed if you then do an error-exit from the realize function, I think). And you don't need to have an s->system_memory -- that can just be a local variable, because it's just caching the pointer to the global system memory MemoryRegion. Otherwise Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM