netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: davem@davemloft.net
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com, Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Subject: [net-next 05/17] i40e: don't enable PTP support on more than one PF per port
Date: Fri, 21 Nov 2014 21:54:56 -0800	[thread overview]
Message-ID: <1416635708-4765-7-git-send-email-jeffrey.t.kirsher@intel.com> (raw)
In-Reply-To: <1416635708-4765-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

Resolve an issue related to images with multiple PFs per physical
port. We cannot fully support 1588 PTP features, since only one port
should control (ie: write) the registers at a time. Doing so can cause
interference of functionality.

It may be possible to partially implement the API for only those
features without side effects. However, this at minimum means non
controlling PFs lose Tx timestamps, frequency atunement, and possibly
SYSTIME adjustment. There may be further impact I did not discover.
Since the API in the kernel expects these features to work, it is
simpler and less dangerous to just disable PTP features on all PFs not
identified as the controlling PF in PRTTSYN_CTL0.PF_ID.

This change also removes the warning printed when hwtstaml IOCTL is
called on the wrong PF. This is actually meaningless now, since only one
PF per port will support it. In addition, the ethtool get_ts_info IOCTL
was updated so that only the controlling port will even indicate support
(so as not to confuse users).

The overall downside is complete loss of functionality on non
controlling PF, vs the possible gain of partial support. The biggest
factor for choosing this approach is simplicity and ensuring that the
main PF will work. There could easily be other portions of the 1588
logic with side effects I am not aware, and the reduced functionality
that might be made available is significantly less useful. In addition,
the API does not allow for proper indication of why particular features
are not supported. These reasons are enough to decide for the simpler
approach to resolving this issue.

