Re: [PATCH v2 7/8] rtc: implement a sysfs interface for clock offset

[Alexandre Belloni]

Hi,

On 04/01/2016 at 10:31:25 -0800, Joshua Clayton wrote :
> The file is called "offset", and may be set and read in decimal.
> For rtcs that do not have read_offset or set_offset implemented,
> read always returns zero and write will return -EINVAL.
> 
Can you expand rtc_attr_is_visible() instead of having the offset file
always present?

Also, you need to expand Documentation/rtc.txt, section SYSFS INTERFACE.

I'm fine with having parts per billion as the unit and I hope we won't
ever need anything more precise :)

> Signed-off-by: Joshua Clayton <stillcompiling@xxxxxxxxx>
> ---
>  drivers/rtc/interface.c |  2 +-
>  drivers/rtc/rtc-sysfs.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 57cd928..c064eb9 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -959,7 +959,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
>  		return -ENODEV;
>  
>  	if (!rtc->ops->set_offset) {
> -		offset = 0;
> +		*offset = 0;
>  		return 0;
>  	}

>  
> diff --git a/drivers/rtc/rtc-sysfs.c b/drivers/rtc/rtc-sysfs.c
> index 7273855..622a0dc 100644
> --- a/drivers/rtc/rtc-sysfs.c
> +++ b/drivers/rtc/rtc-sysfs.c
> @@ -211,6 +211,34 @@ wakealarm_store(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RW(wakealarm);
>  
> +static ssize_t
> +offset_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	ssize_t retval;
> +	long offset;
> +
> +	retval = rtc_read_offset(to_rtc_device(dev), &offset);
> +	if (retval == 0)
> +		retval = sprintf(buf, "%ld\n", offset);
> +
> +	return retval;
> +}
> +
> +static ssize_t
> +offset_store(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t n)
> +{
> +	ssize_t retval;
> +	long offset;
> +
> +	retval = kstrtol(buf, 10, &offset);
> +	if (retval == 0)
> +		retval = rtc_set_offset(to_rtc_device(dev), offset);
> +
> +	return (retval < 0) ? retval : n;
> +}
> +static DEVICE_ATTR_RW(offset);
> +
>  static struct attribute *rtc_attrs[] = {
>  	&dev_attr_name.attr,
>  	&dev_attr_date.attr,
> @@ -219,6 +247,7 @@ static struct attribute *rtc_attrs[] = {
>  	&dev_attr_max_user_freq.attr,
>  	&dev_attr_hctosys.attr,
>  	&dev_attr_wakealarm.attr,
> +	&dev_attr_offset.attr,
>  	NULL,
>  };
>  

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com