Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1581354 > unrolled thread
| Started by | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| First post | 2017-02-15 15:40 +0100 |
| Last post | 2017-02-15 15:40 +0100 |
| Articles | 8 — 1 participant |
Back to article view | Back to linux.kernel
[PATCH 0/8] LinuxPPS: some minor fixes and improvements. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event(). Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
[PATCH 3/8] hardpps: fix some pps_jitter issues. Andrey Drobyshev <immortalguardian1@gmail.com> - 2017-02-15 15:40 +0100
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 0/8] LinuxPPS: some minor fixes and improvements. |
| Message-ID | <tb8Up-1ae-1@gated-at.bofh.it> |
Hello folks, We are using PPS subsystem with kernel consumer in our long-lasting project. (To be precise, kernel consumer and most of PPS drivers was included in vanilla kernel by one of our employee Alexander Gordeev). During the last years we have fixed dozen of issues and made some improvements we'd like to give away. I split the patchset into two parts. Here are some common fixes I'd like to submit. After that I'll post some improvements in filtration algorithms to increase time synchronization precision using low quality PPS sources with high jitter (e.g. x86 box with pps_gen_parport). I would like this patchset to be merged into current devel. [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro. [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal. [PATCH 3/8] hardpps: fix some pps_jitter issues. [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event(). [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal. [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5 sec after previous. [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal. [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource.
[toc] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous. |
| Message-ID | <tb8Uq-1ae-7@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Alexander GQ Gerasiov <gq@cs.msu.su>
On some devices interrupt may be invoked by parasitic assert event produced
while switching from high to low. Such interrupt should be ignored.
Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
drivers/pps/clients/pps_parport.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index 37094b0..39c1fea 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -69,6 +69,8 @@ static void parport_irq(void *handle)
struct pps_event_time ts_assert, ts_clear;
struct pps_client_pp *dev = handle;
struct parport *port = dev->pardev->port;
+ static struct pps_event_time prev;
+ static struct timespec64 ts_delta;
unsigned int i;
unsigned long flags;
@@ -95,9 +97,14 @@ static void parport_irq(void *handle)
/* check the signal (no signal means the pulse is lost this time) */
if (!signal_is_set(port)) {
local_irq_restore(flags);
- dev_err(dev->pps->dev, "lost the signal\n");
+ ts_delta = timespec64_sub(ts_assert.ts_real, prev.ts_real);
+ /* do not print error message in case interrupt handler is
+ * invoked by parasitic assert event */
+ if (timespec64_to_ns(&ts_delta) > (NSEC_PER_SEC / 2))
+ dev_err(dev->pps->dev, "lost the signal\n");
goto out_none;
}
+ prev = ts_assert;
/* poll the port until the signal is unset */
for (i = dev->cw; i; i--)
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal. |
| Message-ID | <tb8Uq-1ae-17@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Alexander GQ Gerasiov <gq@cs.msu.su>
In case pps_dec_valid() is called from second_overflow() in the
absence of pps signal, there is no need to decrease pps_valid.
Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
kernel/time/ntp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d20891e..22f2235 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -154,6 +154,10 @@ static inline void pps_clear(void)
*/
static inline void pps_dec_valid(void)
{
+ /* Silently ignore if PPS was not turned on */
+ if (!(time_status & STA_PPSSIGNAL))
+ return;
+
if (pps_valid > 0)
pps_valid--;
else {
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource. |
| Message-ID | <tb8Uq-1ae-9@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Nikita Edward Baruzdin <nebaruzdin@gmail.com>
This commit is supposed to resolve the issue with hard lockups on systems using
jiffies as their clock source. Namely, it sets limits on number of iterations
clock source may remain unchanged (i. e. not being updated for one reason or
another, as it is with jiffies clock source), and on unsuccessful
getnstimeofday() polls as well. In case limit is reached, we consider clock
source incompatible with this driver or unstable.
Considering this issue to be fixed, un-BROKEN pps_gen_parport.
For explanation of the problem see this thread on lkml:
https://lkml.org/lkml/2011/2/18/310
Signed-off-by: Nikita Edward Baruzdin <nebaruzdin@lvk.cs.msu.su>
Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
drivers/pps/generators/Kconfig | 2 +-
drivers/pps/generators/pps_gen_parport.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
index e4c4f3d..f3a73dd 100644
--- a/drivers/pps/generators/Kconfig
+++ b/drivers/pps/generators/Kconfig
@@ -6,7 +6,7 @@ comment "PPS generators support"
config PPS_GENERATOR_PARPORT
tristate "Parallel port PPS signal generator"
- depends on PARPORT && BROKEN
+ depends on PARPORT
help
If you say yes here you get support for a PPS signal generator which
utilizes STROBE pin of a parallel port to send PPS signals. It uses
diff --git a/drivers/pps/generators/pps_gen_parport.c b/drivers/pps/generators/pps_gen_parport.c
index 7739301..bcb53cd 100644
--- a/drivers/pps/generators/pps_gen_parport.c
+++ b/drivers/pps/generators/pps_gen_parport.c
@@ -53,6 +53,13 @@ MODULE_PARM_DESC(polarity,
"Signal is on the low level (0 - default) or on the high level (1).");
module_param(polarity, uint, 0);
+static unsigned int failure_iterations = 5;
+MODULE_PARM_DESC(failure_iterations,
+ "Number of iterations the clock source may remain unchanged.");
+module_param(failure_iterations, uint, 0);
+
+#define MAX_GETTIME_ATTEMPTS 100000
+
#define SAFETY_INTERVAL 3000 /* set the hrtimer earlier for safety (ns) */
/* internal per port structure */
@@ -79,6 +86,7 @@ static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
struct parport *port;
long lim, delta;
unsigned long flags;
+ unsigned int i;
/* We have to disable interrupts here. The idea is to prevent
* other interrupts on the same processor to introduce random
@@ -106,8 +114,18 @@ static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
}
/* busy loop until the time is right for an assert edge */
+ i = 0;
do {
getnstimeofday(&ts2);
+ i++;
+
+ /* Check if there are problems with clock source
+ * and prevent hard lockups.
+ */
+ if ((i >= failure_iterations &&
+ ts1.tv_sec == ts2.tv_sec &&
+ ts1.tv_nsec == ts2.tv_nsec) || i > MAX_GETTIME_ATTEMPTS)
+ goto error;
} while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
/* set the signal */
@@ -116,8 +134,17 @@ static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
/* busy loop until the time is right for a clear edge */
lim = NSEC_PER_SEC - dev->port_write_time;
+ i = 0;
do {
getnstimeofday(&ts2);
+ i++;
+
+ /* Check if there are problems with clock source
+ * and prevent hard lockups.
+ */
+ if (i > MAX_GETTIME_ATTEMPTS)
+ goto error;
+
} while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
/* unset the signal */
@@ -154,6 +181,11 @@ static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
2 * hrtimer_error)));
return HRTIMER_RESTART;
+
+error:
+ local_irq_restore(flags);
+ pr_err("Clocksource unstable or not compatible with pps_gen_parport.");
+ return HRTIMER_NORESTART;
}
/* calibrate port write time */
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event(). |
| Message-ID | <tb8Uq-1ae-13@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Alexander GQ Gerasiov <gq@redlab-i.ru>
Otherwise we get "scheduling while atomic" problem.
Signed-off-by: Alexander GQ Gerasiov <gq@redlab-i.ru>
---
drivers/pps/kapi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 805c749..a9a111d 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -226,11 +226,11 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
/* Wake up if captured something */
if (captured) {
pps->last_ev++;
+ spin_unlock_irqrestore(&pps->lock, flags);
wake_up_interruptible_all(&pps->queue);
-
kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
- }
- spin_unlock_irqrestore(&pps->lock, flags);
+ } else
+ spin_unlock_irqrestore(&pps->lock, flags);
}
EXPORT_SYMBOL(pps_event);
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal. |
| Message-ID | <tb8Uq-1ae-11@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Alexander GQ Gerasiov <gq@cs.msu.su>
Since clear timeout is non-zero, some clear event capture is requested.
Therefore, if signal is lost we shouldn't generate assert event alone.
Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
drivers/pps/clients/pps_parport.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index 83797d8..37094b0 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -96,7 +96,7 @@ static void parport_irq(void *handle)
if (!signal_is_set(port)) {
local_irq_restore(flags);
dev_err(dev->pps->dev, "lost the signal\n");
- goto out_assert;
+ goto out_none;
}
/* poll the port until the signal is unset */
@@ -118,6 +118,9 @@ static void parport_irq(void *handle)
dev->cw_err = 0;
}
+out_none:
+ return;
+
out_assert:
/* fire assert event */
pps_event(dev->pps, &ts_assert,
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal. |
| Message-ID | <tb8Uq-1ae-19@gated-at.bofh.it> |
| In reply to | #1581354 |
From: Alexander GQ Gerasiov <gq@cs.msu.su> On some devices it may be necessary to transmit inverted data. This commit simply adds polarity parameter to define which state represents presence of signal: it equals 0 if signal is on the low level (default), or 1 if signal is on the high level (inverted signal). Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su> --- drivers/pps/generators/pps_gen_parport.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/pps/generators/pps_gen_parport.c b/drivers/pps/generators/pps_gen_parport.c index dcd39fb..7739301 100644 --- a/drivers/pps/generators/pps_gen_parport.c +++ b/drivers/pps/generators/pps_gen_parport.c @@ -36,8 +36,8 @@ #define DRVDESC "parallel port PPS signal generator" -#define SIGNAL 0 -#define NO_SIGNAL PARPORT_CONTROL_STROBE +#define SIGNAL (polarity?PARPORT_CONTROL_STROBE:0) +#define NO_SIGNAL (polarity?0:PARPORT_CONTROL_STROBE) /* module parameters */ @@ -48,6 +48,10 @@ MODULE_PARM_DESC(delay, "Delay between setting and dropping the signal (ns)"); module_param_named(delay, send_delay, uint, 0); +static unsigned int polarity; +MODULE_PARM_DESC(polarity, + "Signal is on the low level (0 - default) or on the high level (1)."); +module_param(polarity, uint, 0); #define SAFETY_INTERVAL 3000 /* set the hrtimer earlier for safety (ns) */ -- 2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Andrey Drobyshev <immortalguardian1@gmail.com> |
|---|---|
| Date | 2017-02-15 15:40 +0100 |
| Subject | [PATCH 3/8] hardpps: fix some pps_jitter issues. |
| Message-ID | <tb8Uq-1ae-27@gated-at.bofh.it> |
| In reply to | #1581354 |
Handle possible overflow, implementation-defined result of signed right shift
and replace unsuitable constant.
Signed-off-by: Andrey Drobyshev <immortalguardian1@gmail.com>
Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
kernel/time/ntp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 22f2235..9cef1b9 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -91,6 +91,7 @@ static time64_t ntp_next_leap_sec = TIME64_MAX;
*/
#define PPS_VALID 10 /* PPS signal watchdog max (s) */
#define PPS_POPCORN 4 /* popcorn spike threshold (shift) */
+#define PPS_JITUPD 2 /* pps_jitter update factor (shift) */
#define PPS_INTMIN 2 /* min freq interval (s) (shift) */
#define PPS_INTMAX 8 /* max freq interval (s) (shift) */
#define PPS_INTCOUNT 4 /* number of consecutive good intervals to
@@ -941,7 +942,7 @@ static void hardpps_update_phase(long error)
* threshold, the sample is discarded; otherwise, if so enabled,
* the time offset is updated.
*/
- if (jitter > (pps_jitter << PPS_POPCORN)) {
+ if (pps_jitter && (jitter > ((long long)pps_jitter << PPS_POPCORN))) {
printk_deferred(KERN_WARNING
"hardpps: PPSJITTER: jitter=%ld, limit=%ld\n",
jitter, (pps_jitter << PPS_POPCORN));
@@ -955,7 +956,7 @@ static void hardpps_update_phase(long error)
time_adjust = 0;
}
/* update jitter */
- pps_jitter += (jitter - pps_jitter) >> PPS_INTMIN;
+ pps_jitter += shift_right(jitter - pps_jitter, PPS_JITUPD);
}
/*
--
2.1.4
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web