On NTP, RTCs and accurately setting their time

[Jason Gunthorpe]

On Thu, Sep 21, 2017 at 11:45:41PM +0100, Russell King - ARM Linux wrote:
> > I've combined several of your messages together into this reply...
> 
> Sorry about that, I'm currently ill and sleep deprived, so it's been
> a struggle to pick up on everything together.

No worries, your replys were easy to follow, thanks

> > So with v2 of this patch, the driver would specify positive 1s, and
> > then the set will be called when tv_nsec == 0, but the tv_sec will
> > be +1.
> > 
> > Similarly the existing case of 0.5s offset will work more like
> > before, where it will be called with tv_nsec == .5s and the tv_sec
> > will be +1 - before I think this happened reliably 'by accident' due
> > to the rounding.
> 
> It seems to mean (from reading the code) that I'd need to specify
> -470ms for the PCF8523, if I'm understand what you're saying and
> the code correctly.

Okay.. but I guessed a negative number is probably not what most RTCs would
want, eg a positive number would compensate for delays executing an
I2C command, assuming the RTC zeros its divider when it sets the time.

> I should point out, however, that I'm running a modified version
> of your previous patch with 470ms specified - I added a debug
> printk() for the failure cases, and I seem to be getting quite a
> number:
> 
> [  416.966909] rtc_tv_nsec_ok: fail: ts=1506001533.510194975

> then that means we only waited 329s before we next tried to set the
> RTC, not the 11 minutes that we should have.  Not sure yet what's
> going on there, but it seems to suggest that we're struggling to
> set the RTC, taking multiple attempts to achieve it.

Ah..

> Also... don't we want to move the call to rtc_set_ntp_time() from out
> of the "if (abs(now.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec * 5) {"
> check, otherwise how could it be called except at about .5s?

Yes, definitely. If we do not call through to rtc_set_ntp_time then it
does not update target_nsec, and we go back to the 0.5 offset not the
offset you want, which will certainly cause random rtc_tv_nsec_ok
failures.

My bad to not notice that.. But I'm not sure what the revision should
be.. I think this is using tick_nsec similarly to TIME_SET_NSEC_FUZZ,
but I'm not sure where tick_nsec comes from or if we should push it
down in to TIME_SET_NSEC_FUZZ, or something else.

> I'm actually not sure if we are ending up setting the time - my test
> program reports:

Probably not, given the above!

Jason