On Thu, 11 Apr 2024 at 18:23, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> On 11/4/24 15:43, Peter Maydell wrote:
> > On Wed, 21 Aug 2019 at 17:34, Damien Hedde <damien.he...@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-fb5452544...@greensocs.com/
> Updated in this patch (see after '---' description):
> https://lore.kernel.org/qemu-devel/20191018150630.31099-9-damien.he...@greensocs.com/
Hmm, I can't see anything in there that mentions this
TODO or what we'd need more ResetState fields to handle.
I guess I'll go ahead with adding my new ResetType and ignore
this TODO, because I can't see any reason why we need to
do anything in particular for a new ResetType...
> >
> >> +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.
I have some patches which do this; I'll probably send them out
in a series next week once I've figured out whether they fit
better in with other patches that give the motivation.
thanks
-- PMM
