Re: [PATCH 0/3] ppc: Convert (mostly) from device_legacy_reset() to device_cold_reset()

[Peter Maydell]

On Mon, 3 May 2021 at 17:55, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Peter,
>
> +Eduardo/Markus
>
> On 5/3/21 5:18 PM, Peter Maydell wrote:
> > The old function device_legacy_reset() (which was originally
> > device_reset() and got renamed when 3-phase-reset landed) is
> > deprecated, because it has slightly odd semantics -- it resets the
> > device itself, but (unlike when a device is reset as part of system
> > reset) not any qbus it owns (and devices attached to the qbus).  The
> > replacement is device_cold_reset(), which resets the device and its
> > bus (if any).
> >
> > For a device with child bus, the two functions are identical; this
> > patchset changes the PPC code which uses device_legacy_reset() on
> > devices which have no qbus to use device_cold_reset() instead; this
> > should have no functionally visible difference.
>
> So IIUC we should be able to add this check?
>
> -- >8 --
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index cefc5eaa0a9..4e03f964a42 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1121,6 +1122,7 @@ void device_legacy_reset(DeviceState *dev)
>      DeviceClass *klass = DEVICE_GET_CLASS(dev);
>
>      trace_qdev_reset(dev, object_get_typename(OBJECT(dev)));
> +    assert(DEVICE_GET_CLASS(dev)->bus_type);
>      if (klass->reset) {
>          klass->reset(dev);
>      }
> ---

No, because the codepath starting at qdev_reset_all() which
resets the whole qbus-and-devices tree calls device_legacy_reset()
to reset a single device.

There are also probably places where we call device_legacy_reset()
on devices which do have qbuses, where we need to figure out
what to do (eg if they are also manually resetting the qbuses
we could switch to using device_cold_reset(), or perhaps they
aren't resetting the qbuses and this is technically a bug.
Either way, those callsites are harder to convert.

thanks
-- PMM