On 1/23/20 2:28 PM, Damien Hedde wrote: > Deprecate device_legacy_reset(), qdev_reset_all() and > qbus_reset_all() to be replaced by new functions > device_cold_reset() and bus_cold_reset() which uses resettable API. > > Also introduce resettable_cold_reset_fn() which may be used as a > replacement for qdev_reset_all_fn and qbus_reset_all_fn(). > > Following patches will be needed to look at legacy reset call sites > and switch to resettable api. The legacy functions will be removed > when unused. > > Signed-off-by: Damien Hedde <damien.he...@greensocs.com> > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > Reviewed-by: Richard Henderson <richard.hender...@linaro.org> > Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com> > --- > include/hw/qdev-core.h | 27 +++++++++++++++++++++++++++ > include/hw/resettable.h | 9 +++++++++ > hw/core/bus.c | 5 +++++ > hw/core/qdev.c | 5 +++++ > hw/core/resettable.c | 5 +++++ > 5 files changed, 51 insertions(+) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 1b4b420617..b84fcc32bf 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -406,6 +406,13 @@ int qdev_walk_children(DeviceState *dev, > qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn, > void *opaque); > > +/** > + * @qdev_reset_all: > + * Reset @dev. See @qbus_reset_all() for more details. > + * > + * Note: This function is deprecated and will be removed when it becomes > unused. > + * Please use device_cold_reset() now. > + */ > void qdev_reset_all(DeviceState *dev); > void qdev_reset_all_fn(void *opaque); > > @@ -418,10 +425,28 @@ void qdev_reset_all_fn(void *opaque); > * hard reset means that qbus_reset_all will reset all state of the device. > * For PCI devices, for example, this will include the base address registers > * or configuration space. > + * > + * Note: This function is deprecated and will be removed when it becomes > unused. > + * Please use bus_cold_reset() now.
Some time passed, so looking at this with some retrospective. If there is an effort to introduce a new API replacing another one, we should try convert all the uses of the old API to the new one, instead of declaring it legacy. Declare an API legacy/deprecated should be the last resort if there is no way to remove it. I'd recommend to move the deprecated/legacy declarations in a separate header, with the '_legacy' suffix. Else: 1/ we never finish API conversions, 2/ the new API might not be ready for all the legacy API use cases, 3/ we end up having to maintain 2 different APIs. So the recommendation is to use bus_cold_reset(), but it isn't used anywhere...: $ git grep bus_cold_reset docs/devel/reset.rst:64:- ``bus_cold_reset()`` hw/core/bus.c:73:void bus_cold_reset(BusState *bus) include/hw/qdev-core.h:715: * Please use bus_cold_reset() now. include/hw/qdev-core.h:728: * bus_cold_reset: include/hw/qdev-core.h:733:void bus_cold_reset(BusState *bus); IMHO we shouldn't add new public prototypes without callers. I see it is similar to device_cold_reset(), but TBH I'm scared to be the first one using it. Regards, Phil. > */ > void qbus_reset_all(BusState *bus); > void qbus_reset_all_fn(void *opaque); > > +/** > + * device_cold_reset: > + * Reset device @dev and perform a recursive processing using the resettable > + * interface. It triggers a RESET_TYPE_COLD. > + */ > +void device_cold_reset(DeviceState *dev); > + > +/** > + * bus_cold_reset: > + * > + * Reset bus @bus and perform a recursive processing using the resettable > + * interface. It triggers a RESET_TYPE_COLD. > + */ > +void bus_cold_reset(BusState *bus);
