Re: [PATCH] firmware/psci: Convert poweroff to the sys-off handler API

The PSCI poweroff handler is currently registered via pm_power_off,
which fires as the last SYS_OFF_PRIO_DEFAULT handler. However, there
are two problems with this approach: PSCI is a firmware mechanism, so
its priority should be SYS_OFF_PRIO_FIRMWARE, and the point of the
sys-off API is to eliminate pm_power_off and make the ordering and
priority between shutdown mechanisms explicit.

This patch converts PSCI to register_sys_off_handler() so that its
position in the shutdown sequence is explicit and governed by priority.

In an ideal world PSCI would always be registered at SYS_OFF_PRIO_FIRMWARE
since it is a firmware mechanism. However, this approach would lead to
several problems:

  - On platforms with ACPI/EFI and PSCI there would be a clash at FIRMWARE
    level, and the first to register would prevail.

  - Not all platforms have a functional PSCI SYSTEM_OFF callback. This
    happens for example in some Tegra X1 platforms, where shutdown needs
    to be handled by the PMIC driver, explicitly marked as a
    system-power-controller.

As such, the approach here is:

  - On ACPI systems EFI (SYS_OFF_PRIO_FIRMWARE + 1) and ACPI
    (SYS_OFF_PRIO_FIRMWARE) already handle poweroff at higher
    priorities. PSCI registers at SYS_OFF_PRIO_DEFAULT as a fallback
    that runs after both.

  - On DT systems with no system-power-controller, PSCI is the primary
    firmware poweroff path and registers at SYS_OFF_PRIO_FIRMWARE.

  - On DT systems where a system-power-controller node is present, a
    dedicated platform driver is responsible for poweroff. PSCI
    registers at SYS_OFF_PRIO_DEFAULT - 1 so that driver takes
    precedence while PSCI still acts as a backstop.

Signed-off-by: Diogo Ivo <redacted>
---
---
  drivers/firmware/psci/psci.c | 44 +++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index e73bae6cb23a..fc7986b7f12f 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c

@@ -13,7 +13,6 @@
  #include <linux/errno.h>
  #include <linux/linkage.h>
  #include <linux/of.h>
-#include <linux/pm.h>
  #include <linux/printk.h>
  #include <linux/psci.h>
  #include <linux/reboot.h>

@@ -79,6 +78,8 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void)
  static u32 psci_cpu_suspend_feature;
  static bool psci_system_reset2_supported;
  static bool psci_system_off2_hibernate_supported;
+static bool psci_system_off_supported __initdata;
+static bool is_dt __initdata;
  
  static inline bool psci_has_ext_power_state(void)
  {

@@ -329,9 +330,11 @@ static struct notifier_block psci_sys_reset_nb = {
  	.priority = 129,
  };
  
-static void psci_sys_poweroff(void)
+static int psci_sys_poweroff(struct sys_off_data *data)
  {
  	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+
+	return NOTIFY_DONE;
  }
  
  #ifdef CONFIG_HIBERNATION

@@ -665,6 +668,40 @@ static void __init psci_init_smccc(void)
  
  }
  
+static int __init psci_poweroff_init(void)
+{
+	int priority = is_dt ? SYS_OFF_PRIO_FIRMWARE : SYS_OFF_PRIO_DEFAULT;
+	struct sys_off_handler *handler;
+	struct device_node *np;
+
+	if (!psci_system_off_supported)
+		return 0;
+
+	if (is_dt) {
+		/*
+		 * If a system-power-controller is designated in DT, a dedicated
+		 * driver should handle power-off; in that case register PSCI at
+		 * SYS_OFF_PRIO_DEFAULT - 1 as a fallback. This covers both the
+		 * case where that driver fails to power-off the system and the
+		 * case where it fails to register its own handler.
+		 */
+		np = of_find_node_with_property(NULL, "system-power-controller");

+		if (np) {
+			priority = SYS_OFF_PRIO_DEFAULT - 1;
+			of_node_put(np);
+		}
+	}
+
+	handler = register_sys_off_handler(SYS_OFF_MODE_POWER_OFF, priority,
+					   psci_sys_poweroff, NULL);
+	if (IS_ERR(handler))
+		pr_err("Failed to register PSCI power-off handler: %ld\n",
+		       PTR_ERR(handler));
+
+	return PTR_ERR_OR_ZERO(handler);
+}
+subsys_initcall(psci_poweroff_init);
+
  static void __init psci_0_2_set_functions(void)
  {
  	pr_info("Using standard PSCI v0.2 function IDs\n");

@@ -681,7 +718,7 @@ static void __init psci_0_2_set_functions(void)
  
  	register_restart_handler(&psci_sys_reset_nb);
  
-	pm_power_off = psci_sys_poweroff;
+	psci_system_off_supported = true;
  }
  
  /*

@@ -822,6 +859,7 @@ int __init psci_dt_init(void)
  	init_fn = (psci_initcall_t)matched_np->data;
  	ret = init_fn(np);
  
+	is_dt = true;
  	of_node_put(np);
  	return ret;
  }

---
base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
change-id: 20260530-firmware-psci-f4179901d35d

Best regards,
--
Diogo Ivo [off-list ref]