* RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms.
@ 2011-01-20 20:51 David Daney
2011-01-20 21:16 ` John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2011-01-20 20:51 UTC (permalink / raw)
To: John Stultz, Linux Kernel Mailing List, Linus Torvalds,
Alessandro Zummo, Thomas Gleixner
The hwclock program on my MIPS Debian GNU/Linux 5.0 system quit
working with 2.6.38-rc1.
This particular ds1307 has no interrupt connection, so the alarm
feature cannot be used. Because of this the
rtc_class_ops.set_alarm() function always will return -EINVAL
This problem appears to be related to commits:
042620a RTC: Remove UIE emulation
6610e08 RTC: Rework RTC code to use timerqueue for events
My system uses a ds1307 RTC which still works as evidenced by the
kernel boot messages.
.
.
.
rtc-ds1307 0-0068: setting system clock to 2011-01-20 19:58:48 UTC
(1295553528)
.
.
.
But hwclock now fails here is some nice strace output:
.
.
.
open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3
ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = 0
_newselect(4, [3], NULL, NULL, {5, 0}) = 0 (Timeout)
write(2, "select() to /dev/rtc0 to wait for"..., 55select() to /dev/rtc0
to wait for clock tick timed out
) = 55
ioctl(3, PRESTO_SETPID or RTC_UIE_OFF, 0) = 0
close(3) = 0
exit_group(1) = ?
The hwclock program is asking to put the clock in UIE mode and then
does a select() on it. Since the alarm doesn't work, the select times out.
Previously the ioctl(RTC_UIE_ON) would return EINVAL:
.
.
.
open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3
ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = -1 EINVAL (Invalid argument)
ioctl(3, PRESTO_SETOPT or RTC_RD_TIME, {tm_sec=48, tm_min=41,
tm_hour=20, tm_mday=20, tm_mon=0, tm_year=111, ...}) = 0
ioctl(3, PRESTO_SETOPT or RTC_RD_TIME, {tm_sec=48, tm_min=41,
tm_hour=20, tm_mday=20, tm_mon=0, tm_year=111, ...}) = 0
.
.
.
The problem seems to be that rtc_update_irq_enable() no longer returns
-EINVAL when it cannot set the alarm.
David Daney
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms. 2011-01-20 20:51 RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms David Daney @ 2011-01-20 21:16 ` John Stultz 2011-01-20 21:24 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: John Stultz @ 2011-01-20 21:16 UTC (permalink / raw) To: David Daney, Andreas Schwab Cc: Linux Kernel Mailing List, Linus Torvalds, Alessandro Zummo, Thomas Gleixner On Thu, 2011-01-20 at 12:51 -0800, David Daney wrote: > open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3 > ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = 0 > _newselect(4, [3], NULL, NULL, {5, 0}) = 0 (Timeout) > write(2, "select() to /dev/rtc0 to wait for"..., 55select() to /dev/rtc0 > to wait for clock tick timed out > ) = 55 > ioctl(3, PRESTO_SETPID or RTC_UIE_OFF, 0) = 0 > close(3) = 0 > exit_group(1) = ? > > > The hwclock program is asking to put the clock in UIE mode and then > does a select() on it. Since the alarm doesn't work, the select times out. > > Previously the ioctl(RTC_UIE_ON) would return EINVAL: Ah. Good diagnosis! Let me try to get a patch for you and Andreas to test. thanks -john ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms. 2011-01-20 21:16 ` John Stultz @ 2011-01-20 21:24 ` Geert Uytterhoeven 2011-01-20 22:23 ` John Stultz 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2011-01-20 21:24 UTC (permalink / raw) To: John Stultz Cc: David Daney, Andreas Schwab, Linux Kernel Mailing List, Linus Torvalds, Alessandro Zummo, Thomas Gleixner On Thu, Jan 20, 2011 at 22:16, John Stultz <john.stultz@linaro.org> wrote: > On Thu, 2011-01-20 at 12:51 -0800, David Daney wrote: >> open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3 >> ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = 0 >> _newselect(4, [3], NULL, NULL, {5, 0}) = 0 (Timeout) >> write(2, "select() to /dev/rtc0 to wait for"..., 55select() to /dev/rtc0 >> to wait for clock tick timed out >> ) = 55 >> ioctl(3, PRESTO_SETPID or RTC_UIE_OFF, 0) = 0 >> close(3) = 0 >> exit_group(1) = ? >> >> >> The hwclock program is asking to put the clock in UIE mode and then >> does a select() on it. Since the alarm doesn't work, the select times out. >> >> Previously the ioctl(RTC_UIE_ON) would return EINVAL: > > Ah. Good diagnosis! Let me try to get a patch for you and Andreas to > test. I'm also seeing this on m68k (ARAnyM, rtc-generic). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms. 2011-01-20 21:24 ` Geert Uytterhoeven @ 2011-01-20 22:23 ` John Stultz 2011-01-20 22:54 ` David Daney 0 siblings, 1 reply; 6+ messages in thread From: John Stultz @ 2011-01-20 22:23 UTC (permalink / raw) To: Geert Uytterhoeven Cc: David Daney, Andreas Schwab, Linux Kernel Mailing List, Linus Torvalds, Alessandro Zummo, Thomas Gleixner On Thu, 2011-01-20 at 22:24 +0100, Geert Uytterhoeven wrote: > On Thu, Jan 20, 2011 at 22:16, John Stultz <john.stultz@linaro.org> wrote: > > On Thu, 2011-01-20 at 12:51 -0800, David Daney wrote: > >> open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3 > >> ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = 0 > >> _newselect(4, [3], NULL, NULL, {5, 0}) = 0 (Timeout) > >> write(2, "select() to /dev/rtc0 to wait for"..., 55select() to /dev/rtc0 > >> to wait for clock tick timed out > >> ) = 55 > >> ioctl(3, PRESTO_SETPID or RTC_UIE_OFF, 0) = 0 > >> close(3) = 0 > >> exit_group(1) = ? > >> > >> > >> The hwclock program is asking to put the clock in UIE mode and then > >> does a select() on it. Since the alarm doesn't work, the select times out. > >> > >> Previously the ioctl(RTC_UIE_ON) would return EINVAL: > > > > Ah. Good diagnosis! Let me try to get a patch for you and Andreas to > > test. > > I'm also seeing this on m68k (ARAnyM, rtc-generic). Geert, David, Andreas, Could you try the following? Its a bit messy of a patch doing a couple of things: 1) Simplify the timer->enabled management by pushing it into rtc_timer_enqueue/remove (needed cleanup for #2). 2) Properly propagating errors from __rtc_set_alarm back through rtc_timer_enqueue and users. 3) Trivial clenaup making rtc_timer_enqueue/remove static. 4) Fixup virtualized rtc_read_alarm to check hardware capabilities and return errors (also restores zeroing of the rtc_wkalrm stucture). I'll be cleaning these up and breaking them into commits I can send upward, but I wanted to make sure it resolves the issue for you. Let me know if it fixes things. thanks -john NOT FOR INCLUSION! NOT FOR INCLUSION! NOT FOR INCLUSION! Signed-off-by: John Stultz <john.stultz@linaro.org> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 90384b9..db816cd 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -16,6 +16,9 @@ #include <linux/log2.h> #include <linux/workqueue.h> +static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer); +static void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer); + static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm) { int err; @@ -120,12 +123,20 @@ int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm) err = mutex_lock_interruptible(&rtc->ops_lock); if (err) return err; - alarm->enabled = rtc->aie_timer.enabled; - if (alarm->enabled) - alarm->time = rtc_ktime_to_tm(rtc->aie_timer.node.expires); + if (rtc->ops == NULL) + err = -ENODEV; + else if (!rtc->ops->read_alarm) + err = -EINVAL; + else { + memset(alarm, 0, sizeof(struct rtc_wkalrm)); + alarm->enabled = rtc->aie_timer.enabled; + if (alarm->enabled) + alarm->time = + rtc_ktime_to_tm(rtc->aie_timer.node.expires); + } mutex_unlock(&rtc->ops_lock); - return 0; + return err; } EXPORT_SYMBOL_GPL(rtc_read_alarm); @@ -175,16 +186,14 @@ int rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm) return err; if (rtc->aie_timer.enabled) { rtc_timer_remove(rtc, &rtc->aie_timer); - rtc->aie_timer.enabled = 0; } rtc->aie_timer.node.expires = rtc_tm_to_ktime(alarm->time); rtc->aie_timer.period = ktime_set(0, 0); if (alarm->enabled) { - rtc->aie_timer.enabled = 1; - rtc_timer_enqueue(rtc, &rtc->aie_timer); + err = rtc_timer_enqueue(rtc, &rtc->aie_timer); } mutex_unlock(&rtc->ops_lock); - return 0; + return err; } EXPORT_SYMBOL_GPL(rtc_set_alarm); @@ -195,15 +204,15 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled) return err; if (rtc->aie_timer.enabled != enabled) { - if (enabled) { - rtc->aie_timer.enabled = 1; - rtc_timer_enqueue(rtc, &rtc->aie_timer); - } else { + if (enabled) + err = rtc_timer_enqueue(rtc, &rtc->aie_timer); + else rtc_timer_remove(rtc, &rtc->aie_timer); - rtc->aie_timer.enabled = 0; - } } + if (err) + return err; + if (!rtc->ops) err = -ENODEV; else if (!rtc->ops->alarm_irq_enable) @@ -235,12 +244,9 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled) now = rtc_tm_to_ktime(tm); rtc->uie_rtctimer.node.expires = ktime_add(now, onesec); rtc->uie_rtctimer.period = ktime_set(1, 0); - rtc->uie_rtctimer.enabled = 1; - rtc_timer_enqueue(rtc, &rtc->uie_rtctimer); - } else { + err = rtc_timer_enqueue(rtc, &rtc->uie_rtctimer); + } else rtc_timer_remove(rtc, &rtc->uie_rtctimer); - rtc->uie_rtctimer.enabled = 0; - } out: mutex_unlock(&rtc->ops_lock); @@ -488,10 +494,13 @@ EXPORT_SYMBOL_GPL(rtc_irq_set_freq); * Enqueues a timer onto the rtc devices timerqueue and sets * the next alarm event appropriately. * + * Sets the enabled bit on the added timer. + * * Must hold ops_lock for proper serialization of timerqueue */ -void rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer) +static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer) { + timer->enabled = 1; timerqueue_add(&rtc->timerqueue, &timer->node); if (&timer->node == timerqueue_getnext(&rtc->timerqueue)) { struct rtc_wkalrm alarm; @@ -501,7 +510,13 @@ void rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer) err = __rtc_set_alarm(rtc, &alarm); if (err == -ETIME) schedule_work(&rtc->irqwork); + else if (err) { + timerqueue_del(&rtc->timerqueue, &timer->node); + timer->enabled = 0; + return err; + } } + return 0; } /** @@ -512,13 +527,15 @@ void rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer) * Removes a timer onto the rtc devices timerqueue and sets * the next alarm event appropriately. * + * Clears the enabled bit on the removed timer. + * * Must hold ops_lock for proper serialization of timerqueue */ -void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer) +static void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer) { struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue); timerqueue_del(&rtc->timerqueue, &timer->node); - + timer->enabled = 0; if (next == &timer->node) { struct rtc_wkalrm alarm; int err; @@ -626,8 +643,7 @@ int rtc_timer_start(struct rtc_device *rtc, struct rtc_timer* timer, timer->node.expires = expires; timer->period = period; - timer->enabled = 1; - rtc_timer_enqueue(rtc, timer); + ret = rtc_timer_enqueue(rtc, timer); mutex_unlock(&rtc->ops_lock); return ret; @@ -645,7 +661,6 @@ int rtc_timer_cancel(struct rtc_device *rtc, struct rtc_timer* timer) mutex_lock(&rtc->ops_lock); if (timer->enabled) rtc_timer_remove(rtc, timer); - timer->enabled = 0; mutex_unlock(&rtc->ops_lock); return ret; } diff --git a/include/linux/rtc.h b/include/linux/rtc.h index 3c995b4..bd4fbc3 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -246,8 +246,6 @@ int rtc_register(rtc_task_t *task); int rtc_unregister(rtc_task_t *task); int rtc_control(rtc_task_t *t, unsigned int cmd, unsigned long arg); -void rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer); -void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer); void rtc_timer_init(struct rtc_timer *timer, void (*f)(void* p), void* data); int rtc_timer_start(struct rtc_device *rtc, struct rtc_timer* timer, ktime_t expires, ktime_t period); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms. 2011-01-20 22:23 ` John Stultz @ 2011-01-20 22:54 ` David Daney 2011-01-21 10:48 ` Andreas Schwab 0 siblings, 1 reply; 6+ messages in thread From: David Daney @ 2011-01-20 22:54 UTC (permalink / raw) To: John Stultz Cc: Geert Uytterhoeven, Andreas Schwab, Linux Kernel Mailing List, Linus Torvalds, Alessandro Zummo, Thomas Gleixner On 01/20/2011 02:23 PM, John Stultz wrote: > On Thu, 2011-01-20 at 22:24 +0100, Geert Uytterhoeven wrote: >> On Thu, Jan 20, 2011 at 22:16, John Stultz<john.stultz@linaro.org> wrote: >>> On Thu, 2011-01-20 at 12:51 -0800, David Daney wrote: >>>> open("/dev/rtc0", O_RDONLY|O_LARGEFILE) = 3 >>>> ioctl(3, PRESTO_GETMOUNT or RTC_UIE_ON, 0) = 0 >>>> _newselect(4, [3], NULL, NULL, {5, 0}) = 0 (Timeout) >>>> write(2, "select() to /dev/rtc0 to wait for"..., 55select() to /dev/rtc0 >>>> to wait for clock tick timed out >>>> ) = 55 >>>> ioctl(3, PRESTO_SETPID or RTC_UIE_OFF, 0) = 0 >>>> close(3) = 0 >>>> exit_group(1) = ? >>>> >>>> >>>> The hwclock program is asking to put the clock in UIE mode and then >>>> does a select() on it. Since the alarm doesn't work, the select times out. >>>> >>>> Previously the ioctl(RTC_UIE_ON) would return EINVAL: >>> >>> Ah. Good diagnosis! Let me try to get a patch for you and Andreas to >>> test. >> >> I'm also seeing this on m68k (ARAnyM, rtc-generic). > > Geert, David, Andreas, > Could you try the following? Its a bit messy of a patch doing a couple > of things: > > 1) Simplify the timer->enabled management by pushing it into > rtc_timer_enqueue/remove (needed cleanup for #2). > > 2) Properly propagating errors from __rtc_set_alarm back through > rtc_timer_enqueue and users. > > 3) Trivial clenaup making rtc_timer_enqueue/remove static. > > 4) Fixup virtualized rtc_read_alarm to check hardware capabilities and > return errors (also restores zeroing of the rtc_wkalrm stucture). > > I'll be cleaning these up and breaking them into commits I can send > upward, but I wanted to make sure it resolves the issue for you. > > Let me know if it fixes things. > > thanks > -john > John, With that patch, it works now. Thanks. You can add: Tested-by: David Daney <ddaney@caviumnetworks.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms. 2011-01-20 22:54 ` David Daney @ 2011-01-21 10:48 ` Andreas Schwab 0 siblings, 0 replies; 6+ messages in thread From: Andreas Schwab @ 2011-01-21 10:48 UTC (permalink / raw) To: David Daney Cc: John Stultz, Geert Uytterhoeven, Linux Kernel Mailing List, Linus Torvalds, Alessandro Zummo, Thomas Gleixner David Daney <ddaney@caviumnetworks.com> writes: > John, > > With that patch, it works now. Thanks. Works here as well. Thanks, Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-21 10:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-20 20:51 RTC seems broken on 2.6.38-rc1 for RTCs lacking alarms David Daney 2011-01-20 21:16 ` John Stultz 2011-01-20 21:24 ` Geert Uytterhoeven 2011-01-20 22:23 ` John Stultz 2011-01-20 22:54 ` David Daney 2011-01-21 10:48 ` Andreas Schwab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).