[PATCH iwl-next] idpf: add cross timestamping

[PATCH iwl-next] idpf: add cross timestamping

[Milena Olech]

Add cross timestamp support through virtchnl mailbox messages and directly,
through PCIe BAR registers. Cross timestamping assumes that both system
time and device clock time values are cached simultaneously, what is
triggered by HW. Feature is enabled for both ARM and x86 archs.

Signed-off-by: Milena Olech <milena.olech@intel.com>
Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
 drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
 .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
 3 files changed, 210 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
index cb46185da749..f8d320875eb7 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
 							   direct,
 							   mailbox);
 
+	/* Get the cross timestamp */
+	direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
+	mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
+	ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
+							   direct,
+							   mailbox);
+
 	/* Set the device clock time */
 	direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
 	mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
@@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct idpf_adapter *adapter, u64 *src_clk,
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
+/**
+ * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
+ *					  directly
+ * @adapter: Driver specific private structure
+ * @dev_time: 64bit main timer value
+ * @sys_time: 64bit system time value
+ */
+static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter *adapter,
+						 u64 *dev_time, u64 *sys_time)
+{
+	u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
+	struct idpf_ptp *ptp = adapter->ptp;
+
+	spin_lock(&ptp->read_dev_clk_lock);
+
+	idpf_ptp_enable_shtime(adapter);
+
+	dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
+	dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
+
+	sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
+	sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
+
+	*dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
+	*sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
+
+	spin_unlock(&ptp->read_dev_clk_lock);
+}
+
+/**
+ * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
+ *					   through mailbox
+ * @adapter: Driver specific private structure
+ * @dev_time: 64bit main timer value expressed in nanoseconds
+ * @sys_time: 64bit system time value expressed in nanoseconds
+ *
+ * Return: a pair of cross timestamp values on success, -errno otherwise.
+ */
+static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
+						 u64 *dev_time, u64 *sys_time)
+{
+	struct idpf_ptp_dev_timers cross_time;
+	int err;
+
+	err = idpf_ptp_get_cross_time(adapter, &cross_time);
+	if (err)
+		return err;
+
+	*dev_time = cross_time.dev_clk_time_ns;
+	*sys_time = cross_time.sys_time_ns;
+
+	return err;
+}
+
+/**
+ * idpf_ptp_get_sync_device_time - Get the cross time stamp info
+ * @device: Current device time
+ * @system: System counter value read synchronously with device time
+ * @ctx: Context provided by timekeeping code
+ *
+ * Return: the device and the system clocks time read simultaneously on success,
+ * -errno otherwise.
+ */
+static int idpf_ptp_get_sync_device_time(ktime_t *device,
+					 struct system_counterval_t *system,
+					 void *ctx)
+{
+	struct idpf_adapter *adapter = ctx;
+	u64 ns_time_dev, ns_time_sys;
+	int err;
+
+	switch (adapter->ptp->get_cross_tstamp_access) {
+	case IDPF_PTP_NONE:
+		return -EOPNOTSUPP;
+	case IDPF_PTP_DIRECT:
+		idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
+						     &ns_time_sys);
+		break;
+	case IDPF_PTP_MAILBOX:
+		err = idpf_ptp_get_sync_device_time_mailbox(adapter,
+							    &ns_time_dev,
+							    &ns_time_sys);
+		if (err)
+			return err;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	*device = ns_to_ktime(ns_time_dev);
+
+#if IS_ENABLED(CONFIG_X86)
+	system->cs_id = CSID_X86_ART;
+#else
+	system->cs_id = CSID_ARM_ARCH_COUNTER;
+#endif /* CONFIG_X86 */
+	system->cycles = ns_time_sys;
+	system->use_nsecs = true;
+
+	return 0;
+}
+
+/**
+ * idpf_ptp_get_crosststamp - Capture a device cross timestamp
+ * @info: the driver's PTP info structure
+ * @cts: The memory to fill the cross timestamp info
+ *
+ * Capture a cross timestamp between the system time and the device PTP hardware
+ * clock.
+ *
+ * Return: cross timestamp value on success, -errno on failure.
+ */
+static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
+				    struct system_device_crosststamp *cts)
+{
+	struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
+
+	return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
+					     adapter, NULL, cts);
+}
+#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
+
 /**
  * idpf_ptp_gettimex64 - Get the time of the clock
  * @info: the driver's PTP info structure
@@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter *adapter)
 	info->verify = idpf_ptp_verify_pin;
 	info->enable = idpf_ptp_gpio_enable;
 	info->do_aux_work = idpf_ptp_do_aux_work;
+#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
+	info->getcrosststamp = idpf_ptp_get_crosststamp;
+#elif IS_ENABLED(CONFIG_X86)
+	if (pcie_ptm_enabled(adapter->pdev) &&
+	    boot_cpu_has(X86_FEATURE_ART) &&
+	    boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
+		info->getcrosststamp = idpf_ptp_get_crosststamp;
+#endif /* CONFIG_ARM_ARCH_TIMER */
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
index a876749d6116..cd19f65f9fff 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
@@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
  * @dev_clk_ns_h: high part of the device clock register
  * @phy_clk_ns_l: low part of the PHY clock register
  * @phy_clk_ns_h: high part of the PHY clock register
+ * @sys_time_ns_l: low part of the system time register
+ * @sys_time_ns_h: high part of the system time register
  * @incval_l: low part of the increment value register
  * @incval_h: high part of the increment value register
  * @shadj_l: low part of the shadow adjust register
@@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
 	void __iomem *phy_clk_ns_l;
 	void __iomem *phy_clk_ns_h;
 
+	/* System time */
+	void __iomem *sys_time_ns_l;
+	void __iomem *sys_time_ns_h;
+
 	/* Main timer adjustments */
 	void __iomem *incval_l;
 	void __iomem *incval_h;
@@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
  * @dev_clk_regs: the set of registers to access the device clock
  * @caps: PTP capabilities negotiated with the Control Plane
  * @get_dev_clk_time_access: access type for getting the device clock time
+ * @get_cross_tstamp_access: access type for the cross timestamping
  * @set_dev_clk_time_access: access type for setting the device clock time
  * @adj_dev_clk_time_access: access type for the adjusting the device clock
  * @tx_tstamp_access: access type for the Tx timestamp value read
@@ -182,10 +189,11 @@ struct idpf_ptp {
 	struct idpf_ptp_dev_clk_regs dev_clk_regs;
 	u32 caps;
 	enum idpf_ptp_access get_dev_clk_time_access:2;
+	enum idpf_ptp_access get_cross_tstamp_access:2;
 	enum idpf_ptp_access set_dev_clk_time_access:2;
 	enum idpf_ptp_access adj_dev_clk_time_access:2;
 	enum idpf_ptp_access tx_tstamp_access:2;
-	u8 rsv;
+	u8 rsv:6;
 	struct idpf_ptp_secondary_mbx secondary_mbx;
 	spinlock_t read_dev_clk_lock;
 };
@@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
 bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
 int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
 			      struct idpf_ptp_dev_timers *dev_clk_time);
