Re: [Qemu-arm] [PATCH] hw/arm/allwinner-a10: Add the 'A' SRAM and the SRAM controller

[Philippe Mathieu-Daudé]

On Fri, Jan 4, 2019 at 3:10 PM Peter Maydell <address@hidden> wrote:
> On Sun, 9 Dec 2018 at 19:37, Philippe Mathieu-Daudé <address@hidden> wrote:
> > From the "A10 User Manual V1.20" p.29: "3.2. Memory Mapping" and:
> >
> >  7. System Control
> >   7.1. Overview
> >
> >   A10 embeds a high-speed SRAM which has been split into five segments.
> >   See detailed memory mapping in following table:
> >
> >   Area          Address        Size (Bytes)
> >    A1    0x00000000-0x00003FFF 16K
> >    A2    0x00004000-0x00007FFF 16K
> >    A3    0x00008000-0x0000B3FF 13K
> >    A4    0x0000B400-0x0000BFFF  3K
> >
> > Since for emulation purpose we don't need the segmentations, we simply 
> > define
> > the 'A' area as a single 48KB SRAM.
>
> >  static void aw_a10_init(Object *obj)
> >  {
> > @@ -85,6 +86,10 @@ static void aw_a10_realize(DeviceState *dev, Error 
> > **errp)
> >      sysbus_connect_irq(sysbusdev, 4, s->irq[67]);
> >      sysbus_connect_irq(sysbusdev, 5, s->irq[68]);
> >
> > +    memory_region_init_ram(&s->sram_a, NULL, "sram A", 48 * KiB, 
> > &error_fatal);
>
> This creates the memory region with a NULL owner, which is
> OK for board model code, but since we're a device object
> here we should use OBJECT(dev) as the owner.
>
> This is more important if the region creation happens
> in the instance_init function, in which case it will
> result in a leak of the memory region after the device
> is destroyed (see for instance commit 09d98b69804cfd9e),
> but it's worth getting right here anyway.

Thanks, good to know.
I'll fix.
Regards,

Phil.

> > +    memory_region_add_subregion(get_system_memory(), 0x00000000, 
> > &s->sram_a);
> > +    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
> > +
> >      /* FIXME use qdev NIC properties instead of nd_table[] */
> >      if (nd_table[0].used) {
> >          qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);
>
> thanks
> -- PMM