* [PATCH] rtc: cmos: Fix accidentally enabling rtc channel
@ 2013-05-23 1:04 Derek Basehore
2013-05-29 19:27 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Derek Basehore @ 2013-05-23 1:04 UTC (permalink / raw)
To: linux-kernel
Cc: rtc-linux, Alessandro Zummo, akpm, rostedt, gregkh, Derek Basehore
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(-)
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);
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] rtc: cmos: Fix accidentally enabling rtc channel
2013-05-23 1:04 [PATCH] rtc: cmos: Fix accidentally enabling rtc channel Derek Basehore
@ 2013-05-29 19:27 ` Andrew Morton
2013-05-29 20:12 ` dbasehore .
2013-06-03 1:13 ` [PATCH] rtc: cmos: Workaround bios clearing rtc control Derek Basehore
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Morton @ 2013-05-29 19:27 UTC (permalink / raw)
To: Derek Basehore; +Cc: linux-kernel, rtc-linux, Alessandro Zummo, rostedt, gregkh
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rtc: cmos: Fix accidentally enabling rtc channel
2013-05-29 19:27 ` Andrew Morton
@ 2013-05-29 20:12 ` dbasehore .
2013-06-03 1:13 ` [PATCH] rtc: cmos: Workaround bios clearing rtc control Derek Basehore
1 sibling, 0 replies; 4+ messages in thread
From: dbasehore . @ 2013-05-29 20:12 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, rtc-linux, Alessandro Zummo, rostedt, gregkh
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] rtc: cmos: Workaround bios clearing rtc control
2013-05-29 19:27 ` Andrew Morton
2013-05-29 20:12 ` dbasehore .
@ 2013-06-03 1:13 ` Derek Basehore
1 sibling, 0 replies; 4+ messages in thread
From: Derek Basehore @ 2013-06-03 1:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, rtc-linux, Alessandro Zummo, rostedt, gregkh,
Derek Basehore
The bios may clear the rtc control register when resuming the system. Since the
cmos interrupt handler may now be run before the rtc_cmos is resumed, this can
cause the interrupt handler to ignore an alarm since the alarm bit is not set in
the rtc control register. To work around this, check if the rtc_cmos is
suspended and use the stored value for the rtc control register.
Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Reviewed-by: Sameer Nanda <snanda@chromium.org>
---
drivers/rtc/rtc-cmos.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index ee0bc69..22803fe 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -556,17 +556,24 @@ static irqreturn_t cmos_interrupt(int irq, void *p)
rtc_control = CMOS_READ(RTC_CONTROL);
if (is_hpet_enabled())
irqstat = (unsigned long)irq & 0xF0;
- irqstat &= (rtc_control & RTC_IRQMASK) | RTC_IRQF;
+
+ /* If we were suspended, RTC_CONTROL may not be accurate since the
+ * bios may have cleared it.
+ */
+ if (!cmos_rtc.suspend_ctrl)
+ irqstat &= (rtc_control & RTC_IRQMASK) | RTC_IRQF;
+ else
+ irqstat &= (cmos_rtc.suspend_ctrl & RTC_IRQMASK) | RTC_IRQF;
/* All Linux RTC alarms should be treated as if they were oneshot.
* Similar code may be needed in system wakeup paths, in case the
* alarm woke the system.
*/
if (irqstat & RTC_AIE) {
+ cmos_rtc.suspend_ctrl &= ~RTC_AIE;
rtc_control &= ~RTC_AIE;
CMOS_WRITE(rtc_control, RTC_CONTROL);
hpet_mask_rtc_irq_bit(RTC_AIE);
-
CMOS_READ(RTC_INTR_FLAGS);
}
spin_unlock(&rtc_lock);
@@ -839,21 +846,23 @@ static inline int cmos_poweroff(struct device *dev)
static int cmos_resume(struct device *dev)
{
struct cmos_rtc *cmos = dev_get_drvdata(dev);
- unsigned char tmp = cmos->suspend_ctrl;
+ unsigned char tmp;
+ if (cmos->enabled_wake) {
+ if (cmos->wake_off)
+ cmos->wake_off(dev);
+ else
+ disable_irq_wake(cmos->irq);
+ cmos->enabled_wake = 0;
+ }
+
+ spin_lock_irq(&rtc_lock);
+ tmp = cmos->suspend_ctrl;
+ cmos->suspend_ctrl = 0;
/* re-enable any irqs previously active */
if (tmp & RTC_IRQMASK) {
unsigned char mask;
- if (cmos->enabled_wake) {
- if (cmos->wake_off)
- cmos->wake_off(dev);
- else
- disable_irq_wake(cmos->irq);
- cmos->enabled_wake = 0;
- }
-
- spin_lock_irq(&rtc_lock);
if (device_may_wakeup(dev)) {
hpet_rtc_timer_init();
}
@@ -874,8 +883,8 @@ static int cmos_resume(struct device *dev)
tmp &= ~RTC_AIE;
hpet_mask_rtc_irq_bit(RTC_AIE);
} while (mask & RTC_AIE);
- spin_unlock_irq(&rtc_lock);
}
+ spin_unlock_irq(&rtc_lock);
dev_dbg(dev, "resume, ctrl %02x\n", tmp);
--
1.8.2.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-03 1:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-23 1:04 [PATCH] rtc: cmos: Fix accidentally enabling rtc channel Derek Basehore
2013-05-29 19:27 ` Andrew Morton
2013-05-29 20:12 ` dbasehore .
2013-06-03 1:13 ` [PATCH] rtc: cmos: Workaround bios clearing rtc control Derek Basehore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.