All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@armlinux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] rtc: Allow rtc drivers to specify the tv_nsec value for ntp
Date: Mon, 27 Nov 2017 17:52:54 +0000	[thread overview]
Message-ID: <20171127175254.GL31757@n2100.armlinux.org.uk> (raw)
In-Reply-To: <CALAqxLUBWd-mv_3iiFrXLf0ka685t0an89jjduHpT-2jjv5O2Q@mail.gmail.com>

On Mon, Nov 27, 2017 at 09:35:30AM -0800, John Stultz wrote:
> On Thu, Nov 23, 2017 at 1:54 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > Hi,
> >
> > On 13/10/2017 at 11:54:33 -0600, Jason Gunthorpe wrote:
> >> ntp is currently hardwired to try and call the rtc set when wall clock
> >> tv_nsec is 0.5 seconds. This historical behaviour works well with certain
> >> PC RTCs, but is not universal to all rtc hardware.
> >>
> >> Change how this works by introducing the driver specific concept of
> >> set_offset_nsec, the delay between current wall clock time and the target
> >> time to set (with a 0 tv_nsecs).
> >>
> >> For x86-style CMOS set_offset_nsec should be -0.5 s which causes the last
> >> second to be written 0.5 s after it has started.
> >>
> >> For compat with the old rtc_set_ntp_time, the value is defaulted to
> >> + 0.5 s, which causes the next second to be written 0.5s before it starts,
> >> as things were before this patch.
> >>
> >> Testing shows many non-x86 RTCs would like set_offset_nsec ~= 0,
> >> so ultimately each RTC driver should set the set_offset_nsec according
> >> to its needs, and non x86 architectures should stop using
> >> update_persistent_clock64 in order to access this feature.
> >> Future patches will revise the drivers as needed.
> >>
> >> Since CMOS and RTC now have very different handling they are split
> >> into two dedicated code paths, sharing the support code, and ifdefs
> >> are replaced with IS_ENABLED.
> >>
> >> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >> ---
> >>  drivers/rtc/class.c   |   3 +
> >>  drivers/rtc/systohc.c |  53 +++++++++++-----
> >>  include/linux/rtc.h   |  43 ++++++++++++-
> >>  kernel/time/ntp.c     | 166 ++++++++++++++++++++++++++++++++++----------------
> >>  4 files changed, 196 insertions(+), 69 deletions(-)
> >>
> >> Hello All,
> >>
> >> Here is the latest version of this patch that was circulating between
> >> RMK and myself. I have done a few more minor changed and been able to
> >> test it myself on x86 KVM and on the ARM system that motivated the
> >> original CONFIG_RTC_SYSTOHC patch.
> >>
> >> From all my testing, this patch does not change the existing behavior
> >> at all, but provides the base infrastructure to fix individual RTCs
> >> one at a time.
> >>
> >> There are a few followup patches that will set the set_offset_nsec
> >> value for various RTC drivers, and I still have to look at RMK's
> >> hrtimer patch, but those are all incremental on top of this.
> >>
> >
> > So I'm discovering that this patch made it upstream silently. While it
> > somewhat solves the issue for some RTCs, it is not a proper solution for
> > most.
> 
> Sorry. I thought the discussion had finished up and it looked ok, so I
> queued it up. Apologies.

I'm actually rather disappointed that Alexandre Belloni has only now
brought up his dis-satisfaction with the approach after all the effort
that Jason and myself have put in to it.  It's not like Alexandre was
not copied on the patches and discussion.

If Alexandre could not be bothered to bring up his concerns while the
discussion was on-going in September, and didn't bother raising them
in October, I'd say that Alexandre's opinion at this point doesn't
count for much - if it wasn't important to state at the time or for
a couple of months after, why does it become important to state after
the thing has been merged.

Maybe the idea here is basically to waste people's time letting them
develop a patch for an approach, and then object at the last minute
to that approach.  Hardly seems fair or even reasonable.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

  reply	other threads:[~2017-11-27 17:52 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-13 17:54 [PATCH] rtc: Allow rtc drivers to specify the tv_nsec value for ntp Jason Gunthorpe
2017-11-23  9:54 ` Alexandre Belloni
2017-11-23 11:23   ` Russell King - ARM Linux
2017-11-23 12:04     ` Alexandre Belloni
2017-11-23 12:04       ` Alexandre Belloni
2017-11-23 12:53       ` Russell King - ARM Linux
2017-11-23 12:53         ` Russell King - ARM Linux
2017-11-24  0:13         ` J William Piggott
2017-11-24  0:13           ` J William Piggott
2017-11-27 20:18         ` Alexandre Belloni
2017-11-27 20:18           ` Alexandre Belloni
2017-11-27 20:29           ` Jason Gunthorpe
2017-11-27 20:29             ` Jason Gunthorpe
2017-11-28 10:03           ` Russell King - ARM Linux
2017-11-28 10:03             ` Russell King - ARM Linux
2017-11-23 15:04   ` Jason Gunthorpe
2017-11-23 15:36     ` Alexandre Belloni
2017-11-23 16:10       ` Russell King - ARM Linux
2017-11-23 16:25         ` Russell King - ARM Linux
2017-11-27 18:48         ` Alexandre Belloni
2017-11-23 16:33       ` Jason Gunthorpe
2017-11-27 15:43     ` Russell King - ARM Linux
2017-11-27 17:43       ` Russell King - ARM Linux
2017-11-27 18:59         ` Jason Gunthorpe
2017-11-27 17:35   ` John Stultz
2017-11-27 17:52     ` Russell King - ARM Linux [this message]
2017-11-27 18:44       ` Alexandre Belloni
2017-11-27 18:53         ` Russell King - ARM Linux
2017-11-27 19:18           ` Russell King - ARM Linux
2017-11-27 19:31           ` Alexandre Belloni
2017-11-28 10:20             ` Russell King - ARM Linux
2017-11-30 19:39               ` Alexandre Belloni
2017-11-30 19:39               ` Alexandre Belloni
2017-11-30 19:43                 ` 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=20171127175254.GL31757@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.