SK-AM62: PPS Out not acting as expected

Edited Aug 8 2023

I believe this behavior has been observed and replicated by our TI team. Reassigning to another team member to discuss the details, and the plan going forward.  The behavior I was thinking of was AM65x PRU Ethernet PPS output not syncing on SDK 8.6. Totally unrelated to AM62x CPSW Ethernet PPS output, which is syncing correctly with pps_enable.

For future readers, this is a continuation of the discussion at this thread: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1248172/sk-am62-high-precission-pps-input 

Regards,

Nick

Xabier Marquiegui said:

I would expect the PPS out to be upset by this clock manipulation, but this is what I was seeing in the oscilloscope:

I'm not too familar with phc_ctl and what you should see after calling set. Did you try freq instead of set , does that have an impact? Also you are setting the time backwards (just a guess but going backwards might have some issues) do you still see the issue if you set it forwards?

  Pekka

Hello Pekka,

Thank you very much for your time. Let me try to answer some of your questions:

Pekka Varis said:

I'm not too familar with phc_ctl and what you should see after calling set.

Here's what phc_ctl set does in full detail. I hope it's useful:

https://sourceforge.net/p/linuxptp/code/ci/aa60db270be12e7144e7b0cef2f5dde4213b7057/tree/phc_ctl.c#l175

Pekka Varis said:

Did you try freq instead of set , does that have an impact?

Using phc_ctl eth1 freq to upset the syncronization does seem to have the expected effect, and I can see the clock drift. Both speeding up and slowing down.

Pekka Varis said:

Also you are setting the time backwards (just a guess but going backwards might have some issues) do you still see the issue if you set it forwards?

Moving the time forward shows no change in the PPS out signal either:

am62xx-evm:~# phc_ctl eth1 get ; phc_ctl eth1 get ; phc_ctl eth1 set 1666999999.0 ; phc_ctl eth1 get
phc_ctl[2113.457]: clock time is 1666087324.588961617 or Tue Oct 18 10:02:04 2022

phc_ctl[2113.463]: clock time is 1666087324.594667662 or Tue Oct 18 10:02:04 2022

phc_ctl[2113.476]: set clock time to 1666999999.000000000 or Fri Oct 28 23:33:19 2022

phc_ctl[2113.482]: clock time is 1666999999.005754830 or Fri Oct 28 23:33:19 2022

Setting the phc clock is standard procedure whenever two clocks are too far appart. This can be caused by a number of different situations: system startup, system synchronization failure caused by connectivity errors, high cpu load or high network load. Not having the PPS out in sync on clock set operations can lead to errors in syncrhonization stability analisys.

Let me know your thoughts on this.

Best regards,

Xabier.

Thanks this helps. I don't think we test phc_ctl in our automated testing. So to me this looks like set is broken, get and freq work. Others like adjust have not been tested. On quick look of the source would you agree it is clock_settime(clkid, &ts) with this clock that is broken?

  Pekka

Hello Pekka,

I have good news. I have been able to put a concept together that proves that this could be solved.

I don't really like my implementation because it's slow, but I think it should be a good starting point.

Do you think you and your team could have a look at it and propose a faster/cleaner improved version?

From 0b4b5abd5a1badcdee608c00c0a9ace3c9ddf578 Mon Sep 17 00:00:00 2001
From: Xabier Marquiegui <xmarquiegui@ainguraiiot.com>
Date: Wed, 9 Aug 2023 11:32:04 +0200
Subject: [PATCH] net: ethernet: ti: am65-cpts: improve pps support

When CPTS clock is set by running linuxptp (ptp4l, phc_ctl or ts2phc) it
will cause PPS incoherence, as the system PHC will be offset in
comparison to the PPS signal.

The same offset has to be applied to GenF as to the PHC clock to
correct PPS offset and keep them in sync.

More details on: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1256953/sk-am62-pps-out-not-acting-as-expected
---
 drivers/net/ethernet/ti/am65-cpts.c | 44 +++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
index c66618d91c28..8d439ba03f3a 100644
--- a/drivers/net/ethernet/ti/am65-cpts.c
+++ b/drivers/net/ethernet/ti/am65-cpts.c
@@ -492,6 +492,8 @@ static int am65_cpts_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return 0;
 }
 
+static void am65_cpts_ptp_updatepps(struct am65_cpts *cpts, const struct timespec64 *ts, s64 *ns);
+
 static int am65_cpts_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct am65_cpts *cpts = container_of(ptp, struct am65_cpts, ptp_info);
