2009-09-04 16:20:05

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 1/2] rtc: Document the sysfs interface

The sysfs interface to the RTC class drivers is currently undocumented.
Add some basic documentation defining the semantics of the fields.

Signed-off-by: Matthew Garrett <[email protected]>
---
Documentation/rtc.txt | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/Documentation/rtc.txt b/Documentation/rtc.txt
index 8deffcd..216bb8c 100644
--- a/Documentation/rtc.txt
+++ b/Documentation/rtc.txt
@@ -135,6 +135,28 @@ a high functionality RTC is integrated into the SOC. That system might read
the system clock from the discrete RTC, but use the integrated one for all
other tasks, because of its greater functionality.

+SYSFS INTERFACE
+---------------
+
+The sysfs interface under /sys/class/rtc/rtcN provides access to various
+rtc attributes without requiring the use of ioctls. All dates and times
+are in the RTC's timezone, rather than in system time.
+
+date: RTC-provided date
+max_user_freq: The maximum interrupt rate an unprivileged user may request
+ from this RTC.
+name: The name of the RTC corresponding to this sysfs directory
+since_epoch: The number of seconds since the epoch according to the RTC
+time: RTC-provided time
+wakealarm: The time at which the clock will generate a system wakeup
+ event. This is a one shot wakeup event, so must be reset
+ after wake if a daily wakeup is required. Format is either
+ seconds since the epoch or, if there's a leading +, seconds
+ in the future.
+
+IOCTL INTERFACE
+---------------
+
The ioctl() calls supported by /dev/rtc are also supported by the RTC class
framework. However, because the chips and systems are not standardized,
some PC/AT functionality might not be provided. And in the same way, some
--
1.6.4.1


2009-09-04 16:25:32

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, 4 Sep 2009 12:26:08 -0400
Matthew Garrett <[email protected]> wrote:

> CONFIG_RTC_HCTOSYS allows the kernel to read the system time from the RTC
> at boot and resume, avoiding the need for userspace to do so. Unfortunately
> userspace currently has no way to know whether this configuration option
> is enabled and thus cannot sensibly choose whether to run hwclock itself or
> not. Add a boot_timesource sysfs attribute which indicates whether a given
> RTC set the system clock.

This feature is meant to be in addition of the regular
hwclock call to read the system time.

Have you tested if there are drawbacks by not reading it? Like
TZ, DST and similar?

--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-04 16:20:06

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

CONFIG_RTC_HCTOSYS allows the kernel to read the system time from the RTC
at boot and resume, avoiding the need for userspace to do so. Unfortunately
userspace currently has no way to know whether this configuration option
is enabled and thus cannot sensibly choose whether to run hwclock itself or
not. Add a boot_timesource sysfs attribute which indicates whether a given
RTC set the system clock.

Signed-off-by: Matthew Garrett <[email protected]>
---
Documentation/rtc.txt | 2 ++
drivers/rtc/rtc-sysfs.c | 14 ++++++++++++++
2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/rtc.txt b/Documentation/rtc.txt
index 216bb8c..a086cad 100644
--- a/Documentation/rtc.txt
+++ b/Documentation/rtc.txt
@@ -142,6 +142,8 @@ The sysfs interface under /sys/class/rtc/rtcN provides access to various
rtc attributes without requiring the use of ioctls. All dates and times
are in the RTC's timezone, rather than in system time.

+boot_timesource: 1 if the RTC provided the system time at boot via the
+ CONFIG_RTC_HCTOSYS kernel option, 0 otherwise
date: RTC-provided date
max_user_freq: The maximum interrupt rate an unprivileged user may request
from this RTC.
diff --git a/drivers/rtc/rtc-sysfs.c b/drivers/rtc/rtc-sysfs.c
index 2531ce4..dbd6af8 100644
--- a/drivers/rtc/rtc-sysfs.c
+++ b/drivers/rtc/rtc-sysfs.c
@@ -102,6 +102,19 @@ rtc_sysfs_set_max_user_freq(struct device *dev, struct device_attribute *attr,
return n;
}

