[PATCH RFC net-next 0/1] ptp: add pseudo pps ioctl

[PATCH RFC net-next 0/1] ptp: add pseudo pps ioctl

[Richard Cochran]

This patch adds a kind of "poor man's PPS" for use with those PHC
drivers which do not support a true PPS. Using this ioctl, user space
can estimate the system-phc offset and tune one of the clocks as
desired.

This patch has been tested on both the Intel igb (PCIe card) and the
National Semiconductor phyter (PHY via MDIO bus), and the results seem
quite promising.

This patch avoids the "timecompare" code on purpose, since experiments
have shown that code to be quite brittle, having been tuned to only
one specific kind of hardware.

Thanks,
Richard


Richard Cochran (1):
  ptp: add an ioctl to compare PHC time with system time

 drivers/ptp/ptp_chardev.c |   32 ++++++++++++++++++++++++++++++++
 include/linux/ptp_clock.h |   14 ++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

-- 
1.7.2.5

[PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time with system time

[Richard Cochran]

This patch adds an ioctl for PTP Hardware Clock (PHC) devices that allows
user space to measure the time offset between the PHC and the system
clock. Rather than hard coding any kind of estimation algorithm into the
kernel, this patch takes the more flexible approach of just delivering
an array of raw clock readings. In that way, the user space clock servo
may be adapted to new and different hardware clocks.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_chardev.c |   32 ++++++++++++++++++++++++++++++++
 include/linux/ptp_clock.h |   14 ++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index e7f301da2..4f8ae80 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -33,9 +33,13 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 {
 	struct ptp_clock_caps caps;
 	struct ptp_clock_request req;
+	struct ptp_sys_offset sysoff;
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
 	struct ptp_clock_info *ops = ptp->info;
+	struct ptp_clock_time *pct;
+	struct timespec ts;
 	int enable, err = 0;
+	unsigned int i;
 
 	switch (cmd) {
 
@@ -88,6 +92,34 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 		err = ops->enable(ops, &req, enable);
 		break;
 
+	case PTP_SYS_OFFSET:
+		if (copy_from_user(&sysoff, (void __user *)arg,
+				   sizeof(sysoff))) {
+			err = -EFAULT;
+			break;
+		}
+		if (sysoff.n_samples > PTP_MAX_SAMPLES) {
+			err = -EINVAL;
+			break;
+		}
+		pct = &sysoff.ts[0];
+		for (i = 0; i < sysoff.n_samples; i++) {
+			getnstimeofday(&ts);
+			pct->sec = ts.tv_sec;
+			pct->nsec = ts.tv_nsec;
+			pct++;
+			ptp->info->gettime(ptp->info, &ts);
+			pct->sec = ts.tv_sec;
+			pct->nsec = ts.tv_nsec;
+			pct++;
+		}
+		getnstimeofday(&ts);
+		pct->sec = ts.tv_sec;
+		pct->nsec = ts.tv_nsec;
+		if (copy_to_user((void __user *)arg, &sysoff, sizeof(sysoff)))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOTTY;
 		break;
diff --git a/include/linux/ptp_clock.h b/include/linux/ptp_clock.h
index 94e981f..b65c834 100644
--- a/include/linux/ptp_clock.h
+++ b/include/linux/ptp_clock.h
@@ -67,12 +67,26 @@ struct ptp_perout_request {
 	unsigned int rsv[4];          /* Reserved for future use. */
 };
 
+#define PTP_MAX_SAMPLES 25 /* Maximum allowed offset measurement samples. */
+
+struct ptp_sys_offset {
+	unsigned int n_samples; /* Desired number of measurements. */
+	unsigned int rsv[3];    /* Reserved for future use. */
+	/*
+	 * Array of interleaved system/phc time stamps. The kernel
+	 * will provide 2*n_samples + 1 time stamps, with the last
+	 * one as a system time stamp.
+	 */
+	struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1];
+};
+
 #define PTP_CLK_MAGIC '='
 
 #define PTP_CLOCK_GETCAPS  _IOR(PTP_CLK_MAGIC, 1, struct ptp_clock_caps)
 #define PTP_EXTTS_REQUEST  _IOW(PTP_CLK_MAGIC, 2, struct ptp_extts_request)
 #define PTP_PEROUT_REQUEST _IOW(PTP_CLK_MAGIC, 3, struct ptp_perout_request)
 #define PTP_ENABLE_PPS     _IOW(PTP_CLK_MAGIC, 4, int)
+#define PTP_SYS_OFFSET     _IOW(PTP_CLK_MAGIC, 5, struct ptp_sys_offset)
 
 struct ptp_extts_event {
 	struct ptp_clock_time t; /* Time event occured. */
-- 
1.7.2.5

Re: [PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time with system time

[Miroslav Lichvar]

On Thu, Sep 27, 2012 at 08:12:16PM +0200, Richard Cochran wrote:
> This patch adds an ioctl for PTP Hardware Clock (PHC) devices that allows
> user space to measure the time offset between the PHC and the system
> clock. Rather than hard coding any kind of estimation algorithm into the
> kernel, this patch takes the more flexible approach of just delivering
> an array of raw clock readings. In that way, the user space clock servo
> may be adapted to new and different hardware clocks.

Would it make sense to extend the ioctl to allow also comparing the
PHC with another PHC or perhaps even a different system clock than
CLOCK_REALTIME?

I'm thinking if someone wanted to synchronize one PHC to another, it
should be better to work with phc1-phc2 offsets than combine phc1-sys
and sys-phc2 offsets.

-- 
Miroslav Lichvar

Re: [PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time with system time

[Richard Cochran]

On Fri, Sep 28, 2012 at 09:53:03AM +0200, Miroslav Lichvar wrote:
> On Thu, Sep 27, 2012 at 08:12:16PM +0200, Richard Cochran wrote:
> > This patch adds an ioctl for PTP Hardware Clock (PHC) devices that allows
> > user space to measure the time offset between the PHC and the system
> > clock. Rather than hard coding any kind of estimation algorithm into the
> > kernel, this patch takes the more flexible approach of just delivering
> > an array of raw clock readings. In that way, the user space clock servo
> > may be adapted to new and different hardware clocks.
> 
> Would it make sense to extend the ioctl to allow also comparing the
> PHC with another PHC or perhaps even a different system clock than
> CLOCK_REALTIME?
> 
> I'm thinking if someone wanted to synchronize one PHC to another, it
> should be better to work with phc1-phc2 offsets than combine phc1-sys
> and sys-phc2 offsets.

Yes, I did think of that, too. There are some reserved fields in the
ioctl. It would be possible to use one field as a clockid_t for the
second phc device. For the static CLOCK_XYZ clock ids we could have a
switch/case. Then we could read any two clocks in the same way as in
this patch.

I am guessing it would be possible to synchronize two PHC devices to
within a few microseconds this way. Probably that is not good enough
to implement a boundary clock, for example, so I have my doubts about
the utility of this. But in any case, it is possible, and I think that
feature can wait for now.

Thanks,
Richard

Re: [PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time with system time

[Miroslav Lichvar]

On Fri, Sep 28, 2012 at 10:26:38AM +0200, Richard Cochran wrote:
> I am guessing it would be possible to synchronize two PHC devices to
> within a few microseconds this way. Probably that is not good enough
> to implement a boundary clock, for example, so I have my doubts about
> the utility of this. 

I think with two identical PHCs the error would be much smaller, even
if the two fastest consecutive readings took together ~5 microseconds.
The error could be measured with a short cable connecting the two ports
and compared the TX and RX timestamps, and compensated in the
software if it's significant.

> But in any case, it is possible, and I think that
> feature can wait for now.

Ok, thanks.

-- 
Miroslav Lichvar

RE: [PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time with system time

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Richard Cochran
> Sent: Thursday, September 27, 2012 11:12 AM
> To: netdev@vger.kernel.org
> Cc: David Miller; Keller, Jacob E; John Stultz; Miroslav Lichvar
> Subject: [PATCH RFC net-next 1/1] ptp: add an ioctl to compare PHC time
> with system time
> 
> This patch adds an ioctl for PTP Hardware Clock (PHC) devices that allows
> user space to measure the time offset between the PHC and the system
> clock. Rather than hard coding any kind of estimation algorithm into the
> kernel, this patch takes the more flexible approach of just delivering an
> array of raw clock readings. In that way, the user space clock servo may
> be adapted to new and different hardware clocks.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/ptp/ptp_chardev.c |   32 ++++++++++++++++++++++++++++++++
>  include/linux/ptp_clock.h |   14 ++++++++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index
> e7f301da2..4f8ae80 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -33,9 +33,13 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int
> cmd, unsigned long arg)  {
>  	struct ptp_clock_caps caps;
>  	struct ptp_clock_request req;
> +	struct ptp_sys_offset sysoff;
>  	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
>  	struct ptp_clock_info *ops = ptp->info;
> +	struct ptp_clock_time *pct;
> +	struct timespec ts;
>  	int enable, err = 0;
> +	unsigned int i;
> 
>  	switch (cmd) {
> 
> @@ -88,6 +92,34 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int
> cmd, unsigned long arg)
>  		err = ops->enable(ops, &req, enable);
>  		break;
> 
> +	case PTP_SYS_OFFSET:
> +		if (copy_from_user(&sysoff, (void __user *)arg,
> +				   sizeof(sysoff))) {
> +			err = -EFAULT;
> +			break;
> +		}
> +		if (sysoff.n_samples > PTP_MAX_SAMPLES) {
> +			err = -EINVAL;
> +			break;
> +		}
> +		pct = &sysoff.ts[0];
> +		for (i = 0; i < sysoff.n_samples; i++) {
> +			getnstimeofday(&ts);
> +			pct->sec = ts.tv_sec;
> +			pct->nsec = ts.tv_nsec;
> +			pct++;
> +			ptp->info->gettime(ptp->info, &ts);
> +			pct->sec = ts.tv_sec;
> +			pct->nsec = ts.tv_nsec;
> +			pct++;
> +		}
> +		getnstimeofday(&ts);
> +		pct->sec = ts.tv_sec;
> +		pct->nsec = ts.tv_nsec;
> +		if (copy_to_user((void __user *)arg, &sysoff,
> sizeof(sysoff)))
> +			err = -EFAULT;
> +		break;
> +
>  	default:
>  		err = -ENOTTY;
>  		break;
> diff --git a/include/linux/ptp_clock.h b/include/linux/ptp_clock.h index
> 94e981f..b65c834 100644
> --- a/include/linux/ptp_clock.h
> +++ b/include/linux/ptp_clock.h
> @@ -67,12 +67,26 @@ struct ptp_perout_request {
>  	unsigned int rsv[4];          /* Reserved for future use. */
>  };
> 
> +#define PTP_MAX_SAMPLES 25 /* Maximum allowed offset measurement
> +samples. */
> +
> +struct ptp_sys_offset {
> +	unsigned int n_samples; /* Desired number of measurements. */
> +	unsigned int rsv[3];    /* Reserved for future use. */
> +	/*
> +	 * Array of interleaved system/phc time stamps. The kernel
> +	 * will provide 2*n_samples + 1 time stamps, with the last
> +	 * one as a system time stamp.
> +	 */
> +	struct ptp_clock_time ts[2 * PTP_MAX_SAMPLES + 1]; };
> +
>  #define PTP_CLK_MAGIC '='
> 
>  #define PTP_CLOCK_GETCAPS  _IOR(PTP_CLK_MAGIC, 1, struct ptp_clock_caps)
> #define PTP_EXTTS_REQUEST  _IOW(PTP_CLK_MAGIC, 2, struct
> ptp_extts_request)  #define PTP_PEROUT_REQUEST _IOW(PTP_CLK_MAGIC, 3,
> struct ptp_perout_request)
>  #define PTP_ENABLE_PPS     _IOW(PTP_CLK_MAGIC, 4, int)
> +#define PTP_SYS_OFFSET     _IOW(PTP_CLK_MAGIC, 5, struct ptp_sys_offset)
> 
>  struct ptp_extts_event {
>  	struct ptp_clock_time t; /* Time event occured. */
> --
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

This is much nicer than performing the same reads manually in user-space.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>