[PATCH net-next v3 1/2] ptp: introduce PTP_CLOCK_EXTOFF event for the measured external offset

[PATCH net-next v3 1/2] ptp: introduce PTP_CLOCK_EXTOFF event for the measured external offset

[Min Li]

From: Min Li <min.li.xe@renesas.com>

This change is for the PHC devices that can measure the phase offset
between PHC signal and the external signal, such as the 1PPS signal of
GNSS. Reporting PTP_CLOCK_EXTOFF to user space will be piggy-backed to
the existing ptp_extts_event so that application such as ts2phc can
poll the external offset the same way as extts. Hence, ts2phc can use
the offset to achieve the alignment between PHC and the external signal
by the help of either SW or HW filters.

Signed-off-by: Min Li <min.li.xe@renesas.com>
---
-Rebase to net-next tree
-Fix the typo and ns2counters suggested by Simon

 drivers/ptp/ptp_clock.c          | 12 +++++++++---
 include/linux/ptp_clock_kernel.h |  3 +++
 include/uapi/linux/ptp_clock.h   |  9 +++++++--
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 15b804ba4..01b55c1e2 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -48,14 +48,19 @@ static void enqueue_external_timestamp(struct timestamp_event_queue *queue,
 	s64 seconds;
 	u32 remainder;
 
-	seconds = div_u64_rem(src->timestamp, 1000000000, &remainder);
+	if (src->type != PTP_CLOCK_EXTOFF)
+		seconds = div_u64_rem(src->timestamp, 1000000000, &remainder);
 
 	spin_lock_irqsave(&queue->lock, flags);
 
 	dst = &queue->buf[queue->tail];
 	dst->index = src->index;
-	dst->t.sec = seconds;
-	dst->t.nsec = remainder;
+	if (src->type != PTP_CLOCK_EXTOFF) {
+		dst->t.sec = seconds;
+		dst->t.nsec = remainder;
+	} else {
+		dst->offset_ns = src->offset;
+	}
 
 	/* Both WRITE_ONCE() are paired with READ_ONCE() in queue_cnt() */
 	if (!queue_free(queue))
@@ -417,6 +422,7 @@ void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event)
 		break;
 
 	case PTP_CLOCK_EXTTS:
