The ability to read the PHC (Physical Hardware Clock) alongside
multiple system clocks is currently dependent on the specific
hardware architecture. This limitation restricts the use of
PTP_SYS_OFFSET_PRECISE to certain hardware configurations.
The generic soultion which would work across all architectures
is to read the PHC along with the latency to perform PHC-read as
offered by PTP_SYS_OFFSET_EXTENDED which provides pre and post
timestamps. However, these timestamps are currently limited
to the CLOCK_REALTIME timebase. Since CLOCK_REALTIME is affected
by NTP (or similar time synchronization services), it can
experience significant jumps forward or backward. This hinders
the precise latency measurements that PTP_SYS_OFFSET_EXTENDED
is designed to provide.
This problem could be addressed by supporting MONOTONIC_RAW
timestamps within PTP_SYS_OFFSET_EXTENDED. Unlike CLOCK_REALTIME
or CLOCK_MONOTONIC, the MONOTONIC_RAW timebase is unaffected
by NTP adjustments.
This enhancement can be implemented by utilizing one of the three
reserved words within the PTP_SYS_OFFSET_EXTENDED struct to pass
the clock-id for timestamps. The current behavior aligns with
clock-id for CLOCK_REALTIME timebase (value of 0), ensuring
backward compatibility of the UAPI.
Signed-off-by: Mahesh Bandewar <[email protected]>
---
v1 -> v2
* Code-style fixes
v2 -> v3
* Reword commit log
* Fix the compilation issue by using __kernel_clockid instead of clockid_t
which has kernel only scope.
drivers/ptp/ptp_chardev.c | 7 +++++--
include/linux/ptp_clock_kernel.h | 30 ++++++++++++++++++++++++++----
include/uapi/linux/ptp_clock.h | 26 ++++++++++++++++++++------
3 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 7513018c9f9a..c109109c9e8e 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -358,11 +358,14 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
extoff = NULL;
break;
}
- if (extoff->n_samples > PTP_MAX_SAMPLES
- || extoff->rsv[0] || extoff->rsv[1] || extoff->rsv[2]) {
+ if (extoff->n_samples > PTP_MAX_SAMPLES ||
+ extoff->rsv[0] || extoff->rsv[1] ||
+ (extoff->clockid != CLOCK_REALTIME &&
+ extoff->clockid != CLOCK_MONOTONIC_RAW)) {
err = -EINVAL;
break;
}
+ sts.clockid = extoff->clockid;
for (i = 0; i < extoff->n_samples; i++) {
err = ptp->info->gettimex64(ptp->info, &ts, &sts);
if (err)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 6e4b8206c7d0..74ded5f95d95 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -47,10 +47,12 @@ struct system_device_crosststamp;
* struct ptp_system_timestamp - system time corresponding to a PHC timestamp
* @pre_ts: system timestamp before capturing PHC
* @post_ts: system timestamp after capturing PHC
+ * @clockid: clock-base used for capturing the system timestamps
*/
struct ptp_system_timestamp {
struct timespec64 pre_ts;
struct timespec64 post_ts;
+ clockid_t clockid;
};
/**
@@ -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;
+ default:
+ break;
+ }
+ }
}
static inline void ptp_read_system_postts(struct ptp_system_timestamp *sts)
{
- if (sts)
- ktime_get_real_ts64(&sts->post_ts);
+ if (sts) {
+ switch (sts->clockid) {
+ case CLOCK_REALTIME:
+ ktime_get_real_ts64(&sts->post_ts);
+ break;
+ case CLOCK_MONOTONIC_RAW:
+ ktime_get_raw_ts64(&sts->post_ts);
+ break;
+ default:
+ break;
+ }
+ }
}
#endif
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 053b40d642de..02684d7e00e6 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -155,13 +155,27 @@ struct ptp_sys_offset {
struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
};
+/*
+ * 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
+ * backword compatible since v1 expects @rsv[3] to be 0 while the
+ * clockid for CLOCK_REALTIME is '0'.
+ */
struct ptp_sys_offset_extended {
- unsigned int n_samples; /* Desired number of measurements. */
- unsigned int rsv[3]; /* Reserved for future use. */
- /*
- * Array of [system, phc, system] time stamps. The kernel will provide
- * 3*n_samples time stamps.
- */
+ unsigned int n_samples;
+ __kernel_clockid_t clockid;
+ unsigned int rsv[2];
struct ptp_clock_time ts[PTP_MAX_SAMPLES][3];
};
--
2.44.0.769.g3c40516874-goog
On Mon, 2024-04-22 at 15:07 -0700, Mahesh Bandewar wrote:
> diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> index 053b40d642de..02684d7e00e6 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -155,13 +155,27 @@ struct ptp_sys_offset {
> struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
> };
>
> +/*
> + * 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
> + * backword compatible since v1 expects @rsv[3] to be 0 while the
Minor nit ^^^ 'backward'
> + * clockid for CLOCK_REALTIME is '0'.
> + */
> struct ptp_sys_offset_extended {
> - unsigned int n_samples; /* Desired number of measurements. */
> - unsigned int rsv[3]; /* Reserved for future use. */
> - /*
> - * Array of [system, phc, system] time stamps. The kernel will provide
> - * 3*n_samples time stamps.
> - */
> + unsigned int n_samples;
> + __kernel_clockid_t clockid;
> + unsigned int rsv[2];
AFAICS, here you are using the 'old' rsv[0] as 'clockid'
Please either always use rsv[0] instead of rsv[2] or re-order the
fields.
Thanks,
Paolo