Re: [PATCH v4 02/10] hw/core: create Resettable QOM interface

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 11/4/24 15:43, Peter Maydell wrote:
> On Wed, 21 Aug 2019 at 17:34, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> This commit defines an interface allowing multi-phase reset. This aims
>> to solve a problem of the actual single-phase reset (built in
>> DeviceClass and BusClass): reset behavior is dependent on the order
>> in which reset handlers are called. In particular doing external
>> side-effect (like setting an qemu_irq) is problematic because receiving
>> object may not be reset yet.
> 
> So, I wanted to drag up this ancient patch to ask a couple
> of Resettable questions, because I'm working on adding a
> new ResetType (the equivalent of SHUTDOWN_CAUSE_SNAPSHOT_LOAD).
> 
>> +/**
>> + * ResetType:
>> + * Types of reset.
>> + *
>> + * + Cold: reset resulting from a power cycle of the object.
>> + *
>> + * TODO: Support has to be added to handle more types. In particular,
>> + * ResetState structure needs to be expanded.
>> + */
> 
> Does anybody remember what this TODO comment is about? What
> in particular would need to be in the ResetState struct
> to allow another type to be added?

IIRC this comes from this discussion:
https://lore.kernel.org/qemu-devel/7c193b33-8188-2cda-cbf2-fb545254458b@greensocs.com/
Updated in this patch (see after '---' description):
https://lore.kernel.org/qemu-devel/20191018150630.31099-9-damien.hedde@greensocs.com/

> 
>> +typedef enum ResetType {
>> +    RESET_TYPE_COLD,
>> +} ResetType;
> 
>> +typedef void (*ResettableInitPhase)(Object *obj, ResetType type);
>> +typedef void (*ResettableHoldPhase)(Object *obj);
>> +typedef void (*ResettableExitPhase)(Object *obj);
> 
> Was there a reason why we only pass the ResetType to the init
> phase method, and not also to the hold and exit phases ?
> Given that many devices don't need to implement init, it
> seems awkward to require them to do so just to stash the
> ResetType somewhere so they can look at it in the hold
> or exit phase, so I was thinking about adding the argument
> to the other two phase methods.

You are right, the type should be propagated to to all phase
handlers.

> 
> thanks
> -- PMM