Re: [PATCH v2 00/20] rtc: omap: fixes and power-off feature

[Johan Hovold <johan@kernel.org>]

On Tue, Oct 28, 2014 at 03:16:10PM +0000, Russell King - ARM Linux wrote:
> On Tue, Oct 28, 2014 at 02:12:57PM +0100, Johan Hovold wrote:
> > That's not what I was trying to refer to. But the patch set explicitly
> > allows for multiple, prioritised power-off handlers, which can power
> > off a board in different ways and with various degrees of success.
> > Specifically, it allows for fallback handlers in case one or more
> > power-off handlers fail.
> > 
> > So if we allow for that, what is to prevent the final power-off handler
> > from failing? And should this not be logged by arch code in the same way
> > as failure to restart is?
> 
> And how is that different from having a set of power-off handlers, and
> reporting when each individual one fails?  Don't you want to know if
> your primary high priority reboot handler fails, just as much as you
> want to know if your final last-resort power-off handler fails?

Good point. Failed power-off should probably be logged by the power-off
call chain implementation (which seems to makes notifier chains a bad
fit).

And what about any power-off latencies? Should this always be dealt with
in the power-off handler?

Again, if it's predictable and high, as in the OMAP RTC case, it should
go in the handler. But what if it's just normal bus latencies
(peripheral busses, i2c, or whatever people may come up with)?

Should there always be a short delay before calling the next handler?

> Or different from having no power-off handlers.

That is actually quite different, as in that case we call machine_halt
instead (via kernel_halt).

> Here's the x86 code:
> 
> void machine_power_off(void)
> {
>         machine_ops.power_off();
> }
> 
> struct machine_ops machine_ops = {
>         .power_off = native_machine_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);
> }
> 
> void tboot_shutdown(u32 shutdown_type)
> {
>         void (*shutdown)(void);
> 
>         if (!tboot_enabled())
>                 return;
> 
> See - x86 can very well just fall straight back out of machine_power_off()
> if there's no pm_power_off() hook and tboot is not enabled.

I never doubted that, but is the right thing to do? Not all arches do it
that way.

And what about the killing of init? Shall we simply consider that a
systemd bug? 

	case LINUX_REBOOT_CMD_POWER_OFF:
		kernel_power_off();
		do_exit(0);
		break;

If power-off fails (for whatever reason), do_exit(0) will trigger a
panic when called from PID 1.

Johan