From: Jarod Wilson <jarod@redhat.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH net] e1000e: fix PTP on e1000_pch_lpt variants
Date: Wed, 20 Jul 2016 13:05:33 -0400 [thread overview]
Message-ID: <20160720170533.GE37138@redhat.com> (raw)
In-Reply-To: <B2E38BF8-6BB1-4904-B79B-B94B9C360F15@intel.com>
On Tue, Jul 19, 2016 at 08:49:03PM +0000, Rustad, Mark D wrote:
> Jarod Wilson <jarod@redhat.com> wrote:
>
> >I've got reports that the Intel I-218V NIC in Intel NUC5i5RYH systems used
> >as a PTP slave experiences random ~10 hour clock jumps, which are resolved
> >if the same workaround for the 82574 and 82583 is employed. Switching from
> >an if to a select, because the list of NIC types could well grow further
> >and we'd already have to wrap the conditionals.
> >
> >CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >CC: intel-wired-lan at lists.osuosl.org
> >CC: netdev at vger.kernel.org
> >Signed-off-by: Jarod Wilson <jarod@redhat.com>
> >---
> > drivers/net/ethernet/intel/e1000e/netdev.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> >b/drivers/net/ethernet/intel/e1000e/netdev.c
> >index 2b2e2f8..866fea0 100644
> >--- a/drivers/net/ethernet/intel/e1000e/netdev.c
> >+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> >@@ -4335,7 +4335,10 @@ static cycle_t
> >e1000e_cyclecounter_read(const struct cyclecounter *cc)
> > systim = (cycle_t)systimel;
> > systim |= (cycle_t)systimeh << 32;
> >
> >- if ((hw->mac.type == e1000_82574) || (hw->mac.type == e1000_82583)) {
> >+ switch (hw->mac.type) {
> >+ case e1000_82574:
> >+ case e1000_82583:
> >+ case e1000_pch_lpt:
> > u64 time_delta, rem, temp;
> > u32 incvalue;
> > int i;
>
> I don't think that it is acceptable to declare local variables
> inside a switch statement quite like this. At a minimum, a new block
> needs to be opened to allow the declarations.
Gah, sorry, I think testing was done with an if, made a late change to a
switch without doing sufficient re-testing. I'll fix that up and re-test.
--
Jarod Wilson
jarod at redhat.com
WARNING: multiple messages have this Message-ID (diff)
From: Jarod Wilson <jarod@redhat.com>
To: "Rustad, Mark D" <mark.d.rustad@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH net] e1000e: fix PTP on e1000_pch_lpt variants
Date: Wed, 20 Jul 2016 13:05:33 -0400 [thread overview]
Message-ID: <20160720170533.GE37138@redhat.com> (raw)
In-Reply-To: <B2E38BF8-6BB1-4904-B79B-B94B9C360F15@intel.com>
On Tue, Jul 19, 2016 at 08:49:03PM +0000, Rustad, Mark D wrote:
> Jarod Wilson <jarod@redhat.com> wrote:
>
> >I've got reports that the Intel I-218V NIC in Intel NUC5i5RYH systems used
> >as a PTP slave experiences random ~10 hour clock jumps, which are resolved
> >if the same workaround for the 82574 and 82583 is employed. Switching from
> >an if to a select, because the list of NIC types could well grow further
> >and we'd already have to wrap the conditionals.
> >
> >CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >CC: intel-wired-lan@lists.osuosl.org
> >CC: netdev@vger.kernel.org
> >Signed-off-by: Jarod Wilson <jarod@redhat.com>
> >---
> > drivers/net/ethernet/intel/e1000e/netdev.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> >b/drivers/net/ethernet/intel/e1000e/netdev.c
> >index 2b2e2f8..866fea0 100644
> >--- a/drivers/net/ethernet/intel/e1000e/netdev.c
> >+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> >@@ -4335,7 +4335,10 @@ static cycle_t
> >e1000e_cyclecounter_read(const struct cyclecounter *cc)
> > systim = (cycle_t)systimel;
> > systim |= (cycle_t)systimeh << 32;
> >
> >- if ((hw->mac.type == e1000_82574) || (hw->mac.type == e1000_82583)) {
> >+ switch (hw->mac.type) {
> >+ case e1000_82574:
> >+ case e1000_82583:
> >+ case e1000_pch_lpt:
> > u64 time_delta, rem, temp;
> > u32 incvalue;
> > int i;
>
> I don't think that it is acceptable to declare local variables
> inside a switch statement quite like this. At a minimum, a new block
> needs to be opened to allow the declarations.
Gah, sorry, I think testing was done with an if, made a late change to a
switch without doing sufficient re-testing. I'll fix that up and re-test.
--
Jarod Wilson
jarod@redhat.com
next prev parent reply other threads:[~2016-07-20 17:05 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-19 20:25 [Intel-wired-lan] [PATCH net] e1000e: fix PTP on e1000_pch_lpt variants Jarod Wilson
2016-07-19 20:25 ` Jarod Wilson
2016-07-19 20:49 ` [Intel-wired-lan] " Rustad, Mark D
2016-07-19 20:49 ` Rustad, Mark D
2016-07-20 17:05 ` Jarod Wilson [this message]
2016-07-20 17:05 ` Jarod Wilson
2016-07-20 11:01 ` Sergei Shtylyov
2016-07-20 11:01 ` Sergei Shtylyov
2016-07-23 16:44 ` [Intel-wired-lan] [PATCH v2 net-next 0/2] e1000e: fix PTP on e1000_pch_variants Jarod Wilson
2016-07-23 16:44 ` Jarod Wilson
2016-07-23 16:44 ` [Intel-wired-lan] [PATCH net-next 1/2] e1000e: factor out systim sanitization Jarod Wilson
2016-07-23 16:44 ` Jarod Wilson
2016-07-23 16:44 ` [Intel-wired-lan] [PATCH net-next 2/2] e1000e: fix PTP on e1000_pch_lpt variants Jarod Wilson
2016-07-23 16:44 ` Jarod Wilson
2016-07-25 17:56 ` [Intel-wired-lan] [PATCH v2 net-next 0/2] e1000e: fix PTP on e1000_pch_variants Jesse Brandeburg
2016-07-25 17:56 ` Jesse Brandeburg
2016-07-26 17:39 ` [Intel-wired-lan] [PATCH net-next 3/2] e1000e: convert systim overflow check to use flags2 Jarod Wilson
2016-07-26 17:39 ` Jarod Wilson
2016-07-26 17:53 ` [Intel-wired-lan] " Jeff Kirsher
2016-07-26 17:53 ` Jeff Kirsher
2016-07-26 17:58 ` [Intel-wired-lan] " Jarod Wilson
2016-07-26 17:58 ` Jarod Wilson
2016-07-26 18:25 ` [Intel-wired-lan] [PATCH v3 net-next 0/2] e1000e: fix PTP on e1000_pch_variants Jarod Wilson
2016-07-26 18:25 ` Jarod Wilson
2016-07-26 18:25 ` [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out systim sanitization Jarod Wilson
2016-07-26 18:25 ` Jarod Wilson
2016-07-27 14:09 ` [Intel-wired-lan] " Avargil, Raanan
2016-07-27 14:09 ` Avargil, Raanan
2016-07-27 15:01 ` Jarod Wilson
2016-07-27 15:01 ` Jarod Wilson
2016-08-02 1:32 ` Jarod Wilson
2016-08-02 1:32 ` Jarod Wilson
2016-08-02 7:33 ` Brown, Aaron F
2016-08-02 7:33 ` Brown, Aaron F
2016-08-11 15:48 ` Jarod Wilson
2016-08-11 15:48 ` Jarod Wilson
2016-07-27 21:15 ` Woodford, Timothy W.
2016-07-27 21:15 ` Woodford, Timothy W.
2016-07-29 14:40 ` Woodford, Timothy W.
2016-07-29 14:40 ` Woodford, Timothy W.
2016-08-04 19:34 ` Brown, Aaron F
2016-08-04 19:34 ` Brown, Aaron F
2016-08-04 19:30 ` Brown, Aaron F
2016-08-04 19:30 ` Brown, Aaron F
2016-07-26 18:25 ` [Intel-wired-lan] [PATCH net-next v3 2/2] e1000e: fix PTP on e1000_pch_lpt variants Jarod Wilson
2016-07-26 18:25 ` Jarod Wilson
2016-08-04 19:31 ` [Intel-wired-lan] " Brown, Aaron F
2016-08-04 19:31 ` Brown, Aaron F
2016-07-24 20:30 ` [Intel-wired-lan] [PATCH net] " kbuild test robot
2016-07-24 20:30 ` kbuild test robot
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=20160720170533.GE37138@redhat.com \
--to=jarod@redhat.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 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.