Re: [qemu-s390x] [PATCH v3 11/33] hw/s390x/ipl.c: remove qbus_reset_all registration

[Peter Maydell]

On Mon, 29 Jul 2019 at 15:59, Damien Hedde <address@hidden> 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 <address@hidden>
> ---
>  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