All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] PHC frequency fine tuning
@ 2016-11-08 21:49 ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Manfred Rudigier, Stefan Sørensen, Thomas Gleixner,
	Ulrik De Bie, intel-wired-lan

This series expands the PTP Hardware Clock subsystem by adding a
method that passes the frequency tuning word to the the drivers
without dropping the low order bits.  Keeping those bits is useful for
drivers whose frequency resolution is higher than 1 ppb.

The appended script (below) runs a simple demonstration of the
improvement.  This test needs two Intel i210 PCIe cards installed in
the same PC, with their SDP0 pins connected by copper wire.  Measuring
the estimated offset (from the ptp4l servo) and the true offset (from
the PPS) over one hour yields the following statistics.

|        |   Est. Before |    Est. After |   True Before |    True After |
|--------+---------------+---------------+---------------+---------------|
| min    | -5.200000e+01 | -1.600000e+01 | -3.100000e+01 | -1.000000e+00 |
| max    | +5.700000e+01 | +2.500000e+01 | +8.500000e+01 | +4.000000e+01 |
| pk-pk: | +1.090000e+02 | +4.100000e+01 | +1.160000e+02 | +4.100000e+01 |
| mean   | +6.472222e-02 | +1.277778e-02 | +2.422083e+01 | +1.826083e+01 |
| stddev | +1.158006e+01 | +4.581982e+00 | +1.207708e+01 | +4.981435e+00 |

Here the numbers in units of nanoseconds, and the ~20 nanosecond PPS
offset is due to input/output delays on the i210's external interface
logic.

With the series applied, both the peak to peak error and the standard
deviation improve by a factor of more than two.  These two graphs show
the improvement nicely.

  http://linuxptp.sourceforge.net/fine-tuning/fine-est.png

  http://linuxptp.sourceforge.net/fine-tuning/fine-tru.png


Thanks,
Richard

Richard Cochran (3):
  ptp: Introduce a high resolution frequency adjustment method.
  ptp: igb: Use the high resolution frequency method.
  ptp: dp83640: Use the high resolution frequency method.

 drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
 drivers/net/phy/dp83640.c                | 14 +++++++-------
 drivers/ptp/ptp_clock.c                  |  5 ++++-
 include/linux/ptp_clock_kernel.h         |  8 ++++++++
 4 files changed, 27 insertions(+), 16 deletions(-)

-- 
2.1.4

---
#!/bin/sh

set -e
set -x

killall ptp4l || true

DUR=3600
ETHA=eth6
ETHB=eth3
DEVA=/dev/ptp`ethtool -T $ETHA | awk '/PTP/ {print $4}'`
DEVB=/dev/ptp`ethtool -T $ETHB | awk '/PTP/ {print $4}'`

testptp -d $DEVA -p 0

for x in $DEVA $DEVB; do
	testptp -d $x -f 0
	testptp -d $x -s
done

testptp -d $DEVA -L 0,2  # periodic output
testptp -d $DEVB -L 0,1  # external time stamp
testptp -d $DEVA -p 2000000000

ptp4l -m -q -2 -i $ETHA > log.master &
ptp4l -m -q -2 -i $ETHB -s > log.slave &

sleep 60
testptp -d $DEVB -e $DUR > log.pps
tail -n $DUR log.slave > log.est

killall ptp4l

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 0/3] PHC frequency fine tuning
@ 2016-11-08 21:49 ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: intel-wired-lan

This series expands the PTP Hardware Clock subsystem by adding a
method that passes the frequency tuning word to the the drivers
without dropping the low order bits.  Keeping those bits is useful for
drivers whose frequency resolution is higher than 1 ppb.

The appended script (below) runs a simple demonstration of the
improvement.  This test needs two Intel i210 PCIe cards installed in
the same PC, with their SDP0 pins connected by copper wire.  Measuring
the estimated offset (from the ptp4l servo) and the true offset (from
the PPS) over one hour yields the following statistics.

|        |   Est. Before |    Est. After |   True Before |    True After |
|--------+---------------+---------------+---------------+---------------|
| min    | -5.200000e+01 | -1.600000e+01 | -3.100000e+01 | -1.000000e+00 |
| max    | +5.700000e+01 | +2.500000e+01 | +8.500000e+01 | +4.000000e+01 |
| pk-pk: | +1.090000e+02 | +4.100000e+01 | +1.160000e+02 | +4.100000e+01 |
| mean   | +6.472222e-02 | +1.277778e-02 | +2.422083e+01 | +1.826083e+01 |
| stddev | +1.158006e+01 | +4.581982e+00 | +1.207708e+01 | +4.981435e+00 |

Here the numbers in units of nanoseconds, and the ~20 nanosecond PPS
offset is due to input/output delays on the i210's external interface
logic.

With the series applied, both the peak to peak error and the standard
deviation improve by a factor of more than two.  These two graphs show
the improvement nicely.

  http://linuxptp.sourceforge.net/fine-tuning/fine-est.png

  http://linuxptp.sourceforge.net/fine-tuning/fine-tru.png


Thanks,
Richard

Richard Cochran (3):
  ptp: Introduce a high resolution frequency adjustment method.
  ptp: igb: Use the high resolution frequency method.
  ptp: dp83640: Use the high resolution frequency method.

 drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
 drivers/net/phy/dp83640.c                | 14 +++++++-------
 drivers/ptp/ptp_clock.c                  |  5 ++++-
 include/linux/ptp_clock_kernel.h         |  8 ++++++++
 4 files changed, 27 insertions(+), 16 deletions(-)

