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

On Tue, Oct 28, 2014 at 12:25:52AM +0000, Russell King - ARM Linux wrote:

On Mon, Oct 27, 2014 at 04:22:51PM -0700, Andrew Morton wrote:

quoted

On Fri, 24 Oct 2014 21:55:32 +0200 Johan Hovold [off-list ref] wrote:

quoted

I will. :) Just wanted to see whether Andrew preferred I resend the
whole series or just that one patch first.

The diff is minimal:

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index e74750f00b18..e4f97ad9eb21 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c

@@ -423,6 +423,8 @@ static void omap_rtc_power_off(void)
        val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
        rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
                        val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
+
+       mdelay(2000);
 }

Yes, having read this threadlet: we need a very good comment in there
explaining what's going on, please.

Do we even need this delay on anything other than arm?  Or even on all arm?

I think I've already commented on the behaviour of the reboot syscalls
such as power off which can return to userspace, pointing out that
x86 can return to userspace.

As long as x86 can return to userspace, I see no harm in ARM returning
to userspace.  If a driver which is hooking into the power off stuff
is unable to immediately shut off the power (wtf it can't for 2 sec
I've no idea) then having that driver work around that hardware's
specific brokenness with a delay seems entirely reasonable.

Yeah, there are two issues here. If a power-off handler is crazy slow
there really should be a delay in the handler. That was just an
oversight on my part. [ In this case it takes between one and two
seconds due to the resolution of the rtc and they way it's alarm events
are triggered. ]

The other issue is whether arch code should inform the user about failed
power-off, in really exactly the same way as it does for failed reboot,
see:

	ac15e00b1efe ("ARM: restart: move reboot failure handing into
	machine_restart()"

by Russell.

It looks like we're soon to be having power-off call chains, with
configurable priorities, to shut of various parts of the hardware and
this is all at least partly configurable through DT. [1] I think it's
reasonable to expect to see more frequent failures to power off either
due to (DT) misconfiguration or broken or flakey hardware.

Having a short delay (I proposed 1s as for reboot) would also prevent
any oopses when returning to user-space for just quite slow devices
(e.g. millisecond range) without requiring explicit delays in these
handlers.

But as Andrew points out above, this really isn't an arm-specific issue,
and currently various arches deal with this differently, where some
return to user-space, some spin indefinitely (without an error message),
and some spin on failed reboot but not power-off (e.g. arm and arm64).

That allows those SoCs which can do the right thing to do the right
thing without being hindered by such silliness.  And it also stops
the next person coming along and bumping the delay from 2 to 3, to 5,
and then what... 10 seconds?

That wouldn't be an issue then. Arch code would only handle the
non-crazy case and complete power-off failures.

Keeping it in the driver means that the workaround for the broken
hardware is kept with the driver for the broken hardware - exactly
where it should be.

Agreed.

Johan

[1] https://lkml.org/lkml/2014/10/27/506