Re: [PATCH v3 08/11] hw/sparc/sun4m: Realize DMA controller before accessing it

[Mark Cave-Ayland]

On 09/02/2024 11:37, Peter Maydell wrote:

> On Thu, 8 Feb 2024 at 18:14, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > We should not wire IRQs on unrealized device.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/sparc/sun4m.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> > index e782c8ec7a..d52e6a7213 100644
> > --- a/hw/sparc/sun4m.c
> > +++ b/hw/sparc/sun4m.c
> > @@ -312,13 +312,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
> >       dma = qdev_new(TYPE_SPARC32_DMA);
> >       espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
> >                                      OBJECT(dma), "espdma"));
> > -    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
> >
> >       esp = SYSBUS_ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
> >
> >       ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
> >                                    OBJECT(dma), "ledma"));
> > -    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
> >
> >       lance = SYSBUS_PCNET(object_resolve_path_component(
> >                            OBJECT(ledma), "lance"));
> > @@ -332,6 +330,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
> >       }
> >
> >       sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> > +
> > +    sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
> > +
> > +    sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
> > +
>
> (This confused me briefly until I realised that this function
> is reaching into the dma device to find child objects to connect
> the IRQs to. Perhaps it would be nicer if the wrapping 'dma' object
> passed through those IRQs to avoid that.)

FWIW I can't say I'm a fan of passing through IRQs for container devices that contain 
one or more child devices, since you end up with an extra layer of wiring complexity 
that can change depending upon the child device IRQ wiring. I find it much easier to 
access the child devices directly, since then the wiring is always consistent across 
all users.


ATB,

Mark.