On Mon, Nov 29, 2021 at 12:04:01AM +0300, Dmitry Osipenko wrote: > 28.11.2021 03:43, Michał Mirosław пишет: > > On Fri, Nov 26, 2021 at 09:00:44PM +0300, Dmitry Osipenko wrote: > >> SoC platforms often have multiple ways of how to perform system's > >> power-off and restart operations. Meanwhile today's kernel is limited to > >> a single option. Add combined power-off+restart handler call chain API, > >> which is inspired by the restart API. The new API provides both power-off > >> and restart functionality. > >> > >> The old pm_power_off method will be kept around till all users are > >> converted to the new API. > >> > >> Current restart API will be replaced by the new unified API since > >> new API is its superset. The restart functionality of the sys-off handler > >> API is built upon the existing restart-notifier APIs. > >> > >> In order to ease conversion to the new API, convenient helpers are added > >> for the common use-cases. They will reduce amount of boilerplate code and > >> remove global variables. These helpers preserve old behaviour for cases > >> where only one power-off handler is expected, this is what all existing > >> drivers want, and thus, they could be easily converted to the new API. > >> Users of the new API should explicitly enable power-off chaining by > >> setting corresponding flag of the power_handler structure. > > [...] > > > > Hi, > > > > A general question: do we really need three distinct chains for this? > > Hello Michał, > > At minimum this makes code easier to follow. > > > Can't there be only one that chain of callbacks that get a stage > > (RESTART_PREPARE, RESTART, POWER_OFF_PREPARE, POWER_OFF) and can ignore > > them at will? Calling through POWER_OFF_PREPARE would also return > > whether that POWER_OFF is possible (for kernel_can_power_off()). > > I'm having trouble with parsing this comment. Could you please try to > rephrase it? I don't see how you could check whether power-off handler > is available if you'll mix all handlers together. If notify_call_chain() would be fixed to return NOTIFY_OK if any call returned NOTIFY_OK, then this would be a clear way to gather the answer if any of the handlers will attempt the final action (reboot or power off). > > > I would also split this patch into preparation cleanups (like wrapping > > pm_power_off call with a function) and adding the notifier-based > > implementation. > > What's the benefit of this split up will be? Are you suggesting that it > will ease reviewing of this patch or something else? Mainly to ease review, as the wrapping will be a no-op, but the addition of notifier chain changes semantics a bit. Best Regards Michał Mirosław
