[RFC PATCH 1/2] acpi: Use syscore instead of pm_power_off_prepare to prepare for poweroff

[Guenter Roeck]

The syscore shutdown callback seems to be perfectly suited to prepare for system
poweroff. Use it instead of pm_power_off_prepare.

Cc: Rafael J. Wysocki <***@rjwysocki.net>
Cc: Len Brown <***@intel.com>
Signed-off-by: Guenter Roeck <***@roeck-us.net>
---
drivers/acpi/sleep.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 05a31b5..e03c74d 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -17,6 +17,7 @@
#include <linux/interrupt.h>
#include <linux/suspend.h>
#include <linux/reboot.h>
+#include <linux/syscore_ops.h>
#include <linux/acpi.h>
#include <linux/module.h>
#include <asm/io.h>
@@ -820,13 +821,23 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}

-static void acpi_power_off_prepare(void)
+static void acpi_shutdown(void)
{
- /* Prepare to power off the system */
- acpi_sleep_prepare(ACPI_STATE_S5);
- acpi_disable_all_gpes();
+ switch (system_state) {
+ case SYSTEM_POWER_OFF:
+ /* Prepare to power off the system */
+ acpi_sleep_prepare(ACPI_STATE_S5);
+ acpi_disable_all_gpes();
+ break;
+ default:
+ break;
+ }
}

