Re: [PATCH v8 00/27] Introduce power-off+restart call chain API

Rafael J. Wysocki
2022-05-18T14:46:58Z

On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> Problem
> -------
>
> SoC devices require power-off call chaining functionality from kernel.
> We have a widely used restart chaining provided by restart notifier API,
> but nothing for power-off.
>
> Solution
> --------
>
> Introduce new API that provides call chains support for all restart and
> power-off modes. The new API is designed with simplicity and extensibility
> in mind.
>
> This is a third attempt to introduce the new API. First was made by
> Guenter Roeck back in 2014, second was made by Thierry Reding in 2017.
> In fact the work didn't stop and recently arm_pm_restart() was removed
> from v5.14 kernel, which was a part of preparatory work started by
> Guenter Roeck.
>
> Adoption plan
> -------------
>
> This patchset introduces the new API. It also converts multiple drivers
> and arch code to the new API to demonstrate how it all looks in practice,
> removing the pm_power_off_prepare global variable.
>
> The plan is:
>
> 1. Merge the new API and convert arch code to use do_kernel_power_off().
> For now the new API will co-exist with the older API.
>
> 2. Convert all drivers and platform code to the new API.
>
> 3. Remove obsoleted pm_power_off and pm_power_off_prepare variables.
>
> Results
> -------
>
> 1. Devices can be powered off properly.
>
> 2. Global variables are removed from drivers.
>
> 3. Global pm_power_off and pm_power_off_prepare callback variables are
> removed once all users are converted to the new API. The latter callback
> is removed by patch #24 of this series.
>
> 4. Ambiguous call chain ordering is prohibited for non-default priorities.
>
> Changelog:
>
> v8: - Reworked sys-off handler like was suggested by Rafael Wysocki in
> the comments to v7.
>
> - The struct sys-off handler now is private to kernel/reboot.c and
> new API is simplified.
>
> - There is a single sys-off API function for all handler types.
> Users shall pass the required sys-off mode type (restart, power-off
> and etc).
>
> - There is single struct sys_off_data callback argument for all
> handler modes.
>
> - User's callback now must return NOTIFY_DONE or NOTIFY_STOP.
>
> - The default priority level is zero now.
>
> - Multiple handlers now allowed to be registered at the default
> priority level.
>
> - Power-off call chain is atomic now, like the restart chain.
>
> - kernel/reboot.c changes are split up into several logical patches.
>
> - Added r-b from Michał Mirosław to unmodified patches from v7.
>
> - Added acks that were missing in v7 by accident.

The v8 looks much better than the previous versions to me.

I actually don't really have any comments on it except for the minor
remark regarding patch [1/27] sent separately.

Please just send an update of that one patch and I will queue up the
series for 5.19.

However, I'm going to send a pull request with it in the second half
of the merge window, after the majority of the other changes in the
subsystems touched by it have been integrated.

