Skip to contentSkip to conversation

APM resume breakage from 2.6.18-rc1 clocksource changes

BUG New Mikael Pettersson 2 participants 2 messages Jul 10, 2006 – Jul 11, 2006

lore
Share & tools
lore

Mikael Pettersson

author
#0 lore raw
Quoted 11 lines · john stultz

So it seems possible that the timer tick will be enabled before the timekeeping resume code runs. I'm not sure why this isn't seen w/ ACPI suspend/resume, as I think they're using the same sysdev_class .suspend/.resume bits.

Anyway, I think this patch should fix it (I've only compile tested it, as I don't have my laptop on me right now). Would you mind giving it a try?

thanks
-john

With this patch my Latitude resumes OK from APM suspend again.

Thanks John.

Signed-off-by Mikael Pettersson <[email protected]>
Quoted 62 lines
kernel/timer.c +19 -3
index 396a3c0..afaa594 100644
@@ -966,7 +966,7 @@ void __init timekeeping_init(void)
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 }
 
-
+static int timekeeping_suspended;
 /*
 * timekeeping_resume - Resumes the generic timekeeping subsystem.
 * @dev:	unused

@@ -982,6 +982,18 @@ static int timekeeping_resume(struct sys
 	write_seqlock_irqsave(&xtime_lock, flags);
 	/* restart the last cycle value */
 	clock->cycle_last = clocksource_read(clock);
+	clock->error = 0;
+	timekeeping_suspended = 0;
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+	return 0;
+}
+
+static int timekeeping_suspend(struct sys_device *dev, pm_message_t state)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+	timekeeping_suspended = 1;
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 	return 0;
 }

@@ -989,6 +1001,7 @@ static int timekeeping_resume(struct sys
 /* sysfs resume/suspend bits for timekeeping */
 static struct sysdev_class timekeeping_sysclass = {
 	.resume		= timekeeping_resume,
+	.suspend	= timekeeping_suspend,
 	set_kset_name("timekeeping"),
 };
 

@@ -1090,14 +1103,17 @@ static void clocksource_adjust(struct cl
 static void update_wall_time(void)
 {
 	cycle_t offset;
-
-	clock->xtime_nsec += (s64)xtime.tv_nsec << clock->shift;
+	
+	/* avoid timekeeping before we're fully resumed */
+	if (unlikely(timekeeping_suspended))
+		return;
 
 #ifdef CONFIG_GENERIC_TIME
 	offset = (clocksource_read(clock) - clock->cycle_last) & clock->mask;
 #else
 	offset = clock->cycle_interval;
 #endif
+	clock->xtime_nsec += (s64)xtime.tv_nsec << clock->shift;
 
 	/* normally this loop will run just once, however in the
 	 * case of lost or late ticks, it will accumulate correctly.
#1 lore raw

PATCH 1/1 improve timekeeping resume robustness

Quoted 1 line · Mikael Pettersson

With this patch my Latitude resumes OK from APM suspend again.

This patch (against current git tree) resolves problems seen w/ APM suspend.

Due to resume initialization ordering, its possible we could get a timer interrupt before the timekeeping resume() function is called. This patch ensures we don't do any timekeeping accounting before we're fully resumed.

Signed-off-by John Stultz <[email protected]>
kernel/timer.c +19 -2
index 2a87430..6ffff03 100644
@@ -968,6 +968,7 @@ void __init timekeeping_init(void)
 }
 
 
+static int timekeeping_suspended;
 /*
  * timekeeping_resume - Resumes the generic timekeeping subsystem.
  * @dev:	unused

@@ -983,6 +984,18 @@ static int timekeeping_resume(struct sys
 	write_seqlock_irqsave(&xtime_lock, flags);
 	/* restart the last cycle value */
 	clock->cycle_last = clocksource_read(clock);
+	clock->error = 0;
+	timekeeping_suspended = 0;
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+	return 0;
+}
+
+static int timekeeping_suspend(struct sys_device *dev, pm_message_t state)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+	timekeeping_suspended = 1;
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 	return 0;
 }

@@ -990,6 +1003,7 @@ static int timekeeping_resume(struct sys
 /* sysfs resume/suspend bits for timekeeping */
 static struct sysdev_class timekeeping_sysclass = {
 	.resume		= timekeeping_resume,
+	.suspend	= timekeeping_suspend,
 	set_kset_name("timekeeping"),
 };
 

@@ -1099,14 +1113,17 @@ static void clocksource_adjust(struct cl
 static void update_wall_time(void)
 {
 	cycle_t offset;
-
-	clock->xtime_nsec += (s64)xtime.tv_nsec << clock->shift;
+	
+	/* Make sure we're fully resumed: */
+	if (unlikely(timekeeping_suspended))
+		return;
 
 #ifdef CONFIG_GENERIC_TIME
 	offset = (clocksource_read(clock) - clock->cycle_last) & clock->mask;
 #else
 	offset = clock->cycle_interval;
 #endif
+	clock->xtime_nsec += (s64)xtime.tv_nsec << clock->shift;
 
 	/* normally this loop will run just once, however in the
 	 * case of lost or late ticks, it will accumulate correctly.