Skip to contentSkip to conversation

igb: ptp hardware clock

PATCH Superseded Richard Cochran 3 participants 6 messages Dec 26, 2011 – Jan 3, 2012

Subsystems (3)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover igb: ptp hardware clock

* ChangeLog V2
- Fixed wrong bit shifting in the 82576 code.
- Explained the timestamp locking with a comment in the code.
- Preserved the comments from the original timecompare implementation.
- Added an additional test within the overflow counter code to fix a race condition. Details of the problem are given in the commit message.

This patch series implements a PHC driver for the Intel 82576 and 82580 devices, as part of the igb driver.

The first patch adds the PHC driver code as a new source module but does not link it into the main igb driver. Because the system time counter is not so very wide, the code implements an overflow counter in software. Every read operation maintains the overflow counter, as does a "delayed work" watchdog. Only the base clock operations are implemented. The hardware does have some ancillary features, but these can be easily added later.

The second patch removes the timecompare code and links in the new functions.

I have tested the 82580 with good results. However, I don't have the 82576 and so would appreciate testing and feedback.

Thanks,
Richard

Richard Cochran (2):
igb: add PTP Hardware Clock code
igb: offer a PTP Hardware Clock instead of the timecompare method

 drivers/net/ethernet/intel/igb/Makefile   |    2 +-
 drivers/net/ethernet/intel/igb/igb.h      |   21 +-
 drivers/net/ethernet/intel/igb/igb_main.c |  167 +----------
 drivers/net/ethernet/intel/igb/igb_ptp.c  |  486 +++++++++++++++++++++++++++++
 4 files changed, 505 insertions(+), 171 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igb/igb_ptp.c
Signature · 1 line
1.7.2.5
#1 lore raw

PATCH v2 1/2 igb: add PTP Hardware Clock code

This patch adds a source file implementing a PHC. Only the basic clock operations have been implemented, although the hardware would offer some ancillary functions. The code is fairly self contained and is not yet used in the main igb driver.

Every timestamp and clock read operation must consult the overflow counter to form a correct time value. Access to the counter is protected by a spin lock, and this would be sufficient, assuming that the time values are monotonic.

However, this assumption does not hold in general. Consider the following sequence.

1. Hardware latches a receive timestamp (just before counter overflow).
2. User calls clock_gettime() (just after counter overflow).
3. User takes lock, detects 1-0 transition, and increments overflow count.
4. Driver takes lock, incorrectly combines overflow count with timestamp.

A very similar race condition exists between two nearly simultaneous hardware timestamps.

The implementation detects this race by checking the two most significant bits for a transition from 11b to 00b. When this pattern is detected, the code uses the previous value of the overflow count.

The two most significant bits divide the overflow period into four regions, and so the watchdog timeout is set to sample the counter at just over four times per period.

---
 drivers/net/ethernet/intel/igb/igb.h     |    8 +
 drivers/net/ethernet/intel/igb/igb_ptp.c |  420 ++++++++++++++++++++++++++++++
 2 files changed, 428 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igb/igb_ptp.c
drivers/net/ethernet/intel/igb/igb.h +8 -0
index c69feeb..f30458d 100644
@@ -37,6 +37,7 @@
 #include <linux/clocksource.h>
 #include <linux/timecompare.h>
 #include <linux/net_tstamp.h>
+#include <linux/ptp_clock_kernel.h>
 #include <linux/bitops.h>
 #include <linux/if_vlan.h>
 
@@ -364,6 +365,13 @@ struct igb_adapter {
 	u32 wvbr;
 	int node;
 	u32 *shadow_vfta;
+
+	struct ptp_clock *ptp_clock;
+	struct ptp_clock_info caps;
+	struct delayed_work overflow_work;
+	spinlock_t tmreg_lock;
+	u32 overflow_counter;
+	unsigned int last_msb;
 };
 
 #define IGB_FLAG_HAS_MSI           (1 << 0)
drivers/net/ethernet/intel/igb/igb_ptp.c new file +420 -0

420 lines not on this page · raw message

Signature · 14 lines
1.7.2.5


------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired
#2 lore raw

PATCH v2 2/2 igb: offer a PTP Hardware Clock instead of the timecompare method

In reply to Richard Cochran

This commit removes the legacy timecompare code from the igb driver and offers a tunable PHC instead.

---
 drivers/net/ethernet/intel/igb/Makefile   |    2 +-
 drivers/net/ethernet/intel/igb/igb.h      |   13 ++-
 drivers/net/ethernet/intel/igb/igb_main.c |  167 +----------------------------
 drivers/net/ethernet/intel/igb/igb_ptp.c  |   66 +++++++++++
 4 files changed, 77 insertions(+), 171 deletions(-)
drivers/net/ethernet/intel/igb/Makefile +1 -1
index c6e4621..42f0868 100644
@@ -32,6 +32,6 @@
 
 obj-$(CONFIG_IGB) += igb.o
 
-igb-objs := igb_main.o igb_ethtool.o e1000_82575.o \
+igb-objs := igb_main.o igb_ethtool.o igb_ptp.o e1000_82575.o \
 	    e1000_mac.o e1000_nvm.o e1000_phy.o e1000_mbx.o
 
drivers/net/ethernet/intel/igb/igb.h +7 -6
index f30458d..d554d83 100644
@@ -34,8 +34,6 @@
 #include "e1000_mac.h"
 #include "e1000_82575.h"
 
