From: Keller, Jacob E <jacob.e.keller@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v3] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl
Date: Tue, 19 Apr 2016 23:37:03 +0000 [thread overview]
Message-ID: <1461109023.13228.9.camel@intel.com> (raw)
In-Reply-To: <20160419065257.GA4143@hobbes.lan20.walshnetwork.net>
On Tue, 2016-04-19 at 02:52 -0400, Brian Walsh wrote:
> On Fri, Apr 15, 2016 at 03:41:27PM -0700, Jacob Keller wrote:
> >
> > The e1000e_config_hwtstamp function was incorrectly resetting the
> > SYSTIM
> > registers every time the ioctl was being run. If you happened to be
> > running ptp4l and lost the PTP connect (removing cable, or blocking
> > the
> > UDP traffic for example), then ptp4l will eventually perform a
> > restart
> > which involves re-requesting timestamp settings. In e1000e this has
> > the
> > unfortunate and incorrect result of resetting SYSTIME to the kernel
> > time. Since kernel time is usually in UTC, and PTP time is in TAI,
> > this
> > results in the leap second being re-applied.
> >
> > Fix this by extracting the SYSTIME reset out into its own function,
> > e1000e_ptp_reset, which we call during reset to restore the
> > hardware
> > registers. This function will (a) restart the timecounter based on
> > the
> > new system time, (b) restore the previous PPB setting, and (c)
> > restore
> > the previous hwtstamp settings.
> >
> > In order to perform (b), I had to modify the adjfreq ptp function
> > pointer to store the old delta each time it is called. This also
> > has the
> > side effect of restoring the correct base timinca register
> > correctly.
> > The driver does not need to explicitly zero the ptp_delta variable
> > since
> > the entire adapter structure comes zero-initialized.
> >
> > Reported-by: Brian Walsh <brian@walsh.ws>
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >
> > 3rd time's a charm? This time I verified it should compile and run
> > now,
> > without crashes, and without compilation errors.
> >
> > Notes:
> > ????This patch conflicts with Brian's "e1000e fix ptp time reset on
> > newtork
> > ????interruption", and that patch MUST be removed from the queue
> > before applying
> > ????this one.
> > ????
> > ????- Changes since v1
> > ????* move e1000e_init_ptp prior to reset so that PTP clock is
> > created first
> > ????* handle NULL ptp_clock_info gracefully.
> > ????
> > ????- Changes since v2
> > ????* fix stupid compilation error
> > ????* fix stupid null pointer check (who WROTE THIS CODE?!?!??!! ;)
> >
> > ?drivers/net/ethernet/intel/e1000e/e1000.h??|??1 +
> > ?drivers/net/ethernet/intel/e1000e/netdev.c | 65
> > ++++++++++++++++++++++--------
> > ?drivers/net/ethernet/intel/e1000e/ptp.c????|??2 +
> > ?3 files changed, 52 insertions(+), 16 deletions(-)
> >
> Fixes the leapsecond time jump I was trying to fix.
>
> Tested-by: Brian Walsh <brian@walsh.ws>
Thanks for testing this Brian, and sorry for the version thrash. I'm
glad it resolves the issue for you.
Regards,
Jake
next prev parent reply other threads:[~2016-04-19 23:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-15 22:41 [Intel-wired-lan] [PATCH v3] e1000e: don't modify SYSTIM registers during SIOCSHWTSTAMP ioctl Jacob Keller
2016-04-19 6:52 ` Brian Walsh
2016-04-19 23:37 ` Keller, Jacob E [this message]
2016-04-20 17:56 ` Keller, Jacob E
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=1461109023.13228.9.camel@intel.com \
--to=jacob.e.keller@intel.com \
--cc=intel-wired-lan@osuosl.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