Re: [PATCH v4 12/37] serial: start making SerialMM a sysbus device

[Peter Maydell]

On Thu, 21 Nov 2019 at 18:15, Marc-André Lureau
<address@hidden> wrote:
>
> On Thu, Nov 21, 2019 at 5:47 PM Peter Maydell <address@hidden> wrote:
> >
> > On Wed, 20 Nov 2019 at 15:27, Marc-André Lureau
> > <address@hidden> wrote:
> > >
> > > Memory mapped serial device is in fact a sysbus device. The following
> > > patches will make use of sysbus facilities for resource and
> > > registration. In particular, "serial-mm: use sysbus facilities" will
> > > move internal serial realization to serial_mm_realize callback to
> > > follow qdev best practices.
> >
> > What goes wrong if you just put the realize of smm->serial in
> > the right place to start with ?
>
> You mean squash the following patches?

No, I meant just having this patch have

static void serial_mm_realize(DeviceState *dev, Error **errp)
{
    SerialMM *smm = SERIAL_MM(dev);
    SerialState *s = &smm->serial;

    object_property_set_bool(OBJECT(dev), true, "realized", &err);
    if (err) {
        error_propagate(errp, err);
        return;
    }
}

and

+ dc->realize = serial_mm_realize;

rather than manually doing the qdev_init_nofail()
in serial_mm_init(). This seems to me like an integral
part of the change to doing the init of the subdevice in the
init method, so it would be better unless there's a reason
why it breaks something. The rest of patch 15 (which is
what currently makes the equivalent change to realize) is all
about passing through the properties and exposing the
sysbus MMIO/irq regions and should stay a separate patch.

(setting the 'realized' property is better in a realize method
than using qdev_init_nofail() because it means we can propagate
any error outward rather than killing qemu.)

thanks
-- PMM