Re: [Qemu-devel] [PATCH v2] qom: Introduce object_realize_nofail()

[Peter Maydell]

On 13 April 2012 08:17, Paolo Bonzini <address@hidden> wrote:
> Il 12/04/2012 23:08, Andreas Färber ha scritto:
> If you look at the initialization of a typical qdev object, it goes like
> this:
>
>    dev = qdev_create(NULL, "mv88w8618_eth");
>    qdev_set_nic_properties(dev, &nd_table[0]);
>    qdev_init_nofail(dev);
>    sysbus_mmio_map(sysbus_from_qdev(dev), 0, MP_ETH_BASE);
>    sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[MP_ETH_IRQ]);
>
> Now, there is no good reason to have that qdev_init_nofail.  It is there
> because sysbus_init_irq and sysbus_init_mmio are called in the device
> init function (mv88w8618_eth_init):
>
>    static int mv88w8618_eth_init(SysBusDevice *dev)
>    {
>        mv88w8618_eth_state *s = FROM_SYSBUS(mv88w8618_eth_state, dev);
>
>        sysbus_init_irq(dev, &s->irq);
>        s->nic = qemu_new_nic(&net_mv88w8618_info, &s->conf,
>                              object_get_typename(OBJECT(dev)),
>                              dev->qdev.id, s);
>        memory_region_init_io(&s->iomem, &mv88w8618_eth_ops, s,
>                              "mv88w8618-eth", MP_ETH_SIZE);
>        sysbus_init_mmio(dev, &s->iomem);
>        return 0;
>    }
>
> ... and those calls are prerequisites for sysbus_mmio_map and
> sysbus_connect_irq.

...but it's not uncommon for the number of memory regions or
IRQs to depend on some qdev property, so you can't put these
calls in instance_init, that would be too early.

> Similarly, there is no reason why qdev_get_gpio_in can only be called
> after qdev_init_nofail.  It's only like that because qdev_init_gpio_in
> is called later than in the instance_init function.  Only a handful of
> devices need a dynamic number of GPIO pins, everything else can call it
> in instance_init.

I think a model that says "half our devices set up memory and
IRQs in one function and the other half do it somewhere else"
is pretty confusing. I'd rather have a single standard place
where they can all set these up.

-- PMM