Re: [PATCHv4 net-next] ptp/ioctl: support MONOTONIC_RAW timestamps for PTP_SYS_OFFSET_EXTENDED

From: Richard Cochran
Date: Wed May 08 2024 - 00:44:37 EST


On Thu, May 02, 2024 at 02:10:47PM -0700, Mahesh Bandewar wrote:

> @@ -457,14 +459,34 @@ static inline ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp,
>
> static inline void ptp_read_system_prets(struct ptp_system_timestamp *sts)
> {
> - if (sts)
> - ktime_get_real_ts64(&sts->pre_ts);
> + if (sts) {
> + switch (sts->clockid) {
> + case CLOCK_REALTIME:
> + ktime_get_real_ts64(&sts->pre_ts);
> + break;
> + case CLOCK_MONOTONIC_RAW:
> + ktime_get_raw_ts64(&sts->pre_ts);
> + break;

Why not add CLOCK_MONOTONIC as well?
That would be useful in many cases.

> +/*
> + * ptp_sys_offset_extended - data structure for IOCTL operation
> + * PTP_SYS_OFFSET_EXTENDED
> + *
> + * @n_samples: Desired number of measurements.
> + * @clockid: clockid of a clock-base used for pre/post timestamps.
> + * @rsv: Reserved for future use.
> + * @ts: Array of samples in the form [pre-TS, PHC, post-TS]. The
> + * kernel provides @n_samples.
> + *
> + * History:
> + * v1: Initial implementation.
> + *
> + * v2: Use the first word of the reserved-field for @clockid. That's
> + * backward compatible since v1 expects all three reserved words
> + * (@rsv[3]) to be 0 while the clockid (first word in v2) for
> + * CLOCK_REALTIME is '0'.

This is not really appropriate for a source code comment. The
un-merged patch series iterations are preserved at lore.kernel in case
someone needs that.

The "backward compatible" information really wants to be in the commit
message.

Thanks,
Richard