+static ssize_t
+rtc_sysfs_show_hctosys(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+#ifdef CONFIG_RTC_HCTOSYS_DEVICE
+ if (strcmp(dev_name(&to_rtc_device(dev)->dev),
+ CONFIG_RTC_HCTOSYS_DEVICE) == 0)
+ return sprintf(buf, "1\n");
+ else
+#endif
+ return sprintf(buf, "0\n");
+}
+
static struct device_attribute rtc_attrs[] = {
__ATTR(name, S_IRUGO, rtc_sysfs_show_name, NULL),
__ATTR(date, S_IRUGO, rtc_sysfs_show_date, NULL),
@@ -109,6 +122,7 @@ static struct device_attribute rtc_attrs[] = {
__ATTR(since_epoch, S_IRUGO, rtc_sysfs_show_since_epoch, NULL),
__ATTR(max_user_freq, S_IRUGO | S_IWUSR, rtc_sysfs_show_max_user_freq,
rtc_sysfs_set_max_user_freq),
+ __ATTR(boot_timesource, S_IRUGO, rtc_sysfs_show_hctosys, NULL),
{ },
};

--
1.6.4.1

2009-09-04 16:26:19

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 1/2] rtc: Document the sysfs interface

On Fri, 4 Sep 2009 12:26:07 -0400
Matthew Garrett <[email protected]> wrote:

> The sysfs interface to the RTC class drivers is currently undocumented.
> Add some basic documentation defining the semantics of the fields.
>
> Signed-off-by: Matthew Garrett <[email protected]>

thanks!

Acked-by: Alessandro Zummo <[email protected]>
--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-04 16:30:38

by Matthew Garrett

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, Sep 04, 2009 at 06:25:28PM +0200, Alessandro Zummo wrote:

> This feature is meant to be in addition of the regular
> hwclock call to read the system time.
>
> Have you tested if there are drawbacks by not reading it? Like
> TZ, DST and similar?

Current util-linux has a hwclock that can just set the timezone without
performing a full read. However, there's no way of knowing whether it's
safe to do this without knowing that the kernel config option was set
and used. We'd prefer to avoid always doing --hctosys because it's a
noticable performance hit on boot if the work's already been done by the
kernel.

--
Matthew Garrett | [email protected]

2009-09-04 16:35:21

by Mark Brown

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 1/2] rtc: Document the sysfs interface

On Fri, Sep 04, 2009 at 12:26:07PM -0400, Matthew Garrett wrote:

> The sysfs interface to the RTC class drivers is currently undocumented.
> Add some basic documentation defining the semantics of the fields.

> Signed-off-by: Matthew Garrett <[email protected]>

> +SYSFS INTERFACE
> +---------------

Wouldn't this be better placed under Documentation/ABI?

2009-09-04 16:38:29

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, 4 Sep 2009 17:30:29 +0100
Matthew Garrett <[email protected]> wrote:

> We'd prefer to avoid always doing --hctosys because it's a
> noticable performance hit on boot if the work's already been done by the
> kernel.

it shouldn't. maybe your hwclock is waiting for the tick
or something similar. I'd check it.

--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-04 16:39:26

by Matthew Garrett

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 1/2] rtc: Document the sysfs interface

On Fri, Sep 04, 2009 at 05:35:21PM +0100, Mark Brown wrote:
> On Fri, Sep 04, 2009 at 12:26:07PM -0400, Matthew Garrett wrote:
>
> > The sysfs interface to the RTC class drivers is currently undocumented.
> > Add some basic documentation defining the semantics of the fields.
>
> > Signed-off-by: Matthew Garrett <[email protected]>
>
> > +SYSFS INTERFACE
> > +---------------
>
> Wouldn't this be better placed under Documentation/ABI?

Unsure. I'd assumed it made more sense in the existing docs.

--
Matthew Garrett | [email protected]

2009-09-04 16:41:36

by Matthew Garrett

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, Sep 04, 2009 at 06:38:28PM +0200, Alessandro Zummo wrote:
> On Fri, 4 Sep 2009 17:30:29 +0100
> Matthew Garrett <[email protected]> wrote:
>
> > We'd prefer to avoid always doing --hctosys because it's a
> > noticable performance hit on boot if the work's already been done by the
> > kernel.
>
> it shouldn't. maybe your hwclock is waiting for the tick
> or something similar. I'd check it.

I'm just going by what our userspace people are saying. They'd prefer
not to have to run --hctosys if the time's already been set -
https://bugzilla.redhat.com/show_bug.cgi?id=489494 has the discussion.

--
Matthew Garrett | [email protected]

2009-09-04 16:52:08

by Mark Brown

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 1/2] rtc: Document the sysfs interface

On Fri, Sep 04, 2009 at 05:39:18PM +0100, Matthew Garrett wrote:
> On Fri, Sep 04, 2009 at 05:35:21PM +0100, Mark Brown wrote:

> > Wouldn't this be better placed under Documentation/ABI?

> Unsure. I'd assumed it made more sense in the existing docs.

My understanding is that the sysfs ABI (and ABIs in general, I guess)
should be moving towards the more formal style of Documentation/ABI to
help manage userspace expectations - the separation of the documentation
applies to most other things in there as well.

2009-09-04 16:52:49

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, 4 Sep 2009 17:41:27 +0100
Matthew Garrett <[email protected]> wrote:

> > it shouldn't. maybe your hwclock is waiting for the tick
> > or something similar. I'd check it.
>
> I'm just going by what our userspace people are saying. They'd prefer
> not to have to run --hctosys if the time's already been set -
> https://bugzilla.redhat.com/show_bug.cgi?id=489494 has the discussion.
>

that's ok, but nonetheless there's a problem in hwclock
if it takes 3 secs to read the time.

--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-04 16:55:56

by Matthew Garrett

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, Sep 04, 2009 at 06:52:43PM +0200, Alessandro Zummo wrote:

> that's ok, but nonetheless there's a problem in hwclock
> if it takes 3 secs to read the time.

The 3-second case is with virtualised access to the cmos, but you're
right that it does sound excessive. Even so, it seems like a win to be
able to avoid running code that the kernel's already run

--
Matthew Garrett | [email protected]

2009-09-04 17:03:46

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, 4 Sep 2009 17:55:49 +0100
Matthew Garrett <[email protected]> wrote:

> On Fri, Sep 04, 2009 at 06:52:43PM +0200, Alessandro Zummo wrote:
>
> > that's ok, but nonetheless there's a problem in hwclock
> > if it takes 3 secs to read the time.
>
> The 3-second case is with virtualised access to the cmos, but you're
> right that it does sound excessive. Even so, it seems like a win to be
> able to avoid running code that the kernel's already run

ok. I find boot_timesource to be not intuitive. I'd
vote for hctosys or something like that.


--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-04 17:09:35

by Matthew Garrett

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

Ok. How's this?

commit 09895be2ef8528dc33ca8516ca4f5ccbec2ffb05
Author: Matthew Garrett <[email protected]>
Date: Fri Sep 4 12:23:02 2009 -0400

rtc: Add hctosys sysfs attribute

CONFIG_RTC_HCTOSYS allows the kernel to read the system time from the RTC
at boot and resume, avoiding the need for userspace to do so. Unfortunately
userspace currently has no way to know whether this configuration option
is enabled and thus cannot sensibly choose whether to run hwclock itself or
not. Add a hctosys sysfs attribute which indicates whether a given RTC set
the system clock.

Signed-off-by: Matthew Garrett <[email protected]>

diff --git a/Documentation/rtc.txt b/Documentation/rtc.txt
index 216bb8c..baac51f 100644
--- a/Documentation/rtc.txt
+++ b/Documentation/rtc.txt
@@ -143,6 +143,8 @@ rtc attributes without requiring the use of ioctls. All dates and times
are in the RTC's timezone, rather than in system time.

date: RTC-provided date
+hctosys: 1 if the RTC provided the system time at boot via the
+ CONFIG_RTC_HCTOSYS kernel option, 0 otherwise
max_user_freq: The maximum interrupt rate an unprivileged user may request
from this RTC.
name: The name of the RTC corresponding to this sysfs directory
diff --git a/drivers/rtc/rtc-sysfs.c b/drivers/rtc/rtc-sysfs.c
index 2531ce4..7dd23a6 100644
--- a/drivers/rtc/rtc-sysfs.c
+++ b/drivers/rtc/rtc-sysfs.c
@@ -102,6 +102,19 @@ rtc_sysfs_set_max_user_freq(struct device *dev, struct device_attribute *attr,
return n;
}