@@ -501,6 +503,15 @@ static int am65_cpts_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	ns = am65_cpts_gettime(cpts, NULL);
 	ns += delta;
 	am65_cpts_settime(cpts, ns);
+
+
+	if (cpts->pps_enabled) {
+		am65_cpts_ptp_updatepps(cpts, NULL, &ns);
+		// XXX - REMOVE THIS
+		dev_err(cpts->dev, "%s: GenF:%u attempted PPS adjtime\n",
+			__func__, cpts->pps_genf_idx);
+	}
+
 	mutex_unlock(&cpts->ptp_clk_lock);
 
 	return 0;
@@ -543,6 +554,14 @@ static int am65_cpts_ptp_settime(struct ptp_clock_info *ptp,
 	ns = timespec64_to_ns(ts);
 	mutex_lock(&cpts->ptp_clk_lock);
 	am65_cpts_settime(cpts, ns);
+
+	if (cpts->pps_enabled) {
+		am65_cpts_ptp_updatepps(cpts, ts, &ns);
+		// XXX - REMOVE THIS
+		dev_err(cpts->dev, "%s: GenF:%u attempted PPS settime\n",
+			__func__, cpts->pps_genf_idx);
+	}
+
 	mutex_unlock(&cpts->ptp_clk_lock);
 
 	return 0;
@@ -726,6 +745,31 @@ static int am65_cpts_pps_enable(struct am65_cpts *cpts, int on)
 	return ret;
 }
 
+static void am65_cpts_ptp_updatepps(struct am65_cpts *cpts, const struct timespec64 *ts, s64 *ns) {
+	struct ptp_clock_request rq;
+	struct timespec64 localts;
+	struct timespec64 *localtsptr = &localts;
+	if (ts == NULL) {
+		localts = ns_to_timespec64(*ns);
+	} else {
+		localtsptr = (struct timespec64 *)ts;
+	}
+
+	rq.perout.index = cpts->pps_genf_idx;
+	am65_cpts_perout_enable_hw(cpts, &rq.perout, 0);
+	am65_cpts_extts_enable_hw(cpts, cpts->pps_hw_ts_idx, 0);
+
+	am65_cpts_extts_enable_hw(cpts, cpts->pps_hw_ts_idx, cpts->pps_enabled);
+
+	rq.perout.period.sec = 1;
+	rq.perout.period.nsec = 0;
+	rq.perout.start.sec = localtsptr->tv_sec + 2;
+	rq.perout.start.nsec = 0;
+	rq.perout.index = cpts->pps_genf_idx;
+
+	am65_cpts_perout_enable_hw(cpts, &rq.perout, cpts->pps_enabled);
+}
+
 static int am65_cpts_ptp_enable(struct ptp_clock_info *ptp,
 				struct ptp_clock_request *rq, int on)
 {
-- 
2.34.1

The concept seems to not work if I remove the dev_err( print messages that I added for easy debugging, which proves my point that this concept is not good enough for an official fix. Any ideas on how to improve it?

Are you already discussing this patch in some Linux mailing list? To proceed on this I can file a bug on am65-cpts.c , I wanted to see if I can refer to some discussion or just include the above.

  Pekka

I was told by one of your colleagues that this forum was the place for me to propose contributions, so I am not participating in any mailing list. Just include the above. Do you mind sharing the mailing list here?

Sorry for the misunderstanding. I thought the post above looked like copied from a post on some Linux mailing list with some email addresses removed. I'll file the bug with your am65-cpts.c comments.

So this will likely get scheduled to be fixed in 9.1 or 9.2, remain open until then.

  Pekka

No problem. No, the main thing is just the output of a git format-patch I did in my own copy of ti-linux-kernel.

Thank you very much. Is there anyway I can participate in the bug report? I would like to be able to participate in the conversation, if that's possible.

I added a note to include you in the email thread of the patch. Once I get an estimate on the fix I'll update this e2e thread with the schedule.

  Pekka

Hello Pekka,

Has there been any activity in the e-mail thread? I haven't received anything, and I would like to rule out that I'm affected by some kind of span filter or something like that.

Thanks,

Xabier.

Hello Pekka,

It seems there has been some kind of problem with the forum notifications because I haven't gotten notifications about activity on other threads I'm involved in. Maybe you didn't see my previous question. I hope it reaches you this time.

Thank you for your support.

Xabier.

Hello Pekka,

Do you have news on this?

Thanks,

Xabier.

I don't have a confirmation will this be fixed in 9.1 or 9.2.