[PATCH] ARM64: kernel: psci: use restart_handlers instead of arm_pm_restart

[Heiko Stübner]

Instead of hogging the arm_pm_restart callback, register a restart_handler
to make it possible for machines to register more board-specific
restart functionality.

The priority is set to 127, 1 below the "default" to facilitate for
example the use of regular per-soc restart handlers at their default
priority 128 and others like the gpio-restart at priority 129 or above.

Non-psci restarts can be necessary when either the psci implementation
is faulty and does not implement the restart callback or devices need
even more custom restart operations, like recent rk3288-chromebooks.
While the soc-level restart could restart those, an external component
needed to be also reset (via gpio-restart) to allow the device to even
boot again.

Signed-off-by: Heiko Stuebner <***@sntech.de>
---
arch/arm64/kernel/psci.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 869f202..2e0b1e9 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -252,11 +252,18 @@ static int get_set_conduit_method(struct device_node *np)
return 0;
}

-static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
+static int psci_sys_reset(struct notifier_block *this,
+ unsigned long mode, void *cmd)
{
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+ return NOTIFY_DONE;
}

+static struct notifier_block psci_restart_handler = {
+ .notifier_call = psci_sys_reset,
+ .priority = 127,
+};
+
static void psci_sys_poweroff(void)
{
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
@@ -321,7 +328,7 @@ static void __init psci_0_2_set_functions(void)

psci_ops.migrate_info_type = psci_migrate_info_type;

- arm_pm_restart = psci_sys_reset;
+ register_restart_handler(&psci_restart_handler);

pm_power_off = psci_sys_poweroff;
}

[Sudeep Holla]

+Mark, Lorenzo

Post by Heiko Stübner
Instead of hogging the arm_pm_restart callback, register a restart_handler
to make it possible for machines to register more board-specific
restart functionality.

Just curious to know why do you need board specific restart handlers in
Linux. The firmware implementing PSCI is board specific and can deal
with all board specific handling in the firmware.

Post by Heiko Stübner
The priority is set to 127, 1 below the "default" to facilitate for
example the use of regular per-soc restart handlers at their default
priority 128 and others like the gpio-restart at priority 129 or above.
Non-psci restarts can be necessary when either the psci implementation
is faulty and does not implement the restart callback or devices need

Interesting, SYSTEM_RESET is mandatory from PSCIv0.2 and why only
exception for faulty PSCI system reset while it's assumed all other
features are never faulty. IMO it needs to be fixed in the firmware.

Post by Heiko Stübner
even more custom restart operations, like recent rk3288-chromebooks.
While the soc-level restart could restart those, an external component
needed to be also reset (via gpio-restart) to allow the device to even
boot again.

Again firmware implementing PSCI is platform specific and can deal any
such customization required.

By the way, I am not arguing against usage of register_restart_handler
over arm_pm_restart, but the reasoning given here.

Regards,
Sudeep

[Heiko Stübner]

Hi,

Post by Sudeep Holla
+Mark, Lorenzo

Post by Heiko Stübner
Instead of hogging the arm_pm_restart callback, register a restart_handler
to make it possible for machines to register more board-specific
restart functionality.

Just curious to know why do you need board specific restart handlers in
Linux. The firmware implementing PSCI is board specific and can deal
with all board specific handling in the firmware.

I guess in most consumer devices it will be more of a

s/and can deal/and is supposed to deal/

Post by Sudeep Holla

Post by Heiko Stübner
The priority is set to 127, 1 below the "default" to facilitate for
example the use of regular per-soc restart handlers at their default
priority 128 and others like the gpio-restart at priority 129 or above.
Non-psci restarts can be necessary when either the psci implementation
is faulty and does not implement the restart callback or devices need

Interesting, SYSTEM_RESET is mandatory from PSCIv0.2 and why only
exception for faulty PSCI system reset while it's assumed all other
features are never faulty. IMO it needs to be fixed in the firmware.

I've asked Rockchip to also implement the SYSTEM_RESET in their psci firmware,
but of course there are already quite some devices on the market and the psci
firmware part seems to be considered non-replaceable in some devices too.

In general I think people not reading the specification fully, will sadly
happen way to often and in the case of the rk3368 they have added a "nice"

#ifndef CONFIG_ARCH_ROCKCHIP
arm_pm_restart = psci_sys_reset;

pm_power_off = psci_sys_poweroff;
#endif

in their vendor kernel. Also this psci firmware-part sadly is not publically
available, so fixing this myself is also not possible.

Post by Sudeep Holla

Post by Heiko Stübner
even more custom restart operations, like recent rk3288-chromebooks.
While the soc-level restart could restart those, an external component
needed to be also reset (via gpio-restart) to allow the device to even
boot again.

Again firmware implementing PSCI is platform specific and can deal any
such customization required.

I don't believe the common board manufacturer (using a soc-vendor bsp) has the
knowledge or cares to much about following the psci specification and
implementing his board-specific restart method there. And in the case of the
Rockchip psci-implementation, I'm currently not sure if they even get the
sources for the psci code.


So yes, I of course know this is not ideal, switching over to restart handlers
allows to circumvent these lapses in firmware implementation without damaging
sane psci implementations. And I guess once secondary cpu core come up, often
developers will stop reading the spec.

Post by Sudeep Holla
By the way, I am not arguing against usage of register_restart_handler
over arm_pm_restart, but the reasoning given here.

ok - I can of course leave the reasoning out of the patch description, if this
helps yours (or anybodies) conscience :-D


Heiko