On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
> > 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:
> @@ -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.
> > 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
