| Message ID | b1d732324e0b1960b294e90ca5eb2a31b6559188.1478526333.git.richardcochran@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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;
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
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
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
> -----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
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;
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(-)