-- 
2.1.4

---
#!/bin/sh

set -e
set -x

killall ptp4l || true

DUR=3600
ETHA=eth6
ETHB=eth3
DEVA=/dev/ptp`ethtool -T $ETHA | awk '/PTP/ {print $4}'`
DEVB=/dev/ptp`ethtool -T $ETHB | awk '/PTP/ {print $4}'`

testptp -d $DEVA -p 0

for x in $DEVA $DEVB; do
	testptp -d $x -f 0
	testptp -d $x -s
done

testptp -d $DEVA -L 0,2  # periodic output
testptp -d $DEVB -L 0,1  # external time stamp
testptp -d $DEVA -p 2000000000

ptp4l -m -q -2 -i $ETHA > log.master &
ptp4l -m -q -2 -i $ETHB -s > log.slave &

sleep 60
testptp -d $DEVB -e $DUR > log.pps
tail -n $DUR log.slave > log.est

killall ptp4l

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH net-next 1/3] ptp: Introduce a high resolution frequency adjustment method.
  2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 21:49   ` Richard Cochran
  -1 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Manfred Rudigier, Stefan Sørensen, Thomas Gleixner,
	Ulrik De Bie, intel-wired-lan

The internal PTP Hardware Clock (PHC) interface limits the resolution for
frequency adjustments to one part per billion.  However, some hardware
devices allow finer adjustment, and making use of the increased resolution
improves synchronization measurably on such devices.

This patch adds an alternative method that allows finer frequency tuning
by passing the scaled ppm value to PHC drivers.  This value comes from
user space, and it has a resolution of about 0.015 ppb.  We also deprecate
the older method, anticipating its removal once existing drivers have been
converted over.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Suggested-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
---
 drivers/ptp/ptp_clock.c          | 5 ++++-
 include/linux/ptp_clock_kernel.h | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 86280b7..9c13381 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -153,7 +153,10 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 		s32 ppb = scaled_ppm_to_ppb(tx->freq);
 		if (ppb > ops->max_adj || ppb < -ops->max_adj)
 			return -ERANGE;
-		err = ops->adjfreq(ops, ppb);
+		if (ops->adjfine)
+			err = ops->adjfine(ops, tx->freq);
+		else
+			err = ops->adjfreq(ops, ppb);
 		ptp->dialed_frequency = tx->freq;
 	} else if (tx->modes == 0) {
 		tx->freq = ptp->dialed_frequency;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 5ad54fc6..b76d47a 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -58,7 +58,14 @@ struct system_device_crosststamp;
  *
  * clock operations
  *
+ * @adjfine:  Adjusts the frequency of the hardware clock.
+ *            parameter scaled_ppm: Desired frequency offset from
+ *            nominal frequency in parts per million, but with a
+ *            16 bit binary fractional field.
+ *
  * @adjfreq:  Adjusts the frequency of the hardware clock.
+ *            This method is deprecated.  New drivers should implement
+ *            the @adjfine method instead.
  *            parameter delta: Desired frequency offset from nominal frequency
  *            in parts per billion
  *
@@ -108,6 +115,7 @@ struct ptp_clock_info {
 	int n_pins;
 	int pps;
 	struct ptp_pin_desc *pin_config;
+	int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm);
 	int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
 	int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
 	int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 1/3] ptp: Introduce a high resolution frequency adjustment method.
@ 2016-11-08 21:49   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: intel-wired-lan

The internal PTP Hardware Clock (PHC) interface limits the resolution for
frequency adjustments to one part per billion.  However, some hardware
devices allow finer adjustment, and making use of the increased resolution
improves synchronization measurably on such devices.

This patch adds an alternative method that allows finer frequency tuning
by passing the scaled ppm value to PHC drivers.  This value comes from
user space, and it has a resolution of about 0.015 ppb.  We also deprecate
the older method, anticipating its removal once existing drivers have been
converted over.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Suggested-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
---
 drivers/ptp/ptp_clock.c          | 5 ++++-
 include/linux/ptp_clock_kernel.h | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 86280b7..9c13381 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -153,7 +153,10 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 		s32 ppb = scaled_ppm_to_ppb(tx->freq);
 		if (ppb > ops->max_adj || ppb < -ops->max_adj)
 			return -ERANGE;
-		err = ops->adjfreq(ops, ppb);
+		if (ops->adjfine)
+			err = ops->adjfine(ops, tx->freq);
+		else
+			err = ops->adjfreq(ops, ppb);
 		ptp->dialed_frequency = tx->freq;
 	} else if (tx->modes == 0) {
 		tx->freq = ptp->dialed_frequency;
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 5ad54fc6..b76d47a 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -58,7 +58,14 @@ struct system_device_crosststamp;
  *
  * clock operations
  *
+ * @adjfine:  Adjusts the frequency of the hardware clock.
+ *            parameter scaled_ppm: Desired frequency offset from
+ *            nominal frequency in parts per million, but with a
+ *            16 bit binary fractional field.
+ *
  * @adjfreq:  Adjusts the frequency of the hardware clock.
+ *            This method is deprecated.  New drivers should implement
+ *            the @adjfine method instead.
  *            parameter delta: Desired frequency offset from nominal frequency
  *            in parts per billion
  *
@@ -108,6 +115,7 @@ struct ptp_clock_info {
 	int n_pins;
 	int pps;
 	struct ptp_pin_desc *pin_config;
+	int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm);
 	int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
 	int (*adjtime)(struct ptp_clock_info *ptp, s64 delta);
 	int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 21:49   ` Richard Cochran
  -1 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Manfred Rudigier, Stefan Sørensen, Thomas Gleixner,
	Ulrik De Bie, intel-wired-lan

