On Wed, 7 Aug 2019 16:24:30 +0100
Peter Maydell wrote:
> On Mon, 29 Jul 2019 at 15:59, Damien Hedde 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
> > ---
> > 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