+	case PTP_CLOCK_EXTOFF:
 		/* Enqueue timestamp on selected queues */
 		spin_lock_irqsave(&ptp->tsevqs_lock, flags);
 		list_for_each_entry(tsevq, &ptp->tsevqs, qlist) {
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 1ef4e0f9b..6e4b8206c 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -200,6 +200,7 @@ struct ptp_clock;
 enum ptp_clock_events {
 	PTP_CLOCK_ALARM,
 	PTP_CLOCK_EXTTS,
+	PTP_CLOCK_EXTOFF,
 	PTP_CLOCK_PPS,
 	PTP_CLOCK_PPSUSR,
 };
@@ -210,6 +211,7 @@ enum ptp_clock_events {
  * @type:  One of the ptp_clock_events enumeration values.
  * @index: Identifies the source of the event.
  * @timestamp: When the event occurred (%PTP_CLOCK_EXTTS only).
+ * @offset:    When the event occurred (%PTP_CLOCK_EXTOFF only).
  * @pps_times: When the event occurred (%PTP_CLOCK_PPSUSR only).
  */
 
@@ -218,6 +220,7 @@ struct ptp_clock_event {
 	int index;
 	union {
 		u64 timestamp;
+		s64 offset;
 		struct pps_event_time pps_times;
 	};
 };
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index da700999c..66f4dd73a 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -32,6 +32,7 @@
 #define PTP_RISING_EDGE    (1<<1)
 #define PTP_FALLING_EDGE   (1<<2)
 #define PTP_STRICT_FLAGS   (1<<3)
+#define PTP_EXT_OFFSET     (1<<4)
 #define PTP_EXTTS_EDGES    (PTP_RISING_EDGE | PTP_FALLING_EDGE)
 
 /*
@@ -40,7 +41,8 @@
 #define PTP_EXTTS_VALID_FLAGS	(PTP_ENABLE_FEATURE |	\
 				 PTP_RISING_EDGE |	\
 				 PTP_FALLING_EDGE |	\
-				 PTP_STRICT_FLAGS)
+				 PTP_STRICT_FLAGS |	\
+				 PTP_EXT_OFFSET)
 
 /*
  * flag fields valid for the original PTP_EXTTS_REQUEST ioctl.
@@ -228,7 +230,10 @@ struct ptp_pin_desc {
 #define PTP_MASK_EN_SINGLE  _IOW(PTP_CLK_MAGIC, 20, unsigned int)
 
 struct ptp_extts_event {
-	struct ptp_clock_time t; /* Time event occured. */
+	union {
+		struct ptp_clock_time t; /* Time event occurred. */
+		__s64 offset_ns;         /* Offset event occurred. */
+	};
 	unsigned int index;      /* Which channel produced the event. */
 	unsigned int flags;      /* Reserved for future use. */
 	unsigned int rsv[2];     /* Reserved for future use. */
-- 
2.39.2

Re: [PATCH net-next v3 1/2] ptp: introduce PTP_CLOCK_EXTOFF event for the measured external offset

[Richard Cochran]

On Thu, Dec 14, 2023 at 11:36:24AM -0500, Min Li wrote:
> diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> index da700999c..66f4dd73a 100644
> --- a/include/uapi/linux/ptp_clock.h
> +++ b/include/uapi/linux/ptp_clock.h
> @@ -32,6 +32,7 @@
>  #define PTP_RISING_EDGE    (1<<1)
>  #define PTP_FALLING_EDGE   (1<<2)
>  #define PTP_STRICT_FLAGS   (1<<3)
> +#define PTP_EXT_OFFSET     (1<<4)

This isn't going to work.

If user space enables time stamps twice, once with PTP_EXT_OFFSET and
once without, it won't be able to differentiate the two when it reads
a ptp_extts_event.

Thanks,
Richard

RE: [PATCH net-next v3 1/2] ptp: introduce PTP_CLOCK_EXTOFF event for the measured external offset

[Min Li]

> On Thu, Dec 14, 2023 at 11:36:24AM -0500, Min Li wrote:
> > diff --git a/include/uapi/linux/ptp_clock.h
> > b/include/uapi/linux/ptp_clock.h index da700999c..66f4dd73a 100644
> > --- a/include/uapi/linux/ptp_clock.h
> > +++ b/include/uapi/linux/ptp_clock.h
> > @@ -32,6 +32,7 @@
> >  #define PTP_RISING_EDGE    (1<<1)
> >  #define PTP_FALLING_EDGE   (1<<2)
> >  #define PTP_STRICT_FLAGS   (1<<3)
> > +#define PTP_EXT_OFFSET     (1<<4)
> 
> This isn't going to work.
> 
> If user space enables time stamps twice, once with PTP_EXT_OFFSET and
> once without, it won't be able to differentiate the two when it reads a
> ptp_extts_event.
> 
> Thanks,
> Richard

Hi Richard
Would it be Ok if I use the flags to differentiate extts events from extoff?

 struct ptp_extts_event {
-       struct ptp_clock_time t; /* Time event occured. */
+       union {
+               struct ptp_clock_time t; /* Time event occurred. */
+               __s64 offset_ns;         /* Offset event occurred. */
+       };
         unsigned int index;      /* Which channel produced the event. */
         unsigned int flags;      /* Reserved for future use. */
         unsigned int rsv[2];     /* Reserved for future use. */

Re: [PATCH net-next v3 1/2] ptp: introduce PTP_CLOCK_EXTOFF event for the measured external offset

[Richard Cochran]

On Thu, Dec 14, 2023 at 09:59:32PM +0000, Min Li wrote:

> Would it be Ok if I use the flags to differentiate extts events from extoff?

That makes sense to me.  We can return the relevant
ptp_extts_request.flags.  Something like:

#define PTP_EXTTS_FLAGS_VALID	PTP_ENABLE_FEATURE

Then you can return

	ptp_extts_event.flags = PTP_EXTTS_FLAGS_VALID | PTP_EXT_OFFSET;

Later on, other drivers can indicate PTP_[RISING|FALLING]_EDGE, if
they can tell which one happened.

>  struct ptp_extts_event {
> -       struct ptp_clock_time t; /* Time event occured. */
> +       union {
> +               struct ptp_clock_time t; /* Time event occurred. */
> +               __s64 offset_ns;         /* Offset event occurred. */

BTW, please don't make a union here.  Instead just add text to the
comment of `struct ptp_clock_time t`.

The `struct ptp_clock_time t` can be a postive or a negative value
(see comment at the top of the file), and so you can put an offset in
there as well.

Thanks,
Richard