The 82580 and related devices offer a frequency resolution of about
0.029 ppb.  This patch lets users of the device benefit from the
increased frequency resolution when tuning the clock.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index a7895c4..c30eea8 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -226,7 +226,7 @@ static int igb_ptp_adjfreq_82576(struct ptp_clock_info *ptp, s32 ppb)
 	return 0;
 }
 
-static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
+static int igb_ptp_adjfine_82580(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
@@ -235,13 +235,13 @@ static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
 	u64 rate;
 	u32 inca;
 
-	if (ppb < 0) {
+	if (scaled_ppm < 0) {
 		neg_adj = 1;
-		ppb = -ppb;
+		scaled_ppm = -scaled_ppm;
 	}
-	rate = ppb;
-	rate <<= 26;
-	rate = div_u64(rate, 1953125);
+	rate = scaled_ppm;
+	rate <<= 13;
+	rate = div_u64(rate, 15625);
 
 	inca = rate & INCVALUE_MASK;
 	if (neg_adj)
@@ -1103,7 +1103,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
-		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
 		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
@@ -1131,7 +1131,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.n_pins = IGB_N_SDP;
 		adapter->ptp_caps.pps = 1;
 		adapter->ptp_caps.pin_config = adapter->sdp_config;
-		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_i210;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-08 21:49   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: intel-wired-lan

The 82580 and related devices offer a frequency resolution of about
0.029 ppb.  This patch lets users of the device benefit from the
increased frequency resolution when tuning the clock.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index a7895c4..c30eea8 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -226,7 +226,7 @@ static int igb_ptp_adjfreq_82576(struct ptp_clock_info *ptp, s32 ppb)
 	return 0;
 }
 
-static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
+static int igb_ptp_adjfine_82580(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
 					       ptp_caps);
@@ -235,13 +235,13 @@ static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32 ppb)
 	u64 rate;
 	u32 inca;
 
-	if (ppb < 0) {
+	if (scaled_ppm < 0) {
 		neg_adj = 1;
-		ppb = -ppb;
+		scaled_ppm = -scaled_ppm;
 	}
-	rate = ppb;
-	rate <<= 26;
-	rate = div_u64(rate, 1953125);
+	rate = scaled_ppm;
+	rate <<= 13;
+	rate = div_u64(rate, 15625);
 
 	inca = rate & INCVALUE_MASK;
 	if (neg_adj)
@@ -1103,7 +1103,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
 		adapter->ptp_caps.pps = 0;
-		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
 		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
@@ -1131,7 +1131,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.n_pins = IGB_N_SDP;
 		adapter->ptp_caps.pps = 1;
 		adapter->ptp_caps.pin_config = adapter->sdp_config;
-		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
+		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime64 = igb_ptp_settime_i210;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH net-next 3/3] ptp: dp83640: Use the high resolution frequency method.
  2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 21:49   ` Richard Cochran
  -1 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Manfred Rudigier, Stefan Sørensen, Thomas Gleixner,
	Ulrik De Bie, intel-wired-lan

The dp83640 has a frequency resolution of about 0.029 ppb.
This patch lets users of the device benefit from the
increased frequency resolution when tuning the clock.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/phy/dp83640.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 7a240fc..e2460a5 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -375,7 +375,7 @@ static int periodic_output(struct dp83640_clock *clock,
 
 /* ptp clock methods */
 
-static int ptp_dp83640_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+static int ptp_dp83640_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct dp83640_clock *clock =
 		container_of(ptp, struct dp83640_clock, caps);
@@ -384,13 +384,13 @@ static int ptp_dp83640_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	int neg_adj = 0;
 	u16 hi, lo;
 
-	if (ppb < 0) {
+	if (scaled_ppm < 0) {
 		neg_adj = 1;
-		ppb = -ppb;
+		scaled_ppm = -scaled_ppm;
 	}
-	rate = ppb;
-	rate <<= 26;
-	rate = div_u64(rate, 1953125);
+	rate = scaled_ppm;
+	rate <<= 13;
+	rate = div_u64(rate, 15625);
 
 	hi = (rate >> 16) & PTP_RATE_HI_MASK;
 	if (neg_adj)
@@ -1035,7 +1035,7 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	clock->caps.n_per_out	= N_PER_OUT;
 	clock->caps.n_pins	= DP83640_N_PINS;
 	clock->caps.pps		= 0;
-	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
+	clock->caps.adjfine	= ptp_dp83640_adjfine;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
 	clock->caps.gettime64	= ptp_dp83640_gettime;
 	clock->caps.settime64	= ptp_dp83640_settime;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 3/3] ptp: dp83640: Use the high resolution frequency method.
@ 2016-11-08 21:49   ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-08 21:49 UTC (permalink / raw)
  To: intel-wired-lan

The dp83640 has a frequency resolution of about 0.029 ppb.
This patch lets users of the device benefit from the
increased frequency resolution when tuning the clock.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/phy/dp83640.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 7a240fc..e2460a5 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -375,7 +375,7 @@ static int periodic_output(struct dp83640_clock *clock,
 
 /* ptp clock methods */
 
-static int ptp_dp83640_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+static int ptp_dp83640_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct dp83640_clock *clock =
 		container_of(ptp, struct dp83640_clock, caps);
@@ -384,13 +384,13 @@ static int ptp_dp83640_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	int neg_adj = 0;
 	u16 hi, lo;
 
-	if (ppb < 0) {
+	if (scaled_ppm < 0) {
 		neg_adj = 1;
-		ppb = -ppb;
+		scaled_ppm = -scaled_ppm;
 	}
-	rate = ppb;
-	rate <<= 26;
-	rate = div_u64(rate, 1953125);
+	rate = scaled_ppm;
+	rate <<= 13;
+	rate = div_u64(rate, 15625);
 
 	hi = (rate >> 16) & PTP_RATE_HI_MASK;
 	if (neg_adj)
@@ -1035,7 +1035,7 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	clock->caps.n_per_out	= N_PER_OUT;
 	clock->caps.n_pins	= DP83640_N_PINS;
 	clock->caps.pps		= 0;
-	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
+	clock->caps.adjfine	= ptp_dp83640_adjfine;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
 	clock->caps.gettime64	= ptp_dp83640_gettime;
 	clock->caps.settime64	= ptp_dp83640_settime;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 0/3] PHC frequency fine tuning
  2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 21:56   ` Keller, Jacob E
  -1 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 21:56 UTC (permalink / raw)
  To: netdev, richardcochran
  Cc: tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen, davem,
	Kirsher, Jeffrey T, john.stultz, intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> This series expands the PTP Hardware Clock subsystem by adding a
