diff mbox series

[3/8] rtc: cmos: Make rtc_cmos sync offset correct

Message ID 20201206220541.830517160@linutronix.de
State Changes Requested
Headers show
Series ntp/rtc: Fixes and cleanups | expand

Commit Message

Thomas Gleixner Dec. 6, 2020, 9:46 p.m. UTC
The offset for rtc_cmos must be -500ms to work correctly with the current
implementation of rtc_set_ntp_time() due to the following:

  tsched       twrite(t2.tv_sec - 1) 	 t2 (seconds increment)

twrite - tsched is the transport time for the write to hit the device,
which is negligible for this chip because it's accessed directly.

t2 - twrite = 500ms according to the datasheet.

But rtc_set_ntp_time() calculation of tsched is:

    tsched = t2 - 1sec - (t2 - twrite)

The default for the sync offset is 500ms which means that the write happens
at t2 - 1.5 seconds which is obviously off by a second for this device.

Make the offset -500ms so it works correct.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/rtc/rtc-cmos.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Jason Gunthorpe Dec. 7, 2020, 8:50 p.m. UTC | #1
On Sun, Dec 06, 2020 at 10:46:16PM +0100, Thomas Gleixner wrote:
> The offset for rtc_cmos must be -500ms to work correctly with the current
> implementation of rtc_set_ntp_time() due to the following:
> 
>   tsched       twrite(t2.tv_sec - 1) 	 t2 (seconds increment)
> 
> twrite - tsched is the transport time for the write to hit the device,
> which is negligible for this chip because it's accessed directly.
> 
> t2 - twrite = 500ms according to the datasheet.
> 
> But rtc_set_ntp_time() calculation of tsched is:
> 
>     tsched = t2 - 1sec - (t2 - twrite)
> 
> The default for the sync offset is 500ms which means that the write happens
> at t2 - 1.5 seconds which is obviously off by a second for this device.
> 
> Make the offset -500ms so it works correct.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/rtc/rtc-cmos.c |    3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Matches what sync_cmos_clock() does at least.

My recollection is this change was not supposed to change anything, so
either I got things mixed up and this is wrong:

drivers/rtc/class.c:    rtc->set_offset_nsec =  NSEC_PER_SEC / 2;

Or it faithfully preserved some nonsense from before..

Jason
diff mbox series

Patch

--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -868,6 +868,9 @@  cmos_do_probe(struct device *dev, struct
 	if (retval)
 		goto cleanup2;
 
+	/* Set the sync offset for the periodic 11min update correct */
+	cmos_rtc.rtc->set_offset_nsec = -(NSEC_PER_SEC / 2);
+
 	/* export at least the first block of NVRAM */
 	nvmem_cfg.size = address_space - NVRAM_OFFSET;
 	if (rtc_nvmem_register(cmos_rtc.rtc, &nvmem_cfg))