[net-next 0/6][pull request] Intel Wired LAN Driver Updates

[net-next 0/6][pull request] Intel Wired LAN Driver Updates

[Jeff Kirsher]

This series contains updates to igb (specifically PTP code).

The following are changes since commit f6fe569fe056388166575af1cfaed0bcbc688305:
  Revert "usbnet: drop unneeded check for NULL"
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Matthew Vick (6):
  igb: Tidy up wrapping for CONFIG_IGB_PTP.
  igb: Update PTP function names/variables and locations.
  igb: Correct PTP support query from ethtool.
  igb: Store the MAC address in the name in the PTP struct.
  igb: Prevent dropped Tx timestamps via work items and interrupts.
  igb: Add 1588 support to I210/I211.

 drivers/net/ethernet/intel/igb/e1000_defines.h |   8 +
 drivers/net/ethernet/intel/igb/e1000_regs.h    |   2 +
 drivers/net/ethernet/intel/igb/igb.h           |  29 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c   |  84 +--
 drivers/net/ethernet/intel/igb/igb_main.c      | 329 +++---------
 drivers/net/ethernet/intel/igb/igb_ptp.c       | 676 ++++++++++++++++++++-----
 6 files changed, 708 insertions(+), 420 deletions(-)

-- 
1.7.11.4

[net-next 1/6] igb: Tidy up wrapping for CONFIG_IGB_PTP.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

For users without CONFIG_IGB_PTP=y, we should not be compiling any PTP
code into the driver. Tidy up the wrapping in igb to support this.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Jeff Pieper  <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  8 ++++++--
 drivers/net/ethernet/intel/igb/igb_ethtool.c |  4 ++--
 drivers/net/ethernet/intel/igb/igb_main.c    | 23 +++++++++++++++++------
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0c9f62c..a3b5b90 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -34,9 +34,11 @@
 #include "e1000_mac.h"
 #include "e1000_82575.h"
 
+#ifdef CONFIG_IGB_PTP
 #include <linux/clocksource.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
+#endif /* CONFIG_IGB_PTP */
 #include <linux/bitops.h>
 #include <linux/if_vlan.h>
 
@@ -376,12 +378,15 @@ struct igb_adapter {
 	int node;
 	u32 *shadow_vfta;
 
+#ifdef CONFIG_IGB_PTP
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info caps;
 	struct delayed_work overflow_work;
 	spinlock_t tmreg_lock;
 	struct cyclecounter cc;
 	struct timecounter tc;
+#endif /* CONFIG_IGB_PTP */
+
 	char fw_version[32];
 };
 
@@ -436,12 +441,11 @@ extern void igb_set_fw_version(struct igb_adapter *);
 #ifdef CONFIG_IGB_PTP
 extern void igb_ptp_init(struct igb_adapter *adapter);
 extern void igb_ptp_remove(struct igb_adapter *adapter);