> method that passes the frequency tuning word to the the drivers
> without dropping the low order bits.  Keeping those bits is useful
> for
> drivers whose frequency resolution is higher than 1 ppb.
> 

Makes sense.

> The appended script (below) runs a simple demonstration of the
> improvement.  This test needs two Intel i210 PCIe cards installed in
> the same PC, with their SDP0 pins connected by copper
> wire.  Measuring
> the estimated offset (from the ptp4l servo) and the true offset (from
> the PPS) over one hour yields the following statistics.
> 
> > 
> >        |   Est. Before |    Est. After |   True Before |    True
> > After |
> > --------+---------------+---------------+---------------+--------
> > -------|
> > min    | -5.200000e+01 | -1.600000e+01 | -3.100000e+01 |
> > -1.000000e+00 |
> > max    | +5.700000e+01 | +2.500000e+01 | +8.500000e+01 |
> > +4.000000e+01 |
> > pk-pk: | +1.090000e+02 | +4.100000e+01 | +1.160000e+02 |
> > +4.100000e+01 |
> > mean   | +6.472222e-02 | +1.277778e-02 | +2.422083e+01 |
> > +1.826083e+01 |
> > stddev | +1.158006e+01 | +4.581982e+00 | +1.207708e+01 |
> > +4.981435e+00 |
> 
> Here the numbers in units of nanoseconds, and the ~20 nanosecond PPS
> offset is due to input/output delays on the i210's external interface
> logic.
> 
> With the series applied, both the peak to peak error and the standard
> deviation improve by a factor of more than two.  These two graphs
> show
> the improvement nicely.
> 
>   http://linuxptp.sourceforge.net/fine-tuning/fine-est.png
> 
>   http://linuxptp.sourceforge.net/fine-tuning/fine-tru.png
> 

Wow, nice! I'll take a look at the actual patches in a few minutes, but
this is a really nice improvement!

Thanks,
Jake

