Skip to content

Commit 3f2bba7

Browse files
davem330davem330
committed
Merge branch 'ptp-more-accurate-PHC-system-clock-synchronization'
Miroslav Lichvar says: ==================== More accurate PHC<->system clock synchronization RFC->v1: - added new patches - separated PHC timestamp from ptp_system_timestamp - fixed memory leak in PTP_SYS_OFFSET_EXTENDED - changed PTP_SYS_OFFSET_EXTENDED to work with array of arrays - fixed PTP_SYS_OFFSET_EXTENDED to break correctly from loop - fixed timecounter updates in drivers - split gettimex in igb driver - fixed ptp_read_* functions to be available without CONFIG_PTP_1588_CLOCK This series enables a more accurate synchronization between PTP hardware clocks and the system clock. The first two patches are minor cleanup/bug fixes. The third patch adds an extended version of the PTP_SYS_OFFSET ioctl, which returns three timestamps for each measurement. The idea is to shorten the interval between the system timestamps to contain just the reading of the lowest register of the PHC in order to reduce the error in the measured offset and get a smaller upper bound on the maximum error. The fourth patch deprecates the original gettime function. The remaining patches update the gettime function in order to support the new ioctl in the e1000e, igb, ixgbe, and tg3 drivers. Tests with few different NICs in different machines show that: - with an I219 (e1000e) the measured delay was reduced from 2500 to 1300 ns and the error in the measured offset, when compared to the cross timestamping supported by the driver, was reduced by a factor of 5 - with an I210 (igb) the delay was reduced from 5100 to 1700 ns - with an I350 (igb) the delay was reduced from 2300 to 750 ns - with an X550 (ixgbe) the delay was reduced from 1950 to 650 ns - with a BCM5720 (tg3) the delay was reduced from 2400 to 1200 ns ==================== Acked-by: Richard Cochran <richardcochran@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 70e7983 + 6fe42e2 commit 3f2bba7

File tree

10 files changed

+253
-51
lines changed

10 files changed

+253
-51
lines changed

‎drivers/net/ethernet/broadcom/tg3.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6135,10 +6135,16 @@ static int tg3_setup_phy(struct tg3 *tp, bool force_reset)
61356135
}
61366136

61376137
/* tp->lock must be held */
6138-
static u64 tg3_refclk_read(struct tg3 *tp)
6138+
static u64 tg3_refclk_read(struct tg3 *tp, struct ptp_system_timestamp *sts)
61396139
{
6140-
u64 stamp = tr32(TG3_EAV_REF_CLCK_LSB);
6141-
return stamp | (u64)tr32(TG3_EAV_REF_CLCK_MSB) << 32;
6140+
u64 stamp;
6141+
6142+
ptp_read_system_prets(sts);
6143+
stamp = tr32(TG3_EAV_REF_CLCK_LSB);
6144+
ptp_read_system_postts(sts);
6145+
stamp |= (u64)tr32(TG3_EAV_REF_CLCK_MSB) << 32;
6146+
6147+
return stamp;
61426148
}
61436149

61446150
/* tp->lock must be held */
@@ -6229,13 +6235,14 @@ static int tg3_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
62296235
return 0;
62306236
}
62316237

