From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.free-electrons.com (down.free-electrons.com. [37.187.137.238]) by gmr-mx.google.com with ESMTP id p12si228274wiv.1.2015.05.22.03.04.34 for ; Fri, 22 May 2015 03:04:34 -0700 (PDT) Date: Fri, 22 May 2015 12:04:33 +0200 From: Alexandre Belloni To: Huang Adrian Cc: Alessandro Zummo , rtc-linux@googlegroups.com, Brecht Machiels , Thomas Gleixner , John Stultz , Rabin Vincent , Borislav Petkov , Nagananda Chumbalkar , Adrian Huang Subject: [rtc-linux] Re: [RFC PATCH 2/2] rtc: Restore the RTC alarm time to the configured alarm time in BIOS Setup Message-ID: <20150522100433.GK3244@piout.net> References: <1431077706-3560-1-git-send-email-adrianhuang0701@gmail.com> <20150518220542.GW3338@piout.net> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 In-Reply-To: Reply-To: rtc-linux@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , Hi, On 20/05/2015 at 12:30:27 +0800, Huang Adrian wrote : > > > > Actually, I'm not sure that solves your issue. From what I > > understand, what is happening in step 4 is the bug you want to work > > around and it happens because AF gets set even if AIE is not set. > > > > Yes. > > > So, your rtc_alarm_restore() basically calls cmos_set_alarm() which > > calls cmos_irq_disable() finally this one calls cmos_checkintr() which > > does read register C and so clears AF. > > > > The call path is correct. cmos_checkintr() did clear interrupt flags. But, > the problem is that AF is set in the next one second. > > Assume 'n' is the current time. > > 1) The call path is finished at 'n' (current time). It clears the > interrupt flags. > 2) At n + 1, AF is set because the HW alarm time is programmed at n +1. > > This leads to the problem. > > > What I'm not sure is why cmos_do_shutdown() isn't clearing AF properly. > > Is it correctly called at shutdown? > > > > The function cmos_pnp_shutdown() simply returns because > cmos_poweroff() always returns 0 (CONFIG_PM and CONFIG_PNP are > set). The cmos_do_shutdown() is not invoked at shutdown. We did not > get a chance to clear interrupt flag. Here is the callpath: > > kernel_restart_prepare() -> device_shutdown() -> cmos_pnp_shutdown() > > [Experiment 1 - SLES11 SP3 with upstream kernel (without my patch)] > The revised code in cmos_pnp_shutdown() is shown as follows. It > ensures that cmos_do_shutdown() is invoked, which gets the last chance > to clear the interrupt flag before powering off the machine. > > #if 0 > if (system_state == SYSTEM_POWER_OFF && !cmos_poweroff(dev)) > return; > #endif > cmos_poweroff(dev); > > > The issue can be still reproduced. This implies that the time cost > between hwclock and cmos_do_shutdown() is within one second. > So, AF is not set yet. > > [Experiment 2 - SLES11 SP3 with upstream kernel (without my patch)] > Still apply the code snippet in Experiment 1. The system stayed > power off when I intentionally added 'sleep 3' command in the end of > the 'stop' service of /etc/init.d/boot.clock (This script uses the > hwclock utility to set hardware clock > to the current system time). This proves that cmos_do_shutdown() > clears AF generated by RTC HW. > > The system always stayed power off with 10-time shutdown tests. > > [Experiment 3 - SLES11 SP3 with upstream kernel (without my patch)] > Do not apply the code snippet in Experiment 1. Also added 'sleep 3' > command in the end of the 'stop' service of /etc/init.d/boot.clock. > The issue can be reproduced. This proves that cmos_pnp_shutdown() > simply returns without invoking cmos_do_shutdown(). So, AF is not > cleared. > > Experiments 1-3 show that the issue happens if the time cost > between hwclock and cmos_do_shutdown() is within one second. > If we want to clear the interrupt flag for every shutdown, we need > to re-write the cmos_pnp_shutdown() to ensure the > cmos_do_shutdown() is invoke for every shutdown. > > Although my patch can work around the issue, kindly let me know > if you want to re-work cmos_pnp_shutdown(). We can come out a > better way to invoke cmos_do_shutdown() for every shutdown. But, > be remember, the issue still happens even though we make sure > cmos_do_shutdown() is invoked for every shutdown. The problem > is the one second interval. > That's what I understood. How I see it is the following: your patch is reading and setting the alarm then clearing AF at a later point in the shutdown. This may as well be before that one second has elapsed so I don't feel this is a proper fix. Also, other RTCs don't suffer from that issue and this may actually break some of them. What I would do is ensure that AF gets cleared in cmos_do_shutdown() after setting the alarm to now or now - 1 when it is not used for wakeup/restart. Regards, -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.