+static struct syscore_ops acpi_syscore_ops = {
+ .shutdown = acpi_shutdown,
+};
+
static void acpi_power_off(void)
{
/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
@@ -850,7 +861,7 @@ int __init acpi_sleep_init(void)

if (acpi_sleep_state_supported(ACPI_STATE_S5)) {
sleep_states[ACPI_STATE_S5] = 1;
- pm_power_off_prepare = acpi_power_off_prepare;
+ register_syscore_ops(&acpi_syscore_ops);
pm_power_off = acpi_power_off;
}

[Guenter Roeck]

The only user of pm_poweroff_prepare was acpi, which has been converted to use
a syscore callback. Drop pm_poweroff_prepare as it is no longer necessary.

Cc: Rafael J. Wysocki <***@rjwysocki.net>
Cc: Len Brown <***@intel.com>
Cc: Pavel Machek <***@ucw.cz>
Signed-off-by: Guenter Roeck <***@roeck-us.net>
---
include/linux/pm.h | 1 -
kernel/reboot.c | 8 --------
2 files changed, 9 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 383fd68..333506e 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -32,7 +32,6 @@
* Callbacks for platform drivers to implement.
*/
extern void (*pm_power_off)(void);
-extern void (*pm_power_off_prepare)(void);

struct device; /* we have a circular dep with device.h */
#ifdef CONFIG_VT_CONSOLE_SLEEP
diff --git a/kernel/reboot.c b/kernel/reboot.c
index 5925f5a..5ad4f0b 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -44,12 +44,6 @@ int reboot_cpu;
enum reboot_type reboot_type = BOOT_ACPI;
int reboot_force;

-/*
- * If set, this is used for preparing the system to power off.
- */
-
-void (*pm_power_off_prepare)(void);
-
/**
* emergency_restart - reboot the system
*
@@ -257,8 +251,6 @@ EXPORT_SYMBOL_GPL(kernel_halt);
void kernel_power_off(void)
{
kernel_shutdown_prepare(SYSTEM_POWER_OFF);
- if (pm_power_off_prepare)
- pm_power_off_prepare();
migrate_to_reboot_cpu();
syscore_shutdown();
pr_emerg("Power down\n");

[Rafael J. Wysocki]

Post by Guenter Roeck
The syscore shutdown callback seems to be perfectly suited to prepare for system
poweroff. Use it instead of pm_power_off_prepare.

How much testing did that receive?

Post by Guenter Roeck
---
drivers/acpi/sleep.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 05a31b5..e03c74d 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -17,6 +17,7 @@
#include <linux/interrupt.h>
#include <linux/suspend.h>
#include <linux/reboot.h>
+#include <linux/syscore_ops.h>
#include <linux/acpi.h>
#include <linux/module.h>
#include <asm/io.h>
@@ -820,13 +821,23 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
-static void acpi_power_off_prepare(void)
+static void acpi_shutdown(void)
{
- /* Prepare to power off the system */
- acpi_sleep_prepare(ACPI_STATE_S5);
- acpi_disable_all_gpes();
+ switch (system_state) {
+ /* Prepare to power off the system */
+ acpi_sleep_prepare(ACPI_STATE_S5);
+ acpi_disable_all_gpes();
+ break;
+ break;
+ }
}
+static struct syscore_ops acpi_syscore_ops = {
+ .shutdown = acpi_shutdown,
+};
+
static void acpi_power_off(void)
{
/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
@@ -850,7 +861,7 @@ int __init acpi_sleep_init(void)
if (acpi_sleep_state_supported(ACPI_STATE_S5)) {
sleep_states[ACPI_STATE_S5] = 1;
- pm_power_off_prepare = acpi_power_off_prepare;
+ register_syscore_ops(&acpi_syscore_ops);
pm_power_off = acpi_power_off;
}

[Guenter Roeck]

Post by Rafael J. Wysocki

Post by Guenter Roeck
The syscore shutdown callback seems to be perfectly suited to prepare for system
poweroff. Use it instead of pm_power_off_prepare.

How much testing did that receive?

As I mentioned in patch 0/2, compile tested so far only. Before I start playing
with my servers, I wanted to get some feedback if the idea is worth pursuing
further or if I am missing something essential. _If_ it should be worth pursuing
I'll test it with some real systems (servers + laptops) before resubmitting,
and also add log messages for that testing to ensure that acpi_shutdown
is actually called.

Guenter

Post by Rafael J. Wysocki

Post by Guenter Roeck
---
drivers/acpi/sleep.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 05a31b5..e03c74d 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -17,6 +17,7 @@
#include <linux/interrupt.h>
#include <linux/suspend.h>
#include <linux/reboot.h>
+#include <linux/syscore_ops.h>
#include <linux/acpi.h>
#include <linux/module.h>
#include <asm/io.h>
@@ -820,13 +821,23 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
-static void acpi_power_off_prepare(void)
+static void acpi_shutdown(void)
{
- /* Prepare to power off the system */
- acpi_sleep_prepare(ACPI_STATE_S5);
- acpi_disable_all_gpes();
+ switch (system_state) {
+ /* Prepare to power off the system */
+ acpi_sleep_prepare(ACPI_STATE_S5);
+ acpi_disable_all_gpes();
+ break;
+ break;
+ }
}
+static struct syscore_ops acpi_syscore_ops = {
+ .shutdown = acpi_shutdown,
+};
+
static void acpi_power_off(void)
{
/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
@@ -850,7 +861,7 @@ int __init acpi_sleep_init(void)
if (acpi_sleep_state_supported(ACPI_STATE_S5)) {
sleep_states[ACPI_STATE_S5] = 1;
- pm_power_off_prepare = acpi_power_off_prepare;
+ register_syscore_ops(&acpi_syscore_ops);
pm_power_off = acpi_power_off;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Guenter Roeck]

Post by Guenter Roeck

Post by Rafael J. Wysocki

Post by Guenter Roeck
The syscore shutdown callback seems to be perfectly suited to prepare for system
poweroff. Use it instead of pm_power_off_prepare.

How much testing did that receive?

As I mentioned in patch 0/2, compile tested so far only. Before I start playing
with my servers, I wanted to get some feedback if the idea is worth pursuing
further or if I am missing something essential.

Well, it makes sense in principle, but the ordering with respect to the other
syscore shutdown things and the migrate_to_reboot_cpu() may be a problem.
I always get nervous when the ordering of ACPI-related code changes like
that and the reason is quite weak this time to be honest.

Ok, no problem; I'll drop it.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Rafael J. Wysocki]

Post by Guenter Roeck

Post by Rafael J. Wysocki

Post by Guenter Roeck
The syscore shutdown callback seems to be perfectly suited to prepare for system
poweroff. Use it instead of pm_power_off_prepare.

How much testing did that receive?

As I mentioned in patch 0/2, compile tested so far only. Before I start playing
with my servers, I wanted to get some feedback if the idea is worth pursuing
further or if I am missing something essential.

Well, it makes sense in principle, but the ordering with respect to the other
syscore shutdown things and the migrate_to_reboot_cpu() may be a problem.

I always get nervous when the ordering of ACPI-related code changes like
that and the reason is quite weak this time to be honest.