-
 extern void igb_systim_to_hwtstamp(struct igb_adapter *adapter,
 				   struct skb_shared_hwtstamps *hwtstamps,
 				   u64 systim);
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 static inline s32 igb_reset_phy(struct e1000_hw *hw)
 {
 	if (hw->phy.ops.reset)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a294441..0c4e29a 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2338,8 +2338,8 @@ static int igb_ethtool_get_ts_info(struct net_device *dev,
 
 	return 0;
 }
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 static const struct ethtool_ops igb_ethtool_ops = {
 	.get_settings           = igb_get_settings,
 	.set_settings           = igb_set_settings,
@@ -2370,7 +2370,7 @@ static const struct ethtool_ops igb_ethtool_ops = {
 	.complete		= igb_ethtool_complete,
 #ifdef CONFIG_IGB_PTP
 	.get_ts_info		= igb_ethtool_get_ts_info,
-#endif
+#endif /* CONFIG_IGB_PTP */
 };
 
 void igb_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 73cc273..03477d7 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2180,11 +2180,12 @@ static int __devinit igb_probe(struct pci_dev *pdev,
 	}
 
 #endif
+
 #ifdef CONFIG_IGB_PTP
 	/* do hw tstamp init after resetting */
 	igb_ptp_init(adapter);
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 	dev_info(&pdev->dev, "Intel(R) Gigabit Ethernet Network Connection\n");
 	/* print bus type/speed/width info */
 	dev_info(&pdev->dev, "%s: (PCIe:%s:%s) %pM\n",
@@ -2260,8 +2261,8 @@ static void __devexit igb_remove(struct pci_dev *pdev)
 	pm_runtime_get_noresume(&pdev->dev);
 #ifdef CONFIG_IGB_PTP
 	igb_ptp_remove(adapter);
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 	/*
 	 * The watchdog timer may be rescheduled, so explicitly
 	 * disable watchdog from being rescheduled.
@@ -3184,8 +3185,10 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
 	srrctl |= (PAGE_SIZE / 2) >> E1000_SRRCTL_BSIZEPKT_SHIFT;
 #endif
 	srrctl |= E1000_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS;
+#ifdef CONFIG_IGB_PTP
 	if (hw->mac.type >= e1000_82580)
 		srrctl |= E1000_SRRCTL_TIMESTAMP;
+#endif /* CONFIG_IGB_PTP */
 	/* Only set Drop Enable if we are supporting multiple queues */
 	if (adapter->vfs_allocated_count || adapter->num_rx_queues > 1)
 		srrctl |= E1000_SRRCTL_DROP_EN;
@@ -4229,9 +4232,11 @@ static __le32 igb_tx_cmd_type(u32 tx_flags)
 	if (tx_flags & IGB_TX_FLAGS_VLAN)
 		cmd_type |= cpu_to_le32(E1000_ADVTXD_DCMD_VLE);
 
+#ifdef CONFIG_IGB_PTP
 	/* set timestamp bit if present */
 	if (tx_flags & IGB_TX_FLAGS_TSTAMP)
 		cmd_type |= cpu_to_le32(E1000_ADVTXD_MAC_TSTAMP);
+#endif /* CONFIG_IGB_PTP */
 
 	/* set segmentation bits for TSO */
 	if (tx_flags & IGB_TX_FLAGS_TSO)
@@ -4462,10 +4467,12 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 	first->bytecount = skb->len;
 	first->gso_segs = 1;
 
+#ifdef CONFIG_IGB_PTP
 	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 		tx_flags |= IGB_TX_FLAGS_TSTAMP;
 	}
+#endif /* CONFIG_IGB_PTP */
 
 	if (vlan_tx_tag_present(skb)) {
 		tx_flags |= IGB_TX_FLAGS_VLAN;
@@ -5772,8 +5779,8 @@ static void igb_tx_hwtstamp(struct igb_q_vector *q_vector,
 	igb_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
 	skb_tstamp_tx(buffer_info->skb, &shhwtstamps);
 }
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 /**
  * igb_clean_tx_irq - Reclaim resources after transmit completes
  * @q_vector: pointer to q_vector containing needed info
@@ -5821,8 +5828,8 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
 #ifdef CONFIG_IGB_PTP
 		/* retrieve hardware timestamp */
 		igb_tx_hwtstamp(q_vector, tx_buffer);
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 		/* free the skb */
 		dev_kfree_skb_any(tx_buffer->skb);
 		tx_buffer->skb = NULL;
@@ -6033,8 +6040,8 @@ static void igb_rx_hwtstamp(struct igb_q_vector *q_vector,
 
 	igb_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
 }
+#endif /* CONFIG_IGB_PTP */
 
-#endif
 static void igb_rx_vlan(struct igb_ring *ring,
 			union e1000_adv_rx_desc *rx_desc,
 			struct sk_buff *skb)
@@ -6147,7 +6154,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
 
 #ifdef CONFIG_IGB_PTP
 		igb_rx_hwtstamp(q_vector, rx_desc, skb);
-#endif
+#endif /* CONFIG_IGB_PTP */
 		igb_rx_hash(rx_ring, rx_desc, skb);
 		igb_rx_checksum(rx_ring, rx_desc, skb);
 		igb_rx_vlan(rx_ring, rx_desc, skb);
@@ -6340,6 +6347,7 @@ static int igb_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	return 0;
 }
 
+#ifdef CONFIG_IGB_PTP
 /**
  * igb_hwtstamp_ioctl - control hardware time stamping
  * @netdev:
@@ -6514,6 +6522,7 @@ static int igb_hwtstamp_ioctl(struct net_device *netdev,
 	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
 		-EFAULT : 0;
 }
+#endif /* CONFIG_IGB_PTP */
 
 /**
  * igb_ioctl -
@@ -6528,8 +6537,10 @@ static int igb_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
 		return igb_mii_ioctl(netdev, ifr, cmd);
+#ifdef CONFIG_IGB_PTP
 	case SIOCSHWTSTAMP:
 		return igb_hwtstamp_ioctl(netdev, ifr, cmd);
+#endif /* CONFIG_IGB_PTP */
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
1.7.11.4

Re: [net-next 1/6] igb: Tidy up wrapping for CONFIG_IGB_PTP.

[Richard Cochran]

On Wed, Sep 05, 2012 at 04:35:01PM -0700, Jeff Kirsher wrote:
> From: Matthew Vick <matthew.vick@intel.com>
> 
> For users without CONFIG_IGB_PTP=y, we should not be compiling any PTP
> code into the driver. Tidy up the wrapping in igb to support this.

So, you have ignored everything I wrote about time stamping
applications.

(no)Thanks,
Richard

Re: [net-next 1/6] igb: Tidy up wrapping for CONFIG_IGB_PTP.

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Thu, 2012-09-06 at 10:09 +0200, Richard Cochran wrote:
> On Wed, Sep 05, 2012 at 04:35:01PM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> > 
> > For users without CONFIG_IGB_PTP=y, we should not be compiling any
> PTP
> > code into the driver. Tidy up the wrapping in igb to support this.
> 
> So, you have ignored everything I wrote about time stamping
> applications.
> 
> (no)Thanks,
> Richard 

As I recall, he stated he was going to integrate your suggestions in a
new patch set that would follow this one.

In regards to your previous comments about the earlier patch I submitted
to wrap the code.  It was to fix a bug when the driver and time stamping
support are both modules.

Also, IMHO, while I look at pre-processor tags all day and get used to
matching up #ifdef to #endif's, I still see #endif /* CONFIG_IGB_PTP */
useful.  While it is not currently consistent, I see it more useful to
fix as we go, rather than create a single patch to add a comment for
every #endif.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [net-next 1/6] igb: Tidy up wrapping for CONFIG_IGB_PTP.

[Richard Cochran]

On Thu, Sep 06, 2012 at 01:44:27AM -0700, Jeff Kirsher wrote:
> 
> As I recall, he stated he was going to integrate your suggestions in a
> new patch set that would follow this one.

Okay, but won't that mean moving the time stamping functions back, and
removing the #ifdefs again?

> Also, IMHO, while I look at pre-processor tags all day and get used to
> matching up #ifdef to #endif's, I still see #endif /* CONFIG_IGB_PTP */
> useful.  While it is not currently consistent, I see it more useful to
> fix as we go, rather than create a single patch to add a comment for
> every #endif.

Okay, fine.

Thanks,
Richard

Re: [net-next 1/6] igb: Tidy up wrapping for CONFIG_IGB_PTP.

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

On Thu, 2012-09-06 at 10:49 +0200, Richard Cochran wrote:
> On Thu, Sep 06, 2012 at 01:44:27AM -0700, Jeff Kirsher wrote:
> > 
> > As I recall, he stated he was going to integrate your suggestions in a
> > new patch set that would follow this one.
> 
> Okay, but won't that mean moving the time stamping functions back, and
> removing the #ifdefs again?

That maybe, I do not know what Matthew has in mind to integrate your
suggestions.

> > Also, IMHO, while I look at pre-processor tags all day and get used to
> > matching up #ifdef to #endif's, I still see #endif /* CONFIG_IGB_PTP */
> > useful.  While it is not currently consistent, I see it more useful to
> > fix as we go, rather than create a single patch to add a comment for
> > every #endif.
> 
> Okay, fine.
> 
> Thanks,
> Richard



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[net-next 2/6] igb: Update PTP function names/variables and locations.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Where possible, move PTP-related functions into igb_ptp.c and update the
names of functions and variables to match the established coding style
in the files and specify that they are PTP-specific.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Jeff Pieper  <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  17 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c |  34 +-
 drivers/net/ethernet/intel/igb/igb_main.c    | 257 +-------------
 drivers/net/ethernet/intel/igb/igb_ptp.c     | 485 ++++++++++++++++++++-------
 4 files changed, 398 insertions(+), 395 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index a3b5b90..7973469 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -344,7 +344,6 @@ struct igb_adapter {
 
 	/* OS defined structs */
 	struct pci_dev *pdev;
-	struct hwtstamp_config hwtstamp_config;
 
 	spinlock_t stats64_lock;
 	struct rtnl_link_stats64 stats64;
@@ -380,8 +379,8 @@ struct igb_adapter {
 
 #ifdef CONFIG_IGB_PTP
 	struct ptp_clock *ptp_clock;
-	struct ptp_clock_info caps;
-	struct delayed_work overflow_work;
+	struct ptp_clock_info ptp_caps;
+	struct delayed_work ptp_overflow_work;
 	spinlock_t tmreg_lock;
 	struct cyclecounter cc;
 	struct timecounter tc;
@@ -440,10 +439,14 @@ extern void igb_power_up_link(struct igb_adapter *);
 extern void igb_set_fw_version(struct igb_adapter *);
 #ifdef CONFIG_IGB_PTP
 extern void igb_ptp_init(struct igb_adapter *adapter);
-extern void igb_ptp_remove(struct igb_adapter *adapter);
-extern void igb_systim_to_hwtstamp(struct igb_adapter *adapter,
-				   struct skb_shared_hwtstamps *hwtstamps,
-				   u64 systim);
+extern void igb_ptp_stop(struct igb_adapter *adapter);
+extern void igb_ptp_tx_hwtstamp(struct igb_q_vector *q_vector,
+				struct igb_tx_buffer *buffer_info);
+extern void igb_ptp_rx_hwtstamp(struct igb_q_vector *q_vector,
+				union e1000_adv_rx_desc *rx_desc,
+				struct sk_buff *skb);
+extern int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
+				  struct ifreq *ifr, int cmd);
 #endif /* CONFIG_IGB_PTP */
 
 static inline s32 igb_reset_phy(struct e1000_hw *hw)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 0c4e29a..ffed4d0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2295,21 +2295,8 @@ static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	}
 }
 
-static int igb_ethtool_begin(struct net_device *netdev)
-{
-	struct igb_adapter *adapter = netdev_priv(netdev);
-	pm_runtime_get_sync(&adapter->pdev->dev);
-	return 0;
-}
-
-static void igb_ethtool_complete(struct net_device *netdev)
-{
-	struct igb_adapter *adapter = netdev_priv(netdev);
-	pm_runtime_put(&adapter->pdev->dev);
-}
-
 #ifdef CONFIG_IGB_PTP
-static int igb_ethtool_get_ts_info(struct net_device *dev,
+static int igb_get_ts_info(struct net_device *dev,
 				   struct ethtool_ts_info *info)
 {
 	struct igb_adapter *adapter = netdev_priv(dev);
@@ -2340,6 +2327,19 @@ static int igb_ethtool_get_ts_info(struct net_device *dev,
 }
 #endif /* CONFIG_IGB_PTP */
 
+static int igb_ethtool_begin(struct net_device *netdev)
+{
+	struct igb_adapter *adapter = netdev_priv(netdev);
+	pm_runtime_get_sync(&adapter->pdev->dev);
+	return 0;
+}
+
+static void igb_ethtool_complete(struct net_device *netdev)
+{
+	struct igb_adapter *adapter = netdev_priv(netdev);
+	pm_runtime_put(&adapter->pdev->dev);
+}
+
 static const struct ethtool_ops igb_ethtool_ops = {
 	.get_settings           = igb_get_settings,
 	.set_settings           = igb_set_settings,
@@ -2366,11 +2366,11 @@ static const struct ethtool_ops igb_ethtool_ops = {
 	.get_ethtool_stats      = igb_get_ethtool_stats,
 	.get_coalesce           = igb_get_coalesce,
 	.set_coalesce           = igb_set_coalesce,
-	.begin			= igb_ethtool_begin,
-	.complete		= igb_ethtool_complete,
 #ifdef CONFIG_IGB_PTP
-	.get_ts_info		= igb_ethtool_get_ts_info,
+	.get_ts_info            = igb_get_ts_info,
 #endif /* CONFIG_IGB_PTP */
+	.begin			= igb_ethtool_begin,
+	.complete		= igb_ethtool_complete,
 };
 
 void igb_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 03477d7..6e39f0c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2260,7 +2260,7 @@ static void __devexit igb_remove(struct pci_dev *pdev)
 
 	pm_runtime_get_noresume(&pdev->dev);
 #ifdef CONFIG_IGB_PTP
-	igb_ptp_remove(adapter);
+	igb_ptp_stop(adapter);
 #endif /* CONFIG_IGB_PTP */
 
 	/*
@@ -5750,37 +5750,6 @@ static int igb_poll(struct napi_struct *napi, int budget)
 	return 0;
 }
 
-#ifdef CONFIG_IGB_PTP
-/**
- * igb_tx_hwtstamp - utility function which checks for TX time stamp
- * @q_vector: pointer to q_vector containing needed info
- * @buffer: pointer to igb_tx_buffer structure
- *
- * If we were asked to do hardware stamping and such a time stamp is
- * available, then it must have been for this skb here because we only
- * allow only one such packet into the queue.
- */
-static void igb_tx_hwtstamp(struct igb_q_vector *q_vector,
-			    struct igb_tx_buffer *buffer_info)
-{
-	struct igb_adapter *adapter = q_vector->adapter;
-	struct e1000_hw *hw = &adapter->hw;
-	struct skb_shared_hwtstamps shhwtstamps;
-	u64 regval;
-
-	/* if skb does not support hw timestamp or TX stamp not valid exit */
-	if (likely(!(buffer_info->tx_flags & IGB_TX_FLAGS_TSTAMP)) ||
-	    !(rd32(E1000_TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID))
-		return;
-
-	regval = rd32(E1000_TXSTMPL);
-	regval |= (u64)rd32(E1000_TXSTMPH) << 32;
-
-	igb_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
-	skb_tstamp_tx(buffer_info->skb, &shhwtstamps);
-}
-#endif /* CONFIG_IGB_PTP */
-
 /**
  * igb_clean_tx_irq - Reclaim resources after transmit completes
  * @q_vector: pointer to q_vector containing needed info
@@ -5827,7 +5796,7 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
 
 #ifdef CONFIG_IGB_PTP
 		/* retrieve hardware timestamp */
-		igb_tx_hwtstamp(q_vector, tx_buffer);
+		igb_ptp_tx_hwtstamp(q_vector, tx_buffer);
 #endif /* CONFIG_IGB_PTP */
 
 		/* free the skb */
@@ -6001,47 +5970,6 @@ static inline void igb_rx_hash(struct igb_ring *ring,
 		skb->rxhash = le32_to_cpu(rx_desc->wb.lower.hi_dword.rss);
 }
 
-#ifdef CONFIG_IGB_PTP
-static void igb_rx_hwtstamp(struct igb_q_vector *q_vector,
-			    union e1000_adv_rx_desc *rx_desc,
-			    struct sk_buff *skb)
-{
-	struct igb_adapter *adapter = q_vector->adapter;
-	struct e1000_hw *hw = &adapter->hw;
-	u64 regval;
-
-	if (!igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP |
-				       E1000_RXDADV_STAT_TS))
-		return;
-
-	/*
-	 * If this bit is set, then the RX registers contain the time stamp. No
-	 * other packet will be time stamped until we read these registers, so
-	 * read the registers to make them available again. Because only one
-	 * packet can be time stamped at a time, we know that the register
-	 * values must belong to this one here and therefore we don't need to
-	 * compare any of the additional attributes stored for it.
-	 *
-	 * If nothing went wrong, then it should have a shared tx_flags that we
-	 * can turn into a skb_shared_hwtstamps.
-	 */
-	if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
-		u32 *stamp = (u32 *)skb->data;
-		regval = le32_to_cpu(*(stamp + 2));
-		regval |= (u64)le32_to_cpu(*(stamp + 3)) << 32;
-		skb_pull(skb, IGB_TS_HDR_LEN);
-	} else {
-		if(!(rd32(E1000_TSYNCRXCTL) & E1000_TSYNCRXCTL_VALID))
-			return;
-
-		regval = rd32(E1000_RXSTMPL);
-		regval |= (u64)rd32(E1000_RXSTMPH) << 32;
-	}
-
-	igb_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
-}
-#endif /* CONFIG_IGB_PTP */
-
 static void igb_rx_vlan(struct igb_ring *ring,
 			union e1000_adv_rx_desc *rx_desc,
 			struct sk_buff *skb)
@@ -6153,7 +6081,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
 		}
 
 #ifdef CONFIG_IGB_PTP
-		igb_rx_hwtstamp(q_vector, rx_desc, skb);
+		igb_ptp_rx_hwtstamp(q_vector, rx_desc, skb);
 #endif /* CONFIG_IGB_PTP */
 		igb_rx_hash(rx_ring, rx_desc, skb);
 		igb_rx_checksum(rx_ring, rx_desc, skb);
@@ -6347,183 +6275,6 @@ static int igb_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	return 0;
 }
 
-#ifdef CONFIG_IGB_PTP
-/**
- * igb_hwtstamp_ioctl - control hardware time stamping
- * @netdev:
- * @ifreq:
- * @cmd:
- *
- * Outgoing time stamping can be enabled and disabled. Play nice and
- * disable it when requested, although it shouldn't case any overhead
- * when no packet needs it. At most one packet in the queue may be
- * marked for time stamping, otherwise it would be impossible to tell
- * for sure to which packet the hardware time stamp belongs.
- *
- * Incoming time stamping has to be configured via the hardware
- * filters. Not all combinations are supported, in particular event
- * type has to be specified. Matching the kind of event packet is
- * not supported, with the exception of "all V2 events regardless of
- * level 2 or 4".
- *
- **/
-static int igb_hwtstamp_ioctl(struct net_device *netdev,
-			      struct ifreq *ifr, int cmd)
-{
-	struct igb_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	struct hwtstamp_config config;
-	u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED;
-	u32 tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
-	u32 tsync_rx_cfg = 0;
-	bool is_l4 = false;
-	bool is_l2 = false;
-	u32 regval;
-
-	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
-		return -EFAULT;
-
-	/* reserved for future extensions */
-	if (config.flags)
-		return -EINVAL;
-
-	switch (config.tx_type) {
-	case HWTSTAMP_TX_OFF:
-		tsync_tx_ctl = 0;
-	case HWTSTAMP_TX_ON:
-		break;
-	default:
-		return -ERANGE;
-	}
-
-	switch (config.rx_filter) {
-	case HWTSTAMP_FILTER_NONE:
-		tsync_rx_ctl = 0;
-		break;
-	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
-	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
-	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
-	case HWTSTAMP_FILTER_ALL:
-		/*
-		 * register TSYNCRXCFG must be set, therefore it is not
-		 * possible to time stamp both Sync and Delay_Req messages
-		 * => fall back to time stamping all packets
-		 */
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_ALL;
-		config.rx_filter = HWTSTAMP_FILTER_ALL;
-		break;
-	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE;
-		is_l4 = true;
-		break;
-	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE;
-		is_l4 = true;
-		break;
-	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
-	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE;
-		is_l2 = true;
-		is_l4 = true;
-		config.rx_filter = HWTSTAMP_FILTER_SOME;
-		break;
-	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
-	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE;
-		is_l2 = true;
-		is_l4 = true;
-		config.rx_filter = HWTSTAMP_FILTER_SOME;
-		break;
-	case HWTSTAMP_FILTER_PTP_V2_EVENT:
-	case HWTSTAMP_FILTER_PTP_V2_SYNC:
-	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_EVENT_V2;
-		config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
-		is_l2 = true;
-		is_l4 = true;
-		break;
-	default:
-		return -ERANGE;
-	}
-
-	if (hw->mac.type == e1000_82575) {
-		if (tsync_rx_ctl | tsync_tx_ctl)
-			return -EINVAL;
-		return 0;
-	}
-
-	/*
-	 * Per-packet timestamping only works if all packets are
-	 * timestamped, so enable timestamping in all packets as
-	 * long as one rx filter was configured.
-	 */
-	if ((hw->mac.type >= e1000_82580) && tsync_rx_ctl) {
-		tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
-		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_ALL;
-	}
-
-	/* enable/disable TX */
-	regval = rd32(E1000_TSYNCTXCTL);
-	regval &= ~E1000_TSYNCTXCTL_ENABLED;
-	regval |= tsync_tx_ctl;
-	wr32(E1000_TSYNCTXCTL, regval);
-
-	/* enable/disable RX */
-	regval = rd32(E1000_TSYNCRXCTL);
-	regval &= ~(E1000_TSYNCRXCTL_ENABLED | E1000_TSYNCRXCTL_TYPE_MASK);
-	regval |= tsync_rx_ctl;
-	wr32(E1000_TSYNCRXCTL, regval);
-
-	/* define which PTP packets are time stamped */
-	wr32(E1000_TSYNCRXCFG, tsync_rx_cfg);
-
-	/* define ethertype filter for timestamped packets */
-	if (is_l2)
-		wr32(E1000_ETQF(3),
-		                (E1000_ETQF_FILTER_ENABLE | /* enable filter */
-		                 E1000_ETQF_1588 | /* enable timestamping */
-		                 ETH_P_1588));     /* 1588 eth protocol type */
-	else
-		wr32(E1000_ETQF(3), 0);
-
-#define PTP_PORT 319
-	/* L4 Queue Filter[3]: filter by destination port and protocol */
-	if (is_l4) {
-		u32 ftqf = (IPPROTO_UDP /* UDP */
-			| E1000_FTQF_VF_BP /* VF not compared */
-			| E1000_FTQF_1588_TIME_STAMP /* Enable Timestamping */
-			| E1000_FTQF_MASK); /* mask all inputs */
-		ftqf &= ~E1000_FTQF_MASK_PROTO_BP; /* enable protocol check */
-
-		wr32(E1000_IMIR(3), htons(PTP_PORT));
-		wr32(E1000_IMIREXT(3),
-		     (E1000_IMIREXT_SIZE_BP | E1000_IMIREXT_CTRL_BP));
-		if (hw->mac.type == e1000_82576) {
-			/* enable source port check */
-			wr32(E1000_SPQF(3), htons(PTP_PORT));
-			ftqf &= ~E1000_FTQF_MASK_SOURCE_PORT_BP;
-		}
-		wr32(E1000_FTQF(3), ftqf);
-	} else {
-		wr32(E1000_FTQF(3), E1000_FTQF_MASK);
-	}
-	wrfl();
-
-	adapter->hwtstamp_config = config;
-
-	/* clear TX/RX time stamp registers, just to be sure */
-	regval = rd32(E1000_TXSTMPH);
-	regval = rd32(E1000_RXSTMPH);
-
-	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-		-EFAULT : 0;
-}
-#endif /* CONFIG_IGB_PTP */
-
 /**
  * igb_ioctl -
  * @netdev:
@@ -6539,7 +6290,7 @@ static int igb_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 		return igb_mii_ioctl(netdev, ifr, cmd);
 #ifdef CONFIG_IGB_PTP
 	case SIOCSHWTSTAMP:
-		return igb_hwtstamp_ioctl(netdev, ifr, cmd);
+		return igb_ptp_hwtstamp_ioctl(netdev, ifr, cmd);
 #endif /* CONFIG_IGB_PTP */
 	default:
 		return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index c846ea9..34e0d69 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -69,22 +69,22 @@
  *   2^40 * 10^-9 /  60  = 18.3 minutes.
  */
 
-#define IGB_OVERFLOW_PERIOD	(HZ * 60 * 9)
-#define INCPERIOD_82576		(1 << E1000_TIMINCA_16NS_SHIFT)
-#define INCVALUE_82576_MASK	((1 << E1000_TIMINCA_16NS_SHIFT) - 1)
-#define INCVALUE_82576		(16 << IGB_82576_TSYNC_SHIFT)
-#define IGB_NBITS_82580		40
+#define IGB_SYSTIM_OVERFLOW_PERIOD	(HZ * 60 * 9)
+#define INCPERIOD_82576			(1 << E1000_TIMINCA_16NS_SHIFT)
+#define INCVALUE_82576_MASK		((1 << E1000_TIMINCA_16NS_SHIFT) - 1)
+#define INCVALUE_82576			(16 << IGB_82576_TSYNC_SHIFT)
+#define IGB_NBITS_82580			40
 
 /*
  * SYSTIM read access for the 82576
  */
 
-static cycle_t igb_82576_systim_read(const struct cyclecounter *cc)
+static cycle_t igb_ptp_read_82576(const struct cyclecounter *cc)
 {
-	u64 val;
-	u32 lo, hi;
 	struct igb_adapter *igb = container_of(cc, struct igb_adapter, cc);
 	struct e1000_hw *hw = &igb->hw;
+	u64 val;
+	u32 lo, hi;
 
 	lo = rd32(E1000_SYSTIML);
 	hi = rd32(E1000_SYSTIMH);
@@ -99,12 +99,12 @@ static cycle_t igb_82576_systim_read(const struct cyclecounter *cc)
  * SYSTIM read access for the 82580
  */
 
-static cycle_t igb_82580_systim_read(const struct cyclecounter *cc)
+static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 {
-	u64 val;
-	u32 lo, hi, jk;
 	struct igb_adapter *igb = container_of(cc, struct igb_adapter, cc);
 	struct e1000_hw *hw = &igb->hw;
+	u64 val;
+	u32 lo, hi, jk;
 
 	/*
 	 * The timestamp latches on lowest register read. For the 82580
@@ -121,17 +121,63 @@ static cycle_t igb_82580_systim_read(const struct cyclecounter *cc)
 	return val;
 }
 
+/**
+ * igb_ptp_systim_to_hwtstamp - convert system time value to hw timestamp
+ * @adapter: board private structure
+ * @hwtstamps: timestamp structure to update
+ * @systim: unsigned 64bit system time value.
+ *
+ * We need to convert the system time value stored in the RX/TXSTMP registers
+ * into a hwtstamp which can be used by the upper level timestamping functions.
+ *
+ * The 'tmreg_lock' spinlock is used to protect the consistency of the
+ * system time value. This is needed because reading the 64 bit time
+ * value involves reading two (or three) 32 bit registers. The first
+ * read latches the value. Ditto for writing.
+ *
+ * In addition, here have extended the system time with an overflow
+ * counter in software.
+ **/
+static void igb_ptp_systim_to_hwtstamp(struct igb_adapter *adapter,
+				       struct skb_shared_hwtstamps *hwtstamps,
+				       u64 systim)
+{
+	unsigned long flags;
+	u64 ns;
+
+	switch (adapter->hw.mac.type) {
+	case e1000_i210:
+	case e1000_i211:
+	case e1000_i350:
+	case e1000_82580:
+	case e1000_82576:
+		break;
+	default:
+		return;
+	}
+
+	spin_lock_irqsave(&adapter->tmreg_lock, flags);
+
+	ns = timecounter_cyc2time(&adapter->tc, systim);
+
+	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
+
+	memset(hwtstamps, 0, sizeof(*hwtstamps));
+	hwtstamps->hwtstamp = ns_to_ktime(ns);
+}
+
 /*
  * PTP clock operations
  */
 
-static int ptp_82576_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+static int igb_ptp_adjfreq_82576(struct ptp_clock_info *ptp, s32 ppb)
 {
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
+	int neg_adj = 0;
 	u64 rate;
 	u32 incvalue;
-	int neg_adj = 0;
-	struct igb_adapter *igb = container_of(ptp, struct igb_adapter, caps);
-	struct e1000_hw *hw = &igb->hw;
 
 	if (ppb < 0) {
 		neg_adj = 1;
@@ -153,13 +199,14 @@ static int ptp_82576_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	return 0;
 }
 
-static int ptp_82580_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
 {
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
+	int neg_adj = 0;
 	u64 rate;
 	u32 inca;
-	int neg_adj = 0;
-	struct igb_adapter *igb = container_of(ptp, struct igb_adapter, caps);
-	struct e1000_hw *hw = &igb->hw;
 
 	if (ppb < 0) {
 		neg_adj = 1;
@@ -178,11 +225,12 @@ static int ptp_82580_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	return 0;
 }
 
-static int igb_adjtime(struct ptp_clock_info *ptp, s64 delta)
+static int igb_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
-	s64 now;
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
 	unsigned long flags;
-	struct igb_adapter *igb = container_of(ptp, struct igb_adapter, caps);
+	s64 now;
 
 	spin_lock_irqsave(&igb->tmreg_lock, flags);
 
@@ -195,12 +243,13 @@ static int igb_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return 0;
 }
 
-static int igb_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
+static int igb_ptp_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
 {
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	unsigned long flags;
 	u64 ns;
 	u32 remainder;
-	unsigned long flags;
-	struct igb_adapter *igb = container_of(ptp, struct igb_adapter, caps);
 
 	spin_lock_irqsave(&igb->tmreg_lock, flags);
 
@@ -214,11 +263,13 @@ static int igb_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
 	return 0;
 }
 
-static int igb_settime(struct ptp_clock_info *ptp, const struct timespec *ts)
+static int igb_ptp_settime(struct ptp_clock_info *ptp,
+			   const struct timespec *ts)
 {
-	u64 ns;
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
 	unsigned long flags;
-	struct igb_adapter *igb = container_of(ptp, struct igb_adapter, caps);
+	u64 ns;
 
 	ns = ts->tv_sec * 1000000000ULL;
 	ns += ts->tv_nsec;
@@ -232,29 +283,265 @@ static int igb_settime(struct ptp_clock_info *ptp, const struct timespec *ts)
 	return 0;
 }
 
-static int ptp_82576_enable(struct ptp_clock_info *ptp,
-			    struct ptp_clock_request *rq, int on)
+static int igb_ptp_enable(struct ptp_clock_info *ptp,
+			  struct ptp_clock_request *rq, int on)
 {
 	return -EOPNOTSUPP;
 }
 
-static int ptp_82580_enable(struct ptp_clock_info *ptp,
-			    struct ptp_clock_request *rq, int on)
+static void igb_ptp_overflow_check(struct work_struct *work)
 {
-	return -EOPNOTSUPP;
+	struct igb_adapter *igb =
+		container_of(work, struct igb_adapter, ptp_overflow_work.work);
+	struct timespec ts;
+
+	igb_ptp_gettime(&igb->ptp_caps, &ts);
+
+	pr_debug("igb overflow check at %ld.%09lu\n", ts.tv_sec, ts.tv_nsec);
+
+	schedule_delayed_work(&igb->ptp_overflow_work,
+			      IGB_SYSTIM_OVERFLOW_PERIOD);
 }
 
-static void igb_overflow_check(struct work_struct *work)
+/**
+ * igb_ptp_tx_hwtstamp - utility function which checks for TX time stamp
+ * @q_vector: pointer to q_vector containing needed info
+ * @buffer: pointer to igb_tx_buffer structure
+ *
+ * If we were asked to do hardware stamping and such a time stamp is
+ * available, then it must have been for this skb here because we only
+ * allow only one such packet into the queue.
+ */
+void igb_ptp_tx_hwtstamp(struct igb_q_vector *q_vector,
+			 struct igb_tx_buffer *buffer_info)
 {
-	struct timespec ts;
-	struct igb_adapter *igb =
-		container_of(work, struct igb_adapter, overflow_work.work);
+	struct igb_adapter *adapter = q_vector->adapter;
+	struct e1000_hw *hw = &adapter->hw;
+	struct skb_shared_hwtstamps shhwtstamps;
+	u64 regval;
 
-	igb_gettime(&igb->caps, &ts);
+	/* if skb does not support hw timestamp or TX stamp not valid exit */
+	if (likely(!(buffer_info->tx_flags & IGB_TX_FLAGS_TSTAMP)) ||
+	    !(rd32(E1000_TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID))
+		return;
 
-	pr_debug("igb overflow check at %ld.%09lu\n", ts.tv_sec, ts.tv_nsec);
+	regval = rd32(E1000_TXSTMPL);
+	regval |= (u64)rd32(E1000_TXSTMPH) << 32;
 
-	schedule_delayed_work(&igb->overflow_work, IGB_OVERFLOW_PERIOD);
+	igb_ptp_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
+	skb_tstamp_tx(buffer_info->skb, &shhwtstamps);
+}
+
+void igb_ptp_rx_hwtstamp(struct igb_q_vector *q_vector,
+			 union e1000_adv_rx_desc *rx_desc,
+			 struct sk_buff *skb)
+{
+	struct igb_adapter *adapter = q_vector->adapter;
+	struct e1000_hw *hw = &adapter->hw;
+	u64 regval;
+
+	if (!igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP |
+				       E1000_RXDADV_STAT_TS))
+		return;
+
+	/*
+	 * If this bit is set, then the RX registers contain the time stamp. No
+	 * other packet will be time stamped until we read these registers, so
+	 * read the registers to make them available again. Because only one
+	 * packet can be time stamped at a time, we know that the register
+	 * values must belong to this one here and therefore we don't need to
+	 * compare any of the additional attributes stored for it.
+	 *
+	 * If nothing went wrong, then it should have a shared tx_flags that we
+	 * can turn into a skb_shared_hwtstamps.
+	 */
+	if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
+		u32 *stamp = (u32 *)skb->data;
+		regval = le32_to_cpu(*(stamp + 2));
+		regval |= (u64)le32_to_cpu(*(stamp + 3)) << 32;
+		skb_pull(skb, IGB_TS_HDR_LEN);
+	} else {
+		if (!(rd32(E1000_TSYNCRXCTL) & E1000_TSYNCRXCTL_VALID))
+			return;
+
+		regval = rd32(E1000_RXSTMPL);
+		regval |= (u64)rd32(E1000_RXSTMPH) << 32;
+	}
+
+	igb_ptp_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
+}
+
+/**
+ * igb_ptp_hwtstamp_ioctl - control hardware time stamping
+ * @netdev:
+ * @ifreq:
+ * @cmd:
+ *
+ * Outgoing time stamping can be enabled and disabled. Play nice and
+ * disable it when requested, although it shouldn't case any overhead
+ * when no packet needs it. At most one packet in the queue may be
+ * marked for time stamping, otherwise it would be impossible to tell
+ * for sure to which packet the hardware time stamp belongs.
+ *
+ * Incoming time stamping has to be configured via the hardware
+ * filters. Not all combinations are supported, in particular event
+ * type has to be specified. Matching the kind of event packet is
+ * not supported, with the exception of "all V2 events regardless of
+ * level 2 or 4".
+ *
+ **/
+int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
+			   struct ifreq *ifr, int cmd)
+{
+	struct igb_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	struct hwtstamp_config config;
+	u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED;
+	u32 tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
+	u32 tsync_rx_cfg = 0;
+	bool is_l4 = false;
+	bool is_l2 = false;
+	u32 regval;
+
+	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
+		return -EFAULT;
+
+	/* reserved for future extensions */
+	if (config.flags)
+		return -EINVAL;
+
+	switch (config.tx_type) {
+	case HWTSTAMP_TX_OFF:
+		tsync_tx_ctl = 0;
+	case HWTSTAMP_TX_ON:
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	switch (config.rx_filter) {
+	case HWTSTAMP_FILTER_NONE:
+		tsync_rx_ctl = 0;
+		break;
+	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+	case HWTSTAMP_FILTER_ALL:
+		/*
+		 * register TSYNCRXCFG must be set, therefore it is not
+		 * possible to time stamp both Sync and Delay_Req messages
+		 * => fall back to time stamping all packets
+		 */
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_ALL;
+		config.rx_filter = HWTSTAMP_FILTER_ALL;
+		break;
+	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L4_V1;
+		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE;
+		is_l4 = true;
+		break;
+	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L4_V1;
+		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE;
+		is_l4 = true;
+		break;
+	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
+		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE;
+		is_l2 = true;
+		is_l4 = true;
+		config.rx_filter = HWTSTAMP_FILTER_SOME;
+		break;
+	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
+		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE;
+		is_l2 = true;
+		is_l4 = true;
+		config.rx_filter = HWTSTAMP_FILTER_SOME;
+		break;
+	case HWTSTAMP_FILTER_PTP_V2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_EVENT_V2;
+		config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		is_l2 = true;
+		is_l4 = true;
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	if (hw->mac.type == e1000_82575) {
+		if (tsync_rx_ctl | tsync_tx_ctl)
+			return -EINVAL;
+		return 0;
+	}
+
+	/*
+	 * Per-packet timestamping only works if all packets are
+	 * timestamped, so enable timestamping in all packets as
+	 * long as one rx filter was configured.
+	 */
+	if ((hw->mac.type >= e1000_82580) && tsync_rx_ctl) {
+		tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
+		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_ALL;
+	}
+
+	/* enable/disable TX */
+	regval = rd32(E1000_TSYNCTXCTL);
+	regval &= ~E1000_TSYNCTXCTL_ENABLED;
+	regval |= tsync_tx_ctl;
+	wr32(E1000_TSYNCTXCTL, regval);
+
+	/* enable/disable RX */
+	regval = rd32(E1000_TSYNCRXCTL);
+	regval &= ~(E1000_TSYNCRXCTL_ENABLED | E1000_TSYNCRXCTL_TYPE_MASK);
+	regval |= tsync_rx_ctl;
+	wr32(E1000_TSYNCRXCTL, regval);
+
+	/* define which PTP packets are time stamped */
+	wr32(E1000_TSYNCRXCFG, tsync_rx_cfg);
+
+	/* define ethertype filter for timestamped packets */
+	if (is_l2)
+		wr32(E1000_ETQF(3),
+		     (E1000_ETQF_FILTER_ENABLE | /* enable filter */
+		      E1000_ETQF_1588 | /* enable timestamping */
+		      ETH_P_1588));     /* 1588 eth protocol type */
+	else
+		wr32(E1000_ETQF(3), 0);
+
+#define PTP_PORT 319
+	/* L4 Queue Filter[3]: filter by destination port and protocol */
+	if (is_l4) {
+		u32 ftqf = (IPPROTO_UDP /* UDP */
+			| E1000_FTQF_VF_BP /* VF not compared */
+			| E1000_FTQF_1588_TIME_STAMP /* Enable Timestamping */
+			| E1000_FTQF_MASK); /* mask all inputs */
+		ftqf &= ~E1000_FTQF_MASK_PROTO_BP; /* enable protocol check */
+
+		wr32(E1000_IMIR(3), htons(PTP_PORT));
+		wr32(E1000_IMIREXT(3),
+		     (E1000_IMIREXT_SIZE_BP | E1000_IMIREXT_CTRL_BP));
+		if (hw->mac.type == e1000_82576) {
+			/* enable source port check */
+			wr32(E1000_SPQF(3), htons(PTP_PORT));
+			ftqf &= ~E1000_FTQF_MASK_SOURCE_PORT_BP;
+		}
+		wr32(E1000_FTQF(3), ftqf);
+	} else {
+		wr32(E1000_FTQF(3), E1000_FTQF_MASK);
+	}
+	wrfl();
+
+	/* clear TX/RX time stamp registers, just to be sure */
+	regval = rd32(E1000_TXSTMPH);
+	regval = rd32(E1000_RXSTMPH);
+
+	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
+		-EFAULT : 0;
 }
 
 void igb_ptp_init(struct igb_adapter *adapter)
@@ -266,39 +553,39 @@ void igb_ptp_init(struct igb_adapter *adapter)
 	case e1000_i211:
 	case e1000_i350:
 	case e1000_82580:
-		adapter->caps.owner	= THIS_MODULE;
-		strcpy(adapter->caps.name, "igb-82580");
-		adapter->caps.max_adj	= 62499999;
-		adapter->caps.n_ext_ts	= 0;
-		adapter->caps.pps	= 0;
-		adapter->caps.adjfreq	= ptp_82580_adjfreq;
-		adapter->caps.adjtime	= igb_adjtime;
-		adapter->caps.gettime	= igb_gettime;
-		adapter->caps.settime	= igb_settime;
-		adapter->caps.enable	= ptp_82580_enable;
-		adapter->cc.read	= igb_82580_systim_read;
-		adapter->cc.mask	= CLOCKSOURCE_MASK(IGB_NBITS_82580);
-		adapter->cc.mult	= 1;
-		adapter->cc.shift	= 0;
+		adapter->ptp_caps.owner = THIS_MODULE;
+		strcpy(adapter->ptp_caps.name, "igb-82580");
+		adapter->ptp_caps.max_adj = 62499999;
+		adapter->ptp_caps.n_ext_ts = 0;
+		adapter->ptp_caps.pps = 0;
+		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjtime = igb_ptp_adjtime;
+		adapter->ptp_caps.gettime = igb_ptp_gettime;
+		adapter->ptp_caps.settime = igb_ptp_settime;
+		adapter->ptp_caps.enable = igb_ptp_enable;
+		adapter->cc.read = igb_ptp_read_82580;
+		adapter->cc.mask = CLOCKSOURCE_MASK(IGB_NBITS_82580);
+		adapter->cc.mult = 1;
+		adapter->cc.shift = 0;
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
 
 	case e1000_82576:
-		adapter->caps.owner	= THIS_MODULE;
-		strcpy(adapter->caps.name, "igb-82576");
-		adapter->caps.max_adj	= 1000000000;
-		adapter->caps.n_ext_ts	= 0;
-		adapter->caps.pps	= 0;
-		adapter->caps.adjfreq	= ptp_82576_adjfreq;
-		adapter->caps.adjtime	= igb_adjtime;
-		adapter->caps.gettime	= igb_gettime;
-		adapter->caps.settime	= igb_settime;
-		adapter->caps.enable	= ptp_82576_enable;
-		adapter->cc.read	= igb_82576_systim_read;
-		adapter->cc.mask	= CLOCKSOURCE_MASK(64);
-		adapter->cc.mult	= 1;
-		adapter->cc.shift	= IGB_82576_TSYNC_SHIFT;
+		adapter->ptp_caps.owner = THIS_MODULE;
+		strcpy(adapter->ptp_caps.name, "igb-82576");
+		adapter->ptp_caps.max_adj = 1000000000;
+		adapter->ptp_caps.n_ext_ts = 0;
+		adapter->ptp_caps.pps = 0;
+		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82576;
+		adapter->ptp_caps.adjtime = igb_ptp_adjtime;
+		adapter->ptp_caps.gettime = igb_ptp_gettime;
+		adapter->ptp_caps.settime = igb_ptp_settime;
+		adapter->ptp_caps.enable = igb_ptp_enable;
+		adapter->cc.read = igb_ptp_read_82576;
+		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mult = 1;
+		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
 		/* Dial the nominal frequency. */
 		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
 		break;
@@ -313,13 +600,14 @@ void igb_ptp_init(struct igb_adapter *adapter)
 	timecounter_init(&adapter->tc, &adapter->cc,
 			 ktime_to_ns(ktime_get_real()));
 
-	INIT_DELAYED_WORK(&adapter->overflow_work, igb_overflow_check);
+	INIT_DELAYED_WORK(&adapter->ptp_overflow_work, igb_ptp_overflow_check);
 
 	spin_lock_init(&adapter->tmreg_lock);
 
-	schedule_delayed_work(&adapter->overflow_work, IGB_OVERFLOW_PERIOD);
+	schedule_delayed_work(&adapter->ptp_overflow_work,
+			      IGB_SYSTIM_OVERFLOW_PERIOD);
 
-	adapter->ptp_clock = ptp_clock_register(&adapter->caps);
+	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
@@ -328,7 +616,13 @@ void igb_ptp_init(struct igb_adapter *adapter)
 			 adapter->netdev->name);
 }
 
-void igb_ptp_remove(struct igb_adapter *adapter)
+/**
+ * igb_ptp_stop - Disable PTP device and stop the overflow check.
+ * @adapter: Board private structure.
+ *
+ * This function stops the PTP support and cancels the delayed work.
+ **/
+void igb_ptp_stop(struct igb_adapter *adapter)
 {
 	switch (adapter->hw.mac.type) {
 	case e1000_i211:
@@ -336,7 +630,7 @@ void igb_ptp_remove(struct igb_adapter *adapter)
 	case e1000_i350:
 	case e1000_82580:
 	case e1000_82576:
-		cancel_delayed_work_sync(&adapter->overflow_work);
+		cancel_delayed_work_sync(&adapter->ptp_overflow_work);
 		break;
 	default:
 		return;
@@ -348,48 +642,3 @@ void igb_ptp_remove(struct igb_adapter *adapter)
 			 adapter->netdev->name);
 	}
 }
-
-/**
- * igb_systim_to_hwtstamp - convert system time value to hw timestamp
- * @adapter: board private structure
- * @hwtstamps: timestamp structure to update
- * @systim: unsigned 64bit system time value.
- *
- * We need to convert the system time value stored in the RX/TXSTMP registers
- * into a hwtstamp which can be used by the upper level timestamping functions.
- *
- * The 'tmreg_lock' spinlock is used to protect the consistency of the
- * system time value. This is needed because reading the 64 bit time
- * value involves reading two (or three) 32 bit registers. The first
- * read latches the value. Ditto for writing.
- *
- * In addition, here have extended the system time with an overflow
- * counter in software.
- **/
-void igb_systim_to_hwtstamp(struct igb_adapter *adapter,
-			    struct skb_shared_hwtstamps *hwtstamps,
-			    u64 systim)
-{
-	u64 ns;
-	unsigned long flags;
-
-	switch (adapter->hw.mac.type) {
-	case e1000_i210:
-	case e1000_i211:
-	case e1000_i350:
-	case e1000_82580:
-	case e1000_82576:
-		break;
-	default:
-		return;
-	}
-
-	spin_lock_irqsave(&adapter->tmreg_lock, flags);
-
-	ns = timecounter_cyc2time(&adapter->tc, systim);
-
-	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
-
-	memset(hwtstamps, 0, sizeof(*hwtstamps));
-	hwtstamps->hwtstamp = ns_to_ktime(ns);
-}
-- 
1.7.11.4

[net-next 3/6] igb: Correct PTP support query from ethtool.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Update ethtool_get_ts_info to not report any supported functionality on
82575 and add support for V2 Sync and V2 Delay packets. In the case
where CONFIG_IGB_PTP is not defined, we should be reporting default
values.

v2: Correct the function to return EOPNOTSUPP when there is no PTP support
    or the device does not support PTP. Also fix minor whitespace issue.

Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 62 +++++++++++++++++-----------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index ffed4d0..2ea0128 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2295,37 +2295,53 @@ static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	}
 }
 
