linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] LinuxPPS: some minor fixes and improvements.
@ 2017-02-15 14:31 Andrey Drobyshev
  2017-02-15 14:31 ` [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro Andrey Drobyshev
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:36   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal Andrey Drobyshev
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

From: Alexander GQ Gerasiov <gq@cs.msu.su>

Here are some trivial fixes, including:
  * simplify comparisons by using abs() macro
  * remove unnecessary variables
  * remove duplicate headers
  * fix typos

Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>
---
 kernel/time/ntp.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index edf19cc..d20891e 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -16,7 +16,6 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rtc.h>
-#include <linux/math64.h>
 
 #include "ntp_internal.h"
 #include "timekeeping_internal.h"
@@ -874,7 +873,7 @@ static inline void pps_inc_freq_interval(void)
  */
 static long hardpps_update_freq(struct pps_normtime freq_norm)
 {
-	long delta, delta_mod;
+	long delta;
 	s64 ftemp;
 
 	/* check if the frequency interval was too long */
@@ -896,7 +895,7 @@ static long hardpps_update_freq(struct pps_normtime freq_norm)
 			freq_norm.sec);
 	delta = shift_right(ftemp - pps_freq, NTP_SCALE_SHIFT);
 	pps_freq = ftemp;
-	if (delta > PPS_MAXWANDER || delta < -PPS_MAXWANDER) {
+	if (abs(delta) > PPS_MAXWANDER) {
 		printk_deferred(KERN_WARNING
 				"hardpps: PPSWANDER: change=%ld\n", delta);
 		time_status |= STA_PPSWANDER;
@@ -910,10 +909,7 @@ static long hardpps_update_freq(struct pps_normtime freq_norm)
 	 * frequency changes, but is used only for performance
 	 * monitoring
 	 */
-	delta_mod = delta;
-	if (delta_mod < 0)
-		delta_mod = -delta_mod;
-	pps_stabil += (div_s64(((s64)delta_mod) <<
+	pps_stabil += (div_s64(((s64)abs(delta)) <<
 				(NTP_SCALE_SHIFT - SHIFT_USEC),
 				NSEC_PER_USEC) - pps_stabil) >> PPS_INTMIN;
 
@@ -994,10 +990,8 @@ void __hardpps(const struct timespec64 *phase_ts, const struct timespec64 *raw_t
 	freq_norm = pps_normalize_ts(timespec64_sub(*raw_ts, pps_fbase));
 
 	/* check that the signal is in the range
-	 * [1s - MAXFREQ us, 1s + MAXFREQ us], otherwise reject it */
-	if ((freq_norm.sec == 0) ||
-			(freq_norm.nsec > MAXFREQ * freq_norm.sec) ||
-			(freq_norm.nsec < -MAXFREQ * freq_norm.sec)) {
+	 * [1s - MAXFREQ ns, 1s + MAXFREQ ns], otherwise reject it */
+	if (abs(freq_norm.nsec) > MAXFREQ * freq_norm.sec) {
 		time_status |= STA_PPSJITTER;
 		/* restart the frequency calibration interval */
 		pps_fbase = *raw_ts;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
  2017-02-15 14:31 ` [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:36   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 3/8] hardpps: fix some pps_jitter issues Andrey Drobyshev
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/8] hardpps: fix some pps_jitter issues.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
  2017-02-15 14:31 ` [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro Andrey Drobyshev
  2017-02-15 14:31 ` [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:37   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event() Andrey Drobyshev
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event().
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
                   ` (2 preceding siblings ...)
  2017-02-15 14:31 ` [PATCH 3/8] hardpps: fix some pps_jitter issues Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:38   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal Andrey Drobyshev
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
                   ` (3 preceding siblings ...)
  2017-02-15 14:31 ` [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event() Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:39   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous Andrey Drobyshev
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
                   ` (4 preceding siblings ...)
  2017-02-15 14:31 ` [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:40   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal Andrey Drobyshev
  2017-02-15 14:31 ` [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource Andrey Drobyshev
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
                   ` (5 preceding siblings ...)
  2017-02-15 14:31 ` [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:42   ` Rodolfo Giometti
  2017-02-15 14:31 ` [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource Andrey Drobyshev
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource.
  2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
                   ` (6 preceding siblings ...)
  2017-02-15 14:31 ` [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal Andrey Drobyshev
@ 2017-02-15 14:31 ` Andrey Drobyshev
  2017-03-04  7:44   ` Rodolfo Giometti
  7 siblings, 1 reply; 19+ messages in thread
From: Andrey Drobyshev @ 2017-02-15 14:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: gq, giometti

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro.
  2017-02-15 14:31 ` [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro Andrey Drobyshev
@ 2017-03-04  7:36   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:36 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> From: Alexander GQ Gerasiov <gq@cs.msu.su>
>
> Here are some trivial fixes, including:
>   * simplify comparisons by using abs() macro
>   * remove unnecessary variables
>   * remove duplicate headers
>   * fix typos
>
> Signed-off-by: Alexander GQ Gerasiov <gq@cs.msu.su>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal.
  2017-02-15 14:31 ` [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal Andrey Drobyshev
@ 2017-03-04  7:36   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:36 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/8] hardpps: fix some pps_jitter issues.
  2017-02-15 14:31 ` [PATCH 3/8] hardpps: fix some pps_jitter issues Andrey Drobyshev
@ 2017-03-04  7:37   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:37 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event().
  2017-02-15 14:31 ` [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event() Andrey Drobyshev
@ 2017-03-04  7:38   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:38 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal.
  2017-02-15 14:31 ` [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal Andrey Drobyshev
@ 2017-03-04  7:39   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:39 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous.
  2017-02-15 14:31 ` [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous Andrey Drobyshev
@ 2017-03-04  7:40   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:40 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal.
  2017-02-15 14:31 ` [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal Andrey Drobyshev
@ 2017-03-04  7:42   ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:42 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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)

Add spaces:

> +#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) */

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource.
  2017-02-15 14:31 ` [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource Andrey Drobyshev
@ 2017-03-04  7:44   ` Rodolfo Giometti
  2017-03-04  9:19     ` Alexander Gerasiov
  0 siblings, 1 reply; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04  7:44 UTC (permalink / raw)
  To: Andrey Drobyshev, linux-kernel; +Cc: gq

On 02/15/17 15:31, Andrey Drobyshev wrote:
> 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>

Acked-by: Rodolfo Giometti <giometti@enneenne.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource.
  2017-03-04  7:44   ` Rodolfo Giometti
@ 2017-03-04  9:19     ` Alexander Gerasiov
  2017-03-04 13:25       ` Rodolfo Giometti
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Gerasiov @ 2017-03-04  9:19 UTC (permalink / raw)
  To: Rodolfo Giometti; +Cc: Andrey Drobyshev, linux-kernel, Thomas Gleixner

Hello Rodolfo,

I should say, that I found this solution is not the best. For example on
high speed PCs (I think over 3GHz) 5 iteration is not enough and module
stops working after several hours. That makes it not user-friendly,
cause user had to calibrate and set failure_iterations manually.

It would be better to use pre-measured loops_per_jiffy (from delay.h)
some way.

On Sat, 4 Mar 2017 08:44:03 +0100
Rodolfo Giometti <giometti@enneenne.com> wrote:

> On 02/15/17 15:31, Andrey Drobyshev wrote:
> > 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>  
> 
> Acked-by: Rodolfo Giometti <giometti@enneenne.com>



-- 
Best regards,
 Alexander Gerasiov

 Contacts:
 e-mail: gq@cs.msu.su  Homepage: http://gerasiov.net  Skype: gerasiov
 PGP fingerprint: 04B5 9D90 DF7C C2AB CD49  BAEA CA87 E9E8 2AAC 33F1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource.
  2017-03-04  9:19     ` Alexander Gerasiov
@ 2017-03-04 13:25       ` Rodolfo Giometti
  0 siblings, 0 replies; 19+ messages in thread
From: Rodolfo Giometti @ 2017-03-04 13:25 UTC (permalink / raw)
  To: Alexander Gerasiov; +Cc: Andrey Drobyshev, linux-kernel, Thomas Gleixner

On 03/04/17 10:19, Alexander Gerasiov wrote:
> Hello Rodolfo,
>
> I should say, that I found this solution is not the best. For example on
> high speed PCs (I think over 3GHz) 5 iteration is not enough and module
> stops working after several hours. That makes it not user-friendly,
> cause user had to calibrate and set failure_iterations manually.
>
> It would be better to use pre-measured loops_per_jiffy (from delay.h)
> some way.

I see... however I cannot test the patch so if you tried it and you noticed 
these issues we should consider to add it but with the experimental warning?

Ciao,

Rodolfo

-- 

HCE Engineering                      e-mail: giometti@hce-engineering.com
GNU/Linux Solutions                          giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.io
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-03-04 13:25 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15 14:31 [PATCH 0/8] LinuxPPS: some minor fixes and improvements Andrey Drobyshev
2017-02-15 14:31 ` [PATCH 1/8] hardpps: simple fixes replacing clumsy code with abs() macro Andrey Drobyshev
2017-03-04  7:36   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 2/8] ntp/pps: ignore pps_valid decreasing if there is no pps signal Andrey Drobyshev
2017-03-04  7:36   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 3/8] hardpps: fix some pps_jitter issues Andrey Drobyshev
2017-03-04  7:37   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 4/8] LinuxPPS: kapi: Unlock before waking up events queue in pps_event() Andrey Drobyshev
2017-03-04  7:38   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 5/8] LinuxPPS: pps_parport: Do not generate assert in case of lost signal Andrey Drobyshev
2017-03-04  7:39   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 6/8] LinuxPPS: pps_parport: Ignore interrupt invoked less than 0.5sec after previous Andrey Drobyshev
2017-03-04  7:40   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 7/8] LinuxPPS: pps_gen_parport: Add polarity parameter for inverted signal Andrey Drobyshev
2017-03-04  7:42   ` Rodolfo Giometti
2017-02-15 14:31 ` [PATCH 8/8] LinuxPPS: pps_gen_parport: Add check for bad clocksource Andrey Drobyshev
2017-03-04  7:44   ` Rodolfo Giometti
2017-03-04  9:19     ` Alexander Gerasiov
2017-03-04 13:25       ` Rodolfo Giometti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).