On Fri, Apr 12, 2024 at 3:05 PM Peter Maydell <peter.mayd...@linaro.org> wrote: > > 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. >
Hi, I don't remember the details on your first questions but I also agree with adding the type to the other callbacks! Cheers, Edgar