-#ifdef CONFIG_IGB_PTP
 static int igb_get_ts_info(struct net_device *dev,
-				   struct ethtool_ts_info *info)
+			   struct ethtool_ts_info *info)
 {
 	struct igb_adapter *adapter = netdev_priv(dev);
 
-	info->so_timestamping =
-		SOF_TIMESTAMPING_TX_HARDWARE |
-		SOF_TIMESTAMPING_RX_HARDWARE |
-		SOF_TIMESTAMPING_RAW_HARDWARE;
+	switch (adapter->hw.mac.type) {
+#ifdef CONFIG_IGB_PTP
+	case e1000_82576:
+	case e1000_82580:
+	case e1000_i350:
+	case e1000_i210:
+	case e1000_i211:
+		info->so_timestamping =
+			SOF_TIMESTAMPING_TX_HARDWARE |
+			SOF_TIMESTAMPING_RX_HARDWARE |
+			SOF_TIMESTAMPING_RAW_HARDWARE;
 
-	if (adapter->ptp_clock)
-		info->phc_index = ptp_clock_index(adapter->ptp_clock);
-	else
-		info->phc_index = -1;
+		if (adapter->ptp_clock)
+			info->phc_index = ptp_clock_index(adapter->ptp_clock);
+		else
+			info->phc_index = -1;
 
-	info->tx_types =
-		(1 << HWTSTAMP_TX_OFF) |
-		(1 << HWTSTAMP_TX_ON);
+		info->tx_types =
+			(1 << HWTSTAMP_TX_OFF) |
+			(1 << HWTSTAMP_TX_ON);
 
-	info->rx_filters =
-		(1 << HWTSTAMP_FILTER_NONE) |
-		(1 << HWTSTAMP_FILTER_ALL) |
-		(1 << HWTSTAMP_FILTER_SOME) |
-		(1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC) |
-		(1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) |
-		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
+		info->rx_filters = 1 << HWTSTAMP_FILTER_NONE;
 
-	return 0;
-}
+		/* 82576 does not support timestamping all packets. */
+		if (adapter->hw.mac.type >= e1000_82580)
+			info->rx_filters |= 1 << HWTSTAMP_FILTER_ALL;
+		else
+			info->rx_filters |=
+				(1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC) |
+				(1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) |
+				(1 << HWTSTAMP_FILTER_PTP_V2_L2_SYNC) |
+				(1 << HWTSTAMP_FILTER_PTP_V2_L4_SYNC) |
+				(1 << HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ) |
+				(1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) |
+				(1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
+
+		return 0;
 #endif /* CONFIG_IGB_PTP */
+	default:
+		return -EOPNOTSUPP;
+	}
+}
 
 static int igb_ethtool_begin(struct net_device *netdev)
 {
@@ -2366,9 +2382,7 @@ static const struct ethtool_ops igb_ethtool_ops = {
 	.get_ethtool_stats      = igb_get_ethtool_stats,
 	.get_coalesce           = igb_get_coalesce,
 	.set_coalesce           = igb_set_coalesce,
-#ifdef CONFIG_IGB_PTP
 	.get_ts_info            = igb_get_ts_info,
-#endif /* CONFIG_IGB_PTP */
 	.begin			= igb_ethtool_begin,
 	.complete		= igb_ethtool_complete,
 };
-- 
1.7.11.4

[net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Change the name of the adapter in the PTP struct to enable easier
correlation between interface and PTP device.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by:  Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 34e0d69..e69555f 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -547,14 +547,15 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
 void igb_ptp_init(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	struct net_device *netdev = adapter->netdev;
 
 	switch (hw->mac.type) {
 	case e1000_i210:
 	case e1000_i211:
 	case e1000_i350:
 	case e1000_82580:
+		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
-		strcpy(adapter->ptp_caps.name, "igb-82580");
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
@@ -570,10 +571,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-
 	case e1000_82576:
+		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
-		strcpy(adapter->ptp_caps.name, "igb-82576");
 		adapter->ptp_caps.max_adj = 1000000000;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
@@ -589,7 +589,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		/* Dial the nominal frequency. */
 		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
 		break;
-
 	default:
 		adapter->ptp_clock = NULL;
 		return;
-- 
1.7.11.4

Re: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Richard Cochran]

On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> From: Matthew Vick <matthew.vick@intel.com>
> 
> Change the name of the adapter in the PTP struct to enable easier
> correlation between interface and PTP device.

Still think it is better to have the driver name, not the MAC address.
The correlation is clear by using the ethtool method.

Thanks,
Richard

RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Richard Cochran
> Sent: Thursday, September 06, 2012 1:06 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
> 
> On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > From: Matthew Vick <matthew.vick@intel.com>
> >
> > Change the name of the adapter in the PTP struct to enable easier
> > correlation between interface and PTP device.
> 
> Still think it is better to have the driver name, not the MAC address.
> The correlation is clear by using the ethtool method.
> 

I found a method to correct the init code so that the device name can appear here, and I would prefer that over either the driver name or MAC address. It requires moving the ptp initialization into the netdev open handler though.

- Jake

> Thanks,
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Ben Hutchings]

On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> > On Behalf Of Richard Cochran
> > Sent: Thursday, September 06, 2012 1:06 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > gospo@redhat.com; sassmann@redhat.com
> > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name in the
> > PTP struct.
> > 
> > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > From: Matthew Vick <matthew.vick@intel.com>
> > >
> > > Change the name of the adapter in the PTP struct to enable easier
> > > correlation between interface and PTP device.
> > 
> > Still think it is better to have the driver name, not the MAC address.
> > The correlation is clear by using the ethtool method.
> > 
> 
> I found a method to correct the init code so that the device name can
> appear here, and I would prefer that over either the driver name or
> MAC address. It requires moving the ptp initialization into the netdev
> open handler though.

I don't think you should use the net device name for this, as net
devices can be renamed after creation.  (Though you could update the
struct ptp_clock_info whenever this happens.)

I'm starting to wonder what use there is for a 'clock name' distinct
from the clock device name.  What would be more useful is to give it a
parent.  Richard, is there any reason why ptp_clock_register() doesn't
allow specifying the parent device?  You can then make the clock_name
attribute contain the parent device's driver name or whatever it is you
prefer.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Keller, Jacob E]

> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Thursday, September 06, 2012 4:23 PM
> To: Keller, Jacob E; Richard Cochran
> Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
> 
> On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org
> > > [mailto:netdev-owner@vger.kernel.org]
> > > On Behalf Of Richard Cochran
> > > Sent: Thursday, September 06, 2012 1:06 AM
> > > To: Kirsher, Jeffrey T
> > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > gospo@redhat.com; sassmann@redhat.com
> > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name
> > > in the PTP struct.
> > >
> > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > From: Matthew Vick <matthew.vick@intel.com>
> > > >
> > > > Change the name of the adapter in the PTP struct to enable easier
> > > > correlation between interface and PTP device.
> > >
> > > Still think it is better to have the driver name, not the MAC address.
> > > The correlation is clear by using the ethtool method.
> > >
> >
> > I found a method to correct the init code so that the device name can
> > appear here, and I would prefer that over either the driver name or
> > MAC address. It requires moving the ptp initialization into the netdev
> > open handler though.
> 
> I don't think you should use the net device name for this, as net devices
> can be renamed after creation.  (Though you could update the struct
> ptp_clock_info whenever this happens.)
> 
> I'm starting to wonder what use there is for a 'clock name' distinct from
> the clock device name.  What would be more useful is to give it a parent.
> Richard, is there any reason why ptp_clock_register() doesn't allow
> specifying the parent device?  You can then make the clock_name attribute
> contain the parent device's driver name or whatever it is you prefer.
> 
> Ben.

Because ideally the ptp device belongs to multiple ethernet devices...

Sadly current intel hardware doesn't support this.

- Jake

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.

RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Ben Hutchings]

