From e0580929f451e685d92cd10d80b76f39e9b09a97 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Tue, 24 Dec 2019 11:09:34 -0800 Subject: [PATCH] phc2sys: Fix frequency estimation when synchronizing a PHC to the system clock. When synchronizing a PHC to the Linux system clock (CLOCK_REALTIME), the phc2sys uses the sysoff method, reversing the master and slave roles. The offset between a master clock and a slave clock is given by offset = slave_ts - master_ts, and the call to sysoff_measure() provides the 'offset' and 'slave_ts' values. The needed local time stamp on the 'master' is given by master_ts = slave_ts - offset, but the code calcuates master_ts = slave_ts + offset. When passed to the servo, the local time stamp is used to estimate the frequency offset between the two clocks before starting the main synchronization loop. The effect of the bug may be seen with a simple test. Here is a sample output with the existing code. $ sudo testptp -d /dev/ptp1 -f 62400000 frequency adjustment okay $ sudo ./phc2sys -m -q -c eth6 -s CLOCK_REALTIME -O0 phc2sys[90221.239]: eth6 sys offset 191001318 s0 freq -62400000 delay 5547 phc2sys[90222.239]: eth6 sys offset 253380897 s1 freq +8265884 delay 5507 phc2sys[90223.239]: eth6 sys offset -8301685 s2 freq -35801 delay 5487 phc2sys[90224.239]: eth6 sys offset -8297136 s2 freq -2521757 delay 5531 phc2sys[90225.239]: eth6 sys offset -5806117 s2 freq -2519879 delay 5542 phc2sys[90226.239]: eth6 sys offset -3317009 s2 freq -1772606 delay 5495 phc2sys[90227.240]: eth6 sys offset -1575231 s2 freq -1025931 delay 5505 phc2sys[90228.240]: eth6 sys offset -580249 s2 freq -503518 delay 5524 phc2sys[90229.240]: eth6 sys offset -107770 s2 freq -205114 delay 5519 phc2sys[90230.240]: eth6 sys offset 66298 s2 freq -63377 delay 5490 phc2sys[90230.881]: eth6 sys offset 86942 s2 freq -22844 delay 5495 And this is the output with the bug fix in place. $ sudo testptp -d /dev/ptp1 -f 62400000 frequency adjustment okay $ sudo ./phc2sys -m -q -c eth6 -s CLOCK_REALTIME -O0 phc2sys[90365.624]: eth6 sys offset 311912675 s0 freq -62400000 delay 5490 phc2sys[90366.624]: eth6 sys offset 374292766 s1 freq -31098 delay 5642 phc2sys[90367.624]: eth6 sys offset -3825 s2 freq -34923 delay 5617 phc2sys[90368.625]: eth6 sys offset 6 s2 freq -32240 delay 5564 phc2sys[90369.625]: eth6 sys offset 1241 s2 freq -31003 delay 5605 phc2sys[90370.625]: eth6 sys offset 1131 s2 freq -30741 delay 5600 phc2sys[90371.625]: eth6 sys offset 801 s2 freq -30732 delay 5621 phc2sys[90372.625]: eth6 sys offset 458 s2 freq -30834 delay 5640 phc2sys[90373.626]: eth6 sys offset 186 s2 freq -30969 delay 5598 phc2sys[90374.626]: eth6 sys offset 134 s2 freq -30965 delay 5599 phc2sys[90375.626]: eth6 sys offset 43 s2 freq -31016 delay 5595 phc2sys[90375.681]: eth6 sys offset -32 s2 freq -31078 delay 5541 This patch fixes the issue by correcting the calculation of the local time stamp value. Fixes: 8142da41b61f ("phc2sys: Use reversed sysoff when synchronizing to system clock.") Signed-off-by: Richard Cochran Reported-by: Cliff Spradlin Tested-by: Vladimir Oltean --- phc2sys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phc2sys.c b/phc2sys.c index 28c657a..c0b7b3d 100644 --- a/phc2sys.c +++ b/phc2sys.c @@ -770,8 +770,8 @@ static int do_loop(struct node *node, int subscriptions) node->phc_readings, &offset, &ts, &delay) < 0) return -1; - ts += offset; offset = -offset; + ts += offset; } else { /* use phc */ if (!read_phc(node->master->clkid, clock->clkid,