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

[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 <heiko@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;
 }
-- 
2.1.4

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

[Sudeep Holla]

+Mark, Lorenzo

On 26/06/15 00:33, Heiko St?bner wrote:
> 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.

> 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.

> 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

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

[Heiko Stübner]

Hi,

Am Freitag, 26. Juni 2015, 10:08:47 schrieb Sudeep Holla:
> +Mark, Lorenzo
> 
> On 26/06/15 00:33, Heiko St?bner wrote:
> > 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/


> > 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.


> > 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.


> 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