Re: [PATCH v4 1/3] rtc: Add functions to set and read rtc offset

[Joshua Clayton <stillcompiling@gmail.com>]

On Fri,  5 Feb 2016 12:41:11 -0800
Joshua Clayton <stillcompiling@gmail.com> wrote:

Alexandre,
in case you didn't see, I made the change you requested.
The rtc_read_offset function checks for a read_offset function
and returns -EINVAL if not there, instead of an offset of 0.
I just didn't put together a cover letter this time around.
 
> A number of rtc devices, such as the NXP pcf2123 include a facility
> to adjust the clock in order to compensate for temperature or a
> crystal, capacitor, etc, that results in the rtc clock not running
> at exactly 32.768 kHz.
> 
> Data sheets I have seen refer to this as a clock offset, and measure
> it in parts per million, however they often reference ppm to 2 digits
> of precision, which makes integer ppm less than ideal.
> 
> We use parts per billion, which more than covers the precision needed
> and works nicely within 32 bits
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
>  drivers/rtc/interface.c | 54
> +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/rtc.h     |  4 ++++ 2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 5836751..9ef5f6f 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -939,4 +939,58 @@ void
> rtc_t4e40602412d61b7ef4162621a1a15be32126baeeimer_cancel(struct
> rtc_device *rtc, struct rtc_timer *timer)
> mutex_unlock(&rtc->ops_lock); } 
> +/**
> + * rtc_read_offset - Read the amount of rtc offset in parts per
> billion
> + * @ rtc: rtc device to be used
> + * @ offset: the offset in parts per billion
> + *
> + * see below for details.
> + *
> + * Kernel interface to read rtc clock offset
> + * Returns 0 on success, or a negative number on error.
> + * If read_offset() is not implemented for the rtc, return -EINVAL
> + */
> +int rtc_read_offset(struct rtc_device *rtc, long *offset)
> +{
> +	int ret;
> +
> +	if (!rtc->ops)
> +		return -ENODEV;
> +
> +	if (!rtc->ops->read_offset)
> +		return -EINVAL;
> +
> +	mutex_lock(&rtc->ops_lock);
> +	ret = rtc->ops->read_offset(rtc->dev.parent, offset);
> +	mutex_unlock(&rtc->ops_lock);
> +	return ret;
> +}
>  
> +/**
> + * rtc_set_offset - Adjusts the duration of the average second
> + * @ rtc: rtc device to be used
> + * @ offset: the offset in parts per billion
> + *
> + * Some rtc's allow an adjustment to the average duration of a second
> + * to compensate for differences in the actual clock rate due to
> temperature,
> + * the crystal, capacitor, etc.
> + *
> + * Kernel interface to adjust an rtc clock offset.
> + * Return 0 on success, or a negative number on error.
> + * If the rtc offset is not setable (or not implemented), return
> -EINVAL
> + */
> +int rtc_set_offset(struct rtc_device *rtc, long offset)
> +{
> +	int ret;
> +
> +	if (!rtc->ops)
> +		return -ENODEV;
> +
> +	if (!rtc->ops->set_offset)
> +		return -EINVAL;
> +
> +	mutex_lock(&rtc->ops_lock);
> +	ret = rtc->ops->set_offset(rtc->dev.parent, offset);
> +	mutex_unlock(&rtc->ops_lock);
> +	return ret;
> +}
> diff --git a/include/linux/rtc.h b/include/linux/rtc.h
> index 3359f04..b693ada 100644
> --- a/include/linux/rtc.h
> +++ b/include/linux/rtc.h
> @@ -89,6 +89,8 @@ struct rtc_class_ops {
>  	int (*set_mmss)(struct device *, unsigned long secs);
>  	int (*read_callback)(struct device *, int data);
>  	int (*alarm_irq_enable)(struct device *, unsigned int
> enabled);
> +	int (*read_offset)(struct device *, long *offset);
> +	int (*set_offset)(struct device *, long offset);
>  };
>  
>  #define RTC_DEVICE_NAME_SIZE 20
> @@ -208,6 +210,8 @@ 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);
>  void rtc_timer_cancel(struct rtc_device *rtc, struct rtc_timer
> *timer); +int rtc_read_offset(struct rtc_device *rtc, long *offset);
> +int rtc_set_offset(struct rtc_device *rtc, long offset);
>  void rtc_timer_do_work(struct work_struct *work);
>  
>  static inline bool is_leap_year(unsigned int year)

Thanks,

Joshua