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

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 17 Jan 2020 at 18:29, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Jan 17, 2020 at 01:48:06PM +0000, Peter Maydell wrote:
> > On Fri, 10 Jan 2020 at 20:39, Guenter Roeck <linux@roeck-us.net> 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.

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

Yes, it's an existing leak, though it's more of a conceptual leak
than one that valgrind would actually complain about. (The object
isn't totally unreachable because there will be references to it
in the QOM tree. Keeping track of your child objects only becomes
really important if the device is hot-pluggable, because if you
can hot-unplug the device you get a real leak if it hasn't cleaned
up its child objects.)

Aside: I think this code is written this way because although it's
a container device it has been incorrectly written against
the pattern of a board model. Originally board models did not
have any "this is the board" struct that they could keep things
in, so the only way to write board model code that created,
configured and wired up devices was to have it do it in a
way that didn't keep track of the things it created once the
board init function returned. This code is part of an SoC
"container" device, so it does have a state struct that it
could use to hold either embedded device structs or simply
pointers to device objects, but the code is written like the old
board-model code. (These days even board models have a suitable
state struct they can use.)

include/hw/arm/armsse.h is an example of a device state struct
with a lot of embedded device state structs in it. (Not all device
state structs have names ending "State".)

thanks
-- PMM