6232-
static int tg3_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
6238+
static int tg3_ptp_gettimex(struct ptp_clock_info *ptp, struct timespec64 *ts,
6239+
struct ptp_system_timestamp *sts)
62336240
{
62346241
u64 ns;
62356242
struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
62366243

62376244
tg3_full_lock(tp, 0);
6238-
ns = tg3_refclk_read(tp);
6245+
ns = tg3_refclk_read(tp, sts);
62396246
ns += tp->ptp_adjust;
62406247
tg3_full_unlock(tp);
62416248

@@ -6330,7 +6337,7 @@ static const struct ptp_clock_info tg3_ptp_caps = {
63306337
.pps = 0,
63316338
.adjfreq = tg3_ptp_adjfreq,
63326339
.adjtime = tg3_ptp_adjtime,
6333-
.gettime64 = tg3_ptp_gettime,
6340+
.gettimex64 = tg3_ptp_gettimex,
63346341
.settime64 = tg3_ptp_settime,
63356342
.enable = tg3_ptp_enable,
63366343
};

‎drivers/net/ethernet/intel/e1000e/e1000.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,9 @@ extern const struct e1000_info e1000_es2_info;
505505
void e1000e_ptp_init(struct e1000_adapter *adapter);
506506
void e1000e_ptp_remove(struct e1000_adapter *adapter);
507507

508+
u64 e1000e_read_systim(struct e1000_adapter *adapter,
509+
struct ptp_system_timestamp *sts);
510+
508511
static inline s32 e1000_phy_hw_reset(struct e1000_hw *hw)
509512
{
510513
return hw->phy.ops.reset(hw);

‎drivers/net/ethernet/intel/e1000e/netdev.c

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4319,13 +4319,16 @@ void e1000e_reinit_locked(struct e1000_adapter *adapter)
43194319
/**
43204320
* e1000e_sanitize_systim - sanitize raw cycle counter reads
43214321
* @hw: pointer to the HW structure
4322-
* @systim: time value read, sanitized and returned
4322+
* @systim: PHC time value read, sanitized and returned
4323+
* @sts: structure to hold system time before and after reading SYSTIML,
4324+
* may be NULL
43234325
*
43244326
* Errata for 82574/82583 possible bad bits read from SYSTIMH/L:
43254327
* check to see that the time is incrementing at a reasonable
43264328
* rate and is a multiple of incvalue.
43274329
**/
4328-
static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
4330+
static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim,
4331+
struct ptp_system_timestamp *sts)
43294332
{
43304333
u64 time_delta, rem, temp;
43314334
u64 systim_next;
@@ -4335,7 +4338,9 @@ static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
43354338
incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
43364339
for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
43374340
/* latch SYSTIMH on read of SYSTIML */
4341+
ptp_read_system_prets(sts);
43384342
systim_next = (u64)er32(SYSTIML);
4343+
ptp_read_system_postts(sts);
43394344
systim_next |= (u64)er32(SYSTIMH) << 32;
43404345

43414346
time_delta = systim_next - systim;
@@ -4353,27 +4358,32 @@ static u64 e1000e_sanitize_systim(struct e1000_hw *hw, u64 systim)
43534358
}
43544359

43554360
/**
4356-
* e1000e_cyclecounter_read - read raw cycle counter (used by time counter)
4357-
* @cc: cyclecounter structure
4361+
* e1000e_read_systim - read SYSTIM register
4362+
* @adapter: board private structure
4363+
* @sts: structure which will contain system time before and after reading
4364+
* SYSTIML, may be NULL
43584365
**/
4359-
static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
4366+
u64 e1000e_read_systim(struct e1000_adapter *adapter,
4367+
struct ptp_system_timestamp *sts)
43604368
{
4361-
struct e1000_adapter *adapter = container_of(cc, struct e1000_adapter,
4362-
cc);
43634369
struct e1000_hw *hw = &adapter->hw;
4364-
u32 systimel, systimeh;
4370+
u32 systimel, systimel_2, systimeh;
43654371
u64 systim;
43664372
/* SYSTIMH latching upon SYSTIML read does not work well.
43674373
* This means that if SYSTIML overflows after we read it but before
43684374
* we read SYSTIMH, the value of SYSTIMH has been incremented and we
43694375
* will experience a huge non linear increment in the systime value
43704376
* to fix that we test for overflow and if true, we re-read systime.
43714377
*/
4378+
ptp_read_system_prets(sts);
43724379
systimel = er32(SYSTIML);
4380+
ptp_read_system_postts(sts);
43734381
systimeh = er32(SYSTIMH);
43744382
/* Is systimel is so large that overflow is possible? */
43754383
if (systimel >= (u32)0xffffffff - E1000_TIMINCA_INCVALUE_MASK) {
4376-
u32 systimel_2 = er32(SYSTIML);
4384+
ptp_read_system_prets(sts);
4385+
systimel_2 = er32(SYSTIML);
4386+
ptp_read_system_postts(sts);
43774387
if (systimel > systimel_2) {
43784388
/* There was an overflow, read again SYSTIMH, and use
43794389
* systimel_2
@@ -4386,11 +4396,23 @@ static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
43864396
systim |= (u64)systimeh << 32;
43874397

43884398
if (adapter->flags2 & FLAG2_CHECK_SYSTIM_OVERFLOW)
4389-
systim = e1000e_sanitize_systim(hw, systim);
4399+
systim = e1000e_sanitize_systim(hw, systim, sts);
43904400

43914401
return systim;
43924402
}
43934403

4404+
/**
4405+
* e1000e_cyclecounter_read - read raw cycle counter (used by time counter)
4406+
* @cc: cyclecounter structure
4407+
**/
4408+
static u64 e1000e_cyclecounter_read(const struct cyclecounter *cc)
4409+
{
4410+
struct e1000_adapter *adapter = container_of(cc, struct e1000_adapter,
4411+
cc);
4412+
4413+
return e1000e_read_systim(adapter, NULL);
4414+
}
4415+
43944416
/**
43954417
* e1000_sw_init - Initialize general software structures (struct e1000_adapter)
43964418
* @adapter: board private structure to initialize

‎drivers/net/ethernet/intel/e1000e/ptp.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,18 @@ static int e1000e_phc_getcrosststamp(struct ptp_clock_info *ptp,
161161
#endif/*CONFIG_E1000E_HWTS*/
162162

163163
/**
164-
* e1000e_phc_gettime - Reads the current time from the hardware clock
164+
* e1000e_phc_gettimex - Reads the current time from the hardware clock and
165+
* system clock
165166
* @ptp: ptp clock structure
166-
* @ts: timespec structure to hold the current time value
167+
* @ts: timespec structure to hold the current PHC time
168+
* @sts: structure to hold the current system time
167169
*
168170
* Read the timecounter and return the correct value in ns after converting
169171
* it into a struct timespec.
170172
**/
171-
static int e1000e_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
173+
static int e1000e_phc_gettimex(struct ptp_clock_info *ptp,
174+
struct timespec64 *ts,
175+
struct ptp_system_timestamp *sts)
172176
{
173177
struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
174178
ptp_clock_info);
@@ -177,8 +181,8 @@ static int e1000e_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
177181

178182
spin_lock_irqsave(&adapter->systim_lock, flags);
179183

180-
/* Use timecounter_cyc2time() to allow non-monotonic SYSTIM readings */
181-
cycles = adapter->cc.read(&adapter->cc);
184+
/* NOTE: Non-monotonic SYSTIM readings may be returned */
185+
cycles = e1000e_read_systim(adapter, sts);
182186
ns = timecounter_cyc2time(&adapter->tc, cycles);
183187

184188
spin_unlock_irqrestore(&adapter->systim_lock, flags);
@@ -258,7 +262,7 @@ static const struct ptp_clock_info e1000e_ptp_clock_info = {
258262
.pps = 0,
259263
.adjfreq = e1000e_phc_adjfreq,
260264
.adjtime = e1000e_phc_adjtime,
261-
.gettime64 = e1000e_phc_gettime,
265+
.gettimex64 = e1000e_phc_gettimex,
262266
.settime64 = e1000e_phc_settime,
263267
.enable = e1000e_phc_enable,
264268
};

‎drivers/net/ethernet/intel/igb/igb_ptp.c

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -275,17 +275,53 @@ static int igb_ptp_adjtime_i210(struct ptp_clock_info *ptp, s64 delta)
275275
return 0;
276276
}
277277

278-
static int igb_ptp_gettime_82576(struct ptp_clock_info *ptp,
279-
struct timespec64 *ts)
278+
static int igb_ptp_gettimex_82576(struct ptp_clock_info *ptp,
279+
struct timespec64 *ts,
280+
struct ptp_system_timestamp *sts)
280281
{
281282
struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
282283
ptp_caps);
284+
struct e1000_hw *hw = &igb->hw;
283285
unsigned long flags;
286+
u32 lo, hi;
284287
u64 ns;
285288

286289
spin_lock_irqsave(&igb->tmreg_lock, flags);
287290

288-
ns = timecounter_read(&igb->tc);
291+
ptp_read_system_prets(sts);
292+
lo = rd32(E1000_SYSTIML);
293+
ptp_read_system_postts(sts);
294+
hi = rd32(E1000_SYSTIMH);
295+
296+
ns = timecounter_cyc2time(&igb->tc, ((u64)hi << 32) | lo);
297+
298+
spin_unlock_irqrestore(&igb->tmreg_lock, flags);
299+
300+
*ts = ns_to_timespec64(ns);
301+
302+
return 0;
303+
}
304+
305+
static int igb_ptp_gettimex_82580(struct ptp_clock_info *ptp,
306+
struct timespec64 *ts,
307+
struct ptp_system_timestamp *sts)
308+
{
309+
struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
310+
ptp_caps);
311+
struct e1000_hw *hw = &igb->hw;
312+
unsigned long flags;
313+
u32 lo, hi;
314+
u64 ns;
315+
316+
spin_lock_irqsave(&igb->tmreg_lock, flags);
317+
318+
ptp_read_system_prets(sts);
319+
rd32(E1000_SYSTIMR);
320+
ptp_read_system_postts(sts);
321+
lo = rd32(E1000_SYSTIML);
322+
hi = rd32(E1000_SYSTIMH);
323+
324+
ns = timecounter_cyc2time(&igb->tc, ((u64)hi << 32) | lo);
289325

290326
spin_unlock_irqrestore(&igb->tmreg_lock, flags);
291327

@@ -294,16 +330,22 @@ static int igb_ptp_gettime_82576(struct ptp_clock_info *ptp,
294330
return 0;
295331
}
296332

297-
static int igb_ptp_gettime_i210(struct ptp_clock_info *ptp,
298-
struct timespec64 *ts)
333+
static int igb_ptp_gettimex_i210(struct ptp_clock_info *ptp,
334+
struct timespec64 *ts,
335+
struct ptp_system_timestamp *sts)
299336
{
300337
struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
301338
ptp_caps);
339+
struct e1000_hw *hw = &igb->hw;
302340
unsigned long flags;
303341

304342
spin_lock_irqsave(&igb->tmreg_lock, flags);
305343

306-
igb_ptp_read_i210(igb, ts);
344+
ptp_read_system_prets(sts);
345+
rd32(E1000_SYSTIMR);
346+
ptp_read_system_postts(sts);
347+
ts->tv_nsec = rd32(E1000_SYSTIML);
348+
ts->tv_sec = rd32(E1000_SYSTIMH);
307349

308350
spin_unlock_irqrestore(&igb->tmreg_lock, flags);
309351

@@ -656,9 +698,12 @@ static void igb_ptp_overflow_check(struct work_struct *work)
656698
struct igb_adapter *igb =
657699
container_of(work, struct igb_adapter, ptp_overflow_work.work);
658700
struct timespec64 ts;
701+
u64 ns;
659702

660-
igb->ptp_caps.gettime64(&igb->ptp_caps, &ts);
703+
/* Update the timecounter */
704+
ns = timecounter_read(&igb->tc);
661705

706+
ts = ns_to_timespec64(ns);
662707
pr_debug("igb overflow check at %lld.%09lu\n",
663708
(long long) ts.tv_sec, ts.tv_nsec);
664709

@@ -1124,7 +1169,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
11241169
adapter->ptp_caps.pps = 0;
11251170
adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82576;
11261171
adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
1127-
adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
1172+
adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_82576;
11281173
adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
11291174
adapter->ptp_caps.enable = igb_ptp_feature_enable;
11301175
adapter->cc.read = igb_ptp_read_82576;
@@ -1143,7 +1188,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
11431188
adapter->ptp_caps.pps = 0;
11441189
adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
11451190
adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
1146-
adapter->ptp_caps.gettime64 = igb_ptp_gettime_82576;
1191+
adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_82580;
11471192
adapter->ptp_caps.settime64 = igb_ptp_settime_82576;
11481193
adapter->ptp_caps.enable = igb_ptp_feature_enable;
11491194
adapter->cc.read = igb_ptp_read_82580;
@@ -1171,7 +1216,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
11711216
adapter->ptp_caps.pin_config = adapter->sdp_config;
11721217
adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
11731218
adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
1174-
adapter->ptp_caps.gettime64 = igb_ptp_gettime_i210;
1219+
adapter->ptp_caps.gettimex64 = igb_ptp_gettimex_i210;
11751220
adapter->ptp_caps.settime64 = igb_ptp_settime_i210;
11761221
adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
11771222
adapter->ptp_caps.verify = igb_ptp_verify_pin;

0 commit comments

Comments
 (0)