On Thu, 2012-09-06 at 23:24 +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > Sent: Thursday, September 06, 2012 4:23 PM
> > To: Keller, Jacob E; Richard Cochran
> > Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> > netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> > Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> > PTP struct.
> > 
> > On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > > -----Original Message-----
> > > > From: netdev-owner@vger.kernel.org
> > > > [mailto:netdev-owner@vger.kernel.org]
> > > > On Behalf Of Richard Cochran
> > > > Sent: Thursday, September 06, 2012 1:06 AM
> > > > To: Kirsher, Jeffrey T
> > > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > > gospo@redhat.com; sassmann@redhat.com
> > > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the name
> > > > in the PTP struct.
> > > >
> > > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > > From: Matthew Vick <matthew.vick@intel.com>
> > > > >
> > > > > Change the name of the adapter in the PTP struct to enable easier
> > > > > correlation between interface and PTP device.
> > > >
> > > > Still think it is better to have the driver name, not the MAC address.
> > > > The correlation is clear by using the ethtool method.
> > > >
> > >
> > > I found a method to correct the init code so that the device name can
> > > appear here, and I would prefer that over either the driver name or
> > > MAC address. It requires moving the ptp initialization into the netdev
> > > open handler though.
> > 
> > I don't think you should use the net device name for this, as net devices
> > can be renamed after creation.  (Though you could update the struct
> > ptp_clock_info whenever this happens.)
> > 
> > I'm starting to wonder what use there is for a 'clock name' distinct from
> > the clock device name.  What would be more useful is to give it a parent.
> > Richard, is there any reason why ptp_clock_register() doesn't allow
> > specifying the parent device?  You can then make the clock_name attribute
> > contain the parent device's driver name or whatever it is you prefer.
> > 
> > Ben.
> 
> Because ideally the ptp device belongs to multiple ethernet devices...
> 
> Sadly current intel hardware doesn't support this.

