From patchwork Wed Jan 4 20:15:14 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?J=C3=A9r=C3=A9my_Compostella?= X-Patchwork-Id: 134354 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail-vx0-f184.google.com (mail-vx0-f184.google.com [209.85.220.184]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id 0E3071007D6 for ; Thu, 5 Jan 2012 07:15:40 +1100 (EST) Received: by vcbfl15 with SMTP id fl15sf10612870vcb.11 for ; Wed, 04 Jan 2012 12:15:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=beta; h=x-beenthere:received-spf:mime-version:in-reply-to:references:from :date:message-id:subject:to:cc:x-original-sender :x-original-authentication-results:reply-to:precedence:mailing-list :list-id:x-google-group-id:list-post:list-help:list-archive:sender :list-subscribe:list-unsubscribe:content-type; bh=ahGb70mGn6Su3gGAnAhd8tkCWG+c48NIl1EfFz3VMlk=; b=D0TCjyey+XTN/H9MvupUW+wb8uxGcEuUf766K6WJFXv/gnnqtwr/IpS7hpw66tDgqK +eh8Rbi71IbZob0BkQa+Wo3skhHx9LW3PTXCEm/gLannF4umLb4AaalUh3rA7nGAB5BP t9Bpr4qTPPt9JaQM1L4WjmwbH78oRhzvQ07LA= Received: by 10.52.174.80 with SMTP id bq16mr6929736vdc.11.1325708135755; Wed, 04 Jan 2012 12:15:35 -0800 (PST) X-BeenThere: rtc-linux@googlegroups.com Received: by 10.220.228.195 with SMTP id jf3ls2872529vcb.4.gmail; Wed, 04 Jan 2012 12:15:35 -0800 (PST) Received: by 10.52.77.97 with SMTP id r1mr47348250vdw.4.1325708135325; Wed, 04 Jan 2012 12:15:35 -0800 (PST) Received: by 10.52.77.97 with SMTP id r1mr47348249vdw.4.1325708135308; Wed, 04 Jan 2012 12:15:35 -0800 (PST) Received: from mail-vw0-f53.google.com (mail-vw0-f53.google.com [209.85.212.53]) by gmr-mx.google.com with ESMTPS id q3si987726vdf.0.2012.01.04.12.15.35 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 04 Jan 2012 12:15:35 -0800 (PST) Received-SPF: pass (google.com: domain of jeremy.compostella@gmail.com designates 209.85.212.53 as permitted sender) client-ip=209.85.212.53; Received: by vbih1 with SMTP id h1so7082492vbi.26 for ; Wed, 04 Jan 2012 12:15:35 -0800 (PST) Received: by 10.52.27.236 with SMTP id w12mr28029131vdg.110.1325708135210; Wed, 04 Jan 2012 12:15:35 -0800 (PST) MIME-Version: 1.0 Received: by 10.52.29.209 with HTTP; Wed, 4 Jan 2012 12:15:14 -0800 (PST) In-Reply-To: References: <8762h08ttb.fsf@Apollo.jerryland.fr> <1325639328.3037.71.camel@work-vm> From: =?ISO-8859-1?Q?J=E9r=E9my_Compostella?= Date: Wed, 4 Jan 2012 21:15:14 +0100 Message-ID: Subject: Re: [rtc-linux] [PATCH] rtc: Fix unexpected poweron and unnecessary alarm disabling To: rtc-linux@googlegroups.com Cc: Linus Walleij , Alessandro Zummo , Rabin VINCENT X-Original-Sender: jeremy.compostella@gmail.com X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of jeremy.compostella@gmail.com designates 209.85.212.53 as permitted sender) smtp.mail=jeremy.compostella@gmail.com; dkim=pass (test mode) header.i=@gmail.com Reply-To: rtc-linux@googlegroups.com Precedence: list Mailing-list: list rtc-linux@googlegroups.com; contact rtc-linux+owners@googlegroups.com List-ID: X-Google-Group-Id: 712029733259 List-Post: , List-Help: , List-Archive: Sender: rtc-linux@googlegroups.com List-Subscribe: , List-Unsubscribe: , 2012/1/4 Jérémy Compostella > 2012/1/4 John Stultz > >> On Wed, 2012-01-04 at 01:59 +0100, Linus Walleij wrote: >> > 2011/12/28 Jérémy Compostella : >> > >> > > On my laptop with 3.2.0-rc7 I have unexpected poweron after shutdown >> > > 300 seconds later. I found out that the "hwclock --systohw" plays with >> > > UIE which lead to the rtc_disable_rtc() function call. This function >> > > disables the alarm by setting the alarm 300 seconds later with the >> > > disable flag which does not seems to be the best way since we could >> > > use the alarm_irq_enable handler. >> > > >> > > I found out that the real problem is not really related to a bad RTC >> > > management but to a bad ACPI compliance. Actually, the platform should >> > > not wake up when the PM1_ENABLE RTC_EN ACPI flag isn't set. But it >> does >> > > on some machine like mine if there is a RTC CMOS alarm set in future >> even >> > > with the AIE disabled. >> > > >> > > Moreover, the rtc_timer_do_work calls the rtc_alarm_disable even if >> > > there is no alarm to disable. Therefore, during the boot time, a >> > > disabled alarm is set 300 seconds in the future which does not make >> > > sense. >> > >> > The patch looks very similar to a patch submitted by Rabin. >> > I took a look at the Rabin patch. I do agree it's very similar except that > my > patch avoid useless alarm disabling too. > > FYI, I successfully passed the rtc_test program with this patch applied. > > > I would prefer if John could say something about the proper solution... >> >> For 3.2 I've requested the problematic patch be reverted. We can try >> again for a proper fix for 3.3. >> >> thanks >> -john >> > I took a look at the whole RTC drivers and some of them does not implement the alarm_irq_enable callback and I'm not sure it makes sense and it's realistic to implement it for these one. So, I propose a new patch which disables the appropriate alarm instead of the arbitrary 300 seconds in the future alarm. This patch is compatible with the removed problematic patch and passed with success the rtctest test suite. Please review it, Best regards, Jeremy Compostella From fa83fa95ffb2279f2af80316583b18b784537245 Mon Sep 17 00:00:00 2001 From: Jeremy Compostella Date: Wed, 4 Jan 2012 21:03:38 +0100 Subject: [PATCH] rtc: Fix unexpected poweron and unnecessary alarm disabling On my laptop 3.2.0-rc6 I have unexpected poweron after shutdown 300 seconds later. I found out that the "hwclock --systohw" plays with UIE which lead to the rtc_disable_rtc() function call. This function disables the alarm by setting the alarm 300 seconds later with the disable flag which does not seems to be the best way since we could disable the appropriate alarm instead. Moreover, the rtc_timer_do_work calls the rtc_alarm_disable event if there is no alarm to disable. Therefore, during the boot time, a disabled alarm is set 300 seconds in the future which does not make sense. Signed-off-by: Jeremy Compostella --- drivers/rtc/interface.c | 16 ++++++---------- 1 files changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 3bcc7cf..7241b3f 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -776,15 +776,11 @@ static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer) return 0; } -static void rtc_alarm_disable(struct rtc_device *rtc) +static void rtc_alarm_disable(struct rtc_device *rtc, ktime_t time) { 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.time = rtc_ktime_to_tm(time); alarm.enabled = 0; ___rtc_set_alarm(rtc, &alarm); @@ -812,7 +808,7 @@ static void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer) int err; next = timerqueue_getnext(&rtc->timerqueue); if (!next) { - rtc_alarm_disable(rtc); + rtc_alarm_disable(rtc, timer->node.expires); return; } alarm.time = rtc_ktime_to_tm(next->expires); @@ -835,7 +831,7 @@ static void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer) */ void rtc_timer_do_work(struct work_struct *work) { - struct rtc_timer *timer; + struct rtc_timer *timer = NULL; struct timerqueue_node *next; ktime_t now; struct rtc_time tm; @@ -876,8 +872,8 @@ again: err = __rtc_set_alarm(rtc, &alarm); if (err == -ETIME) goto again; - } else - rtc_alarm_disable(rtc); + } else if (timer && timer->enabled == 0) + rtc_alarm_disable(rtc, timer->node.expires); mutex_unlock(&rtc->ops_lock); } -- 1.7.2.5