+int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
+			    struct idpf_ptp_dev_timers *cross_time);
 int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
 int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
 int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
@@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
 	return -EOPNOTSUPP;
 }
 
+static inline int
+idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
+			struct idpf_ptp_dev_timers *cross_time)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
 					    u64 time)
 {
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
index bdcc54a5fb56..4f1fb0cefe51 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
@@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 		.send_buf.iov_len = sizeof(send_ptp_caps_msg),
 		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
 	};
+	struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
 	struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
 	struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
 	struct idpf_ptp_secondary_mbx *scnd_mbx;
@@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 
 	access_type = ptp->get_dev_clk_time_access;
 	if (access_type != IDPF_PTP_DIRECT)
-		goto discipline_clock;
+		goto cross_tstamp;
 
 	clock_offsets = recv_ptp_caps_msg->clk_offsets;
 
@@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
 	temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
 	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
 
+cross_tstamp:
+	access_type = ptp->get_cross_tstamp_access;
+	if (access_type != IDPF_PTP_DIRECT)
+		goto discipline_clock;
+
+	cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
+
+	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
+	ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
+							    temp_offset);
+	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
+	ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
+							    temp_offset);
+	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
+	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
+
 discipline_clock:
 	access_type = ptp->adj_dev_clk_time_access;
 	if (access_type != IDPF_PTP_DIRECT)
@@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
 	return 0;
 }
 