The PTP clock is instantiated by some driver, bound to some device,
whether that's a PCI device or a platform device or something else.  I
think that device (not a net device) should be the parent of the clock
device.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Keller, Jacob E]

> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Thursday, September 06, 2012 4:40 PM
> To: Keller, Jacob E
> Cc: Richard Cochran; Kirsher, Jeffrey T; davem@davemloft.net; Vick,
> Matthew; netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> Subject: RE: [net-next 4/6] igb: Store the MAC address in the name in the
> PTP struct.
> 
> On Thu, 2012-09-06 at 23:24 +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > > Sent: Thursday, September 06, 2012 4:23 PM
> > > To: Keller, Jacob E; Richard Cochran
> > > Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew;
> > > netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
> > > Subject: RE: [net-next 4/6] igb: Store the MAC address in the name
> > > in the PTP struct.
> > >
> > > On Thu, 2012-09-06 at 22:59 +0000, Keller, Jacob E wrote:
> > > > > -----Original Message-----
> > > > > From: netdev-owner@vger.kernel.org
> > > > > [mailto:netdev-owner@vger.kernel.org]
> > > > > On Behalf Of Richard Cochran
> > > > > Sent: Thursday, September 06, 2012 1:06 AM
> > > > > To: Kirsher, Jeffrey T
> > > > > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org;
> > > > > gospo@redhat.com; sassmann@redhat.com
> > > > > Subject: Re: [net-next 4/6] igb: Store the MAC address in the
> > > > > name in the PTP struct.
> > > > >
> > > > > On Wed, Sep 05, 2012 at 04:35:04PM -0700, Jeff Kirsher wrote:
> > > > > > From: Matthew Vick <matthew.vick@intel.com>
> > > > > >
> > > > > > Change the name of the adapter in the PTP struct to enable
> > > > > > easier correlation between interface and PTP device.
> > > > >
> > > > > Still think it is better to have the driver name, not the MAC
> address.
> > > > > The correlation is clear by using the ethtool method.
> > > > >
> > > >
> > > > I found a method to correct the init code so that the device name
> > > > can appear here, and I would prefer that over either the driver
> > > > name or MAC address. It requires moving the ptp initialization
> > > > into the netdev open handler though.
> > >
> > > I don't think you should use the net device name for this, as net
> > > devices can be renamed after creation.  (Though you could update the
> > > struct ptp_clock_info whenever this happens.)
> > >
> > > I'm starting to wonder what use there is for a 'clock name' distinct
> > > from the clock device name.  What would be more useful is to give it a
> parent.
> > > Richard, is there any reason why ptp_clock_register() doesn't allow
> > > specifying the parent device?  You can then make the clock_name
> > > attribute contain the parent device's driver name or whatever it is
> you prefer.
> > >
> > > Ben.
> >
> > Because ideally the ptp device belongs to multiple ethernet devices...
> >
> > Sadly current intel hardware doesn't support this.
> 
> The PTP clock is instantiated by some driver, bound to some device,
> whether that's a PCI device or a platform device or something else.  I
> think that device (not a net device) should be the parent of the clock
> device.
> 

