Re: [PATCH 1/4] new timeofday core subsystem (v. B2)

john stultz
2005-06-14T00:53:25Z

On Thu, 2005-06-09 at 16:15 +0300, Pekka Enberg wrote:
> Hi John,
>
> Some coding style comments below.

[snip]
> Please drop this redundant comment.
>
> > + interval_sum += interval;
> > +
> > + write_seqlock_irqsave(&ntp_lock, flags);
> > +
> > + /* decrement singleshot offset interval */
>
> Ditto.
>
> > + ss_offset_len -= interval;
> > + if(ss_offset_len < 0) /* make sure it doesn't go negative */
>
> Ditto.
>
> > + ss_offset_len = 0;
> > +
> > + /* Do second overflow code */
>
> Drop redundant comment.
>
> > + ntp_maxerror += shiftR(ntp_tolerance, SHIFT_USEC);
> > + if (ntp_maxerror > NTP_PHASE_LIMIT) {
> > + ntp_maxerror = NTP_PHASE_LIMIT;
> > + ntp_status |= STA_UNSYNC;
> > + }
> > +
> > + /* Calculate offset_adj for the next second */
>
> Ditto.
>
> > + tmp = ntp_offset;
>
> tmp could use a more descriptive name.
>
> > + if (!(ntp_status & STA_FLL))
> > + tmp = shiftR(tmp, SHIFT_KG + ntp_constant);
> > +
> > + /* bound the adjustment to MAXPHASE/MINSEC */
>
> Redundant comment.

Thanks for the uh, redundant redundant-comment comments ;)

Much is from the transition from psudo-code to actual code, so I
appreciate you pointing it out. There are a few spots where I feel there
is still value in the comments, so I've left them, but overall I think
you'll see an improvement.

thanks again for the feedback!
-john