Re: [PATCH for-9.2 10/10] hw: Remove device_phases_reset()

[Richard Henderson]

On 8/14/24 02:52, Peter Maydell wrote:
> Currently we have transitional machinery between legacy reset
> and three phase reset that works in two directions:
>   * if you invoke three phase reset on a device which has set
>     theDeviceClass::legacy_reset method, we detect this in
>     device_get_transitional_reset() and arrange that we call
>     the legacy_reset method during the hold phase of reset
>   * if you invoke legacy reset on a device which implements
>     three phase reset, the default legacy_reset method is
>     device_phases_reset(), which does a three-phase reset
>     of the device
>
> However, we have now eliminated all the places which could invoke
> legacy reset on a device, which means that the function
> device_phases_reset() is never called -- it serves only as the value
> ofDeviceClass::legacy_reset that indicates that the subclass never
> overrode the legacy reset method.  So we can delete it, and instead
> check for legacy_reset != NULL.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/core/qdev.c | 51 ++++++++++++--------------------------------------
>   1 file changed, 12 insertions(+), 39 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


Is the reason we prioritize resettable_get_tr_func over rc->phases to allow for a legacy 
device to be attached to a resettable bus?

I wonder if device_class_set_legacy_reset can simplify that, with

static void do_legacy_reset(...)
{
    dc->legacy_reset(...);
}

void device_class_set_legacy_reset(DeviceClass *dc, DeviceReset dev_reset)
{
    dc->legacy_reset = dev_reset;

    /* Parent enter/exit are not invoked with a legacy child. */
    dc->resettable.enter = NULL;
    dc->resettable.exit = NULL;
    dc->resettable.hold = do_legacy_reset;
}

Which would eliminate resettable_get_tr_func and the supporting layers 
completely.


r~