Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Richard Cochran <richardcochran@gmail.com>
Date: 2016-11-30 22:17:47
Also in:
linux-devicetree, linux-omap, netdev
On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
quoted
In order to produce the PPS edge correctly, you would have to adjust the comparison value whenever cc.mult changes,yes. And that is done in cpts_ptp_adjfreq() if (cpts->ts_comp_enabled) cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC); ^^^ re-calculate reload value for cpts_ts_comp_settime(cpts, ns); ^^^ adjust the ts_comp
And it races with the pulse itself. You forgot about this part:
quoted hunk ↗ jump to hunk
@@ -172,14 +232,31 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb) adj *= ppb; diff = div_u64(adj, 1000000000ULL); + mutex_lock(&cpts->ptp_clk_mutex); + spin_lock_irqsave(&cpts->lock, flags); + if (cpts->ts_comp_enabled) { + cpts_ts_comp_disable(cpts);
Sorry, but this is a train wreck.
quoted
but of course this is unworkable.Sry, but this is questionable - code for pps comes from TI internal branches (SDK releases) where it survived for a pretty long time.
That doesn't mean the code is any good. If you adjust at the right moment, then no pulse occurs at all!
I'm, of course, agree that without HW support for freq adjustment this PPS feature is not super precise and has some limitation, but that is what we agree to live with.
I do NOT agree to live with this. I am one who is going to have to explain to the world why their beagle bone PPS sucks. Thanks, Richard