[RFC][PATCH] timers: Make alarmtimer depend on CONFIG_RTC_CLASS

[RFC][PATCH] timers: Make alarmtimer depend on CONFIG_RTC_CLASS

[Richard Weinberger]

The alarmtimer interface makes IMHO only sense when a RTC device
is available.
On systems with !CONFIG_RTC_CLASS (like UML) the warning
"Kernel not built with RTC support, ALARM timers will not wake from suspend"
is annoying.

Reported-by: Toralf Förster <toralf.foerster@gmx.de>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 kernel/time/Makefile     |    3 ++-
 kernel/time/alarmtimer.c |   19 +------------------
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index e2fd74b..224f833 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,6 +1,7 @@
 obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
-obj-y += timeconv.o posix-clock.o alarmtimer.o
+obj-y += timeconv.o posix-clock.o
 
+obj-$(CONFIG_RTC_CLASS) += alarmtimer.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST)	+= tick-broadcast.o
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 2d96624..c1f2898 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -42,11 +42,9 @@ static struct alarm_base {
 	clockid_t		base_clockid;
 } alarm_bases[ALARM_NUMTYPE];
 
-#ifdef CONFIG_RTC_CLASS
 /* rtc timer and device for setting alarm wakeups at suspend */
 static struct rtc_timer		rtctimer;
 static struct rtc_device	*rtcdev;
-#endif
 
 /* freezer delta & lock used to handle clock_nanosleep triggered wakeups */
 static ktime_t freezer_delta;
@@ -150,7 +148,6 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer)
 
 }
 
-#ifdef CONFIG_RTC_CLASS
 /**
  * alarmtimer_suspend - Suspend time callback
  * @dev: unused
@@ -208,12 +205,6 @@ static int alarmtimer_suspend(struct device *dev)
 
 	return 0;
 }
-#else
-static int alarmtimer_suspend(struct device *dev)
-{
-	return 0;
-}
-#endif
 
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 {
@@ -638,7 +629,6 @@ static int __init alarmtimer_init(void)
 }
 device_initcall(alarmtimer_init);
 
-#ifdef CONFIG_RTC_CLASS
 /**
  * has_wakealarm - check rtc device has wakealarm ability
  * @dev: current device
@@ -691,12 +681,5 @@ static int __init alarmtimer_init_late(void)
 
 	return 0;
 }
-#else
-static int __init alarmtimer_init_late(void)
-{
-	printk(KERN_WARNING "Kernel not built with RTC support, ALARM timers"
-		" will not wake from suspend");
-	return 0;
-}
-#endif
+
 late_initcall(alarmtimer_init_late);
-- 
1.7.5.3

Re: [RFC][PATCH] timers: Make alarmtimer depend on CONFIG_RTC_CLASS

[John Stultz]

On Sun, 2011-05-29 at 12:48 +0200, Richard Weinberger wrote:
> The alarmtimer interface makes IMHO only sense when a RTC device
> is available.
> On systems with !CONFIG_RTC_CLASS (like UML) the warning
> "Kernel not built with RTC support, ALARM timers will not wake from suspend"
> is annoying.

Yea. 

The tradeoff with this patch is that applications that use
CLOCK_REALTIME_ALARM or CLOCK_BOOTTIME_ALARM will then get -EINVAL.

I'm mixed here, since we probably want to communicate to the application
that the alarm timers aren't going to wake us up, but also I suspect
most applications won't handle the -EINVAL properly, so I had allowed
for the clockids to still work as long as we didn't suspend.

I'm leaning more towards just returning EINVAL as you suggest, since
really the functionality isn't there. But I'm thinking possibly doing so
if no RTCs are detected at runtime (rather then using all the ifdefs you
do).

Thoughts from anyone else?

thanks
-john

Re: [RFC][PATCH] timers: Make alarmtimer depend on CONFIG_RTC_CLASS

[Peter Zijlstra]

On Tue, 2011-05-31 at 23:38 -0700, John Stultz wrote:
> On Sun, 2011-05-29 at 12:48 +0200, Richard Weinberger wrote:
> > The alarmtimer interface makes IMHO only sense when a RTC device
> > is available.
> > On systems with !CONFIG_RTC_CLASS (like UML) the warning
> > "Kernel not built with RTC support, ALARM timers will not wake from suspend"
> > is annoying.
> 
> Yea. 
> 
> The tradeoff with this patch is that applications that use
> CLOCK_REALTIME_ALARM or CLOCK_BOOTTIME_ALARM will then get -EINVAL.
> 
> I'm mixed here, since we probably want to communicate to the application
> that the alarm timers aren't going to wake us up, but also I suspect
> most applications won't handle the -EINVAL properly, so I had allowed
> for the clockids to still work as long as we didn't suspend.
> 
> I'm leaning more towards just returning EINVAL as you suggest, since
> really the functionality isn't there. But I'm thinking possibly doing so
> if no RTCs are detected at runtime (rather then using all the ifdefs you
> do).
> 
> Thoughts from anyone else?

Wouldn't -ENOTSUPP be a better return value than -EINVAL? But yeah, I
think simply returning an error is fine, if apps don't check for that,
they're broken, simple as that, we can't possibly avoid all userspace
problems by adding more kernel code.