Messages in this thread | | | Subject | Re: [PATCHv3 12/16] ntp: add hardpps implementation | From | john stultz <> | Date | Wed, 04 Aug 2010 15:49:47 -0700 |
| |
On Thu, 2010-08-05 at 01:06 +0400, Alexander Gordeev wrote: > This commit adds hardpps() implementation based upon the original one > from the NTPv4 reference kernel code from David Mills. However, it is > highly optimized towards very fast syncronization and maximum stickness > to PPS signal. The typical error is less then a microsecond. > To make it sync faster I had to throw away exponential phase filter so > that the full phase offset is corrected immediately. Then I also had to > throw away median phase filter because it gives a bigger error itself > if used without exponential filter. > Maybe we will find an appropriate filtering scheme in the future but > it's not necessary if the signal quality is ok. > > Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
[snip]
> +#ifdef CONFIG_NTP_PPS > + > +struct pps_normtime { > + __kernel_time_t sec; /* seconds */ > + long nsec; /* nanoseconds */ > +};
I don't quite remember the history here (it may be I suggested you use this instead of overloading a timespec? I honestly don't recall), but could you add some extra context in a comment here for what a pps_normtime structure represents and why its used instead of a timespec? The comment below sort of hints at it, but it would be useful if it was more explicit.
> +/* normalize the timestamp so that nsec is in the > + ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */ > +static inline struct pps_normtime pps_normalize_ts(struct timespec ts) > +{ > + struct pps_normtime norm = { > + .sec = ts.tv_sec, > + .nsec = ts.tv_nsec > + }; > + > + if (norm.nsec > (NSEC_PER_SEC >> 1)) { > + norm.nsec -= NSEC_PER_SEC; > + norm.sec++; > + } > + > + return norm; > +}
Otherwise the code looks pretty good to me.
Acked-by: John Stultz <johnstul@us.ibm.com>
thanks -john
| |