Skip to contentSkip to conversation

ARM64: kernel: psci: use restart_handlers instead of arm_pm_restart

PATCH New Heiko Stübner 2 participants 3 messages Jun 25, 2015 – Jun 26, 2015 arm64

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM64: kernel: psci: use restart_handlers instead of arm_pm_restart

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.

---
 arch/arm64/kernel/psci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
arch/arm64/kernel/psci.c +9 -2
index 869f202..2e0b1e9 100644
@@ -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;
 }
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/1 ARM64: kernel: psci: use restart_handlers instead of arm_pm_restart

+Mark, Lorenzo

Quoted 4 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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

#2 lore raw

PATCH 1/1 ARM64: kernel: psci: use restart_handlers instead of arm_pm_restart

Hi,

Am Freitag, 26. Juni 2015, 10:08:47 schrieb Sudeep Holla:

Quoted 10 lines

+Mark, Lorenzo

Quoted 3 lines · 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/

Quoted 10 lines
Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 2 lines

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