Patchwork rtc: cmos: Fix accidentally enabling rtc channel

login
register
mail settings
Submitter Derek Basehore
Date May 23, 2013, 1:04 a.m.
Message ID <1369271045-17410-1-git-send-email-dbasehore@chromium.org>
Download mbox | patch
Permalink /patch/245794/
State New
Headers show

Comments

Derek Basehore - May 23, 2013, 1:04 a.m.
During resume, we call hpet_rtc_timer_init after masking an irq bit in hpet.
This will cause the call to hpet_disable_rtc_channel to be undone if RTC_AIE is
the only bit not masked.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
---
 drivers/rtc/rtc-cmos.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Andrew Morton - May 29, 2013, 7:27 p.m.
On Wed, 22 May 2013 18:04:05 -0700 Derek Basehore <dbasehore@chromium.org> wrote:

> During resume, we call hpet_rtc_timer_init after masking an irq bit in hpet.
> This will cause the call to hpet_disable_rtc_channel to be undone if RTC_AIE is
> the only bit not masked.

What were the user-visible runtime effects of this bug?
Derek Basehore - May 29, 2013, 8:12 p.m.
I'm still debugging all of the issues, but allowing the cmos interrupt
handler to run before resuming caused some issues where the timer for
the alarm was not removed. This would cause other, later timers to not
be cleared, so utilities such as hwclock would time out when waiting
for the update interrupt.

There's another patch needed that I am currently testing that works
around an issue seen in coreboot (and probably other firmware) where
the RTC_CONTROL register is cleared before we get to the kernel. This
prevents the cmos interrupt handler from clearing an alarm.

On Wed, May 29, 2013 at 12:27 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 22 May 2013 18:04:05 -0700 Derek Basehore <dbasehore@chromium.org> wrote:
>
>> During resume, we call hpet_rtc_timer_init after masking an irq bit in hpet.
>> This will cause the call to hpet_disable_rtc_channel to be undone if RTC_AIE is
>> the only bit not masked.
>
> What were the user-visible runtime effects of this bug?

Patch

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index cc5bea9..ee0bc69 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -854,6 +854,10 @@  static int cmos_resume(struct device *dev)
 		}
 
 		spin_lock_irq(&rtc_lock);
+		if (device_may_wakeup(dev)) {
+			hpet_rtc_timer_init();
+		}
+
 		do {
 			CMOS_WRITE(tmp, RTC_CONTROL);
 			hpet_set_rtc_irq_bit(tmp & RTC_IRQMASK);
@@ -869,7 +873,6 @@  static int cmos_resume(struct device *dev)
 			rtc_update_irq(cmos->rtc, 1, mask);
 			tmp &= ~RTC_AIE;
 			hpet_mask_rtc_irq_bit(RTC_AIE);
-			hpet_rtc_timer_init();
 		} while (mask & RTC_AIE);
 		spin_unlock_irq(&rtc_lock);
 	}