The ptp4l/phc2sys BC can stuck in sync fault while port is in transitional
UNCALIBRATED state:
 MASTER -> UNCALIBRATED
 UNCALIBRATED -> SLAVE
in the later case state change sequence is
 UNCALIBRATED -> SLAVE -> SYNCHRONIZATION_FAULT -> UNCALIBRATED.
Once issue happens the BC or GM has to be restarted to recover.

To trigger an issue the GM clock has to be restarted few times.

The cause of the issue is that both ptp4l and phc2sys could be syncing the
same PHC clock while Port is in UNCALIBRATED state. This happens because in
reconfigure() the check [1]

[1]     if (dst_cnt > 1 && !src) {
        ...
        if (src_cnt > 1) {
        ...
[2]     if (src_cnt > 0 && !src) {
        ...
        if (!src_cnt && !dst_cnt) {
        ...

is defined first and selects last clock in the clocks list (usually first
defined in ptp4l cfg file) as the master clock even if there is some other
clock(Port) in transitional UNCALIBRATED state. As result, phc2sys will
start syncing the same clock as ptp4l, and ptp4l has no chances to
stabilize Port in UNCALIBRATED state.
Moreover, check [1] bypasses most of follow up checks unless CLOCK_REALTIME
is allowed to be master clock.

The original commit 46a0b281b915 ("phc2sys: autoconfiguration") gave ptp4l
chance to proceed by introducing check [2]. But follow up commit
2ab2fbbdda86 ("phc2sys: default to the first clock in automatic mode.")
introduced check [1] and so this issue.

To fix an issue the check [2] is moved down which makes phc2sys to wait for
ptp4l to settle.

Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com>
---
 phc2sys.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index b15127d..fd80afd 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -419,19 +419,6 @@ static void reconfigure(struct phc2sys_private *priv)
                }
                last = c;
        }
-       if (dst_cnt > 1 && !src) {
-               if (!rt || rt->dest_only) {
-                       priv->master = last;
-                       /* Reset to original state in next reconfiguration. */
-                       priv->master->new_state = priv->master->state;
-                       priv->master->state = PS_SLAVE;
-                       if (rt)
-                               rt->state = PS_SLAVE;
-                       pr_info("no source, selecting %s as the default clock",
-                               last->device);
-                       return;
-               }
-       }
        if (src_cnt > 1) {
                pr_info("multiple master clocks available, postponing sync...");
                priv->master = NULL;
@@ -447,6 +434,21 @@ static void reconfigure(struct phc2sys_private *priv)
                priv->master = NULL;
                return;
        }
+
+       if (dst_cnt > 1 && !src) {
+               if (!rt || rt->dest_only) {
+                       priv->master = last;
+                       /* Reset to original state in next reconfiguration. */
+                       priv->master->new_state = priv->master->state;
+                       priv->master->state = PS_SLAVE;
+                       if (rt)
+                               rt->state = PS_SLAVE;
+                       pr_info("no source, selecting %s as the default clock",
+                               last->device);
+                       return;
+               }
+       }
+
        if ((!src_cnt && (!rt || rt->dest_only)) ||
            (!dst_cnt && !rt)) {
                pr_info("nothing to synchronize");
-- 
2.17.1



_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to