> 
> Thanks,
> Richard
> 
> Richard Cochran (3):
>   ptp: Introduce a high resolution frequency adjustment method.
>   ptp: igb: Use the high resolution frequency method.
>   ptp: dp83640: Use the high resolution frequency method.
> 
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
>  drivers/net/phy/dp83640.c                | 14 +++++++-------
>  drivers/ptp/ptp_clock.c                  |  5 ++++-
>  include/linux/ptp_clock_kernel.h         |  8 ++++++++
>  4 files changed, 27 insertions(+), 16 deletions(-)
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 0/3] PHC frequency fine tuning
@ 2016-11-08 21:56   ` Keller, Jacob E
  0 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 21:56 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> This series expands the PTP Hardware Clock subsystem by adding a
> method that passes the frequency tuning word to the the drivers
> without dropping the low order bits.??Keeping those bits is useful
> for
> drivers whose frequency resolution is higher than 1 ppb.
> 

Makes sense.

> The appended script (below) runs a simple demonstration of the
> improvement.??This test needs two Intel i210 PCIe cards installed in
> the same PC, with their SDP0 pins connected by copper
> wire.??Measuring
> the estimated offset (from the ptp4l servo) and the true offset (from
> the PPS) over one hour yields the following statistics.
> 
> > 
> > ???????|???Est. Before |????Est. After |???True Before |????True
> > After |
> > --------+---------------+---------------+---------------+--------
> > -------|
> > min????| -5.200000e+01 | -1.600000e+01 | -3.100000e+01 |
> > -1.000000e+00 |
> > max????| +5.700000e+01 | +2.500000e+01 | +8.500000e+01 |
> > +4.000000e+01 |
> > pk-pk: | +1.090000e+02 | +4.100000e+01 | +1.160000e+02 |
> > +4.100000e+01 |
> > mean???| +6.472222e-02 | +1.277778e-02 | +2.422083e+01 |
> > +1.826083e+01 |
> > stddev | +1.158006e+01 | +4.581982e+00 | +1.207708e+01 |
> > +4.981435e+00 |
> 
> Here the numbers in units of nanoseconds, and the ~20 nanosecond PPS
> offset is due to input/output delays on the i210's external interface
> logic.
> 
> With the series applied, both the peak to peak error and the standard
> deviation improve by a factor of more than two.??These two graphs
> show
> the improvement nicely.
> 
> ? http://linuxptp.sourceforge.net/fine-tuning/fine-est.png
> 
> ? http://linuxptp.sourceforge.net/fine-tuning/fine-tru.png
> 

Wow, nice! I'll take a look at the actual patches in a few minutes, but
this is a really nice improvement!

Thanks,
Jake

> 
> Thanks,
> Richard
> 
> Richard Cochran (3):
> ? ptp: Introduce a high resolution frequency adjustment method.
> ? ptp: igb: Use the high resolution frequency method.
> ? ptp: dp83640: Use the high resolution frequency method.
> 
> ?drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
> ?drivers/net/phy/dp83640.c????????????????| 14 +++++++-------
> ?drivers/ptp/ptp_clock.c??????????????????|??5 ++++-
> ?include/linux/ptp_clock_kernel.h?????????|??8 ++++++++
> ?4 files changed, 27 insertions(+), 16 deletions(-)
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-08 21:49   ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 22:02     ` Keller, Jacob E
  -1 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 22:02 UTC (permalink / raw)
  To: netdev, richardcochran
  Cc: tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen, davem,
	Kirsher, Jeffrey T, john.stultz, intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> The 82580 and related devices offer a frequency resolution of about