Agreed, that would make the most sense.

> Ben.
> 
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.

Re: [net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Richard Cochran]

On Thu, Sep 06, 2012 at 11:40:23PM +0000, Keller, Jacob E wrote:
> > -----Original Message-----
> > From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> > 
> > The PTP clock is instantiated by some driver, bound to some device,
> > whether that's a PCI device or a platform device or something else.  I
> > think that device (not a net device) should be the parent of the clock
> > device.
> > 
> 
> Agreed, that would make the most sense.

Okay, makes sense, but that will require a change to the phc core.

For the present, I can buy Jacob's argument that the MAC address is a
way to clarify to the user that there are "unfortunate" multiple clock
instances.

Thanks,
Richard

[net-next 5/6] igb: Prevent dropped Tx timestamps via work items and interrupts.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

In rare circumstances, it's possible a descriptor writeback will occur
before a timestamped Tx packet will go out on the wire, leading to the
driver believing the hardware failed to timestamp the packet. Schedule a
work item for 82576 and use the available time sync interrupt registers
on 82580 and above to account for this.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |   5 ++
 drivers/net/ethernet/intel/igb/e1000_regs.h    |   2 +
 drivers/net/ethernet/intel/igb/igb.h           |   8 +-
 drivers/net/ethernet/intel/igb/igb_main.c      |  61 +++++++++++++--
 drivers/net/ethernet/intel/igb/igb_ptp.c       | 102 +++++++++++++++++++++----
 5 files changed, 153 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index ec7e4fe..0b27e8f 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -360,6 +360,7 @@
 #define E1000_ICR_RXDMT0        0x00000010 /* rx desc min. threshold (0) */
 #define E1000_ICR_RXT0          0x00000080 /* rx timer intr (ring 0) */
 #define E1000_ICR_VMMB          0x00000100 /* VM MB event */
+#define E1000_ICR_TS            0x00080000 /* Time Sync Interrupt */
 #define E1000_ICR_DRSTA         0x40000000 /* Device Reset Asserted */
 /* If this bit asserted, the driver should claim the interrupt */
 #define E1000_ICR_INT_ASSERTED  0x80000000
@@ -399,6 +400,7 @@
 #define E1000_IMS_TXDW      E1000_ICR_TXDW      /* Transmit desc written back */
 #define E1000_IMS_LSC       E1000_ICR_LSC       /* Link Status Change */
 #define E1000_IMS_VMMB      E1000_ICR_VMMB      /* Mail box activity */
+#define E1000_IMS_TS        E1000_ICR_TS        /* Time Sync Interrupt */
 #define E1000_IMS_RXSEQ     E1000_ICR_RXSEQ     /* rx sequence error */
 #define E1000_IMS_RXDMT0    E1000_ICR_RXDMT0    /* rx desc min. threshold */
 #define E1000_IMS_RXT0      E1000_ICR_RXT0      /* rx timer intr */
@@ -510,6 +512,9 @@
 
 #define E1000_TIMINCA_16NS_SHIFT 24
 
+#define E1000_TSICR_TXTS 0x00000002
+#define E1000_TSIM_TXTS 0x00000002
+
 #define E1000_MDICNFG_EXT_MDIO    0x80000000      /* MDI ext/int destination */
 #define E1000_MDICNFG_COM_MDIO    0x40000000      /* MDI shared w/ lan 0 */
 #define E1000_MDICNFG_PHY_MASK    0x03E00000
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index 28394be..faec840 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -91,6 +91,8 @@
 #define E1000_TIMINCA    0x0B608 /* Increment attributes register - RW */
 #define E1000_TSAUXC     0x0B640 /* Timesync Auxiliary Control register */
 #define E1000_SYSTIMR    0x0B6F8 /* System time register Residue */
