Use restart_handlers instead of arm_pm_restart in odroid-reboot

[Cheaterman]

Very inspired by this patch.

[Cheaterman]

Now, as to whether or how much the odroid-reboot is needed in the first place, I'm honestly unsure ; I fixed it because I saw my N2+ fail to reboot once, but it doesn't seem to be consistent, so I have no idea :-)

[pyavitz]

I use the following and other than an oddity here and there its been consistent.

diff -Naur a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
--- a/arch/arm64/include/asm/system_misc.h	2022-02-27 17:36:33.000000000 -0500
+++ b/arch/arm64/include/asm/system_misc.h	2022-03-03 13:51:34.031068479 -0500
@@ -32,6 +32,8 @@
 struct mm_struct;
 extern void __show_regs(struct pt_regs *);
 
+extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
+
 #endif	/* __ASSEMBLY__ */
 
 #endif	/* __ASM_SYSTEM_MISC_H */
diff -Naur a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
--- a/arch/arm64/kernel/process.c	2022-02-27 17:36:33.000000000 -0500
+++ b/arch/arm64/kernel/process.c	2022-03-03 13:53:35.651284725 -0500
@@ -68,6 +68,8 @@
 void (*pm_power_off)(void);
 EXPORT_SYMBOL_GPL(pm_power_off);
 
+void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
+
 #ifdef CONFIG_HOTPLUG_CPU
 void arch_cpu_idle_dead(void)
 {
@@ -138,7 +140,10 @@
 		efi_reboot(reboot_mode, NULL);
 
 	/* Now call the architecture specific reboot code. */
-	do_kernel_restart(cmd);
+	if (arm_pm_restart)
+		arm_pm_restart(reboot_mode, cmd);
+	else
+		do_kernel_restart(cmd);
 
 	/*
 	 * Whoops - the architecture was unable to reboot.

[ginkage]

I second @Cheaterman's approach, it makes more sense than revert of ab6cef1.

[pyavitz]

I'm actually starting to lean towards odroid-reboot isn't needed at all when on Linux 6.x.y. I was recently trying to squash a problem I was having with an N2 2GB variant (random lock ups) and started with using a clean mainline kernel. To my surprise the board rebooted and powered down with no issue. No patches applied.

I still need to test this on the N2+ and C4, but that looks like a pretty good sign to me.

[ashh87]

I've recently pushed a similar patch to Armbian (armbian/build#4824). The odroid-reboot driver is mainly useful for fixing reboot when the boards are used with UHS SD cards, as it resets their voltages to a state which the bootloader can use them from.

@Cheaterman I found that I had to set the notifier_block priority to 130 so that the driver is called before the standard PSCI functions. Otherwise, the standard functions reboot the board before the odroid_card_reset function is called. If you're not using a UHS card for boot, then the N2+ reboot issue you saw is probably unrelated, and I think this code will now compile, but won't get called.

@pyavitz I continued to have this issue on Armbian with 6.x kernels before I patched them. I was thinking of tidying up my Armbian patch and submitting it to the mainline kernel, but I did wonder if it might be better implemented as some optional device-tree properties of the SD card driver, so boards which needed it could add it to their device tree there, rather than compile this special driver. Something for me to look into anyway! Also, as you have an N2+, are you able to do some testing on it with a UHS card please? I only have a C4, so I haven't added the device-tree node for odroid,reboot back into Armbian for the N2/N2+ as I can't test them.

@tobetter The other thing this driver does is make what looks to me like an undocumented PSCI call before powering off the board: "__invoke_psci_fn_smc(0x82000042, 1, 0, 0);" Are you able to tell me what this call is for?

[rpardini]

@pyavitz : works with pure-mainline, sans reboot driver, cos pure mainline does not enable UHS modes for C4/N2. If you patch DT for high speed, then reboot, it hangs in early boot (if booting from SD/eMMC). If booting from SPI it works no problem too.

[pyavitz]

@pyavitz : works with pure-mainline, sans reboot driver, cos pure mainline does not enable UHS modes for C4/N2. If you patch DT for high speed, then reboot, it hangs in early boot (if booting from SD/eMMC). If booting from SPI it works no problem too.

With and without the patching, my N2 reboots fine. In my testing, this is not the case on the N2+, N2L and C4. A quick scan of the Armbian forum shows most peps are having reboot issues with those units.

[rpardini]

Interesting, would like to get to the bottom of this. @tobetter certainly knows

[pyavitz]

Interesting, would like to get to the bottom of this. @tobetter certainly knows

Yeah it's mystery. I'm at the point where I'm tired of messing with it, so I just use what I know works. If it requires patching, so be it.

[Cheaterman]

If you're not using a UHS card for boot, then the N2+ reboot issue you saw is probably unrelated, and I think this code will now compile, but won't get called.

I'm using eMMCs for booting my N2+s, I don't know if that applies too. We are now noticing production devices that are randomly refusing to reboot - they stop but won't restart. I'll check whether I applied my patch on their kernel or not, but I wonder if this could be related.

EDIT: And I do mean randomly ; we tested the reboot by hand yesterday with no issues, but crontab triggering it later during the night made the device hang.

[gitmeister]

My 2 cents after studying the C2 and C4 schematics.

The Odroid C2 has a pull-down on the TF_3V3N_1V8_EN signal which governs the IO voltage levels (U13 TF_IO Regulator) used by the sdcard. According to the sdcard specs (shown below) the card needs 3.3V to initiate a proper reboot. If the voltage switching pin is left floating at boot time (ie. via the open-drain DTS directive), it will default the voltage to 3.3V. This patch has resulted in proper reboots where previously the board has solidly refused to reboot). Could the issue be related to the sdcards being inadvertently rebooted at the lower voltage (1.8V) after running in UHS-I mode and not being reset to the higher voltage prior to boot?

The Odroid C4 (which I do not have) has a different means of generating the voltage switch but it does have pull-ups/downs that could be exploited via a similar DTS patch to achieve the 3.3V IO voltage at boot-time. In that case TF_3V3N_1V8_EN could also be made open-drain and thus default to enabling the U16 VDDIO regulator to 3.3V.

SD memory cards have used a 3.3V signaling interface since the SD standard was introduced in 2000 and through SD Specification 3.0, when 1.8V signaling was added with the UHS-I bus mode, the ultimate removable single-ended interface. UHS-I adopted 1.8V signaling because it is suitable for faster rise/fall time and lower electromagnetic interference. However, UHS memory cards still require 3.3V signaling to initialize the card so hosts need to support 3.3V signaling, too.

https://www.sdcard.org/developers/sd-standard-overview/low-voltage-signaling/

Edit: Curiously, I checked the DTS for the Odroid Bananapi5 which happens to have the same VDDIO circuit as at the C4, and the DTS already incorporates the open-drain on TF_3V3N_1V8_EN.