> 0.029 ppb.  This patch lets users of the device benefit from the
> increased frequency resolution when tuning the clock.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index a7895c4..c30eea8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -226,7 +226,7 @@ static int igb_ptp_adjfreq_82576(struct
> ptp_clock_info *ptp, s32 ppb)
>  	return 0;
>  }
>  
> -static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32
> ppb)
> +static int igb_ptp_adjfine_82580(struct ptp_clock_info *ptp, long
> scaled_ppm)
>  {
>  	struct igb_adapter *igb = container_of(ptp, struct
> igb_adapter,
>  					       ptp_caps);
> @@ -235,13 +235,13 @@ static int igb_ptp_adjfreq_82580(struct
> ptp_clock_info *ptp, s32 ppb)
>  	u64 rate;
>  	u32 inca;
>  
> -	if (ppb < 0) {
> +	if (scaled_ppm < 0) {
>  		neg_adj = 1;
> -		ppb = -ppb;
> +		scaled_ppm = -scaled_ppm;
>  	}
> -	rate = ppb;
> -	rate <<= 26;
> -	rate = div_u64(rate, 1953125);
> +	rate = scaled_ppm;
> +	rate <<= 13;
> +	rate = div_u64(rate, 15625);
>  

I'm curious how you generate the new math here, since this can be
tricky, and I could use more examples in order to port to some of the
other drivers implementations. I'm not quit sure how to handle the
value when the lower 16 bits are fractional.

Thanks,
Jake

>  	inca = rate & INCVALUE_MASK;
>  	if (neg_adj)
> @@ -1103,7 +1103,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  		adapter->ptp_caps.max_adj = 62499999;
>  		adapter->ptp_caps.n_ext_ts = 0;
>  		adapter->ptp_caps.pps = 0;
> -		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
> +		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
>  		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
>  		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
>  		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
> @@ -1131,7 +1131,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  		adapter->ptp_caps.n_pins = IGB_N_SDP;
>  		adapter->ptp_caps.pps = 1;
>  		adapter->ptp_caps.pin_config = adapter->sdp_config;
> -		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
> +		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
>  		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
>  		adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
>  		adapter->ptp_caps.settime64 = igb_ptp_settime_i210;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-08 22:02     ` Keller, Jacob E
  0 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 22:02 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> The 82580 and related devices offer a frequency resolution of about
> 0.029 ppb.??This patch lets users of the device benefit from the
> increased frequency resolution when tuning the clock.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
> ?drivers/net/ethernet/intel/igb/igb_ptp.c | 16 ++++++++--------
> ?1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index a7895c4..c30eea8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -226,7 +226,7 @@ static int igb_ptp_adjfreq_82576(struct
> ptp_clock_info *ptp, s32 ppb)
> ?	return 0;
> ?}
> ?
> -static int igb_ptp_adjfreq_82580(struct ptp_clock_info *ptp, s32
> ppb)
> +static int igb_ptp_adjfine_82580(struct ptp_clock_info *ptp, long
> scaled_ppm)
> ?{
> ?	struct igb_adapter *igb = container_of(ptp, struct
> igb_adapter,
> ?					???????ptp_caps);
> @@ -235,13 +235,13 @@ static int igb_ptp_adjfreq_82580(struct
> ptp_clock_info *ptp, s32 ppb)
> ?	u64 rate;
> ?	u32 inca;
> ?
> -	if (ppb < 0) {
> +	if (scaled_ppm < 0) {
> ?		neg_adj = 1;
> -		ppb = -ppb;
> +		scaled_ppm = -scaled_ppm;
> ?	}
> -	rate = ppb;
> -	rate <<= 26;
> -	rate = div_u64(rate, 1953125);
> +	rate = scaled_ppm;
> +	rate <<= 13;
> +	rate = div_u64(rate, 15625);
> ?

I'm curious how you generate the new math here, since this can be
tricky, and I could use more examples in order to port to some of the
other drivers implementations. I'm not quit sure how to handle the
value when the lower 16 bits are fractional.

Thanks,
Jake

> ?	inca = rate & INCVALUE_MASK;
> ?	if (neg_adj)
> @@ -1103,7 +1103,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
> ?		adapter->ptp_caps.max_adj = 62499999;
> ?		adapter->ptp_caps.n_ext_ts = 0;
> ?		adapter->ptp_caps.pps = 0;
> -		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
> +		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
> ?		adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
> ?		adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
> ?		adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
> @@ -1131,7 +1131,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
> ?		adapter->ptp_caps.n_pins = IGB_N_SDP;
> ?		adapter->ptp_caps.pps = 1;
> ?		adapter->ptp_caps.pin_config = adapter->sdp_config;
> -		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
> +		adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
> ?		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
> ?		adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
> ?		adapter->ptp_caps.settime64 = igb_ptp_settime_i210;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-08 21:49   ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-08 22:04     ` Keller, Jacob E
  -1 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 22:04 UTC (permalink / raw)
  To: netdev, richardcochran
  Cc: tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen, davem,
	Kirsher, Jeffrey T, john.stultz, intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> The 82580 and related devices offer a frequency resolution of about
> 0.029 ppb.  This patch lets users of the device benefit from the
> increased frequency resolution when tuning the clock.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---

Additionally, what about min/max frequency check? Wouldn't this need to
be updated for the new adjfine operation?

Thanks,
Jake

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-08 22:04     ` Keller, Jacob E
  0 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-08 22:04 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> The 82580 and related devices offer a frequency resolution of about
> 0.029 ppb.??This patch lets users of the device benefit from the
> increased frequency resolution when tuning the clock.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---

Additionally, what about min/max frequency check? Wouldn't this need to
be updated for the new adjfine operation?

Thanks,
Jake

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-08 22:02     ` [Intel-wired-lan] " Keller, Jacob E
@ 2016-11-09 13:11       ` Richard Cochran
  -1 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-09 13:11 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: netdev, tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen,
	davem, Kirsher, Jeffrey T, john.stultz, intel-wired-lan

On Tue, Nov 08, 2016 at 10:02:22PM +0000, Keller, Jacob E wrote:
> On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> > -	rate = ppb;
> > -	rate <<= 26;
> > -	rate = div_u64(rate, 1953125);
> > +	rate = scaled_ppm;
> > +	rate <<= 13;
> > +	rate = div_u64(rate, 15625);
> 
> I'm curious how you generate the new math here, since this can be
> tricky, and I could use more examples in order to port to some of the
> other drivers implementations. I'm not quit sure how to handle the
> value when the lower 16 bits are fractional.

TL;DR version:

In ptp_clock.c we convert scaled_ppm to ppb like this.

	ppb = scaled_ppm * 10^3 * 2^-16

If you already have a working driver that does

	regval = ppb * SOMEMATH;

then just substitute

	regval = (scaled_ppm * 10^3 * 2^-16) * SOMEMATH;
	       = (scaled_ppm *  5^3 * 2^-13) * SOMEMATH;

and simplify by combining the 5^3 and 2^-13 constants into SOMEMATH.

Longer explanation:

You have to consider how the frequency adjustment HW works, case by
case.  Both the i210 and the phyter have an adjustment register that
holds units of 2^-32 nanoseconds per 8 nanosecond clock period, and so
the rate from adjustment value 1 is (2^-32 / 8).

Then with the old interface, the conversion from "adjustment unit" to
ppb was (2^-32 / 8 * 10^9) or (2^-26 * 5^9).  The conversion the other
way needs the inverse, and so the code did (ppb << 26) / 5^9.

With the new interface, the conversion from "adjustment unit" to
scaled_ppm is (2^-32 / 8 * 10^6 * 2^16) or (2^-13 * 5^6).  The code
converts the other direction using the inverse, (s_ppm << 13) / 5^6.

HTH,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-09 13:11       ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-09 13:11 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, Nov 08, 2016 at 10:02:22PM +0000, Keller, Jacob E wrote:
> On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> > -	rate = ppb;
> > -	rate <<= 26;
> > -	rate = div_u64(rate, 1953125);
> > +	rate = scaled_ppm;
> > +	rate <<= 13;
> > +	rate = div_u64(rate, 15625);
> 
> I'm curious how you generate the new math here, since this can be
> tricky, and I could use more examples in order to port to some of the
> other drivers implementations. I'm not quit sure how to handle the
> value when the lower 16 bits are fractional.

TL;DR version:

In ptp_clock.c we convert scaled_ppm to ppb like this.

	ppb = scaled_ppm * 10^3 * 2^-16

If you already have a working driver that does

	regval = ppb * SOMEMATH;

then just substitute

	regval = (scaled_ppm * 10^3 * 2^-16) * SOMEMATH;
	       = (scaled_ppm *  5^3 * 2^-13) * SOMEMATH;

and simplify by combining the 5^3 and 2^-13 constants into SOMEMATH.

Longer explanation:

You have to consider how the frequency adjustment HW works, case by
case.  Both the i210 and the phyter have an adjustment register that
holds units of 2^-32 nanoseconds per 8 nanosecond clock period, and so
the rate from adjustment value 1 is (2^-32 / 8).

Then with the old interface, the conversion from "adjustment unit" to
ppb was (2^-32 / 8 * 10^9) or (2^-26 * 5^9).  The conversion the other
way needs the inverse, and so the code did (ppb << 26) / 5^9.

With the new interface, the conversion from "adjustment unit" to
scaled_ppm is (2^-32 / 8 * 10^6 * 2^16) or (2^-13 * 5^6).  The code
converts the other direction using the inverse, (s_ppm << 13) / 5^6.

HTH,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-08 22:04     ` [Intel-wired-lan] " Keller, Jacob E
@ 2016-11-09 13:15       ` Richard Cochran
  -1 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-09 13:15 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: netdev, tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen,
	davem, Kirsher, Jeffrey T, john.stultz, intel-wired-lan

On Tue, Nov 08, 2016 at 10:04:23PM +0000, Keller, Jacob E wrote:
> Additionally, what about min/max frequency check? Wouldn't this need to
> be updated for the new adjfine operation?

In theory you might increase the max by some sub-ppb value, but we
cannot express that as the resolution of the user space interface is
in ppb, and that little extra is not important anyhow.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-09 13:15       ` Richard Cochran
  0 siblings, 0 replies; 22+ messages in thread
From: Richard Cochran @ 2016-11-09 13:15 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, Nov 08, 2016 at 10:04:23PM +0000, Keller, Jacob E wrote:
> Additionally, what about min/max frequency check? Wouldn't this need to
> be updated for the new adjfine operation?

In theory you might increase the max by some sub-ppb value, but we
cannot express that as the resolution of the user space interface is
in ppb, and that little extra is not important anyhow.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
  2016-11-09 13:11       ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-09 21:40         ` Keller, Jacob E
  -1 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-09 21:40 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, tglx, Manfred.Rudigier, ulrik.debie-os, stefan.sorensen,
	davem, Kirsher, Jeffrey T, john.stultz, intel-wired-lan

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Wednesday, November 09, 2016 5:12 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; tglx@linutronix.de; Manfred.Rudigier@omicron.at;
> ulrik.debie-os@e2big.org; stefan.sorensen@spectralink.com;
> davem@davemloft.net; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> john.stultz@linaro.org; intel-wired-lan@lists.osuosl.org
> Subject: Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency
> method.
> 
> On Tue, Nov 08, 2016 at 10:02:22PM +0000, Keller, Jacob E wrote:
> > On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> > > -	rate = ppb;
> > > -	rate <<= 26;
> > > -	rate = div_u64(rate, 1953125);
> > > +	rate = scaled_ppm;
> > > +	rate <<= 13;
> > > +	rate = div_u64(rate, 15625);
> >
> > I'm curious how you generate the new math here, since this can be
> > tricky, and I could use more examples in order to port to some of the
> > other drivers implementations. I'm not quit sure how to handle the
> > value when the lower 16 bits are fractional.
> 
> TL;DR version:
> 
> In ptp_clock.c we convert scaled_ppm to ppb like this.
> 
> 	ppb = scaled_ppm * 10^3 * 2^-16
> 
> If you already have a working driver that does
> 
> 	regval = ppb * SOMEMATH;
> 
> then just substitute
> 
> 	regval = (scaled_ppm * 10^3 * 2^-16) * SOMEMATH;
> 	       = (scaled_ppm *  5^3 * 2^-13) * SOMEMATH;
> 
> and simplify by combining the 5^3 and 2^-13 constants into SOMEMATH.
> 

Thanks, this makes more sense :)

> Longer explanation:
> 
> You have to consider how the frequency adjustment HW works, case by
> case.  Both the i210 and the phyter have an adjustment register that
> holds units of 2^-32 nanoseconds per 8 nanosecond clock period, and so
> the rate from adjustment value 1 is (2^-32 / 8).
> 
> Then with the old interface, the conversion from "adjustment unit" to
> ppb was (2^-32 / 8 * 10^9) or (2^-26 * 5^9).  The conversion the other
> way needs the inverse, and so the code did (ppb << 26) / 5^9.
> 
> With the new interface, the conversion from "adjustment unit" to
> scaled_ppm is (2^-32 / 8 * 10^6 * 2^16) or (2^-13 * 5^6).  The code
> converts the other direction using the inverse, (s_ppm << 13) / 5^6.
> 

Right. This helps.

Thanks,
Jake

> HTH,
> Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method.
@ 2016-11-09 21:40         ` Keller, Jacob E
  0 siblings, 0 replies; 22+ messages in thread
From: Keller, Jacob E @ 2016-11-09 21:40 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran at gmail.com]
> Sent: Wednesday, November 09, 2016 5:12 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev at vger.kernel.org; tglx at linutronix.de; Manfred.Rudigier at omicron.at;
> ulrik.debie-os at e2big.org; stefan.sorensen at spectralink.com;
> davem at davemloft.net; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> john.stultz at linaro.org; intel-wired-lan at lists.osuosl.org
> Subject: Re: [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency
> method.
> 
> On Tue, Nov 08, 2016 at 10:02:22PM +0000, Keller, Jacob E wrote:
> > On Tue, 2016-11-08 at 22:49 +0100, Richard Cochran wrote:
> > > -	rate = ppb;
> > > -	rate <<= 26;
> > > -	rate = div_u64(rate, 1953125);
> > > +	rate = scaled_ppm;
> > > +	rate <<= 13;
> > > +	rate = div_u64(rate, 15625);
> >
> > I'm curious how you generate the new math here, since this can be
> > tricky, and I could use more examples in order to port to some of the
> > other drivers implementations. I'm not quit sure how to handle the
> > value when the lower 16 bits are fractional.
> 
> TL;DR version:
> 
> In ptp_clock.c we convert scaled_ppm to ppb like this.
> 
> 	ppb = scaled_ppm * 10^3 * 2^-16
> 
> If you already have a working driver that does
> 
> 	regval = ppb * SOMEMATH;
> 
> then just substitute
> 
> 	regval = (scaled_ppm * 10^3 * 2^-16) * SOMEMATH;
> 	       = (scaled_ppm *  5^3 * 2^-13) * SOMEMATH;
> 
> and simplify by combining the 5^3 and 2^-13 constants into SOMEMATH.
> 

Thanks, this makes more sense :)

> Longer explanation:
> 
> You have to consider how the frequency adjustment HW works, case by
> case.  Both the i210 and the phyter have an adjustment register that
> holds units of 2^-32 nanoseconds per 8 nanosecond clock period, and so
> the rate from adjustment value 1 is (2^-32 / 8).
> 
> Then with the old interface, the conversion from "adjustment unit" to
> ppb was (2^-32 / 8 * 10^9) or (2^-26 * 5^9).  The conversion the other
> way needs the inverse, and so the code did (ppb << 26) / 5^9.
> 
> With the new interface, the conversion from "adjustment unit" to
> scaled_ppm is (2^-32 / 8 * 10^6 * 2^16) or (2^-13 * 5^6).  The code
> converts the other direction using the inverse, (s_ppm << 13) / 5^6.
> 

Right. This helps.

Thanks,
Jake

> HTH,
> Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH net-next 0/3] PHC frequency fine tuning
  2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
