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

[Philippe Mathieu-Daudé]

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);
     }
---

> 
> There is one other use of device_legacy_reset() in PPC code which I
> didn't change: in hw/ppc/spapr_pci.c:spapr_phb_children_reset().  I
> couldn't figure out what the children being reset here are and if
> they might own buses.  I suspect that even if they do own buses the
> right thing would be to change to device_cold_reset(), but I stuck to
> only the changes I felt reasonably sure were definitely
> no-behaviour-change.
> 
> NB: tested with 'make check' and 'make check-acceptance' only.
> 
> thanks
> -- PMM