-#include <linux/clocksource.h>
-#include <linux/timecompare.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/bitops.h>
@@ -329,9 +327,6 @@ struct igb_adapter {
 
 	/* OS defined structs */
 	struct pci_dev *pdev;
-	struct cyclecounter cycles;
-	struct timecounter clock;
-	struct timecompare compare;
 	struct hwtstamp_config hwtstamp_config;
 
 	spinlock_t stats64_lock;
@@ -386,7 +381,6 @@ struct igb_adapter {
 #define IGB_DMCTLX_DCFLUSH_DIS     0x80000000  /* Disable DMA Coal Flush */
 
 #define IGB_82576_TSYNC_SHIFT 19
-#define IGB_82580_TSYNC_SHIFT 24
 #define IGB_TS_HDR_LEN        16
 enum e1000_state_t {
 	__IGB_TESTING,
@@ -423,6 +417,13 @@ extern bool igb_has_link(struct igb_adapter *adapter);
 extern void igb_set_ethtool_ops(struct net_device *);
 extern void igb_power_up_link(struct igb_adapter *);
 
+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);
+
 static inline s32 igb_reset_phy(struct e1000_hw *hw)
 {
 	if (hw->phy.ops.reset)
drivers/net/ethernet/intel/igb/igb_main.c +3 -164

203 lines not on this page · raw message

drivers/net/ethernet/intel/igb/igb_ptp.c +66 -0

87 lines not on this page · raw message

Signature · 14 lines
1.7.2.5


------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired
#3 lore raw
In reply to Richard Cochran
Quoted 33 lines · Richard Cochran

* ChangeLog V2
- Fixed wrong bit shifting in the 82576 code.
- Explained the timestamp locking with a comment in the code.
- Preserved the comments from the original timecompare implementation.
- Added an additional test within the overflow counter code to fix a race condition. Details of the problem are given in the commit message.

This patch series implements a PHC driver for the Intel 82576 and 82580 devices, as part of the igb driver.

The first patch adds the PHC driver code as a new source module but does not link it into the main igb driver. Because the system time counter is not so very wide, the code implements an overflow counter in software. Every read operation maintains the overflow counter, as does a "delayed work" watchdog. Only the base clock operations are implemented. The hardware does have some ancillary features, but these can be easily added later.

The second patch removes the timecompare code and links in the new functions.

I have tested the 82580 with good results. However, I don't have the 82576 and so would appreciate testing and feedback.

Thanks,
Richard

Richard Cochran (2):
igb: add PTP Hardware Clock code
igb: offer a PTP Hardware Clock instead of the timecompare method

Quoted 7 lines
 drivers/net/ethernet/intel/igb/Makefile   |    2 +-
 drivers/net/ethernet/intel/igb/igb.h      |   21 +-
 drivers/net/ethernet/intel/igb/igb_main.c |  167 +----------
 drivers/net/ethernet/intel/igb/igb_ptp.c  |  486 +++++++++++++++++++++++++++++
 4 files changed, 505 insertions(+), 171 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igb/igb_ptp.c

Thanks Richard, I have added this series to my queue.

#4 lore raw
In reply to Richard Cochran
Quoted 3 lines

I have tested the 82580 with good results. However, I don't have the 82576 and so would appreciate testing and feedback.

Richard,

We'll test this on an 82576 but that won't happen until next week we staff is back from the holidays.

Cheers,
John

#5 lore raw
In reply to Richard Cochran
Quoted 33 lines · Richard Cochran

* ChangeLog V2
- Fixed wrong bit shifting in the 82576 code.
- Explained the timestamp locking with a comment in the code.
- Preserved the comments from the original timecompare implementation.
- Added an additional test within the overflow counter code to fix a race condition. Details of the problem are given in the commit message.

This patch series implements a PHC driver for the Intel 82576 and 82580 devices, as part of the igb driver.

The first patch adds the PHC driver code as a new source module but does not link it into the main igb driver. Because the system time counter is not so very wide, the code implements an overflow counter in software. Every read operation maintains the overflow counter, as does a "delayed work" watchdog. Only the base clock operations are implemented. The hardware does have some ancillary features, but these can be easily added later.

The second patch removes the timecompare code and links in the new functions.

I have tested the 82580 with good results. However, I don't have the 82576 and so would appreciate testing and feedback.

Thanks,
Richard

Richard Cochran (2):
igb: add PTP Hardware Clock code
igb: offer a PTP Hardware Clock instead of the timecompare method

Quoted 7 lines
 drivers/net/ethernet/intel/igb/Makefile   |    2 +-
 drivers/net/ethernet/intel/igb/igb.h      |   21 +-
 drivers/net/ethernet/intel/igb/igb_main.c |  167 +----------
 drivers/net/ethernet/intel/igb/igb_ptp.c  |  486 +++++++++++++++++++++++++++++
 4 files changed, 505 insertions(+), 171 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igb/igb_ptp.c

We found an issue with this series of patches, if we did not have CONFIG_PTP_1588_CLOCK enabled in the kernel, the igb driver would not compile.

So I would suggest wrapping the portions of code with #ifdef CONFIG_PTP_1588_CLOCK.

I will await v3 of the series, thanks Richard for this work!