On Wed, 7 Aug 2019 16:24:30 +0100 Peter Maydell <peter.mayd...@linaro.org> wrote:
> On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.he...@greensocs.com> wrote: > > > > Replace deprecated qbus_reset_all by resettable_reset_cold_fn for > > the ipl registration in the main reset handlers. > > > > This does not impact the behavior. > > > > Signed-off-by: Damien Hedde <damien.he...@greensocs.com> > > --- > > hw/s390x/ipl.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > > index 60bd081d3e..402770a2c9 100644 > > --- a/hw/s390x/ipl.c > > +++ b/hw/s390x/ipl.c > > @@ -234,7 +234,11 @@ static void s390_ipl_realize(DeviceState *dev, Error > > **errp) > > */ > > ipl->compat_start_addr = ipl->start_addr; > > ipl->compat_bios_start_addr = ipl->bios_start_addr; > > - qemu_register_reset(qdev_reset_all_fn, dev); > > + /* > > + * TODO: when we add some kind of main reset container / domain > > + * switch to it to really benefit from multi-phase. > > + */ > > I think this comment misses the mark a bit. Here's my suggestion: > > /* > * Because this Device is not on any bus in the qbus tree (it is > * not a sysbus device and it's not on some other bus like a PCI > * bus) it will not be automatically reset by the 'reset the > * sysbus' hook registered by vl.c like most devices. So we must > * manually register a reset hook for it. > * TODO: there should be a better way to do this. > */ Agreed, that explains much better why we're doing this. > > > + qemu_register_reset(resettable_reset_cold_fn, dev); This is fine for the conversion done within this series; but resetting the ipl device is one case where the cold vs. warm distinction falls a bit short (there's a s390_reset enum which covers more cases). Not sure if we want some custom reset types? > > error: > > error_propagate(errp, err); > > } > > -- > > 2.22.0 > > > > thanks > -- PMM