Change-ID: If4696bae686fc18aef6552b67dd417213d987c16
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  4 ++++
 drivers/net/ethernet/intel/i40e/i40e_ptp.c     | 32 +++++++++++++++++---------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index b2402851..4053b34 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1325,6 +1325,10 @@ static int i40e_get_ts_info(struct net_device *dev,
 {
 	struct i40e_pf *pf = i40e_netdev_to_pf(dev);
 
+	/* only report HW timestamping if PTP is enabled */
+	if (!(pf->flags & I40E_FLAG_PTP))
+		return ethtool_op_get_ts_info(dev, info);
+
 	info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
 				SOF_TIMESTAMPING_RX_SOFTWARE |
 				SOF_TIMESTAMPING_SOFTWARE |
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index f9151037..6d1ec92 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -424,6 +424,9 @@ int i40e_ptp_get_ts_config(struct i40e_pf *pf, struct ifreq *ifr)
 {
 	struct hwtstamp_config *config = &pf->tstamp_config;
 
+	if (!(pf->flags & I40E_FLAG_PTP))
+		return -EOPNOTSUPP;
+
 	return copy_to_user(ifr->ifr_data, config, sizeof(*config)) ?
 		-EFAULT : 0;
 }
@@ -444,22 +447,12 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
 				       struct hwtstamp_config *config)
 {
 	struct i40e_hw *hw = &pf->hw;
-	u32 pf_id, tsyntype, regval;
+	u32 tsyntype, regval;
 
 	/* Reserved for future extensions. */
 	if (config->flags)
 		return -EINVAL;
 
-	/* Confirm that 1588 is supported on this PF. */
-	pf_id = (rd32(hw, I40E_PRTTSYN_CTL0) & I40E_PRTTSYN_CTL0_PF_ID_MASK) >>
-		I40E_PRTTSYN_CTL0_PF_ID_SHIFT;
-	if (hw->pf_id != pf_id) {
-		dev_err(&pf->pdev->dev,
-			"PF %d attempted to control timestamp mode on port %d, which is owned by PF %d\n",
-			hw->pf_id, hw->port, pf_id);
-		return -EPERM;
-	}
-
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
 		pf->ptp_tx = false;
@@ -562,6 +555,9 @@ int i40e_ptp_set_ts_config(struct i40e_pf *pf, struct ifreq *ifr)
 	struct hwtstamp_config config;
 	int err;
 
+	if (!(pf->flags & I40E_FLAG_PTP))
+		return -EOPNOTSUPP;
+
 	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
 		return -EFAULT;
 
@@ -631,8 +627,22 @@ void i40e_ptp_init(struct i40e_pf *pf)
 {
 	struct net_device *netdev = pf->vsi[pf->lan_vsi]->netdev;
 	struct i40e_hw *hw = &pf->hw;
+	u32 pf_id;
 	long err;
 
+	/* Only one PF is assigned to control 1588 logic per port. Do not
+	 * enable any support for PFs not assigned via PRTTSYN_CTL0.PF_ID
+	 */
+	pf_id = (rd32(hw, I40E_PRTTSYN_CTL0) & I40E_PRTTSYN_CTL0_PF_ID_MASK) >>
+		I40E_PRTTSYN_CTL0_PF_ID_SHIFT;
+	if (hw->pf_id != pf_id) {
+		pf->flags &= ~I40E_FLAG_PTP;
+		dev_info(&pf->pdev->dev, "%s: PTP not supported on %s\n",
+			 __func__,
+			 netdev->name);
+		return;
+	}
+
 	/* we have to initialize the lock first, since we can't control
 	 * when the user will enter the PHC device entry points
 	 */
-- 
1.9.3

  parent reply	other threads:[~2014-11-22  5:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-22  5:54 [net-next 00/17][pull request] Intel Wired LAN Driver Updates 2014-11-21 Jeff Kirsher
2014-11-22  5:54 ` [net-next 01/17] i40e: Remove unneeded break statement Jeff Kirsher
2014-11-22  5:58   ` Jeff Kirsher
2014-11-22  5:54 ` Jeff Kirsher
2014-11-22  5:54 ` [net-next 02/17] i40e: remove useless debug noise Jeff Kirsher
2014-11-22  5:54 ` [net-next 03/17] i40e: allow various base numbers in debugfs aq commands Jeff Kirsher
2014-11-24 16:50   ` David Laight
2014-11-24 17:26     ` Nelson, Shannon
2014-11-22  5:54 ` [net-next 04/17] i40e: Add description to misc and fd interrupts Jeff Kirsher
2014-11-22  5:54 ` Jeff Kirsher [this message]
2014-11-22  5:54 ` [net-next 06/17] i40e: Add a virtual channel op to config RSS Jeff Kirsher
2014-11-22  5:54 ` [net-next 07/17] i40e: Define and use i40e_is_vf macro Jeff Kirsher
2014-11-22  5:54 ` [net-next 08/17] i40e: fix netdev_stat macro definition Jeff Kirsher
2014-11-22  5:55 ` [net-next 09/17] i40evf: make early init sequence even more robust Jeff Kirsher
2014-11-22  5:55 ` [net-next 10/17] i40e: Increase reset delay Jeff Kirsher
2014-11-22  5:55 ` [net-next 11/17] i40e: Add new update VSI flow to accommodate FW fix with VSI Loopback mode Jeff Kirsher
2014-11-22  5:55 ` [net-next 12/17] i40e: Re enable Main VSI loopback setting in the reset path Jeff Kirsher
2014-11-22  5:55 ` [net-next 13/17] i40evf: refactor ethtool RSS handling Jeff Kirsher
2014-11-23  3:44   ` Ben Hutchings
2014-11-22  5:55 ` [net-next 14/17] i40e: implement ethtool RSS config Jeff Kirsher
2014-11-22 18:22   ` Eric Dumazet
2014-11-23  3:52     ` Ben Hutchings
2014-11-23  3:53   ` Ben Hutchings
2014-11-22  5:55 ` [net-next 15/17] i40e: increase ARQ size Jeff Kirsher
2014-11-22  5:55 ` [net-next 16/17] i40e: get pf_id from HW rather than PCI function Jeff Kirsher
2014-11-22  5:55 ` [net-next 17/17] i40e: Bump i40e version to 1.2.2 and i40evf version to 1.0.6 Jeff Kirsher
2014-11-23 19:22 ` [net-next 00/17][pull request] Intel Wired LAN Driver Updates 2014-11-21 David Miller

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=1416635708-4765-7-git-send-email-jeffrey.t.kirsher@intel.com \
    --to=jeffrey.t.kirsher@intel.com \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jogreene@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@redhat.com \
    --cc=sassmann@redhat.com \
    /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).