+#define E1000_TSICR      0x0B66C /* Interrupt Cause Register */
+#define E1000_TSIM       0x0B674 /* Interrupt Mask Register */
 
 /* Filtering Registers */
 #define E1000_SAQF(_n) (0x5980 + 4 * (_n))
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 7973469..43c8e29 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -381,6 +381,8 @@ struct igb_adapter {
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
 	struct delayed_work ptp_overflow_work;
+	struct work_struct ptp_tx_work;
+	struct sk_buff *ptp_tx_skb;
 	spinlock_t tmreg_lock;
 	struct cyclecounter cc;
 	struct timecounter tc;
@@ -394,6 +396,7 @@ struct igb_adapter {
 #define IGB_FLAG_QUAD_PORT_A       (1 << 2)
 #define IGB_FLAG_QUEUE_PAIRS       (1 << 3)
 #define IGB_FLAG_DMAC              (1 << 4)
+#define IGB_FLAG_PTP               (1 << 5)
 
 /* DMA Coalescing defines */
 #define IGB_MIN_TXPBSIZE           20408
@@ -440,8 +443,9 @@ extern void igb_set_fw_version(struct igb_adapter *);
 #ifdef CONFIG_IGB_PTP
 extern void igb_ptp_init(struct igb_adapter *adapter);
 extern void igb_ptp_stop(struct igb_adapter *adapter);
-extern void igb_ptp_tx_hwtstamp(struct igb_q_vector *q_vector,
-				struct igb_tx_buffer *buffer_info);
+extern void igb_ptp_reset(struct igb_adapter *adapter);
+extern void igb_ptp_tx_work(struct work_struct *work);
+extern void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter);
 extern void igb_ptp_rx_hwtstamp(struct igb_q_vector *q_vector,
 				union e1000_adv_rx_desc *rx_desc,
 				struct sk_buff *skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6e39f0c..19d7666 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1751,6 +1751,11 @@ void igb_reset(struct igb_adapter *adapter)
 	/* Enable h/w to recognize an 802.1Q VLAN Ethernet packet */
 	wr32(E1000_VET, ETHERNET_IEEE_VLAN_TYPE);
 
+#ifdef CONFIG_IGB_PTP
+	/* Re-enable PTP, where applicable. */
+	igb_ptp_reset(adapter);
+#endif /* CONFIG_IGB_PTP */
+
 	igb_get_phy_info(hw);
 }
 
@@ -4234,7 +4239,7 @@ static __le32 igb_tx_cmd_type(u32 tx_flags)
 
 #ifdef CONFIG_IGB_PTP
 	/* set timestamp bit if present */
-	if (tx_flags & IGB_TX_FLAGS_TSTAMP)
+	if (unlikely(tx_flags & IGB_TX_FLAGS_TSTAMP))
 		cmd_type |= cpu_to_le32(E1000_ADVTXD_MAC_TSTAMP);
 #endif /* CONFIG_IGB_PTP */
 
@@ -4445,6 +4450,9 @@ static inline int igb_maybe_stop_tx(struct igb_ring *tx_ring, const u16 size)
 netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 				struct igb_ring *tx_ring)
 {
+#ifdef CONFIG_IGB_PTP
+	struct igb_adapter *adapter = netdev_priv(tx_ring->netdev);
+#endif /* CONFIG_IGB_PTP */
 	struct igb_tx_buffer *first;
 	int tso;
 	u32 tx_flags = 0;
@@ -4468,9 +4476,14 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 	first->gso_segs = 1;
 
 #ifdef CONFIG_IGB_PTP
-	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
+	if (unlikely((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+		     !(adapter->ptp_tx_skb))) {
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 		tx_flags |= IGB_TX_FLAGS_TSTAMP;
+
+		adapter->ptp_tx_skb = skb_get(skb);
+		if (adapter->hw.mac.type == e1000_82576)
+			schedule_work(&adapter->ptp_tx_work);
 	}
 #endif /* CONFIG_IGB_PTP */
 
@@ -4859,6 +4872,19 @@ static irqreturn_t igb_msix_other(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
+#ifdef CONFIG_IGB_PTP
+	if (icr & E1000_ICR_TS) {
+		u32 tsicr = rd32(E1000_TSICR);
+
+		if (tsicr & E1000_TSICR_TXTS) {
+			/* acknowledge the interrupt */
+			wr32(E1000_TSICR, E1000_TSICR_TXTS);
+			/* retrieve hardware timestamp */
+			schedule_work(&adapter->ptp_tx_work);
+		}
+	}
+#endif /* CONFIG_IGB_PTP */
+
 	wr32(E1000_EIMS, adapter->eims_other);
 
 	return IRQ_HANDLED;
@@ -5650,6 +5676,19 @@ static irqreturn_t igb_intr_msi(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
+#ifdef CONFIG_IGB_PTP
+	if (icr & E1000_ICR_TS) {
+		u32 tsicr = rd32(E1000_TSICR);
+
+		if (tsicr & E1000_TSICR_TXTS) {
+			/* acknowledge the interrupt */
+			wr32(E1000_TSICR, E1000_TSICR_TXTS);
+			/* retrieve hardware timestamp */
+			schedule_work(&adapter->ptp_tx_work);
+		}
+	}
+#endif /* CONFIG_IGB_PTP */
+
 	napi_schedule(&q_vector->napi);
 
 	return IRQ_HANDLED;
@@ -5691,6 +5730,19 @@ static irqreturn_t igb_intr(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
+#ifdef CONFIG_IGB_PTP
+	if (icr & E1000_ICR_TS) {
+		u32 tsicr = rd32(E1000_TSICR);
+
+		if (tsicr & E1000_TSICR_TXTS) {
+			/* acknowledge the interrupt */
+			wr32(E1000_TSICR, E1000_TSICR_TXTS);
+			/* retrieve hardware timestamp */
+			schedule_work(&adapter->ptp_tx_work);
+		}
+	}
+#endif /* CONFIG_IGB_PTP */
+
 	napi_schedule(&q_vector->napi);
 
 	return IRQ_HANDLED;
@@ -5794,11 +5846,6 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
 		total_bytes += tx_buffer->bytecount;
 		total_packets += tx_buffer->gso_segs;
 
-#ifdef CONFIG_IGB_PTP
-		/* retrieve hardware timestamp */
-		igb_ptp_tx_hwtstamp(q_vector, tx_buffer);
-#endif /* CONFIG_IGB_PTP */
-
 		/* free the skb */
 		dev_kfree_skb_any(tx_buffer->skb);
 		tx_buffer->skb = NULL;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index e69555f..d57060c 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -289,6 +289,31 @@ static int igb_ptp_enable(struct ptp_clock_info *ptp,
 	return -EOPNOTSUPP;
 }
 
+/**
+ * igb_ptp_tx_work
+ * @work: pointer to work struct
+ *
+ * This work function polls the TSYNCTXCTL valid bit to determine when a
+ * timestamp has been taken for the current stored skb.
+ */
+void igb_ptp_tx_work(struct work_struct *work)
+{
+	struct igb_adapter *adapter = container_of(work, struct igb_adapter,
+						   ptp_tx_work);
+	struct e1000_hw *hw = &adapter->hw;
+	u32 tsynctxctl;
+
+	if (!adapter->ptp_tx_skb)
+		return;
+
+	tsynctxctl = rd32(E1000_TSYNCTXCTL);
+	if (tsynctxctl & E1000_TSYNCTXCTL_VALID)
+		igb_ptp_tx_hwtstamp(adapter);
+	else
+		/* reschedule to check later */
+		schedule_work(&adapter->ptp_tx_work);
+}
+
 static void igb_ptp_overflow_check(struct work_struct *work)
 {
 	struct igb_adapter *igb =
@@ -305,31 +330,25 @@ static void igb_ptp_overflow_check(struct work_struct *work)
 
 /**
  * igb_ptp_tx_hwtstamp - utility function which checks for TX time stamp
- * @q_vector: pointer to q_vector containing needed info
- * @buffer: pointer to igb_tx_buffer structure
+ * @adapter: Board private structure.
  *
  * If we were asked to do hardware stamping and such a time stamp is
  * available, then it must have been for this skb here because we only
  * allow only one such packet into the queue.
  */
-void igb_ptp_tx_hwtstamp(struct igb_q_vector *q_vector,
-			 struct igb_tx_buffer *buffer_info)
+void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter)
 {
-	struct igb_adapter *adapter = q_vector->adapter;
 	struct e1000_hw *hw = &adapter->hw;
 	struct skb_shared_hwtstamps shhwtstamps;
 	u64 regval;
 
-	/* if skb does not support hw timestamp or TX stamp not valid exit */
-	if (likely(!(buffer_info->tx_flags & IGB_TX_FLAGS_TSTAMP)) ||
-	    !(rd32(E1000_TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID))
-		return;
-
 	regval = rd32(E1000_TXSTMPL);
 	regval |= (u64)rd32(E1000_TXSTMPH) << 32;
 
 	igb_ptp_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
-	skb_tstamp_tx(buffer_info->skb, &shhwtstamps);
+	skb_tstamp_tx(adapter->ptp_tx_skb, &shhwtstamps);
+	dev_kfree_skb_any(adapter->ptp_tx_skb);
+	adapter->ptp_tx_skb = NULL;
 }
 
 void igb_ptp_rx_hwtstamp(struct igb_q_vector *q_vector,
@@ -603,16 +622,26 @@ void igb_ptp_init(struct igb_adapter *adapter)
 
 	spin_lock_init(&adapter->tmreg_lock);
 
+	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
+
 	schedule_delayed_work(&adapter->ptp_overflow_work,
 			      IGB_SYSTIM_OVERFLOW_PERIOD);
 
+	/* Initialize the time sync interrupts for devices that support it. */
+	if (hw->mac.type >= e1000_82580) {
+		wr32(E1000_TSIM, E1000_TSIM_TXTS);
+		wr32(E1000_IMS, E1000_IMS_TS);
+	}
+
 	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
-	} else
+	} else {
 		dev_info(&adapter->pdev->dev, "added PHC on %s\n",
 			 adapter->netdev->name);
+		adapter->flags |= IGB_FLAG_PTP;
+	}
 }
 
 /**
@@ -624,20 +653,61 @@ void igb_ptp_init(struct igb_adapter *adapter)
 void igb_ptp_stop(struct igb_adapter *adapter)
 {
 	switch (adapter->hw.mac.type) {
-	case e1000_i211:
-	case e1000_i210:
-	case e1000_i350:
-	case e1000_82580:
 	case e1000_82576:
+	case e1000_82580:
+	case e1000_i350:
 		cancel_delayed_work_sync(&adapter->ptp_overflow_work);
 		break;
+	case e1000_i210:
+	case e1000_i211:
+		/* No delayed work to cancel. */
+		break;
 	default:
 		return;
 	}
 
+	cancel_work_sync(&adapter->ptp_tx_work);
+
 	if (adapter->ptp_clock) {
 		ptp_clock_unregister(adapter->ptp_clock);
 		dev_info(&adapter->pdev->dev, "removed PHC on %s\n",
 			 adapter->netdev->name);
+		adapter->flags &= ~IGB_FLAG_PTP;
 	}
 }
+
+/**
+ * igb_ptp_reset - Re-enable the adapter for PTP following a reset.
+ * @adapter: Board private structure.
+ *
+ * This function handles the reset work required to re-enable the PTP device.
+ **/
+void igb_ptp_reset(struct igb_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+
+	if (!(adapter->flags & IGB_FLAG_PTP))
+		return;
+
+	switch (adapter->hw.mac.type) {
+	case e1000_82576:
+		/* Dial the nominal frequency. */
+		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
+		break;
+	case e1000_82580:
+	case e1000_i350:
+	case e1000_i210:
+	case e1000_i211:
+		/* Enable the timer functions and interrupts. */
+		wr32(E1000_TSAUXC, 0x0);
+		wr32(E1000_TSIM, E1000_TSIM_TXTS);
+		wr32(E1000_IMS, E1000_IMS_TS);
+		break;
+	default:
+		/* No work to do. */
+		return;
+	}
+
+	timecounter_init(&adapter->tc, &adapter->cc,
+			 ktime_to_ns(ktime_get_real()));
+}
-- 
1.7.11.4

[net-next 6/6] igb: Add 1588 support to I210/I211.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Previously I210/I211 followed the same code flow as 82580/I350 for 1588.
However, since the register sets have changed, we must update the
implementation to accommodate the register changes.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_defines.h |   3 +
 drivers/net/ethernet/intel/igb/igb_ptp.c       | 216 +++++++++++++++++++------
 2 files changed, 174 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 0b27e8f..cae3070 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -322,6 +322,9 @@
 #define E1000_FCRTC_RTH_COAL_SHIFT      4
 #define E1000_PCIEMISC_LX_DECISION      0x00000080 /* Lx power decision */
 
+/* Timestamp in Rx buffer */
+#define E1000_RXPBS_CFG_TS_EN           0x80000000
+
 /* SerDes Control */
 #define E1000_SCTL_DISABLE_SERDES_LOOPBACK 0x0400
 
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index d57060c..e13ba1d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -121,6 +121,41 @@ static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 	return val;
 }
 
+/*
+ * SYSTIM read access for I210/I211
+ */
+
+static void igb_ptp_read_i210(struct igb_adapter *adapter, struct timespec *ts)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	u32 sec, nsec, jk;
+
+	/*
+	 * The timestamp latches on lowest register read. For I210/I211, the
+	 * lowest register is SYSTIMR. Since we only need to provide nanosecond
+	 * resolution, we can ignore it.
+	 */
+	jk = rd32(E1000_SYSTIMR);
+	nsec = rd32(E1000_SYSTIML);
+	sec = rd32(E1000_SYSTIMH);
+
+	ts->tv_sec = sec;
+	ts->tv_nsec = nsec;
+}
+
+static void igb_ptp_write_i210(struct igb_adapter *adapter,
+			       const struct timespec *ts)
+{
+	struct e1000_hw *hw = &adapter->hw;
+
+	/*
+	 * Writing the SYSTIMR register is not necessary as it only provides
+	 * sub-nanosecond resolution.
+	 */
+	wr32(E1000_SYSTIML, ts->tv_nsec);
+	wr32(E1000_SYSTIMH, ts->tv_sec);
+}
+
 /**
  * igb_ptp_systim_to_hwtstamp - convert system time value to hw timestamp
  * @adapter: board private structure
@@ -146,24 +181,28 @@ static void igb_ptp_systim_to_hwtstamp(struct igb_adapter *adapter,
 	u64 ns;
 
 	switch (adapter->hw.mac.type) {
+	case e1000_82576:
+	case e1000_82580:
+	case e1000_i350:
+		spin_lock_irqsave(&adapter->tmreg_lock, flags);
+
+		ns = timecounter_cyc2time(&adapter->tc, systim);
+
+		spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
+
+		memset(hwtstamps, 0, sizeof(*hwtstamps));
+		hwtstamps->hwtstamp = ns_to_ktime(ns);
+		break;
 	case e1000_i210:
 	case e1000_i211:
-	case e1000_i350:
-	case e1000_82580:
-	case e1000_82576:
+		memset(hwtstamps, 0, sizeof(*hwtstamps));
+		/* Upper 32 bits contain s, lower 32 bits contain ns. */
+		hwtstamps->hwtstamp = ktime_set(systim >> 32,
+						systim & 0xFFFFFFFF);
 		break;
 	default:
-		return;
+		break;
 	}
-
-	spin_lock_irqsave(&adapter->tmreg_lock, flags);
-
-	ns = timecounter_cyc2time(&adapter->tc, systim);
-
-	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
-
-	memset(hwtstamps, 0, sizeof(*hwtstamps));
-	hwtstamps->hwtstamp = ns_to_ktime(ns);
 }
 
 /*
@@ -225,7 +264,7 @@ static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
 	return 0;
 }
 
-static int igb_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+static int igb_ptp_adjtime_82576(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
@@ -243,7 +282,26 @@ static int igb_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return 0;
 }
 
-static int igb_ptp_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
+static int igb_ptp_adjtime_i210(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	unsigned long flags;
+	struct timespec now, then = ns_to_timespec(delta);
+
+	spin_lock_irqsave(&igb->tmreg_lock, flags);
+
+	igb_ptp_read_i210(igb, &now);
+	now = timespec_add(now, then);
+	igb_ptp_write_i210(igb, (const struct timespec *)&now);
+
+	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+
+	return 0;
+}
+
+static int igb_ptp_gettime_82576(struct ptp_clock_info *ptp,
+				 struct timespec *ts)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
@@ -263,8 +321,24 @@ static int igb_ptp_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
 	return 0;
 }
 
-static int igb_ptp_settime(struct ptp_clock_info *ptp,
-			   const struct timespec *ts)
+static int igb_ptp_gettime_i210(struct ptp_clock_info *ptp,
+				struct timespec *ts)
+{
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	unsigned long flags;
+
+	spin_lock_irqsave(&igb->tmreg_lock, flags);
+
+	igb_ptp_read_i210(igb, ts);
+
+	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+
+	return 0;
+}
+
+static int igb_ptp_settime_82576(struct ptp_clock_info *ptp,
+				 const struct timespec *ts)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
@@ -283,6 +357,22 @@ static int igb_ptp_settime(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
+				const struct timespec *ts)
+{
+	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
+					       ptp_caps);
+	unsigned long flags;
+
+	spin_lock_irqsave(&igb->tmreg_lock, flags);
+
+	igb_ptp_write_i210(igb, ts);
+
+	spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+
+	return 0;
+}
+
 static int igb_ptp_enable(struct ptp_clock_info *ptp,
 			  struct ptp_clock_request *rq, int on)
 {
@@ -320,7 +410,7 @@ static void igb_ptp_overflow_check(struct work_struct *work)
 		container_of(work, struct igb_adapter, ptp_overflow_work.work);
 	struct timespec ts;
 
-	igb_ptp_gettime(&igb->ptp_caps, &ts);
+	igb->ptp_caps.gettime(&igb->ptp_caps, &ts);
 
 	pr_debug("igb overflow check at %ld.%09lu\n", ts.tv_sec, ts.tv_nsec);
 
@@ -506,6 +596,13 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
 	if ((hw->mac.type >= e1000_82580) && tsync_rx_ctl) {
 		tsync_rx_ctl = E1000_TSYNCRXCTL_ENABLED;
 		tsync_rx_ctl |= E1000_TSYNCRXCTL_TYPE_ALL;
+
+		if ((hw->mac.type == e1000_i210) ||
+		    (hw->mac.type == e1000_i211)) {
+			regval = rd32(E1000_RXPBS);
+			regval |= E1000_RXPBS_CFG_TS_EN;
+			wr32(E1000_RXPBS, regval);
+		}
 	}
 
 	/* enable/disable TX */
@@ -556,7 +653,9 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
 	wrfl();
 
 	/* clear TX/RX time stamp registers, just to be sure */
