Re: [PATCH v4 08/25] kernel: Add combined power-off+restart handler call chain API

On Mon, Nov 29, 2021 at 12:04:01AM +0300, Dmitry Osipenko wrote:

28.11.2021 03:43, Michał Mirosław пишет:

quoted

On Fri, Nov 26, 2021 at 09:00:44PM +0300, Dmitry Osipenko wrote:

quoted

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.

quoted

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).

quoted

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