@ 2016-11-10  2:20   ` David Miller
  -1 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2016-11-10  2:20 UTC (permalink / raw)
  To: richardcochran
  Cc: netdev, jacob.e.keller, jeffrey.t.kirsher, john.stultz,
	Manfred.Rudigier, stefan.sorensen, tglx, ulrik.debie-os,
	intel-wired-lan

From: Richard Cochran <richardcochran@gmail.com>
Date: Tue,  8 Nov 2016 22:49:15 +0100

> This series expands the PTP Hardware Clock subsystem by adding a
> method that passes the frequency tuning word to the the drivers
> without dropping the low order bits.  Keeping those bits is useful for
> drivers whose frequency resolution is higher than 1 ppb.

Series applied, thanks Richard.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Intel-wired-lan] [PATCH net-next 0/3] PHC frequency fine tuning
@ 2016-11-10  2:20   ` David Miller
  0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2016-11-10  2:20 UTC (permalink / raw)
  To: intel-wired-lan

From: Richard Cochran <richardcochran@gmail.com>
Date: Tue,  8 Nov 2016 22:49:15 +0100

> This series expands the PTP Hardware Clock subsystem by adding a
> method that passes the frequency tuning word to the the drivers
> without dropping the low order bits.  Keeping those bits is useful for
> drivers whose frequency resolution is higher than 1 ppb.

