[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_in
From: |
Alistair Francis |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting |
Date: |
Tue, 2 Jul 2019 09:59:29 -0700 |
On Tue, Jul 2, 2019 at 10:00 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> On 7/2/19 6:18 PM, Alistair Francis wrote:
> > On Mon, Jul 1, 2019 at 5:32 AM Philippe Mathieu-Daudé <address@hidden>
> > wrote:
> >>
> >> As explained in commit aff39be0ed97:
> >>
> >> Both functions, object_initialize() and object_property_add_child()
> >> increase the reference counter of the new object, so one of the
> >> references has to be dropped afterwards to get the reference
> >> counting right. Otherwise the child object will not be properly
> >> cleaned up when the parent gets destroyed.
> >> Thus let's use now object_initialize_child() instead to get the
> >> reference counting here right.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> >
> > Reviewed-by: Alistair Francis <address@hidden>
> >
> > Alistair
> >
> >> ---
> >> hw/dma/xilinx_axidma.c | 16 ++++++++--------
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> >> index 921be178d9..91f5ec587f 100644
> >> --- a/hw/dma/xilinx_axidma.c
> >> +++ b/hw/dma/xilinx_axidma.c
> >> @@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
> >> XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
> >> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> >>
> >> - object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
> >> - TYPE_XILINX_AXI_DMA_DATA_STREAM);
> >> - object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
> >> - TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
> >> - object_property_add_child(OBJECT(s), "axistream-connected-target",
> >> - (Object *)&s->rx_data_dev, &error_abort);
> >> - object_property_add_child(OBJECT(s),
> >> "axistream-control-connected-target",
> >> - (Object *)&s->rx_control_dev, &error_abort);
> >> + object_initialize_child(OBJECT(s), "axistream-connected-target",
> >> + &s->rx_data_dev, sizeof(s->rx_data_dev),
> >> + TYPE_XILINX_AXI_ENET_DATA_STREAM,
> >> &error_abort,
> >> + NULL);
> >> + object_initialize_child(OBJECT(s),
> >> "axistream-control-connected-target",
> >> + &s->rx_control_dev, sizeof(s->rx_control_dev),
> >> + TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
> >> &error_abort,
> >> + NULL);
> >>
> >> sysbus_init_irq(sbd, &s->streams[0].irq);
> >> sysbus_init_irq(sbd, &s->streams[1].irq);
> >> --
> >> 2.20.1
>
> As GCC suggested, this patch requires this snippet squashed:
>
> -- >8 --
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> @@ -565,11 +565,11 @@ static void xilinx_axidma_init(Object *obj)
>
> object_initialize_child(OBJECT(s), "axistream-connected-target",
> &s->rx_data_dev, sizeof(s->rx_data_dev),
> - TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> + TYPE_XILINX_AXI_DMA_DATA_STREAM, &error_abort,
> NULL);
> object_initialize_child(OBJECT(s),
> "axistream-control-connected-target",
> &s->rx_control_dev, sizeof(s->rx_control_dev),
> - TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
> &error_abort,
> + TYPE_XILINX_AXI_DMA_CONTROL_STREAM,
> &error_abort,
> NULL);
>
> sysbus_init_irq(sbd, &s->streams[0].irq);
> ---
Ah I missed that
>
> Since it was commented in reply to the cover, I assume your R-b stands
> with the snippet applied.
Yeah it does.
Alistair
>
> Thanks for reviewing,
>
> Phil.