+	regval = rd32(E1000_TXSTMPL);
 	regval = rd32(E1000_TXSTMPH);
+	regval = rd32(E1000_RXSTMPL);
 	regval = rd32(E1000_RXSTMPH);
 
 	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
@@ -569,19 +668,35 @@ void igb_ptp_init(struct igb_adapter *adapter)
 	struct net_device *netdev = adapter->netdev;
 
 	switch (hw->mac.type) {
-	case e1000_i210:
-	case e1000_i211:
-	case e1000_i350:
+	case e1000_82576:
+		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
+		adapter->ptp_caps.owner = THIS_MODULE;
+		adapter->ptp_caps.max_adj = 1000000000;
+		adapter->ptp_caps.n_ext_ts = 0;
+		adapter->ptp_caps.pps = 0;
+		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82576;
+		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
+		adapter->ptp_caps.gettime = igb_ptp_gettime_82576;
+		adapter->ptp_caps.settime = igb_ptp_settime_82576;
+		adapter->ptp_caps.enable = igb_ptp_enable;
+		adapter->cc.read = igb_ptp_read_82576;
+		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mult = 1;
+		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
+		/* Dial the nominal frequency. */
+		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
+		break;
 	case e1000_82580:
+	case e1000_i350:
 		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
-		adapter->ptp_caps.adjtime = igb_ptp_adjtime;
-		adapter->ptp_caps.gettime = igb_ptp_gettime;
-		adapter->ptp_caps.settime = igb_ptp_settime;
+		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
+		adapter->ptp_caps.gettime = igb_ptp_gettime_82576;
+		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_enable;
 		adapter->cc.read = igb_ptp_read_82580;
 		adapter->cc.mask = CLOCKSOURCE_MASK(IGB_NBITS_82580);
@@ -590,23 +705,20 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-	case e1000_82576:
+	case e1000_i210:
+	case e1000_i211:
 		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
-		adapter->ptp_caps.max_adj = 1000000000;
+		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
-		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82576;
-		adapter->ptp_caps.adjtime = igb_ptp_adjtime;
-		adapter->ptp_caps.gettime = igb_ptp_gettime;
-		adapter->ptp_caps.settime = igb_ptp_settime;
+		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
+		adapter->ptp_caps.gettime = igb_ptp_gettime_i210;
+		adapter->ptp_caps.settime = igb_ptp_settime_i210;
 		adapter->ptp_caps.enable = igb_ptp_enable;
-		adapter->cc.read = igb_ptp_read_82576;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
-		adapter->cc.mult = 1;
-		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
-		/* Dial the nominal frequency. */
-		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
+		/* Enable the timer functions by clearing bit 31. */
+		wr32(E1000_TSAUXC, 0x0);
 		break;
 	default:
 		adapter->ptp_clock = NULL;
@@ -615,17 +727,24 @@ void igb_ptp_init(struct igb_adapter *adapter)
 
 	wrfl();
 
-	timecounter_init(&adapter->tc, &adapter->cc,
-			 ktime_to_ns(ktime_get_real()));
+	spin_lock_init(&adapter->tmreg_lock);
+	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
 
-	INIT_DELAYED_WORK(&adapter->ptp_overflow_work, igb_ptp_overflow_check);
+	/* Initialize the clock and overflow work for devices that need it. */
+	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211)) {
+		struct timespec ts = ktime_to_timespec(ktime_get_real());
 
-	spin_lock_init(&adapter->tmreg_lock);
+		igb_ptp_settime_i210(&adapter->ptp_caps, &ts);
+	} else {
+		timecounter_init(&adapter->tc, &adapter->cc,
+				 ktime_to_ns(ktime_get_real()));
 
-	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
+		INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
+				  igb_ptp_overflow_check);
 
-	schedule_delayed_work(&adapter->ptp_overflow_work,
-			      IGB_SYSTIM_OVERFLOW_PERIOD);
+		schedule_delayed_work(&adapter->ptp_overflow_work,
+				      IGB_SYSTIM_OVERFLOW_PERIOD);
+	}
 
 	/* Initialize the time sync interrupts for devices that support it. */
 	if (hw->mac.type >= e1000_82580) {
@@ -708,6 +827,13 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 		return;
 	}
 
-	timecounter_init(&adapter->tc, &adapter->cc,
-			 ktime_to_ns(ktime_get_real()));
+	/* Re-initialize the timer. */
+	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211)) {
+		struct timespec ts = ktime_to_timespec(ktime_get_real());
+
+		igb_ptp_settime_i210(&adapter->ptp_caps, &ts);
+	} else {
+		timecounter_init(&adapter->tc, &adapter->cc,
+				 ktime_to_ns(ktime_get_real()));
+	}
 }
-- 
1.7.11.4

Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates

[Richard Cochran]

On Wed, Sep 05, 2012 at 04:35:00PM -0700, Jeff Kirsher wrote:
> 
> Matthew Vick (6):
>   igb: Tidy up wrapping for CONFIG_IGB_PTP.
>   igb: Update PTP function names/variables and locations.
>   igb: Correct PTP support query from ethtool.
>   igb: Store the MAC address in the name in the PTP struct.

These four still look like unnecessary churn to me, and possibly the
wrong direction WRT time stamping in the 82580.

>   igb: Prevent dropped Tx timestamps via work items and interrupts.
>   igb: Add 1588 support to I210/I211.

But these two look important, so I don't know what to say.

Sorry,
Richard

RE: [net-next 0/6][pull request] Intel Wired LAN Driver Updates

[Vick, Matthew]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Richard Cochran
> Sent: Thursday, September 06, 2012 1:13 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com
> Subject: Re: [net-next 0/6][pull request] Intel Wired LAN Driver
> Updates
> 
> On Wed, Sep 05, 2012 at 04:35:00PM -0700, Jeff Kirsher wrote:
> >
> > Matthew Vick (6):
> >   igb: Tidy up wrapping for CONFIG_IGB_PTP.
> >   igb: Update PTP function names/variables and locations.
> >   igb: Correct PTP support query from ethtool.
> >   igb: Store the MAC address in the name in the PTP struct.
> 
> These four still look like unnecessary churn to me, and possibly the
> wrong direction WRT time stamping in the 82580.

(I apologize for the delayed response--I was out of the office. I've read through the discussions and will respond here in the interest of brevity.)

I still fail to see how they're churn. I'm making the code make sense for what is actually supported today. I understand that, theoretically, some of this work may be reverted if/when we pull out hardware timestamping as a separate feature from the PTP support in the driver, but I don't think we should leave it general because someone someday might get around to doing something with it. It's confusing and more difficult to maintain for no benefit. It should be part of the development effort to generalize and consolidate where possible when adding a feature.

> >   igb: Prevent dropped Tx timestamps via work items and interrupts.
> >   igb: Add 1588 support to I210/I211.
> 
> But these two look important, so I don't know what to say.
> 
> Sorry,
> Richard

Those two are definitely important. :)

Cheers,
Matthew

Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

On Wed, 2012-09-05 at 16:35 -0700, Kirsher, Jeffrey T wrote:
> This series contains updates to igb (specifically PTP code).
> 
> The following are changes since commit f6fe569fe056388166575af1cfaed0bcbc688305:
>   Revert "usbnet: drop unneeded check for NULL"
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
> 
> Matthew Vick (6):
>   igb: Tidy up wrapping for CONFIG_IGB_PTP.
>   igb: Update PTP function names/variables and locations.
>   igb: Correct PTP support query from ethtool.
>   igb: Store the MAC address in the name in the PTP struct.
>   igb: Prevent dropped Tx timestamps via work items and interrupts.
>   igb: Add 1588 support to I210/I211.
> 
>  drivers/net/ethernet/intel/igb/e1000_defines.h |   8 +
>  drivers/net/ethernet/intel/igb/e1000_regs.h    |   2 +
>  drivers/net/ethernet/intel/igb/igb.h           |  29 +-
>  drivers/net/ethernet/intel/igb/igb_ethtool.c   |  84 +--
>  drivers/net/ethernet/intel/igb/igb_main.c      | 329 +++---------
>  drivers/net/ethernet/intel/igb/igb_ptp.c       | 676 ++++++++++++++++++++-----
>  6 files changed, 708 insertions(+), 420 deletions(-)
> 
> --

Dave-
I see you have set this series to "Changes Requested" in patchworks, and
I am assuming that is from the discussion that occurred on patch 04 of
the series.  That discussion came to the conclusion that changes should
happen in the PTP core, and that the patch is fine as is currently.

If there was something else you want changed, let me/Matthew know so
that we can make the necessary changes.

Cheers,
Jeff

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 13 Sep 2012 14:05:59 -0700

> On Wed, 2012-09-05 at 16:35 -0700, Kirsher, Jeffrey T wrote:
>> This series contains updates to igb (specifically PTP code).
>> 
>> The following are changes since commit f6fe569fe056388166575af1cfaed0bcbc688305:
>>   Revert "usbnet: drop unneeded check for NULL"
>> and are available in the git repository at:
>>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
>> 
>> Matthew Vick (6):
>>   igb: Tidy up wrapping for CONFIG_IGB_PTP.
>>   igb: Update PTP function names/variables and locations.
>>   igb: Correct PTP support query from ethtool.
>>   igb: Store the MAC address in the name in the PTP struct.
>>   igb: Prevent dropped Tx timestamps via work items and interrupts.
>>   igb: Add 1588 support to I210/I211.
 ...
> I see you have set this series to "Changes Requested" in patchworks, and
> I am assuming that is from the discussion that occurred on patch 04 of
> the series.  That discussion came to the conclusion that changes should
> happen in the PTP core, and that the patch is fine as is currently.
> 
> If there was something else you want changed, let me/Matthew know so
> that we can make the necessary changes.

Thanks for updating me on this, pulled and pushed out.

[net-next 0/6][pull request] Intel Wired LAN Driver Updates

[Jeff Kirsher]

This series contains updates to igb, specifically the PTP code.
There have been no changes since the last pull request other than
the update to the git tree to the latest net-next.

There was discussion on patch 4 of the series which resulted in
a suggested change to the PTP core.  Once these get accepted,
Matthew Vick will continue on with the suggested changes made by
Richard Cochran, Ben Hutchings and Jacob Keller.
  
The following are changes since commit ba01dfe18241bf89b058fd8a60218b218ad2bb30:
  Merge branch 'for-davem' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Matthew Vick (6):
  igb: Tidy up wrapping for CONFIG_IGB_PTP.
  igb: Update PTP function names/variables and locations.
  igb: Correct PTP support query from ethtool.
  igb: Store the MAC address in the name in the PTP struct.
  igb: Prevent dropped Tx timestamps via work items and interrupts.
  igb: Add 1588 support to I210/I211.

 drivers/net/ethernet/intel/igb/e1000_defines.h |   8 +
 drivers/net/ethernet/intel/igb/e1000_regs.h    |   2 +
 drivers/net/ethernet/intel/igb/igb.h           |  29 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c   |  84 +--
 drivers/net/ethernet/intel/igb/igb_main.c      | 329 +++---------
 drivers/net/ethernet/intel/igb/igb_ptp.c       | 676 ++++++++++++++++++++-----
 6 files changed, 708 insertions(+), 420 deletions(-)

-- 
1.7.11.4

[net-next 4/6] igb: Store the MAC address in the name in the PTP struct.

[Jeff Kirsher]

From: Matthew Vick <matthew.vick@intel.com>

Change the name of the adapter in the PTP struct to enable easier
correlation between interface and PTP device.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Matthew Vick <matthew.vick@intel.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by:  Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 34e0d69..e69555f 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -547,14 +547,15 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev,
 void igb_ptp_init(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	struct net_device *netdev = adapter->netdev;
 
 	switch (hw->mac.type) {
 	case e1000_i210:
 	case e1000_i211:
 	case e1000_i350:
 	case e1000_82580:
+		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
-		strcpy(adapter->ptp_caps.name, "igb-82580");
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
@@ -570,10 +571,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-
 	case e1000_82576:
+		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
-		strcpy(adapter->ptp_caps.name, "igb-82576");
 		adapter->ptp_caps.max_adj = 1000000000;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
@@ -589,7 +589,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		/* Dial the nominal frequency. */
 		wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576);
 		break;
-
 	default:
 		adapter->ptp_clock = NULL;
 		return;
-- 
1.7.11.4