[PATCH v1 1/1] net/e1000: use device timestamp for igc read_clock() operation

[PATCH v1 1/1] net/e1000: use device timestamp for igc read_clock() operation

[Song Yoong Siang]

Change eth_igc_read_clock() to read from hardware timestamp registers
(E1000_SYSTIML/E1000_SYSTIMH) instead of using system clock_gettime().

This ensures that the clock reading is consistent with the hardware's
internal time base used for Qbv cycle and launch time scheduling,
providing better accuracy for Time-Sensitive Networking applications.

Fixes: 9630f7c71ecd ("net/igc: enable launch time offloading")
Cc: stable@dpdk.org

Signed-off-by: David Zage <david.zage@intel.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
---
 drivers/net/intel/e1000/igc_ethdev.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/intel/e1000/igc_ethdev.c b/drivers/net/intel/e1000/igc_ethdev.c
index b9c91d2446..045f7c784d 100644
--- a/drivers/net/intel/e1000/igc_ethdev.c
+++ b/drivers/net/intel/e1000/igc_ethdev.c
@@ -2972,10 +2972,18 @@ eth_igc_timesync_disable(struct rte_eth_dev *dev)
 static int
 eth_igc_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t *clock)
 {
-	struct timespec system_time;
+	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
+	uint32_t nsec, sec;
 
-	clock_gettime(CLOCK_REALTIME, &system_time);
-	*clock = system_time.tv_sec * NSEC_PER_SEC + system_time.tv_nsec;
+	/*
+	 * Reading the SYSTIML register latches the upper 32 bits to the SYSTIMH
+	 * shadow register for coherent access. As long as we read SYSTIML first
+	 * followed by SYSTIMH, we avoid race conditions where the time rolls
+	 * over between the two register reads.
+	 */
+	nsec = E1000_READ_REG(hw, E1000_SYSTIML);
+	sec = E1000_READ_REG(hw, E1000_SYSTIMH);
+	*clock = (uint64_t)sec * NSEC_PER_SEC + (uint64_t)nsec;
 
 	return 0;
 }
-- 
2.48.1

RE: [PATCH v1 1/1] net/e1000: use device timestamp for igc read_clock() operation

[Hore, Soumyadeep]

Hi Song, on closer inspection I see the code implementation is similar to eth_igc_timesync_read_time().

Please reuse the existing code as much as possible.

Change eth_igc_read_clock() to read from hardware timestamp registers
(E1000_SYSTIML/E1000_SYSTIMH) instead of using system clock_gettime().

This ensures that the clock reading is consistent with the hardware's internal time base used for Qbv cycle and launch time scheduling, providing better accuracy for Time-Sensitive Networking applications.

Fixes: 9630f7c71ecd ("net/igc: enable launch time offloading")
Cc: stable@dpdk.org

Signed-off-by: David Zage <david.zage@intel.com>
Signed-off-by: Song Yoong Siang <yoong.siang.song@intel.com>
---
 drivers/net/intel/e1000/igc_ethdev.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/intel/e1000/igc_ethdev.c b/drivers/net/intel/e1000/igc_ethdev.c
index b9c91d2446..045f7c784d 100644
--- a/drivers/net/intel/e1000/igc_ethdev.c
+++ b/drivers/net/intel/e1000/igc_ethdev.c
@@ -2972,10 +2972,18 @@ eth_igc_timesync_disable(struct rte_eth_dev *dev)  static int  eth_igc_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t *clock)  {
-	struct timespec system_time;
+	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
+	uint32_t nsec, sec;
 
-	clock_gettime(CLOCK_REALTIME, &system_time);
-	*clock = system_time.tv_sec * NSEC_PER_SEC + system_time.tv_nsec;
+	/*
+	 * Reading the SYSTIML register latches the upper 32 bits to the SYSTIMH
+	 * shadow register for coherent access. As long as we read SYSTIML first
+	 * followed by SYSTIMH, we avoid race conditions where the time rolls
+	 * over between the two register reads.
+	 */
+	nsec = E1000_READ_REG(hw, E1000_SYSTIML);
+	sec = E1000_READ_REG(hw, E1000_SYSTIMH);
+	*clock = (uint64_t)sec * NSEC_PER_SEC + (uint64_t)nsec;
 
 	return 0;
 }
--
2.48.1