| Message ID | 20190729145654.14644-12-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.hedde@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.hedde@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. */ > + qemu_register_reset(resettable_reset_cold_fn, dev); > error: > error_propagate(errp, err); > } > -- > 2.22.0 > thanks -- PMM
On Wed, 7 Aug 2019 16:24:30 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.hedde@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.hedde@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
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. + */ + qemu_register_reset(resettable_reset_cold_fn, dev); error: error_propagate(errp, err); }
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.hedde@greensocs.com> --- hw/s390x/ipl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)