Re: [PATCH] RTC classdev: Add sysfs support for wakeup alarm (r/w)

[David Brownell <david-b@pacbell.net>]

On Sunday 17 December 2006 11:30 am, Paul Sokolovsky wrote:

>     Small battery-powered systems, like PDAs, need a way to be
> suspended most of the time and woken up just from time to time to
> process pending tasks. 

Sounds like you're thinking of this from a userspace perspective...

Could you share some examples of such "pending tasks"?  I suspect
beaconing once or twice a second in a wireless network isn't quite
what you mean, although it fits your description.  On some Linux
platforms with dynamic tick support, Linux can enter suspend modes
between periodic wakeups needed for such beaconing.


> Obvious way to achieve this is to use timer, or 
> alarm, wakeup. Unfortunately, this matter is bit confusing in Linux.
> There's only one "good" "supported" way to set alarm - via ioctl() on
> an RTC device fd. Unfortunately, this alarm is not persistent - as soon
> as fd is closed, alarm id discharged.

I don't think that's true in general.  Most RTCs don't even care
whether userspace did an open() or close().  I see the S3C one does,
and that explicitly leaves the alarm active. 

But I see that only the SA1100/PXA and SH RTCs turn off all IRQs
after RTC_WKALM_* requests ... that's a distinct minority.

So judging implementations as votes ... only two implementations
that implement the RTC_WKALM_* call follow that rule, and most
don't.  However, a few implementations ignore rtc_wkalrm.enabled,
or otherwise mistreat that flag (e.g. rtc-ds1553 doesn't disable
AIE when enabled==0), so it's clear there are some issues there.

My vote would be that closing the FD should not turn off the alarm.
It's supposed to be a one-shot deal anyway.

And also, that someone audits the drivers/rtc code to make sure that
alarm-capable drivers handle the rtc_wkalrm.enabled flag correctly;
your patch sort of presumes that will happen, anyway.  And hmm, it'd
be good to have rtctest.c (in Documentation/rtc.txt) test for that...
it doesn't actually use RTC_WKALM_* calls, so it's too easy for folk
to goof up their implementations.


> Formal part
> ===========
> 
> Implement "alarm" attribute group for RTC classdevs. At this time,
> add "since_epoch", "wakeup_enabled", and "pending" attributes. First
> two support both read and write.

I think you shouldn't add this group unless the RTC has methods
to read and write the alarm; there are RTCs that don't have that
feature.

Also, I'd rather see a much simpler interface.  Like a single
"alarm" attribute.  It would display as the empty string unless
it was enabled, in which case the alarm time wouhd show.  To
disable it, write an empty string; to enable an alarm, just write
a valid time (in the future).  The other parameters aren't needed;
"wakeup" is PM infrastructure (/sys/devices/.../power/wakeup),
since it's easy to have an alarm that's not wakeup-capable.

- Dave