[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

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

[Song, Yoong Siang]

On Saturday, November 8, 2025 1:11 AM, Hore, Soumyadeep <soumyadeep.hore@intel.com> wrote:
>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.
>

Hi Soumyadeep,

Thank for your comment. I will reuse the existing code and resubmit v2.

Thanks & Regards
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