Series applied, thanks Richard.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-11-10  2:20 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08 21:49 [PATCH net-next 0/3] PHC frequency fine tuning Richard Cochran
2016-11-08 21:49 ` [Intel-wired-lan] " Richard Cochran
2016-11-08 21:49 ` [PATCH net-next 1/3] ptp: Introduce a high resolution frequency adjustment method Richard Cochran
2016-11-08 21:49   ` [Intel-wired-lan] " Richard Cochran
2016-11-08 21:49 ` [PATCH net-next 2/3] ptp: igb: Use the high resolution frequency method Richard Cochran
2016-11-08 21:49   ` [Intel-wired-lan] " Richard Cochran
2016-11-08 22:02   ` Keller, Jacob E
2016-11-08 22:02     ` [Intel-wired-lan] " Keller, Jacob E
2016-11-09 13:11     ` Richard Cochran
2016-11-09 13:11       ` [Intel-wired-lan] " Richard Cochran
2016-11-09 21:40       ` Keller, Jacob E
2016-11-09 21:40         ` [Intel-wired-lan] " Keller, Jacob E
2016-11-08 22:04   ` Keller, Jacob E
2016-11-08 22:04     ` [Intel-wired-lan] " Keller, Jacob E
2016-11-09 13:15     ` Richard Cochran
2016-11-09 13:15       ` [Intel-wired-lan] " Richard Cochran
2016-11-08 21:49 ` [PATCH net-next 3/3] ptp: dp83640: " Richard Cochran
2016-11-08 21:49   ` [Intel-wired-lan] " Richard Cochran
2016-11-08 21:56 ` [PATCH net-next 0/3] PHC frequency fine tuning Keller, Jacob E
2016-11-08 21:56   ` [Intel-wired-lan] " Keller, Jacob E
2016-11-10  2:20 ` David Miller
2016-11-10  2:20   ` [Intel-wired-lan] " David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.