+/**
+ * idpf_ptp_get_cross_time - Send virtchnl get cross time message
+ * @adapter: Driver specific private structure
+ * @cross_time: Pointer to the device clock structure where the value is set
+ *
+ * Send virtchnl get cross time message to get the time of the clock and the
+ * system time.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
+			    struct idpf_ptp_dev_timers *cross_time)
+{
+	struct virtchnl2_ptp_get_cross_time cross_time_msg;
+	struct idpf_vc_xn_params xn_params = {
+		.vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
+		.send_buf.iov_base = &cross_time_msg,
+		.send_buf.iov_len = sizeof(cross_time_msg),
+		.recv_buf.iov_base = &cross_time_msg,
+		.recv_buf.iov_len = sizeof(cross_time_msg),
+		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
+	};
+	int reply_sz;
+
+	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
+	if (reply_sz < 0)
+		return reply_sz;
+	if (reply_sz != sizeof(cross_time_msg))
+		return -EIO;
+
+	cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
+	cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
+
+	return 0;
+}
+
 /**
  * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
  * @adapter: Driver specific private structure
-- 
2.43.5

Re: [PATCH iwl-next] idpf: add cross timestamping

[Willem de Bruijn]

Milena Olech wrote:
> Add cross timestamp support through virtchnl mailbox messages and directly,
> through PCIe BAR registers. Cross timestamping assumes that both system
> time and device clock time values are cached simultaneously, what is
> triggered by HW. Feature is enabled for both ARM and x86 archs.
> 
> Signed-off-by: Milena Olech <milena.olech@intel.com>
> Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [Intel-wired-lan] [PATCH iwl-next] idpf: add cross timestamping

[Paul Menzel]

Dear Milena,


Thank you for your patch.


Am 23.05.25 um 17:58 schrieb Milena Olech:
> Add cross timestamp support through virtchnl mailbox messages and directly,
> through PCIe BAR registers. Cross timestamping assumes that both system
> time and device clock time values are cached simultaneously, what is
> triggered by HW. Feature is enabled for both ARM and x86 archs.

How can this be tested?

> Signed-off-by: Milena Olech <milena.olech@intel.com>
> Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
>   drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
>   3 files changed, 210 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> index cb46185da749..f8d320875eb7 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> @@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
>   							   direct,
>   							   mailbox);
>   
> +	/* Get the cross timestamp */
> +	direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
> +	mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
> +	ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
> +							   direct,
> +							   mailbox);
> +
>   	/* Set the device clock time */
>   	direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>   	mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
> @@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct idpf_adapter *adapter, u64 *src_clk,
>   	return 0;
>   }
>   
> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
> +/**
> + * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
> + *					  directly
> + * @adapter: Driver specific private structure
> + * @dev_time: 64bit main timer value
> + * @sys_time: 64bit system time value
> + */
> +static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter *adapter,
> +						 u64 *dev_time, u64 *sys_time)
> +{
> +	u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
> +	struct idpf_ptp *ptp = adapter->ptp;
> +
> +	spin_lock(&ptp->read_dev_clk_lock);
> +
> +	idpf_ptp_enable_shtime(adapter);
> +
> +	dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
> +	dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
> +
> +	sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
> +	sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
> +
> +	*dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
> +	*sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
> +
> +	spin_unlock(&ptp->read_dev_clk_lock);
> +}
> +
> +/**
> + * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
> + *					   through mailbox
> + * @adapter: Driver specific private structure
> + * @dev_time: 64bit main timer value expressed in nanoseconds
> + * @sys_time: 64bit system time value expressed in nanoseconds
> + *
> + * Return: a pair of cross timestamp values on success, -errno otherwise.

0 is returned on success?

> + */
> +static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
> +						 u64 *dev_time, u64 *sys_time)
> +{
> +	struct idpf_ptp_dev_timers cross_time;
> +	int err;
> +
> +	err = idpf_ptp_get_cross_time(adapter, &cross_time);
> +	if (err)
> +		return err;
> +
> +	*dev_time = cross_time.dev_clk_time_ns;
> +	*sys_time = cross_time.sys_time_ns;
> +
> +	return err;
> +}
> +
> +/**
> + * idpf_ptp_get_sync_device_time - Get the cross time stamp info
> + * @device: Current device time
> + * @system: System counter value read synchronously with device time
> + * @ctx: Context provided by timekeeping code
> + *
> + * Return: the device and the system clocks time read simultaneously on success,
> + * -errno otherwise.

Also mention 0 return?

> + */
> +static int idpf_ptp_get_sync_device_time(ktime_t *device,
> +					 struct system_counterval_t *system,
> +					 void *ctx)
> +{
> +	struct idpf_adapter *adapter = ctx;
> +	u64 ns_time_dev, ns_time_sys;
> +	int err;
> +
> +	switch (adapter->ptp->get_cross_tstamp_access) {
> +	case IDPF_PTP_NONE:
> +		return -EOPNOTSUPP;
> +	case IDPF_PTP_DIRECT:
> +		idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
> +						     &ns_time_sys);
> +		break;
> +	case IDPF_PTP_MAILBOX:
> +		err = idpf_ptp_get_sync_device_time_mailbox(adapter,
> +							    &ns_time_dev,
> +							    &ns_time_sys);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	*device = ns_to_ktime(ns_time_dev);
> +
> +#if IS_ENABLED(CONFIG_X86)
> +	system->cs_id = CSID_X86_ART;
> +#else
> +	system->cs_id = CSID_ARM_ARCH_COUNTER;
> +#endif /* CONFIG_X86 */
> +	system->cycles = ns_time_sys;
> +	system->use_nsecs = true;
> +
> +	return 0;
> +}
> +
> +/**
> + * idpf_ptp_get_crosststamp - Capture a device cross timestamp
> + * @info: the driver's PTP info structure
> + * @cts: The memory to fill the cross timestamp info
> + *
> + * Capture a cross timestamp between the system time and the device PTP hardware
> + * clock.
> + *
> + * Return: cross timestamp value on success, -errno on failure.
> + */
> +static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
> +				    struct system_device_crosststamp *cts)
> +{
> +	struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
> +
> +	return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
> +					     adapter, NULL, cts);
> +}
> +#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
> +
>   /**
>    * idpf_ptp_gettimex64 - Get the time of the clock
>    * @info: the driver's PTP info structure
> @@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter *adapter)
>   	info->verify = idpf_ptp_verify_pin;
>   	info->enable = idpf_ptp_gpio_enable;
>   	info->do_aux_work = idpf_ptp_do_aux_work;
> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
> +	info->getcrosststamp = idpf_ptp_get_crosststamp;
> +#elif IS_ENABLED(CONFIG_X86)
> +	if (pcie_ptm_enabled(adapter->pdev) &&
> +	    boot_cpu_has(X86_FEATURE_ART) &&
> +	    boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
> +		info->getcrosststamp = idpf_ptp_get_crosststamp;
> +#endif /* CONFIG_ARM_ARCH_TIMER */
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> index a876749d6116..cd19f65f9fff 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> @@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
>    * @dev_clk_ns_h: high part of the device clock register
>    * @phy_clk_ns_l: low part of the PHY clock register
>    * @phy_clk_ns_h: high part of the PHY clock register
> + * @sys_time_ns_l: low part of the system time register
> + * @sys_time_ns_h: high part of the system time register
>    * @incval_l: low part of the increment value register
>    * @incval_h: high part of the increment value register
>    * @shadj_l: low part of the shadow adjust register
> @@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
>   	void __iomem *phy_clk_ns_l;
>   	void __iomem *phy_clk_ns_h;
>   
> +	/* System time */
> +	void __iomem *sys_time_ns_l;
> +	void __iomem *sys_time_ns_h;
> +
>   	/* Main timer adjustments */
>   	void __iomem *incval_l;
>   	void __iomem *incval_h;
> @@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
>    * @dev_clk_regs: the set of registers to access the device clock
>    * @caps: PTP capabilities negotiated with the Control Plane
>    * @get_dev_clk_time_access: access type for getting the device clock time
> +  * @get_cross_tstamp_access: access type for the cross timestamping
>    * @set_dev_clk_time_access: access type for setting the device clock time
>    * @adj_dev_clk_time_access: access type for the adjusting the device clock
>    * @tx_tstamp_access: access type for the Tx timestamp value read
> @@ -182,10 +189,11 @@ struct idpf_ptp {
>   	struct idpf_ptp_dev_clk_regs dev_clk_regs;
>   	u32 caps;
>   	enum idpf_ptp_access get_dev_clk_time_access:2;
> +	enum idpf_ptp_access get_cross_tstamp_access:2;
>   	enum idpf_ptp_access set_dev_clk_time_access:2;
>   	enum idpf_ptp_access adj_dev_clk_time_access:2;
>   	enum idpf_ptp_access tx_tstamp_access:2;
> -	u8 rsv;
> +	u8 rsv:6;

Why limit this? Mention it in the commit message?

>   	struct idpf_ptp_secondary_mbx secondary_mbx;
>   	spinlock_t read_dev_clk_lock;
>   };
> @@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
>   bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
>   int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   			      struct idpf_ptp_dev_timers *dev_clk_time);
> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			    struct idpf_ptp_dev_timers *cross_time);
>   int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
>   int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
>   int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
> @@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int
> +idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			struct idpf_ptp_dev_timers *cross_time)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
>   					    u64 time)
>   {
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> index bdcc54a5fb56..4f1fb0cefe51 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> @@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   		.send_buf.iov_len = sizeof(send_ptp_caps_msg),
>   		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>   	};
> +	struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
>   	struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
>   	struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
>   	struct idpf_ptp_secondary_mbx *scnd_mbx;
> @@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   
>   	access_type = ptp->get_dev_clk_time_access;
>   	if (access_type != IDPF_PTP_DIRECT)
> -		goto discipline_clock;
> +		goto cross_tstamp;
>   
>   	clock_offsets = recv_ptp_caps_msg->clk_offsets;
>   
> @@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   	temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
>   	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>   
> +cross_tstamp:
> +	access_type = ptp->get_cross_tstamp_access;
> +	if (access_type != IDPF_PTP_DIRECT)
> +		goto discipline_clock;
> +
> +	cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
> +
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
> +	ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
> +							    temp_offset);
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
> +	ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
> +							    temp_offset);
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
> +	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
> +
>   discipline_clock:
>   	access_type = ptp->adj_dev_clk_time_access;
>   	if (access_type != IDPF_PTP_DIRECT)
> @@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   	return 0;
>   }
>   
> +/**
> + * idpf_ptp_get_cross_time - Send virtchnl get cross time message
> + * @adapter: Driver specific private structure
> + * @cross_time: Pointer to the device clock structure where the value is set
> + *
> + * Send virtchnl get cross time message to get the time of the clock and the
> + * system time.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			    struct idpf_ptp_dev_timers *cross_time)
> +{
> +	struct virtchnl2_ptp_get_cross_time cross_time_msg;
> +	struct idpf_vc_xn_params xn_params = {
> +		.vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
> +		.send_buf.iov_base = &cross_time_msg,
> +		.send_buf.iov_len = sizeof(cross_time_msg),
> +		.recv_buf.iov_base = &cross_time_msg,
> +		.recv_buf.iov_len = sizeof(cross_time_msg),
> +		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> +	};
> +	int reply_sz;
> +
> +	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> +	if (reply_sz < 0)
> +		return reply_sz;
> +	if (reply_sz != sizeof(cross_time_msg))
> +		return -EIO;
> +
> +	cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
> +	cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
> +
> +	return 0;
> +}
> +
>   /**
>    * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
>    * @adapter: Driver specific private structure


Kind regards,

Paul

Re: [PATCH iwl-next] idpf: add cross timestamping

[Vadim Fedorenko]

On 23/05/2025 16:58, Milena Olech wrote:
> Add cross timestamp support through virtchnl mailbox messages and directly,
> through PCIe BAR registers. Cross timestamping assumes that both system
> time and device clock time values are cached simultaneously, what is
> triggered by HW. Feature is enabled for both ARM and x86 archs.
> 
> Signed-off-by: Milena Olech <milena.olech@intel.com>
> Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
>   drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
>   3 files changed, 210 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> index cb46185da749..f8d320875eb7 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> @@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
>   							   direct,
>   							   mailbox);
>   
> +	/* Get the cross timestamp */
> +	direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
> +	mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
> +	ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
> +							   direct,
> +							   mailbox);
> +
>   	/* Set the device clock time */
>   	direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>   	mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
> @@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct idpf_adapter *adapter, u64 *src_clk,
>   	return 0;
>   }
>   
> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
> +/**
> + * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
> + *					  directly
> + * @adapter: Driver specific private structure
> + * @dev_time: 64bit main timer value
> + * @sys_time: 64bit system time value
> + */
> +static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter *adapter,
> +						 u64 *dev_time, u64 *sys_time)
> +{
> +	u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
> +	struct idpf_ptp *ptp = adapter->ptp;
> +
> +	spin_lock(&ptp->read_dev_clk_lock);
> +
> +	idpf_ptp_enable_shtime(adapter);
> +
> +	dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
> +	dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
> +
> +	sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
> +	sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
> +
> +	*dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
> +	*sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
> +
> +	spin_unlock(&ptp->read_dev_clk_lock);

nit: I would say the calc part should be outside of spinlock scope.
For this part of code it's just several CPU cycles, but still looks
cleaner.

> +}
> +
> +/**
> + * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
> + *					   through mailbox
> + * @adapter: Driver specific private structure
> + * @dev_time: 64bit main timer value expressed in nanoseconds
> + * @sys_time: 64bit system time value expressed in nanoseconds
> + *
> + * Return: a pair of cross timestamp values on success, -errno otherwise.
> + */
> +static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
> +						 u64 *dev_time, u64 *sys_time)
> +{
> +	struct idpf_ptp_dev_timers cross_time;
> +	int err;
> +
> +	err = idpf_ptp_get_cross_time(adapter, &cross_time);
> +	if (err)
> +		return err;
> +
> +	*dev_time = cross_time.dev_clk_time_ns;
> +	*sys_time = cross_time.sys_time_ns;
> +
> +	return err;
> +}
> +
> +/**
> + * idpf_ptp_get_sync_device_time - Get the cross time stamp info
> + * @device: Current device time
> + * @system: System counter value read synchronously with device time
> + * @ctx: Context provided by timekeeping code
> + *
> + * Return: the device and the system clocks time read simultaneously on success,
> + * -errno otherwise.
> + */
> +static int idpf_ptp_get_sync_device_time(ktime_t *device,
> +					 struct system_counterval_t *system,
> +					 void *ctx)
> +{
> +	struct idpf_adapter *adapter = ctx;
> +	u64 ns_time_dev, ns_time_sys;
> +	int err;
> +
> +	switch (adapter->ptp->get_cross_tstamp_access) {
> +	case IDPF_PTP_NONE:
> +		return -EOPNOTSUPP;
> +	case IDPF_PTP_DIRECT:
> +		idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
> +						     &ns_time_sys);
> +		break;
> +	case IDPF_PTP_MAILBOX:
> +		err = idpf_ptp_get_sync_device_time_mailbox(adapter,
> +							    &ns_time_dev,
> +							    &ns_time_sys);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	*device = ns_to_ktime(ns_time_dev);
> +
> +#if IS_ENABLED(CONFIG_X86)
> +	system->cs_id = CSID_X86_ART;
> +#else
> +	system->cs_id = CSID_ARM_ARCH_COUNTER;
> +#endif /* CONFIG_X86 */

AFAIR, IS_ENABLED() can be used in normal code flow, so this code can be 
written in one line (well, 2 lines actually) without explicit macro:

         system->cs_id = IS_ENABLED(CONFIG_X86) ? CSID_X86_ART
                                                : CSID_ARM_ARCH_COUNTER

> +	system->cycles = ns_time_sys;
> +	system->use_nsecs = true;
> +
> +	return 0;
> +}
> +
> +/**
> + * idpf_ptp_get_crosststamp - Capture a device cross timestamp
> + * @info: the driver's PTP info structure
> + * @cts: The memory to fill the cross timestamp info
> + *
> + * Capture a cross timestamp between the system time and the device PTP hardware
> + * clock.
> + *
> + * Return: cross timestamp value on success, -errno on failure.
> + */
> +static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
> +				    struct system_device_crosststamp *cts)
> +{
> +	struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
> +
> +	return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
> +					     adapter, NULL, cts);
> +}
> +#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
> +
>   /**
>    * idpf_ptp_gettimex64 - Get the time of the clock
>    * @info: the driver's PTP info structure
> @@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter *adapter)
>   	info->verify = idpf_ptp_verify_pin;
>   	info->enable = idpf_ptp_gpio_enable;
>   	info->do_aux_work = idpf_ptp_do_aux_work;
> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
> +	info->getcrosststamp = idpf_ptp_get_crosststamp;
> +#elif IS_ENABLED(CONFIG_X86)
> +	if (pcie_ptm_enabled(adapter->pdev) &&
> +	    boot_cpu_has(X86_FEATURE_ART) &&
> +	    boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
> +		info->getcrosststamp = idpf_ptp_get_crosststamp;
> +#endif /* CONFIG_ARM_ARCH_TIMER */
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> index a876749d6116..cd19f65f9fff 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
> @@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
>    * @dev_clk_ns_h: high part of the device clock register
>    * @phy_clk_ns_l: low part of the PHY clock register
>    * @phy_clk_ns_h: high part of the PHY clock register
> + * @sys_time_ns_l: low part of the system time register
> + * @sys_time_ns_h: high part of the system time register
>    * @incval_l: low part of the increment value register
>    * @incval_h: high part of the increment value register
>    * @shadj_l: low part of the shadow adjust register
> @@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
>   	void __iomem *phy_clk_ns_l;
>   	void __iomem *phy_clk_ns_h;
>   
> +	/* System time */
> +	void __iomem *sys_time_ns_l;
> +	void __iomem *sys_time_ns_h;
> +
>   	/* Main timer adjustments */
>   	void __iomem *incval_l;
>   	void __iomem *incval_h;
> @@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
>    * @dev_clk_regs: the set of registers to access the device clock
>    * @caps: PTP capabilities negotiated with the Control Plane
>    * @get_dev_clk_time_access: access type for getting the device clock time
> + * @get_cross_tstamp_access: access type for the cross timestamping
>    * @set_dev_clk_time_access: access type for setting the device clock time
>    * @adj_dev_clk_time_access: access type for the adjusting the device clock
>    * @tx_tstamp_access: access type for the Tx timestamp value read
> @@ -182,10 +189,11 @@ struct idpf_ptp {
>   	struct idpf_ptp_dev_clk_regs dev_clk_regs;
>   	u32 caps;
>   	enum idpf_ptp_access get_dev_clk_time_access:2;
> +	enum idpf_ptp_access get_cross_tstamp_access:2;
>   	enum idpf_ptp_access set_dev_clk_time_access:2;
>   	enum idpf_ptp_access adj_dev_clk_time_access:2;
>   	enum idpf_ptp_access tx_tstamp_access:2;
> -	u8 rsv;
> +	u8 rsv:6;
>   	struct idpf_ptp_secondary_mbx secondary_mbx;
>   	spinlock_t read_dev_clk_lock;
>   };
> @@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
>   bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
>   int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   			      struct idpf_ptp_dev_timers *dev_clk_time);
> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			    struct idpf_ptp_dev_timers *cross_time);
>   int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
>   int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
>   int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
> @@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int
> +idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			struct idpf_ptp_dev_timers *cross_time)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
>   					    u64 time)
>   {
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> index bdcc54a5fb56..4f1fb0cefe51 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> @@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   		.send_buf.iov_len = sizeof(send_ptp_caps_msg),
>   		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>   	};
> +	struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
>   	struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
>   	struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
>   	struct idpf_ptp_secondary_mbx *scnd_mbx;
> @@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   
>   	access_type = ptp->get_dev_clk_time_access;
>   	if (access_type != IDPF_PTP_DIRECT)
> -		goto discipline_clock;
> +		goto cross_tstamp;
>   
>   	clock_offsets = recv_ptp_caps_msg->clk_offsets;
>   
> @@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>   	temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
>   	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>   
> +cross_tstamp:
> +	access_type = ptp->get_cross_tstamp_access;
> +	if (access_type != IDPF_PTP_DIRECT)
> +		goto discipline_clock;
> +
> +	cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
> +
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
> +	ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
> +							    temp_offset);
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
> +	ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
> +							    temp_offset);
> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
> +	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
> +
>   discipline_clock:
>   	access_type = ptp->adj_dev_clk_time_access;
>   	if (access_type != IDPF_PTP_DIRECT)
> @@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>   	return 0;
>   }
>   
> +/**
> + * idpf_ptp_get_cross_time - Send virtchnl get cross time message
> + * @adapter: Driver specific private structure
> + * @cross_time: Pointer to the device clock structure where the value is set
> + *
> + * Send virtchnl get cross time message to get the time of the clock and the
> + * system time.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
> +			    struct idpf_ptp_dev_timers *cross_time)
> +{
> +	struct virtchnl2_ptp_get_cross_time cross_time_msg;
> +	struct idpf_vc_xn_params xn_params = {
> +		.vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
> +		.send_buf.iov_base = &cross_time_msg,
> +		.send_buf.iov_len = sizeof(cross_time_msg),
> +		.recv_buf.iov_base = &cross_time_msg,
> +		.recv_buf.iov_len = sizeof(cross_time_msg),
> +		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> +	};
> +	int reply_sz;
> +
> +	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> +	if (reply_sz < 0)
> +		return reply_sz;
> +	if (reply_sz != sizeof(cross_time_msg))
> +		return -EIO;
> +
> +	cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
> +	cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
> +
> +	return 0;
> +}
> +
>   /**
>    * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
>    * @adapter: Driver specific private structure

RE: [PATCH iwl-next] idpf: add cross timestamping

[Olech, Milena]

On 26/05/2025 6:24PM, Vadim Fedorenko wrote:
>On 23/05/2025 16:58, Milena Olech wrote:
>> Add cross timestamp support through virtchnl mailbox messages and directly,
>> through PCIe BAR registers. Cross timestamping assumes that both system
>> time and device clock time values are cached simultaneously, what is
>> triggered by HW. Feature is enabled for both ARM and x86 archs.
>> 
>> Signed-off-by: Milena Olech <milena.olech@intel.com>
>> Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>
>> ---
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
>>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
>>   3 files changed, 210 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> index cb46185da749..f8d320875eb7 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> @@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
>>   							   direct,
>>   							   mailbox);
>>   
>> +	/* Get the cross timestamp */
>> +	direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
>> +	mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
>> +	ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
>> +							   direct,
>> +							   mailbox);
>> +
>>   	/* Set the device clock time */
>>   	direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>>   	mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>> @@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct idpf_adapter *adapter, u64 *src_clk,
>>   	return 0;
>>   }
>>   
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
>> +/**
>> + * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
>> + *					  directly
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value
>> + * @sys_time: 64bit system time value
>> + */
>> +static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter *adapter,
>> +						 u64 *dev_time, u64 *sys_time)
>> +{
>> +	u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
>> +	struct idpf_ptp *ptp = adapter->ptp;
>> +
>> +	spin_lock(&ptp->read_dev_clk_lock);
>> +
>> +	idpf_ptp_enable_shtime(adapter);
>> +
>> +	dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
>> +	dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
>> +
>> +	sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
>> +	sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
>> +
>> +	*dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
>> +	*sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
>> +
>> +	spin_unlock(&ptp->read_dev_clk_lock);
>
>nit: I would say the calc part should be outside of spinlock scope.
>For this part of code it's just several CPU cycles, but still looks
>cleaner.
>

Sure, makes sense.
Will fix it in v2

>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
>> + *					   through mailbox
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value expressed in nanoseconds
>> + * @sys_time: 64bit system time value expressed in nanoseconds
>> + *
>> + * Return: a pair of cross timestamp values on success, -errno otherwise.
>> + */
>> +static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
>> +						 u64 *dev_time, u64 *sys_time)
>> +{
>> +	struct idpf_ptp_dev_timers cross_time;
>> +	int err;
>> +
>> +	err = idpf_ptp_get_cross_time(adapter, &cross_time);
>> +	if (err)
>> +		return err;
>> +
>> +	*dev_time = cross_time.dev_clk_time_ns;
>> +	*sys_time = cross_time.sys_time_ns;
>> +
>> +	return err;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time - Get the cross time stamp info
>> + * @device: Current device time
>> + * @system: System counter value read synchronously with device time
>> + * @ctx: Context provided by timekeeping code
>> + *
>> + * Return: the device and the system clocks time read simultaneously on success,
>> + * -errno otherwise.
>> + */
>> +static int idpf_ptp_get_sync_device_time(ktime_t *device,
>> +					 struct system_counterval_t *system,
>> +					 void *ctx)
>> +{
>> +	struct idpf_adapter *adapter = ctx;
>> +	u64 ns_time_dev, ns_time_sys;
>> +	int err;
>> +
>> +	switch (adapter->ptp->get_cross_tstamp_access) {
>> +	case IDPF_PTP_NONE:
>> +		return -EOPNOTSUPP;
>> +	case IDPF_PTP_DIRECT:
>> +		idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
>> +						     &ns_time_sys);
>> +		break;
>> +	case IDPF_PTP_MAILBOX:
>> +		err = idpf_ptp_get_sync_device_time_mailbox(adapter,
>> +							    &ns_time_dev,
>> +							    &ns_time_sys);
>> +		if (err)
>> +			return err;
>> +		break;
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	*device = ns_to_ktime(ns_time_dev);
>> +
>> +#if IS_ENABLED(CONFIG_X86)
>> +	system->cs_id = CSID_X86_ART;
>> +#else
>> +	system->cs_id = CSID_ARM_ARCH_COUNTER;
>> +#endif /* CONFIG_X86 */
>
>AFAIR, IS_ENABLED() can be used in normal code flow, so this code can be 
>written in one line (well, 2 lines actually) without explicit macro:

Ok!

>
>         system->cs_id = IS_ENABLED(CONFIG_X86) ? CSID_X86_ART
>                                                : CSID_ARM_ARCH_COUNTER
>
>> +	system->cycles = ns_time_sys;
>> +	system->use_nsecs = true;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_crosststamp - Capture a device cross timestamp
>> + * @info: the driver's PTP info structure
>> + * @cts: The memory to fill the cross timestamp info
>> + *
>> + * Capture a cross timestamp between the system time and the device PTP hardware
>> + * clock.
>> + *
>> + * Return: cross timestamp value on success, -errno on failure.
>> + */
>> +static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
>> +				    struct system_device_crosststamp *cts)
>> +{
>> +	struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
>> +
>> +	return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
>> +					     adapter, NULL, cts);
>> +}
>> +#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
>> +
>>   /**
>>    * idpf_ptp_gettimex64 - Get the time of the clock
>>    * @info: the driver's PTP info structure
>> @@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter *adapter)
>>   	info->verify = idpf_ptp_verify_pin;
>>   	info->enable = idpf_ptp_gpio_enable;
>>   	info->do_aux_work = idpf_ptp_do_aux_work;
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
>> +	info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#elif IS_ENABLED(CONFIG_X86)
>> +	if (pcie_ptm_enabled(adapter->pdev) &&
>> +	    boot_cpu_has(X86_FEATURE_ART) &&
>> +	    boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
>> +		info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#endif /* CONFIG_ARM_ARCH_TIMER */
>>   }
>>   
>>   /**
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> index a876749d6116..cd19f65f9fff 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> @@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
>>    * @dev_clk_ns_h: high part of the device clock register
>>    * @phy_clk_ns_l: low part of the PHY clock register
>>    * @phy_clk_ns_h: high part of the PHY clock register
>> + * @sys_time_ns_l: low part of the system time register
>> + * @sys_time_ns_h: high part of the system time register
>>    * @incval_l: low part of the increment value register
>>    * @incval_h: high part of the increment value register
>>    * @shadj_l: low part of the shadow adjust register
>> @@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
>>   	void __iomem *phy_clk_ns_l;
>>   	void __iomem *phy_clk_ns_h;
>>   
>> +	/* System time */
>> +	void __iomem *sys_time_ns_l;
>> +	void __iomem *sys_time_ns_h;
>> +
>>   	/* Main timer adjustments */
>>   	void __iomem *incval_l;
>>   	void __iomem *incval_h;
>> @@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
>>    * @dev_clk_regs: the set of registers to access the device clock
>>    * @caps: PTP capabilities negotiated with the Control Plane
>>    * @get_dev_clk_time_access: access type for getting the device clock time
>> + * @get_cross_tstamp_access: access type for the cross timestamping
>>    * @set_dev_clk_time_access: access type for setting the device clock time
>>    * @adj_dev_clk_time_access: access type for the adjusting the device clock
>>    * @tx_tstamp_access: access type for the Tx timestamp value read
>> @@ -182,10 +189,11 @@ struct idpf_ptp {
>>   	struct idpf_ptp_dev_clk_regs dev_clk_regs;
>>   	u32 caps;
>>   	enum idpf_ptp_access get_dev_clk_time_access:2;
>> +	enum idpf_ptp_access get_cross_tstamp_access:2;
>>   	enum idpf_ptp_access set_dev_clk_time_access:2;
>>   	enum idpf_ptp_access adj_dev_clk_time_access:2;
>>   	enum idpf_ptp_access tx_tstamp_access:2;
>> -	u8 rsv;
>> +	u8 rsv:6;
>>   	struct idpf_ptp_secondary_mbx secondary_mbx;
>>   	spinlock_t read_dev_clk_lock;
>>   };
>> @@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
>>   bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
>>   int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   			      struct idpf_ptp_dev_timers *dev_clk_time);
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			    struct idpf_ptp_dev_timers *cross_time);
>>   int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
>>   int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
>>   int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
>> @@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   	return -EOPNOTSUPP;
>>   }
>>   
>> +static inline int
>> +idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
>>   static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
>>   					    u64 time)
>>   {
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> index bdcc54a5fb56..4f1fb0cefe51 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> @@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   		.send_buf.iov_len = sizeof(send_ptp_caps_msg),
>>   		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>>   	};
>> +	struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
>>   	struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
>>   	struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
>>   	struct idpf_ptp_secondary_mbx *scnd_mbx;
>> @@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   
>>   	access_type = ptp->get_dev_clk_time_access;
>>   	if (access_type != IDPF_PTP_DIRECT)
>> -		goto discipline_clock;
>> +		goto cross_tstamp;
>>   
>>   	clock_offsets = recv_ptp_caps_msg->clk_offsets;
>>   
>> @@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   	temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
>>   	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>>   
>> +cross_tstamp:
>> +	access_type = ptp->get_cross_tstamp_access;
>> +	if (access_type != IDPF_PTP_DIRECT)
>> +		goto discipline_clock;
>> +
>> +	cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
>> +
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
>> +	ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
>> +							    temp_offset);
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
>> +	ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
>> +							    temp_offset);
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
>> +	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>> +
>>   discipline_clock:
>>   	access_type = ptp->adj_dev_clk_time_access;
>>   	if (access_type != IDPF_PTP_DIRECT)
>> @@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   	return 0;
>>   }
>>   
>> +/**
>> + * idpf_ptp_get_cross_time - Send virtchnl get cross time message
>> + * @adapter: Driver specific private structure
>> + * @cross_time: Pointer to the device clock structure where the value is set
>> + *
>> + * Send virtchnl get cross time message to get the time of the clock and the
>> + * system time.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			    struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +	struct virtchnl2_ptp_get_cross_time cross_time_msg;
>> +	struct idpf_vc_xn_params xn_params = {
>> +		.vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
>> +		.send_buf.iov_base = &cross_time_msg,
>> +		.send_buf.iov_len = sizeof(cross_time_msg),
>> +		.recv_buf.iov_base = &cross_time_msg,
>> +		.recv_buf.iov_len = sizeof(cross_time_msg),
>> +		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>> +	};
>> +	int reply_sz;
>> +
>> +	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
>> +	if (reply_sz < 0)
>> +		return reply_sz;
>> +	if (reply_sz != sizeof(cross_time_msg))
>> +		return -EIO;
>> +
>> +	cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
>> +	cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
>>    * @adapter: Driver specific private structure

RE: [Intel-wired-lan] [PATCH iwl-next] idpf: add cross timestamping

[Olech, Milena]

On 05/25/2025 7:31AM, Paul Menzel wrote:

>Dear Milena,
>
>
>Thank you for your patch.
>
>
>Am 23.05.25 um 17:58 schrieb Milena Olech:
>> Add cross timestamp support through virtchnl mailbox messages and directly,
>> through PCIe BAR registers. Cross timestamping assumes that both system
>> time and device clock time values are cached simultaneously, what is
>> triggered by HW. Feature is enabled for both ARM and x86 archs.
>
>How can this be tested?

You'd need - for x86 case - Intel CPU with PTM support, like Sapphire
Rapids and have PTM support on the PCI.

Additionally, there's a HW mechanism that provides system time and device
time latched exactly at the same time.

When all these requirements are fulfilled, you can run phc2sys app to
synchronize your system time with the device clock, and the "delay"
printed by the app should be equal to 0 - that's because there's no
delay between reading system time and device time values.

>
>> Signed-off-by: Milena Olech <milena.olech@intel.com>
>> Reviewed-by: Karol Kolacinski <karol.kolacinski@intel.com>
>> ---
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 138 ++++++++++++++++++
>>   drivers/net/ethernet/intel/idpf/idpf_ptp.h    |  19 ++-
>>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  55 ++++++-
>>   3 files changed, 210 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> index cb46185da749..f8d320875eb7 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
>> @@ -42,6 +42,13 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter)
>>   							   direct,
>>   							   mailbox);
>>   
>> +	/* Get the cross timestamp */
>> +	direct = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME;
>> +	mailbox = VIRTCHNL2_CAP_PTP_GET_CROSS_TIME_MB;
>> +	ptp->get_cross_tstamp_access = idpf_ptp_get_access(adapter,
>> +							   direct,
>> +							   mailbox);
>> +
>>   	/* Set the device clock time */
>>   	direct = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>>   	mailbox = VIRTCHNL2_CAP_PTP_SET_DEVICE_CLK_TIME;
>> @@ -171,6 +178,129 @@ static int idpf_ptp_read_src_clk_reg(struct idpf_adapter *adapter, u64 *src_clk,
>>   	return 0;
>>   }
>>   
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER) || IS_ENABLED(CONFIG_X86)
>> +/**
>> + * idpf_ptp_get_sync_device_time_direct - Get the cross time stamp values
>> + *					  directly
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value
>> + * @sys_time: 64bit system time value
>> + */
>> +static void idpf_ptp_get_sync_device_time_direct(struct idpf_adapter *adapter,
>> +						 u64 *dev_time, u64 *sys_time)
>> +{
>> +	u32 dev_time_lo, dev_time_hi, sys_time_lo, sys_time_hi;
>> +	struct idpf_ptp *ptp = adapter->ptp;
>> +
>> +	spin_lock(&ptp->read_dev_clk_lock);
>> +
>> +	idpf_ptp_enable_shtime(adapter);
>> +
>> +	dev_time_lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
>> +	dev_time_hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
>> +
>> +	sys_time_lo = readl(ptp->dev_clk_regs.sys_time_ns_l);
>> +	sys_time_hi = readl(ptp->dev_clk_regs.sys_time_ns_h);
>> +
>> +	*dev_time = (u64)dev_time_hi << 32 | dev_time_lo;
>> +	*sys_time = (u64)sys_time_hi << 32 | sys_time_lo;
>> +
>> +	spin_unlock(&ptp->read_dev_clk_lock);
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time_mailbox - Get the cross time stamp values
>> + *					   through mailbox
>> + * @adapter: Driver specific private structure
>> + * @dev_time: 64bit main timer value expressed in nanoseconds
>> + * @sys_time: 64bit system time value expressed in nanoseconds
>> + *
>> + * Return: a pair of cross timestamp values on success, -errno otherwise.
>
>0 is returned on success?
>
>> + */
>> +static int idpf_ptp_get_sync_device_time_mailbox(struct idpf_adapter *adapter,
>> +						 u64 *dev_time, u64 *sys_time)
>> +{
>> +	struct idpf_ptp_dev_timers cross_time;
>> +	int err;
>> +
>> +	err = idpf_ptp_get_cross_time(adapter, &cross_time);
>> +	if (err)
>> +		return err;
>> +
>> +	*dev_time = cross_time.dev_clk_time_ns;
>> +	*sys_time = cross_time.sys_time_ns;
>> +
>> +	return err;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_sync_device_time - Get the cross time stamp info
>> + * @device: Current device time
>> + * @system: System counter value read synchronously with device time
>> + * @ctx: Context provided by timekeeping code
>> + *
>> + * Return: the device and the system clocks time read simultaneously on success,
>> + * -errno otherwise.
>
>Also mention 0 return?

Right, will fix it.

>
>> + */
>> +static int idpf_ptp_get_sync_device_time(ktime_t *device,
>> +					 struct system_counterval_t *system,
>> +					 void *ctx)
>> +{
>> +	struct idpf_adapter *adapter = ctx;
>> +	u64 ns_time_dev, ns_time_sys;
>> +	int err;
>> +
>> +	switch (adapter->ptp->get_cross_tstamp_access) {
>> +	case IDPF_PTP_NONE:
>> +		return -EOPNOTSUPP;
>> +	case IDPF_PTP_DIRECT:
>> +		idpf_ptp_get_sync_device_time_direct(adapter, &ns_time_dev,
>> +						     &ns_time_sys);
>> +		break;
>> +	case IDPF_PTP_MAILBOX:
>> +		err = idpf_ptp_get_sync_device_time_mailbox(adapter,
>> +							    &ns_time_dev,
>> +							    &ns_time_sys);
>> +		if (err)
>> +			return err;
>> +		break;
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	*device = ns_to_ktime(ns_time_dev);
>> +
>> +#if IS_ENABLED(CONFIG_X86)
>> +	system->cs_id = CSID_X86_ART;
>> +#else
>> +	system->cs_id = CSID_ARM_ARCH_COUNTER;
>> +#endif /* CONFIG_X86 */
>> +	system->cycles = ns_time_sys;
>> +	system->use_nsecs = true;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * idpf_ptp_get_crosststamp - Capture a device cross timestamp
>> + * @info: the driver's PTP info structure
>> + * @cts: The memory to fill the cross timestamp info
>> + *
>> + * Capture a cross timestamp between the system time and the device PTP hardware
>> + * clock.
>> + *
>> + * Return: cross timestamp value on success, -errno on failure.
>> + */
>> +static int idpf_ptp_get_crosststamp(struct ptp_clock_info *info,
>> +				    struct system_device_crosststamp *cts)
>> +{
>> +	struct idpf_adapter *adapter = idpf_ptp_info_to_adapter(info);
>> +
>> +	return get_device_system_crosststamp(idpf_ptp_get_sync_device_time,
>> +					     adapter, NULL, cts);
>> +}
>> +#endif /* CONFIG_ARM_ARCH_TIMER || CONFIG_X86 */
>> +
>>   /**
>>    * idpf_ptp_gettimex64 - Get the time of the clock
>>    * @info: the driver's PTP info structure
>> @@ -664,6 +794,14 @@ static void idpf_ptp_set_caps(const struct idpf_adapter *adapter)
>>   	info->verify = idpf_ptp_verify_pin;
>>   	info->enable = idpf_ptp_gpio_enable;
>>   	info->do_aux_work = idpf_ptp_do_aux_work;
>> +#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER)
>> +	info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#elif IS_ENABLED(CONFIG_X86)
>> +	if (pcie_ptm_enabled(adapter->pdev) &&
>> +	    boot_cpu_has(X86_FEATURE_ART) &&
>> +	    boot_cpu_has(X86_FEATURE_TSC_KNOWN_FREQ))
>> +		info->getcrosststamp = idpf_ptp_get_crosststamp;
>> +#endif /* CONFIG_ARM_ARCH_TIMER */
>>   }
>>   
>>   /**
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.h b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> index a876749d6116..cd19f65f9fff 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.h
>> @@ -21,6 +21,8 @@ struct idpf_ptp_cmd {
>>    * @dev_clk_ns_h: high part of the device clock register
>>    * @phy_clk_ns_l: low part of the PHY clock register
>>    * @phy_clk_ns_h: high part of the PHY clock register
>> + * @sys_time_ns_l: low part of the system time register
>> + * @sys_time_ns_h: high part of the system time register
>>    * @incval_l: low part of the increment value register
>>    * @incval_h: high part of the increment value register
>>    * @shadj_l: low part of the shadow adjust register
>> @@ -42,6 +44,10 @@ struct idpf_ptp_dev_clk_regs {
>>   	void __iomem *phy_clk_ns_l;
>>   	void __iomem *phy_clk_ns_h;
>>   
>> +	/* System time */
>> +	void __iomem *sys_time_ns_l;
>> +	void __iomem *sys_time_ns_h;
>> +
>>   	/* Main timer adjustments */
>>   	void __iomem *incval_l;
>>   	void __iomem *incval_h;
>> @@ -162,6 +168,7 @@ struct idpf_ptp_vport_tx_tstamp_caps {
>>    * @dev_clk_regs: the set of registers to access the device clock
>>    * @caps: PTP capabilities negotiated with the Control Plane
>>    * @get_dev_clk_time_access: access type for getting the device clock time
>> +  * @get_cross_tstamp_access: access type for the cross timestamping
>>    * @set_dev_clk_time_access: access type for setting the device clock time
>>    * @adj_dev_clk_time_access: access type for the adjusting the device clock
>>    * @tx_tstamp_access: access type for the Tx timestamp value read
>> @@ -182,10 +189,11 @@ struct idpf_ptp {
>>   	struct idpf_ptp_dev_clk_regs dev_clk_regs;
>>   	u32 caps;
>>   	enum idpf_ptp_access get_dev_clk_time_access:2;
>> +	enum idpf_ptp_access get_cross_tstamp_access:2;
>>   	enum idpf_ptp_access set_dev_clk_time_access:2;
>>   	enum idpf_ptp_access adj_dev_clk_time_access:2;
>>   	enum idpf_ptp_access tx_tstamp_access:2;
>> -	u8 rsv;
>> +	u8 rsv:6;
>
>Why limit this? Mention it in the commit message?
>
>>   	struct idpf_ptp_secondary_mbx secondary_mbx;
>>   	spinlock_t read_dev_clk_lock;
>>   };
>> @@ -264,6 +272,8 @@ void idpf_ptp_get_features_access(const struct idpf_adapter *adapter);
>>   bool idpf_ptp_get_txq_tstamp_capability(struct idpf_tx_queue *txq);
>>   int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   			      struct idpf_ptp_dev_timers *dev_clk_time);
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			    struct idpf_ptp_dev_timers *cross_time);
>>   int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time);
>>   int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval);
>>   int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta);
>> @@ -305,6 +315,13 @@ idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   	return -EOPNOTSUPP;
>>   }
>>   
>> +static inline int
>> +idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
>>   static inline int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter,
>>   					    u64 time)
>>   {
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> index bdcc54a5fb56..4f1fb0cefe51 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
>> @@ -30,6 +30,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   		.send_buf.iov_len = sizeof(send_ptp_caps_msg),
>>   		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>>   	};
>> +	struct virtchnl2_ptp_cross_time_reg_offsets cross_tstamp_offsets;
>>   	struct virtchnl2_ptp_clk_adj_reg_offsets clk_adj_offsets;
>>   	struct virtchnl2_ptp_clk_reg_offsets clock_offsets;
>>   	struct idpf_ptp_secondary_mbx *scnd_mbx;
>> @@ -71,7 +72,7 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   
>>   	access_type = ptp->get_dev_clk_time_access;
>>   	if (access_type != IDPF_PTP_DIRECT)
>> -		goto discipline_clock;
>> +		goto cross_tstamp;
>>   
>>   	clock_offsets = recv_ptp_caps_msg->clk_offsets;
>>   
>> @@ -90,6 +91,22 @@ int idpf_ptp_get_caps(struct idpf_adapter *adapter)
>>   	temp_offset = le32_to_cpu(clock_offsets.cmd_sync_trigger);
>>   	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>>   
>> +cross_tstamp:
>> +	access_type = ptp->get_cross_tstamp_access;
>> +	if (access_type != IDPF_PTP_DIRECT)
>> +		goto discipline_clock;
>> +
>> +	cross_tstamp_offsets = recv_ptp_caps_msg->cross_time_offsets;
>> +
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_l);
>> +	ptp->dev_clk_regs.sys_time_ns_l = idpf_get_reg_addr(adapter,
>> +							    temp_offset);
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.sys_time_ns_h);
>> +	ptp->dev_clk_regs.sys_time_ns_h = idpf_get_reg_addr(adapter,
>> +							    temp_offset);
>> +	temp_offset = le32_to_cpu(cross_tstamp_offsets.cmd_sync_trigger);
>> +	ptp->dev_clk_regs.cmd_sync = idpf_get_reg_addr(adapter, temp_offset);
>> +
>>   discipline_clock:
>>   	access_type = ptp->adj_dev_clk_time_access;
>>   	if (access_type != IDPF_PTP_DIRECT)
>> @@ -162,6 +179,42 @@ int idpf_ptp_get_dev_clk_time(struct idpf_adapter *adapter,
>>   	return 0;
>>   }
>>   
>> +/**
>> + * idpf_ptp_get_cross_time - Send virtchnl get cross time message
>> + * @adapter: Driver specific private structure
>> + * @cross_time: Pointer to the device clock structure where the value is set
>> + *
>> + * Send virtchnl get cross time message to get the time of the clock and the
>> + * system time.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int idpf_ptp_get_cross_time(struct idpf_adapter *adapter,
>> +			    struct idpf_ptp_dev_timers *cross_time)
>> +{
>> +	struct virtchnl2_ptp_get_cross_time cross_time_msg;
>> +	struct idpf_vc_xn_params xn_params = {
>> +		.vc_op = VIRTCHNL2_OP_PTP_GET_CROSS_TIME,
>> +		.send_buf.iov_base = &cross_time_msg,
>> +		.send_buf.iov_len = sizeof(cross_time_msg),
>> +		.recv_buf.iov_base = &cross_time_msg,
>> +		.recv_buf.iov_len = sizeof(cross_time_msg),
>> +		.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
>> +	};
>> +	int reply_sz;
>> +
>> +	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
>> +	if (reply_sz < 0)
>> +		return reply_sz;
>> +	if (reply_sz != sizeof(cross_time_msg))
>> +		return -EIO;
>> +
>> +	cross_time->dev_clk_time_ns = le64_to_cpu(cross_time_msg.dev_time_ns);
>> +	cross_time->sys_time_ns = le64_to_cpu(cross_time_msg.sys_time_ns);
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * idpf_ptp_set_dev_clk_time - Send virtchnl set device time message
>>    * @adapter: Driver specific private structure
>
>
>Kind regards,
>
>Paul
>