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
> 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. */
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