+static ssize_t
+rtc_sysfs_show_hctosys(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+#ifdef CONFIG_RTC_HCTOSYS_DEVICE
+ if (strcmp(dev_name(&to_rtc_device(dev)->dev),
+ CONFIG_RTC_HCTOSYS_DEVICE) == 0)
+ return sprintf(buf, "1\n");
+ else
+#endif
+ return sprintf(buf, "0\n");
+}
+
static struct device_attribute rtc_attrs[] = {
__ATTR(name, S_IRUGO, rtc_sysfs_show_name, NULL),
__ATTR(date, S_IRUGO, rtc_sysfs_show_date, NULL),
@@ -109,6 +122,7 @@ static struct device_attribute rtc_attrs[] = {
__ATTR(since_epoch, S_IRUGO, rtc_sysfs_show_since_epoch, NULL),
__ATTR(max_user_freq, S_IRUGO | S_IWUSR, rtc_sysfs_show_max_user_freq,
rtc_sysfs_set_max_user_freq),
+ __ATTR(hctosys, S_IRUGO, rtc_sysfs_show_hctosys, NULL),
{ },
};


--
Matthew Garrett | [email protected]

2009-09-04 23:23:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 1/2] rtc: Document the sysfs interface

On Fri, 4 Sep 2009 17:52:06 +0100
Mark Brown <[email protected]> wrote:

> On Fri, Sep 04, 2009 at 05:39:18PM +0100, Matthew Garrett wrote:
> > On Fri, Sep 04, 2009 at 05:35:21PM +0100, Mark Brown wrote:
>
> > > Wouldn't this be better placed under Documentation/ABI?
>
> > Unsure. I'd assumed it made more sense in the existing docs.
>
> My understanding is that the sysfs ABI (and ABIs in general, I guess)
> should be moving towards the more formal style of Documentation/ABI to
> help manage userspace expectations - the separation of the documentation
> applies to most other things in there as well.

Yes, I think you're right.

But I'm not going to pass on a documentation patch just because it's in
the wrong place ;)

2009-09-05 21:11:06

by Alessandro Zummo

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, 4 Sep 2009 18:09:28 +0100
Matthew Garrett <[email protected]> wrote:

> Ok. How's this?
>
> commit 09895be2ef8528dc33ca8516ca4f5ccbec2ffb05
> Author: Matthew Garrett <[email protected]>
> Date: Fri Sep 4 12:23:02 2009 -0400

perfect, ty.

Acked-by: Alessandro Zummo <[email protected]>

--

Best regards,

Alessandro Zummo,
Tower Technologies - Torino, Italy

http://www.towertech.it

2009-09-06 01:34:38

by Mark Brown

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, Sep 04, 2009 at 06:09:28PM +0100, Matthew Garrett wrote:

> +hctosys: 1 if the RTC provided the system time at boot via the
> + CONFIG_RTC_HCTOSYS kernel option, 0 otherwise

...

> +static ssize_t
> +rtc_sysfs_show_hctosys(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> +#ifdef CONFIG_RTC_HCTOSYS_DEVICE
> + if (strcmp(dev_name(&to_rtc_device(dev)->dev),
> + CONFIG_RTC_HCTOSYS_DEVICE) == 0)
> + return sprintf(buf, "1\n");
> + else
> +#endif

Strictly speaking this only reports if the RTC was configured to provide
the system time at boot, it does not check to see if it actually
succeeded in doing so. Obviously failures here are a relatively rare
occurrance but they can happen. I don't think it's worth changing the
code since it's unlikely userspace will be able to do any better than
the kernel did but it might be worth updating the documentation just in
case.

2009-09-14 22:59:34

by Karel Zak

[permalink] [raw]
Subject: Re: [rtc-linux] Re: [PATCH 2/2] rtc: Add boot_timesource sysfs attribute

On Fri, Sep 04, 2009 at 06:52:43PM +0200, Alessandro Zummo wrote:
>
> On Fri, 4 Sep 2009 17:41:27 +0100
> Matthew Garrett <[email protected]> wrote:
>
> > > it shouldn't. maybe your hwclock is waiting for the tick
> > > or something similar. I'd check it.
> >
> > I'm just going by what our userspace people are saying. They'd prefer
> > not to have to run --hctosys if the time's already been set -
> > https://bugzilla.redhat.com/show_bug.cgi?id=489494 has the discussion.
> >
>
> that's ok, but nonetheless there's a problem in hwclock
> if it takes 3 secs to read the time.

IMHO 3 seconds is nonsense. It's usually 0.5 - 1 second.

Anyway, the "hwclock --hctosys" in userspace is a mystical voodoo and
it's definitely better to move this thing to kernel.

Karel

--
Karel Zak <[email protected]>