[rtc-linux] Re: [RFC PATCH 2/2] rtc: Restore the RTC alarm time to the configured alarm time in BIOS Setup

[Alexandre Belloni <alexandre.belloni@free-electrons.com>]

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.