linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: jgunthorpe@obsidianresearch.com (Jason Gunthorpe)
To: linux-arm-kernel@lists.infradead.org
Subject: On NTP, RTCs and accurately setting their time
Date: Thu, 21 Sep 2017 17:20:40 -0600	[thread overview]
Message-ID: <20170921232040.GA15130@obsidianresearch.com> (raw)
In-Reply-To: <20170921224541.GC20805@n2100.armlinux.org.uk>

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

  reply	other threads:[~2017-09-21 23:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-20 11:21 On NTP, RTCs and accurately setting their time Russell King - ARM Linux
2017-09-20 13:23 ` Russell King - ARM Linux
2017-09-20 16:22 ` Jason Gunthorpe
2017-09-20 16:51   ` Russell King - ARM Linux
2017-09-20 17:16     ` Jason Gunthorpe
2017-09-20 22:45     ` Jason Gunthorpe
2017-09-21  7:59       ` Russell King - ARM Linux
2017-09-21  9:32         ` Russell King - ARM Linux
2017-09-21 11:30           ` Russell King - ARM Linux
2017-09-21 22:05           ` Jason Gunthorpe
2017-09-21 22:45             ` Russell King - ARM Linux
2017-09-21 23:20               ` Jason Gunthorpe [this message]
2017-09-22  9:57                 ` Russell King - ARM Linux
2017-09-22 12:24                   ` Russell King - ARM Linux
2017-09-22 16:28                     ` Russell King - ARM Linux
2017-09-22 16:48                   ` Jason Gunthorpe
2017-09-22 17:20                     ` Russell King - ARM Linux
2017-09-22 18:24                       ` Jason Gunthorpe
2017-09-23 18:23                         ` Russell King - ARM Linux
2017-09-23 19:05                           ` Russell King - ARM Linux
2017-09-24  1:30                           ` Jason Gunthorpe
2017-09-21  9:46       ` Russell King - ARM Linux
2017-09-21 11:29       ` Russell King - ARM Linux
2017-09-21 12:28       ` Russell King - ARM Linux
2017-09-26 11:58         ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170921232040.GA15130@obsidianresearch.com \
    --to=jgunthorpe@obsidianresearch.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).