Re: [PATCH net-next v3 3/3] gve: implement PTP gettimex64

[Jacob Keller]

On 4/3/2026 12:44 PM, Harshitha Ramamurthy wrote:
> From: Jordan Rhee <jordanrhee@xxxxxxxxxx>
> 
> Enable chrony and phc2sys to synchronize system clock to NIC clock.
> 
> The system cycle counters are sampled by the device to minimize the
> uncertainty window. If the system times are sampled in the host, the
> delta between pre and post readings is 100us or more due to AQ command
> latency. The system times returned by the device have a delta of ~1us,
> which enables significantly more accurate clock synchronization.
> 
> Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
> Reviewed-by: Kevin Yang <yyd@xxxxxxxxxx>
> Reviewed-by: Naman Gulati <namangulati@xxxxxxxxxx>
> Signed-off-by: Jordan Rhee <jordanrhee@xxxxxxxxxx>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> ---

> +/*
> + * Convert a raw cycle count (e.g. from get_cycles()) to the system clock
> + * type specified by clockid. The system_time_snapshot must be taken before
> + * the cycle counter is sampled.
> + */
> +static int gve_cycles_to_timespec64(struct gve_priv *priv, clockid_t clockid,
> +				    struct system_time_snapshot *snap,
> +				    u64 cycles, struct timespec64 *ts)
> +{
> +	struct gve_cycles_to_clock_callback_ctx ctx = {0};
> +	struct system_device_crosststamp xtstamp;
> +	int err;
> +
> +	ctx.cycles = cycles;
> +	err = get_device_system_crosststamp(gve_cycles_to_clock_fn, &ctx, snap,
> +					    &xtstamp);
> +	if (err) {
> +		dev_err_ratelimited(&priv->pdev->dev,
> +				    "get_device_system_crosststamp() failed to convert %lld cycles to system time: %d\n",
> +				    cycles,
> +				    err);
> +		return err;
> +	}
> + 

This looks a lot like a cross timestamp (i.e. something like PCIe PTM)
Why not just implement the .crosstimestamp and PTP_SYS_OFF_PRECISE? Does
that not work properly? Or is this not really a cross timestamp despite
use of the get_device_system_crosststamp handler? :D

Thanks,
Jake

> +	switch (clockid) {
> +	case CLOCK_REALTIME:
> +		*ts = ktime_to_timespec64(xtstamp.sys_realtime);
> +		break;
> +	case CLOCK_MONOTONIC_RAW:
> +		*ts = ktime_to_timespec64(xtstamp.sys_monoraw);
> +		break;
> +	default:
> +		dev_err_ratelimited(&priv->pdev->dev,
> +				    "Cycle count conversion to clockid %d not supported\n",
> +				    clockid);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +