From: John Stultz <john.stultz@linaro.org>
To: George Spelvin <linux@horizon.com>
Cc: linux-kernel@vger.kernel.org, Alexander Gordeev <lasaine@lvk.cs.msu.su>
Subject: Re: [PATCH] ntp: make is_error_status() use its argument
Date: Mon, 12 May 2014 10:54:29 -0700 [thread overview]
Message-ID: <53710AD5.504@linaro.org> (raw)
In-Reply-To: <20140512133548.31421.qmail@ns.horizon.com>
On 05/12/2014 06:35 AM, George Spelvin wrote:
> It's an inline function always called with the global time_status as an
> argument, so there's zero functional difference, but the non-CONFIG_SMP
> version uses the passed-in argument, while the CONFIG_SMP one ignores
> its argument and uses the global.
CONFIG_NTP_PPS not CONFIG_SMP, right?
Good catch though, looks like the check code was re-factored out, but
someone forgot to use the local variable.
Adding Alexander since he submitted the pps logic.
If there's no objections, I'll queue this (with a more verbose commit
message) for 3.16
thanks
-john
>
> Make it use the argument always; shorter variable names are good.
>
> Signed-off-by: George Spelvin <linux@horizon.com>
> ---
> While poking about in the code, I came across this rather odd bit.
> It looked worth fixing, on general principles.
>
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 419a52cecd..1a2aad3fff 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -165,21 +165,21 @@ static inline void pps_set_freq(s64 freq)
>
> static inline int is_error_status(int status)
> {
> - return (time_status & (STA_UNSYNC|STA_CLOCKERR))
> + return (status & (STA_UNSYNC|STA_CLOCKERR))
> /* PPS signal lost when either PPS time or
> * PPS frequency synchronization requested
> */
> - || ((time_status & (STA_PPSFREQ|STA_PPSTIME))
> - && !(time_status & STA_PPSSIGNAL))
> + || ((status & (STA_PPSFREQ|STA_PPSTIME))
> + && !(status & STA_PPSSIGNAL))
> /* PPS jitter exceeded when
> * PPS time synchronization requested */
> - || ((time_status & (STA_PPSTIME|STA_PPSJITTER))
> + || ((status & (STA_PPSTIME|STA_PPSJITTER))
> == (STA_PPSTIME|STA_PPSJITTER))
> /* PPS wander exceeded or calibration error when
> * PPS frequency synchronization requested
> */
> - || ((time_status & STA_PPSFREQ)
> - && (time_status & (STA_PPSWANDER|STA_PPSERROR)));
> + || ((status & STA_PPSFREQ)
> + && (status & (STA_PPSWANDER|STA_PPSERROR)));
> }
>
> static inline void pps_fill_timex(struct timex *txc)
next prev parent reply other threads:[~2014-05-12 17:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-12 13:35 [PATCH] ntp: make is_error_status() use its argument George Spelvin
2014-05-12 17:54 ` John Stultz [this message]
2014-05-12 20:10 ` Alexander Gordeev
2014-05-13 3:01 ` George Spelvin
2014-05-13 3:15 ` John Stultz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53710AD5.504@linaro.org \
--to=john.stultz@linaro.org \
--cc=lasaine@lvk.cs.msu.su \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@horizon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox