Hi On Thu, Nov 21, 2019 at 10:24 PM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Thu, 21 Nov 2019 at 18:15, Marc-André Lureau > <marcandre.lur...@gmail.com> wrote: > > > > On Thu, Nov 21, 2019 at 5:47 PM Peter Maydell <peter.mayd...@linaro.org> > > wrote: > > > > > > On Wed, 20 Nov 2019 at 15:27, Marc-André Lureau > > > <marcandre.lur...@redhat.com> 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.)
Ok, but I implemented realize and moved serial init in "serial: make SerialIO a sysbus device". I propose to add another patch to follow your suggestion to use set_boot("realize", true, errp) on top. -- Marc-André Lureau