Re: REGRESSION 3.2-rcX: RTC auto poweron after 5 minutes

[Rabin Vincent]

On Tue, Dec 27, 2011 at 16:27, Andreas Friedrich <afrie@xxxxxxx> wrote:
> On Mo, Dez 26, 2011 at 03:58:41 +0100, Andreas Friedrich wrote:
> the 5 minutes auto-poweron problem was caused by the new function
> drivers/rtc/interface.c -> rtc_alarm_disable():
>   ...
>   alarm.time = rtc_ktime_to_tm(ktime_add(rtc_tm_to_ktime(tm),
>                                        ktime_set(300, 0)));
>   alarm.enabled = 0;
>   ...
>
> In this function the RTC alarm shall be disabled. Why do you set up a
> 5 minutes interval just before disabling the alarm?

The 5 minutes is taken from rtc-sysfs.c ("Provide a valid future alarm
time" etc.).  Although perhaps the "future" part is just to get past the
check in __rtc_set_alarm(), which we anyway don't use in
rtc_alarm_disable().

>
> I have changed
>   ktime_set(300, 0)));
> to
>   ktime_set(0, 0)));
> and all worked fine with my notebook.
>
> Please check if this could be a common solution of the problem.

Probably not, because if your hardware really doesn't disable the alarm
then you could get the same problem if someone say changes the RTC time
to a few minutes earlier after a call to rtc_alarm_disable().

Perhaps we can avoid your five-minute problem by just attempting
to disable the irq without setting a new alarm time (not yet tested):

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 3bcc7cf..54a3b5e 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -778,16 +778,10 @@ static int rtc_timer_enqueue(struct rtc_device
*rtc, struct rtc_timer *timer)

 static void rtc_alarm_disable(struct rtc_device *rtc)
 {
-	struct rtc_wkalrm alarm;
-	struct rtc_time tm;
-
-	__rtc_read_time(rtc, &tm);
-
-	alarm.time = rtc_ktime_to_tm(ktime_add(rtc_tm_to_ktime(tm),
-				     ktime_set(300, 0)));
-	alarm.enabled = 0;
+	if (!rtc->ops || !rtc->ops->alarm_irq_enable)
+		return;

-	___rtc_set_alarm(rtc, &alarm);
+	rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
 }

btw, if you haven't done so yet, could you please confirm that it's not
something in your userspace which is _asking_ for the alarm to be
enabled?  You could add a printk() of the 'enabled' argument into
cmos_alarm_irq_enable() to do this (and one in cmos_set_alarm()
wouldn't hurt too).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/