Re: [PATCH 6/6] hw/arm/exynos4210: Connect serial port DMA busy signals with pl330

[Guenter Roeck]

On Fri, Jan 17, 2020 at 01:48:06PM +0000, Peter Maydell wrote:
> On Fri, 10 Jan 2020 at 20:39, Guenter Roeck <address@hidden> wrote:
> >
> > The Exynos4210 serial driver uses an interrupt line to signal if receive
> > data is available. Connect that interrupt with the DMA controller's
> > 'peripheral busy' gpio pin.
> >
> > Signed-off-by: Guenter Roeck <address@hidden>
> > ---
> >  hw/arm/exynos4210.c | 39 ++++++++++++++++++++++++++-------------
> >  1 file changed, 26 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> > index c7b5c587b1..498d79ebb2 100644
> > --- a/hw/arm/exynos4210.c
> > +++ b/hw/arm/exynos4210.c
> > @@ -166,8 +166,8 @@ static uint64_t exynos4210_calc_affinity(int cpu)
> >      return (0x9 << ARM_AFF1_SHIFT) | cpu;
> >  }
> >
> > -static void pl330_create(uint32_t base, qemu_irq irq, int nreq, int 
> > nevents,
> > -                         int width)
> > +static DeviceState *pl330_create(uint32_t base, qemu_irq irq, int nreq,
> > +                                 int nevents, int width)
> >  {
> >      SysBusDevice *busdev;
> >      DeviceState *dev;
> > @@ -191,6 +191,7 @@ static void pl330_create(uint32_t base, qemu_irq irq, 
> > int nreq, int nevents,
> >      for (i = 0; i < nevents; i++) {
> >          sysbus_connect_irq(busdev, i + 1, irq); /* event irq lines */
> >      }
> > +    return dev;
> >  }
> >
> >  static void exynos4210_realize(DeviceState *socdev, Error **errp)
> > @@ -199,7 +200,7 @@ static void exynos4210_realize(DeviceState *socdev, 
> > Error **errp)
> >      MemoryRegion *system_mem = get_system_memory();
> >      qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
> >      SysBusDevice *busdev;
> > -    DeviceState *dev;
> > +    DeviceState *dev, *uart[4], *pl330[3];
> 
> Rather than having the uart and pl330 pointers be locals,
> they should be fields in Exynos4210State. (Otherwise technically
> we leak them, though this is unnoticeable in practice because there's
> no way to destroy an Exynos4210State.)
> 
Out of curiosity: Is that a new leak because they are now tied together,
or is it an existing leak ? I don't find many DeviceState entries
in xxxState structures, so find it difficult to determine if/when/why
there is such a leak.

Thanks,
Guenter