[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0gpu2ezMhWr=grg6M8aWAx58DQozbXHoZaiPqUaZxJi4Q@mail.gmail.com>
Date: Thu, 28 Oct 2021 11:59:18 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Dmitry Osipenko <digetx@...il.com>
Cc: Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Lee Jones <lee.jones@...aro.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Mark Brown <broonie@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Guenter Roeck <linux@...ck-us.net>,
Russell King <linux@...linux.org.uk>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Ulf Hansson <ulf.hansson@...aro.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Guo Ren <guoren@...nel.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Greg Ungerer <gerg@...ux-m68k.org>,
Joshua Thompson <funaho@...ai.org>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Greentime Hu <green.hu@...il.com>,
Vincent Chen <deanbo422@...il.com>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
Helge Deller <deller@....de>,
Michael Ellerman <mpe@...erman.id.au>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>,
Rich Felker <dalias@...c.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"the arch/x86 maintainers" <x86@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Len Brown <lenb@...nel.org>,
Santosh Shilimkar <ssantosh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
Linus Walleij <linus.walleij@...aro.org>,
Chen-Yu Tsai <wens@...e.org>,
Jonathan Neuschäfer <j.neuschaefer@....net>,
Tony Lindgren <tony@...mide.com>,
Liam Girdwood <lgirdwood@...il.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Vladimir Zapolskiy <vz@...ia.com>,
Avi Fishman <avifishman70@...il.com>,
Tomer Maimon <tmaimon77@...il.com>,
Tali Perry <tali.perry1@...il.com>,
Patrick Venture <venture@...gle.com>,
Nancy Yuen <yuenn@...gle.com>,
Benjamin Fair <benjaminfair@...gle.com>,
Pavel Machek <pavel@....cz>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-csky@...r.kernel.org, linux-ia64@...r.kernel.org,
linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
linux-parisc@...r.kernel.org,
linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
linux-riscv@...ts.infradead.org,
Linux-sh list <linux-sh@...r.kernel.org>,
xen-devel@...ts.xenproject.org,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
openbmc@...ts.ozlabs.org,
linux-tegra <linux-tegra@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 08/45] kernel: Add combined power-off+restart handler
call chain API
On Wed, Oct 27, 2021 at 11:18 PM Dmitry Osipenko <digetx@...il.com> wrote:
>
> SoC platforms often have multiple options 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 power-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 executed, this is what 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.
>
> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> ---
> include/linux/reboot.h | 176 +++++++++++-
> kernel/power/hibernate.c | 2 +-
> kernel/reboot.c | 601 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 768 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index b7fa25726323..0ec835338c27 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -8,10 +8,16 @@
>
> struct device;
>
> -#define SYS_DOWN 0x0001 /* Notify of system down */
> -#define SYS_RESTART SYS_DOWN
> -#define SYS_HALT 0x0002 /* Notify of system halt */
> -#define SYS_POWER_OFF 0x0003 /* Notify of system power off */
> +enum reboot_prepare_mode {
> + SYS_DOWN = 1, /* Notify of system down */
> + SYS_RESTART = SYS_DOWN,
> + SYS_HALT, /* Notify of system halt */
> + SYS_POWER_OFF, /* Notify of system power off */
> +};
> +
> +#define RESTART_PRIO_RESERVED 0
> +#define RESTART_PRIO_DEFAULT 128
> +#define RESTART_PRIO_HIGH 192
>
> enum reboot_mode {
> REBOOT_UNDEFINED = -1,
> @@ -49,6 +55,167 @@ int register_restart_handler(struct notifier_block *);
> int unregister_restart_handler(struct notifier_block *);
> void do_kernel_restart(char *cmd);
>
> +/*
> + * Unified poweroff + restart API.
> + */
> +
> +#define POWEROFF_PRIO_RESERVED 0
> +#define POWEROFF_PRIO_PLATFORM 1
> +#define POWEROFF_PRIO_DEFAULT 128
> +#define POWEROFF_PRIO_HIGH 192
> +#define POWEROFF_PRIO_FIRMWARE 224
Also I'm wondering why these particular numbers were chosen, here and above?
> +
> +enum poweroff_mode {
> + POWEROFF_NORMAL = 0,
> + POWEROFF_PREPARE,
> +};
> +
> +struct power_off_data {
> + void *cb_data;
> +};
> +
> +struct power_off_prep_data {
> + void *cb_data;
> +};
> +
> +struct restart_data {
> + void *cb_data;
> + const char *cmd;
> + enum reboot_mode mode;
> +};
> +
> +struct reboot_prep_data {
> + void *cb_data;
> + const char *cmd;
> + enum reboot_prepare_mode mode;
> +};
> +
> +struct power_handler_private_data {
> + struct notifier_block reboot_prep_nb;
> + struct notifier_block power_off_nb;
> + struct notifier_block restart_nb;
> + void (*trivial_power_off_cb)(void);
> + void (*simple_power_off_cb)(void *data);
> + void *simple_power_off_cb_data;
> + bool registered;
> +};
> +
> +/**
> + * struct power_handler - Machine power-off + restart handler
> + *
> + * Describes power-off and restart handlers which are invoked by kernel
> + * to power off or restart this machine. Supports prioritized chaining for
> + * both restart and power-off handlers. Callback's priority must be unique.
> + * Intended to be used by device drivers that are responsible for restarting
> + * and powering off hardware which kernel is running on.
> + *
> + * Struct power_handler can be static. Members of this structure must not be
> + * altered while handler is registered.
> + *
> + * Fill the structure members and pass it to register_power_handler().
> + */
> +struct power_handler {
> + /**
> + * @cb_data:
> + *
> + * User data included in callback's argument.
> + */
And here I would document the structure fields in the main kerneldoc
comment above.
As is, it is a bit hard to grasp the whole definition.
> + void *cb_data;
> +
> + /**
> + * @power_off_cb:
> + *
> + * Callback that should turn off machine. Inactive if NULL.
> + */
> + void (*power_off_cb)(struct power_off_data *data);
> +
> + /**
> + * @power_off_prepare_cb:
> + *
> + * Power-off preparation callback. All power-off preparation callbacks
> + * are invoked before @restart_cb. Inactive if NULL.
> + */
> + void (*power_off_prepare_cb)(struct power_off_prep_data *data);
> +
> + /**
> + * @power_off_priority:
> + *
> + * Power-off callback priority, must be unique. Zero value is
> + * reassigned to default priority. Inactive if @power_off_cb is NULL.
> + */
> + int power_off_priority;
> +
> + /**
> + * @power_off_chaining_allowed:
> + *
> + * False if callbacks execution should stop when @power_off_cb fails
> + * to power off machine. True if further lower priority power-off
> + * callback should be executed.
> + */
> + bool power_off_chaining_allowed;
> +
> + /**
> + * @restart_cb:
> + *
> + * Callback that should reboot machine. Inactive if NULL.
> + */
> + void (*restart_cb)(struct restart_data *data);
> +
> + /**
> + * @restart_priority:
> + *
> + * Restart callback priority, must be unique. Zero value is reassigned
> + * to default priority. Inactive if @restart_cb is NULL.
> + */
> + int restart_priority;
> +
> + /**
> + * @reboot_prepare_cb:
> + *
> + * Reboot preparation callback. All reboot preparation callbacks are
> + * invoked before @restart_cb. Inactive if NULL.
> + */
> + void (*reboot_prepare_cb)(struct reboot_prep_data *data);
> +
> + /**
> + * @priv:
> + *
> + * Internal data. Shouldn't be touched.
> + */
> + const struct power_handler_private_data priv;
> +};
Powered by blists - more mailing lists