netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Cochran <richardcochran@gmail.com>
To: "Keller, Jacob E" <jacob.e.keller@intel.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"Allan, Bruce W" <bruce.w.allan@intel.com>,
	"Ronciak, John" <john.ronciak@intel.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>,
	"Vick, Matthew" <matthew.vick@intel.com>
Subject: Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.
Date: Thu, 20 Nov 2014 10:18:04 +0100	[thread overview]
Message-ID: <20141120091804.GA4587@localhost.localdomain> (raw)
In-Reply-To: <1416431178.15933.38.camel@jekeller-desk1.amr.corp.intel.com>

On Wed, Nov 19, 2014 at 09:06:19PM +0000, Keller, Jacob E wrote:
> On Wed, 2014-11-19 at 21:26 +0100, Richard Cochran wrote:
> > On Wed, Nov 19, 2014 at 07:32:33PM +0000, Keller, Jacob E wrote:
> > > Good catch :)

I have not been able to reproduce the crash, and so the cause is not
what I thought it was. Maybe it was my patch that preserved the
enabled interrupts in igb_ptp_reset(). I didn't notice that the driver
frees and reallocates the ptp_clock. I would never do that, myself.

> I think you need something here, but it should be clearing that register
> after a MAC reset, so it needs to be re-initialized. I'm not sure if
> that reset path was used in the same place in the past.

Okay, lets figure this out. Why is there a PTP reset function at all?
I don't know, lets see who calls it...

  Finding functions calling: igb_ptp_reset
  ----------------------------------------
  *** drivers/net/ethernet/intel/igb/igb_main.c:
  igb_reset[2033]                igb_ptp_reset(adapter);

Easy enough to understand. But who is calling igb_reset?

  Finding functions calling: igb_reset
  ------------------------------------
  *** drivers/net/ethernet/intel/igb/igb_ethtool.c:
  igb_set_settings[345]          igb_reset(adapter);
  igb_set_pauseparam[409]        igb_reset(adapter);
  igb_diag_test[2016]            igb_reset(adapter);
  igb_set_eee[2729]              igb_reset(adapter);
  *** drivers/net/ethernet/intel/igb/igb_main.c:
  igb_down[1814]                 igb_reset(adapter);
  igb_set_features[2069]         igb_reset(adapter);
  igb_probe[2526]                igb_reset(adapter);
  __igb_open[3110]               igb_reset(adapter);
  igb_watchdog_task[4231]        igb_reset(adapter);
  igb_change_mtu[5189]           igb_reset(adapter);
  igb_resume[7460]               igb_reset(adapter);
  igb_sriov_reinit[7545]         igb_reset(adapter);
  igb_io_slot_reset[7678]        igb_reset(adapter);

Wow, that is quite much. So, whenever any random parameter is changed,
we reset the PTP clock. Great.

Really, wouldn't better to reset the clock functions only when
absolutely necessary?

Thanks,
Richard

  reply	other threads:[~2014-11-20  9:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 23:06 [PATCH net-next 0/4] igb: auxiliary PHC functions for the i210 Richard Cochran
2014-11-17 23:06 ` [PATCH net-next 1/4] igb: refactor time sync interrupt handling Richard Cochran
2014-11-17 23:06 ` [PATCH net-next 2/4] igb: do not clobber the TSAUXC bits on reset Richard Cochran
2014-11-17 23:22   ` Keller, Jacob E
2014-11-17 23:06 ` [PATCH net-next 3/4] igb: enable internal PPS for the i210 Richard Cochran
2014-11-17 23:24   ` Keller, Jacob E
2014-11-19  6:37   ` Richard Cochran
2014-11-19 19:32     ` Keller, Jacob E
2014-11-19 20:26       ` Richard Cochran
2014-11-19 21:06         ` Keller, Jacob E
2014-11-20  9:18           ` Richard Cochran [this message]
2014-11-20 21:28             ` Keller, Jacob E
2014-11-21  5:49               ` Richard Cochran
2014-11-17 23:06 ` [PATCH net-next 4/4] igb: enable auxiliary PHC functions " Richard Cochran
2014-11-17 23:28   ` Keller, Jacob E
2014-11-17 23:15 ` [PATCH net-next 0/4] igb: " Jeff Kirsher
2014-11-19  6:31   ` Richard Cochran
2014-11-19  8:43     ` Jeff Kirsher

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=20141120091804.GA4587@localhost.localdomain \
    --to=richardcochran@gmail.com \
    --cc=bruce.w.allan@intel.com \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=john.ronciak@intel.com \
    --cc=matthew.vick@intel.com \
    --cc=netdev@vger.kernel.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).