> v7: - Rebased on a recent linux-next. Dropped the recently removed
> NDS32 architecture. Only SH and x86 arches left un-acked.
>
> - Added acks from Thomas Bogendoerfer and Krzysztof Kozlowski
> to the MIPS and memory/emif patches respectively.
>
> - Made couple minor cosmetic improvements to the new API.
>
> - A month ago I joined Collabora and continuing to work on this series
> on the company's time, so changed my email address to collabora.com
>
> v6: - Rebased on a recent linux-next.
>
> - Made minor couple cosmetic changes.
>
> v5: - Dropped patches which cleaned up notifier/reboot headers, as was
> requested by Rafael Wysocki.
>
> - Dropped WARN_ON() from the code, as was requested by Rafael Wysocki.
> Replaced it with pr_err() appropriately.
>
> - Dropped *_notifier_has_unique_priority() functions and added
> *_notifier_chain_register_unique_prio() instead, as was suggested
> by Michał Mirosław and Rafael Wysocki.
>
> - Dropped export of blocking_notifier_call_chain_is_empty() symbol,
> as was suggested by Rafael Wysocki.
>
> - Michał Mirosław suggested that will be better to split up patch
> that adds the new API to ease reviewing, but Rafael Wysocki asked
> not add more patches, so I kept it as a single patch.
>
> - Added temporary "weak" stub for pm_power_off() which fixes linkage
> failure once symbol is removed from arch/* code. Previously I missed
> this problem because was only compile-testing object files.
>
> v4: - Made a very minor improvement to doc comments, clarifying couple
> default values.
>
> - Corrected list of emails recipient by adding Linus, Sebastian,
> Philipp and more NDS people. Removed bouncing emails.
>
> - Added acks that were given to v3.
>
> v3: - Renamed power_handler to sys_off_handler as was suggested by
> Rafael Wysocki.
>
> - Improved doc-comments as was suggested by Rafael Wysocki. Added more
> doc-comments.
>
> - Implemented full set of 180 patches which convert whole kernel in
> accordance to the plan, see link [1] above. Slightly adjusted API to
> better suit for the remaining converted drivers.
>
> * Added unregister_sys_off_handler() that is handy for a couple old
> platform drivers.
>
> * Dropped devm_register_trivial_restart_handler(), 'simple' variant
> is enough to have.
>
> - Improved "Add atomic/blocking_notifier_has_unique_priority()" patch,
> as was suggested by Andy Shevchenko. Also replaced down_write() with
> down_read() and factored out common notifier_has_unique_priority().
>
> - Added stop_chain field to struct restart_data and reboot_prep_data
> after discovering couple drivers wanting that feature.
>
> - Added acks that were given to v2.
>
> v2: - Replaced standalone power-off call chain demo-API with the combined
> power-off+restart API because this is what drivers want. It's a more
> comprehensive solution.
>
> - Converted multiple drivers and arch code to the new API. Suggested by
> Andy Shevchenko. I skimmed through the rest of drivers, verifying that
> new API suits them. The rest of the drivers will be converted once we
> will settle on the new API, otherwise will be too many patches here.
>
> - v2 API doesn't expose notifier to users and require handlers to
> have unique priority. Suggested by Guenter Roeck.
>
> - v2 API has power-off chaining disabled by default and require
> drivers to explicitly opt-in to the chaining. This preserves old
> behaviour for existing drivers once they are converted to the new
> API.
>
> Dmitry Osipenko (27):
> notifier: Add atomic_notifier_call_chain_is_empty()
> notifier: Add blocking/atomic_notifier_chain_register_unique_prio()
> kernel/reboot: Introduce sys-off handler API
> kernel/reboot: Wrap legacy power-off callbacks into sys-off handlers
> kernel/reboot: Add do_kernel_power_off()
> kernel/reboot: Add stub for pm_power_off
> kernel/reboot: Add kernel_can_power_off()
> kernel/reboot: Add register_platform_power_off()
> ARM: Use do_kernel_power_off()
> csky: Use do_kernel_power_off()
> riscv: Use do_kernel_power_off()
> arm64: Use do_kernel_power_off()
> parisc: Use do_kernel_power_off()
> xen/x86: Use do_kernel_power_off()
> powerpc: Use do_kernel_power_off()
> m68k: Switch to new sys-off handler API
> sh: Use do_kernel_power_off()
> x86: Use do_kernel_power_off()
> ia64: Use do_kernel_power_off()
> mips: Use do_kernel_power_off()
> memory: emif: Use kernel_can_power_off()
> ACPI: power: Switch to sys-off handler API
> regulator: pfuze100: Use devm_register_sys_off_handler()
> reboot: Remove pm_power_off_prepare()
> soc/tegra: pmc: Use sys-off handler API to power off Nexus 7 properly
> kernel/reboot: Add devm_register_power_off_handler()
> kernel/reboot: Add devm_register_restart_handler()
>
> arch/arm/kernel/reboot.c | 4 +-
> arch/arm64/kernel/process.c | 3 +-
> arch/csky/kernel/power.c | 6 +-
> arch/ia64/kernel/process.c | 4 +-
> arch/m68k/emu/natfeat.c | 3 +-
> arch/m68k/include/asm/machdep.h | 1 -
> arch/m68k/kernel/process.c | 5 +-
> arch/m68k/kernel/setup_mm.c | 1 -
> arch/m68k/kernel/setup_no.c | 1 -
> arch/m68k/mac/config.c | 4 +-
> arch/mips/kernel/reset.c | 3 +-
> arch/parisc/kernel/process.c | 4 +-
> arch/powerpc/kernel/setup-common.c | 4 +-
> arch/powerpc/xmon/xmon.c | 3 +-
> arch/riscv/kernel/reset.c | 12 +-
> arch/sh/kernel/reboot.c | 3 +-
> arch/x86/kernel/reboot.c | 4 +-
> arch/x86/xen/enlighten_pv.c | 4 +-
> drivers/acpi/sleep.c | 16 +-
> drivers/memory/emif.c | 2 +-
> drivers/regulator/pfuze100-regulator.c | 42 ++-
> drivers/soc/tegra/pmc.c | 87 +++++--
> include/linux/notifier.h | 7 +
> include/linux/pm.h | 1 -
> include/linux/reboot.h | 91 +++++++
> kernel/notifier.c | 101 +++++--
> kernel/reboot.c | 347 ++++++++++++++++++++++++-
> 27 files changed, 639 insertions(+), 124 deletions(-)
>
> --
> 2.35.1
>