* 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).