On Wed, Apr 16, 2014 at 09:28:28AM -0700, Sebastian Capella wrote:
On 15 April 2014 14:18, Pavel Machek pavel@ucw.cz wrote:
On Tue 2014-04-15 21:54:53, Russell King - ARM Linux wrote:
What I'm basically saying is that I see no reason for ARM to do something different to what x86 does.
What is pretty clear to me is that ARM is compatible with x86, which is compatible with kernel/reboot.c, and it's the hibernate code which is the odd one out.
I'm pretty sure the original code did not return. Anyway, the best solution, given how many platforms are out there, would be to
a) document that it should not return
b) fix hibernation to handle the returning case, anyway.
Thanks Russell and Pavel!
This sounds fine to me. Any objections?
Here is the i386 code from 2.2.20:
void machine_halt(void) { }
void machine_power_off(void) { if (acpi_power_off) acpi_power_off(); }
Both can return. On the other hand, machine_restart() can never return as the final attempt to perform that action in machine_real_restart is a jump to 16-bit code.
2.4 kernels then modified it to this:
void machine_halt(void) { }
void machine_power_off(void) { if (pm_power_off) pm_power_off(); }
with machine_restart() doing similar to v2.2.
2.6.0 also did the same as 2.4 kernels. 2.6.16 then changed to this:
void machine_restart(char * __unused) { machine_shutdown(); machine_emergency_restart(); }
void machine_halt(void) { }
void machine_power_off(void) { if (pm_power_off) { machine_shutdown(); pm_power_off(); } }
which starts adding some extra stuff into the power off hook - but again, it's a no-op unless the pm_power_off function pointer is initialised.
Today, it's:
void machine_power_off(void) { machine_ops.power_off(); }
which calls native_power_off():
static void native_machine_power_off(void) { if (pm_power_off) { if (!reboot_force) machine_shutdown(); pm_power_off(); } /* A fallback in case there is no PM info available */ tboot_shutdown(TB_SHUTDOWN_HALT); }
and tboot_shutdown():
void tboot_shutdown(u32 shutdown_type) { void (*shutdown)(void);
if (!tboot_enabled()) return;
so it's entirely possible today for machine_power_off() on x86 to return.
So... the i386 code has had this "machine_power_off() can return" behaviour for a significant number of years and persists to this day.
I'd say scrap (a) _unless_ we're going to add while (1) loops to all the architectures. Alternatively, we could just accept that machine_power_off() may return and deal with that case (iow, not crash) in generic code.