This patchset contains several changes that improve an overall
design/performance of PPS subsystem. I'd like these patches to be
merged mainline if no one objects.
Patches 1-4 are bugfixes.
Patches 5-12 are other improvements to PPS subsystem.
Patches 13-15 add kernel consumer support.
Patch 16 adds parallel port PPS client.
Patch 17 adds parallel port PPS generator.
You can find description for my previous patchset (it describes patches
13-17 in more detailed) here: http://lkml.org/lkml/2010/2/24/189
This patchset is tested against the vanilla 2.6.36 kernel. But we are
actually using it on 2.6.33.7-rt29 rt-preempt kernel most of the time.
Those who are interested in other versions of the patchset can find
them in my git repository:
git://github.com/ago/linux-2.6.git
There is one problem however: kernel consumer works bad (if enabled)
when CONFIG_NO_HZ is enabled. The reason for this is commit
a092ff0f90cae22b2ac8028ecd2c6f6c1a9e4601. Without it hardpps() is able
to sync to 1us precision in about 10 seconds. With CONFIG_NO_HZ it is
not syncing at all. This only affects patches 13-15, others are ok.
Changelog
v4 -> v5:
* drop patch that adds PPS workqueue, it needs some more thinking
* replace patch that stops disabling interrupts with a lighter version
* add patch 5 which ensures that dcd_change() will never be called
before tty_ldisc_open() or after tty_ldisc_close()
* fix patch 7: convert all remaining prints
* use BUG_ON only in pps_event
* use "canonical" error-handling style in pps_tty_open
Alexander Gordeev (17):
pps: trivial fixes
pps: declare variables where they are used in switch
pps: fix race in PPS_FETCH handler
pps: unify timestamp gathering
tty: don't allow ldisc dcd_change() after ldisc halt
pps: access pps device by direct pointer
pps: convert printk/pr_* to dev_*
pps: move idr stuff to pps.c
pps: do not disable interrupts for idr operations
pps: use BUG_ON for kernel API safety checks
pps: simplify conditions a bit
pps: timestamp is always passed to dcd_change()
ntp: add hardpps implementation
pps: capture MONOTONIC_RAW timestamps as well
pps: add kernel consumer support
pps: add parallel port PPS client
pps: add parallel port PPS signal generator
Documentation/ioctl/ioctl-number.txt | 2 +-
Documentation/pps/pps.txt | 46 ++++
Documentation/serial/tty.txt | 2 +-
drivers/char/tty_io.c | 1 +
drivers/char/tty_ldisc.c | 7 +
drivers/pps/Kconfig | 10 +
drivers/pps/Makefile | 2 +-
drivers/pps/clients/Kconfig | 7 +
drivers/pps/clients/Makefile | 1 +
drivers/pps/clients/pps-ktimer.c | 44 ++--
drivers/pps/clients/pps-ldisc.c | 54 ++---
drivers/pps/clients/pps_parport.c | 247 +++++++++++++++++
drivers/pps/generators/Kconfig | 17 ++
drivers/pps/generators/Makefile | 9 +
drivers/pps/generators/pps_gen_parport.c | 275 +++++++++++++++++++
drivers/pps/kapi.c | 221 +++++-----------
drivers/pps/pps.c | 177 ++++++++++---
include/linux/pps.h | 7 +
include/linux/pps_kernel.h | 54 +++-
include/linux/serial_core.h | 23 +-
include/linux/time.h | 2 +
include/linux/timex.h | 1 +
include/linux/tty.h | 1 +
include/linux/tty_ldisc.h | 7 +-
kernel/time/ntp.c | 425 ++++++++++++++++++++++++++++-
kernel/time/timekeeping.c | 38 +++
26 files changed, 1387 insertions(+), 293 deletions(-)
create mode 100644 drivers/pps/clients/pps_parport.c
create mode 100644 drivers/pps/generators/Kconfig
create mode 100644 drivers/pps/generators/Makefile
create mode 100644 drivers/pps/generators/pps_gen_parport.c
--
1.7.2.3
Add an optional feature of PPSAPI, kernel consumer support, which uses
the added hardpps() function.
Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/pps/Kconfig | 1 +
drivers/pps/kapi.c | 27 +++++++++++++++
drivers/pps/pps.c | 62 +++++++++++++++++++++++++++++++++-
include/linux/pps.h | 7 ++++
include/linux/pps_kernel.h | 6 +++
6 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 33223ff..de2b113 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -249,7 +249,7 @@ Code Seq#(hex) Include File Comments
'p' 40-7F linux/nvram.h
'p' 80-9F linux/ppdev.h user-space parport
<mailto:[email protected]>
-'p' A1-A4 linux/pps.h LinuxPPS
+'p' A1-A5 linux/pps.h LinuxPPS
<mailto:[email protected]>
'q' 00-1F linux/serio.h
'q' 80-FF linux/telephony.h Internet PhoneJACK, Internet LineJACK
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 33c9126..4823e47 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -7,6 +7,7 @@ menu "PPS support"
config PPS
tristate "PPS support"
depends on EXPERIMENTAL
+ select NTP_PPS
---help---
PPS (Pulse Per Second) is a special pulse provided by some GPS
antennae. Userland can use it to get a high-precision time
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index ebf3374..ed78f6d 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -26,12 +26,23 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/time.h>
+#include <linux/timex.h>
#include <linux/spinlock.h>
#include <linux/fs.h>
#include <linux/pps_kernel.h>
#include <linux/slab.h>
/*
+ * Global variables
+ */
+
+/* state variables to bind kernel consumer */
+/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
+DEFINE_SPINLOCK(pps_kc_hardpps_lock);
+void *pps_kc_hardpps_dev; /* some unique pointer to device */
+int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
+
+/*
* Local functions
*/
@@ -139,6 +150,16 @@ EXPORT_SYMBOL(pps_register_source);
void pps_unregister_source(struct pps_device *pps)
{
+ spin_lock_irq(&pps_kc_hardpps_lock);
+ if (pps == pps_kc_hardpps_dev) {
+ pps_kc_hardpps_mode = 0;
+ pps_kc_hardpps_dev = NULL;
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+ dev_info(pps->dev, "unbound kernel consumer"
+ " on device removal\n");
+ } else
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+
pps_unregister_cdev(pps);
/* don't have to kfree(pps) here because it will be done on
@@ -211,6 +232,12 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
captured = ~0;
}
+ /* Pass some events to kernel consumer if activated */
+ spin_lock(&pps_kc_hardpps_lock);
+ if (pps == pps_kc_hardpps_dev && event & pps_kc_hardpps_mode)
+ hardpps(&ts->ts_real, &ts->ts_raw);
+ spin_unlock(&pps_kc_hardpps_lock);
+
/* Wake up if captured something */
if (captured) {
pps->last_ev++;
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 54964a5..76f4b5e 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -197,9 +197,69 @@ static long pps_cdev_ioctl(struct file *file,
break;
}
+ case PPS_KC_BIND: {
+ struct pps_bind_args bind_args;
+
+ dev_dbg(pps->dev, "PPS_KC_BIND\n");
+
+ /* Check the capabilities */
+ if (!capable(CAP_SYS_TIME))
+ return -EPERM;
+
+ if (copy_from_user(&bind_args, uarg,
+ sizeof(struct pps_bind_args)))
+ return -EFAULT;
+
+ /* Check for supported capabilities */
+ if ((bind_args.edge & ~pps->info.mode) != 0) {
+ dev_err(pps->dev, "unsupported capabilities (%x)\n",
+ bind_args.edge);
+ return -EINVAL;
+ }
+
+ /* Validate parameters roughly */
+ if (bind_args.tsformat != PPS_TSFMT_TSPEC ||
+ (bind_args.edge & ~PPS_CAPTUREBOTH) != 0 ||
+ bind_args.consumer != PPS_KC_HARDPPS) {
+ dev_err(pps->dev, "invalid kernel consumer bind"
+ " parameters (%x)\n", bind_args.edge);
+ return -EINVAL;
+ }
+
+ /* Check if another consumer is already bound */
+ spin_lock_irq(&pps_kc_hardpps_lock);
+
+ if (bind_args.edge == 0)
+ if (pps_kc_hardpps_dev == pps) {
+ pps_kc_hardpps_mode = 0;
+ pps_kc_hardpps_dev = NULL;
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+ dev_info(pps->dev, "unbound kernel"
+ " consumer\n");
+ } else {
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+ dev_err(pps->dev, "selected kernel consumer"
+ " is not bound\n");
+ return -EINVAL;
+ }
+ else
+ if (pps_kc_hardpps_dev == NULL ||
+ pps_kc_hardpps_dev == pps) {
+ pps_kc_hardpps_mode = bind_args.edge;
+ pps_kc_hardpps_dev = pps;
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+ dev_info(pps->dev, "bound kernel consumer: "
+ "edge=0x%x\n", bind_args.edge);
+ } else {
+ spin_unlock_irq(&pps_kc_hardpps_lock);
+ dev_err(pps->dev, "another kernel consumer"
+ " is already bound\n");
+ return -EINVAL;
+ }
+ break;
+ }
default:
return -ENOTTY;
- break;
}
return 0;
diff --git a/include/linux/pps.h b/include/linux/pps.h
index 0194ab0..a9bb1d9 100644
--- a/include/linux/pps.h
+++ b/include/linux/pps.h
@@ -114,11 +114,18 @@ struct pps_fdata {
struct pps_ktime timeout;
};
+struct pps_bind_args {
+ int tsformat; /* format of time stamps */
+ int edge; /* selected event type */
+ int consumer; /* selected kernel consumer */
+};
+
#include <linux/ioctl.h>
#define PPS_GETPARAMS _IOR('p', 0xa1, struct pps_kparams *)
#define PPS_SETPARAMS _IOW('p', 0xa2, struct pps_kparams *)
#define PPS_GETCAP _IOR('p', 0xa3, int *)
#define PPS_FETCH _IOWR('p', 0xa4, struct pps_fdata *)
+#define PPS_KC_BIND _IOW('p', 0xa5, struct pps_bind_args *)
#endif /* _PPS_H_ */
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 6ae8898..2302274 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -79,6 +79,12 @@ struct pps_device {
extern struct device_attribute pps_attrs[];
+/* state variables to bind kernel consumer */
+/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
+extern spinlock_t pps_kc_hardpps_lock;
+extern void *pps_kc_hardpps_dev; /* some unique pointer to device */
+extern int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
+
/*
* Exported functions
*/
--
1.7.2.3
Since now idr is only used to manage char device id's and not used in
kernel API anymore it should be moved to pps.c. This also makes it
possible to release id only at actual device freeing so nobody can
register a pps device with the same id while our device is not freed
yet.
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 56 ++-------------------------------------------------
drivers/pps/pps.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 52 insertions(+), 54 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index e8847c1..c42d3cb 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -27,19 +27,11 @@
#include <linux/sched.h>
#include <linux/time.h>
#include <linux/spinlock.h>
-#include <linux/idr.h>
#include <linux/fs.h>
#include <linux/pps_kernel.h>
#include <linux/slab.h>
/*
- * Local variables
- */
-
-static DEFINE_SPINLOCK(pps_idr_lock);
-static DEFINE_IDR(pps_idr);
-
-/*
* Local functions
*/
@@ -76,7 +68,6 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
int default_params)
{
struct pps_device *pps;
- int id;
int err;
/* Sanity checks */
@@ -117,54 +108,18 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
init_waitqueue_head(&pps->queue);
spin_lock_init(&pps->lock);
- /* Get new ID for the new PPS source */
- if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
- err = -ENOMEM;
- goto kfree_pps;
- }
-
- spin_lock_irq(&pps_idr_lock);
-
- /* Now really allocate the PPS source.
- * After idr_get_new() calling the new source will be freely available
- * into the kernel.
- */
- err = idr_get_new(&pps_idr, pps, &id);
- if (err < 0) {
- spin_unlock_irq(&pps_idr_lock);
- goto kfree_pps;
- }
-
- id = id & MAX_ID_MASK;
- if (id >= PPS_MAX_SOURCES) {
- spin_unlock_irq(&pps_idr_lock);
-
- pr_err("%s: too many PPS sources in the system\n",
- info->name);
- err = -EBUSY;
- goto free_idr;
- }
- pps->id = id;
-
- spin_unlock_irq(&pps_idr_lock);
-
/* Create the char device */
err = pps_register_cdev(pps);
if (err < 0) {
pr_err("%s: unable to create char device\n",
info->name);
- goto free_idr;
+ goto kfree_pps;
}
dev_info(pps->dev, "new PPS source %s\n", info->name);
return pps;
-free_idr:
- spin_lock_irq(&pps_idr_lock);
- idr_remove(&pps_idr, id);
- spin_unlock_irq(&pps_idr_lock);
-
kfree_pps:
kfree(pps);
@@ -184,15 +139,10 @@ EXPORT_SYMBOL(pps_register_source);
void pps_unregister_source(struct pps_device *pps)
{
- unsigned int id = pps->id;
-
pps_unregister_cdev(pps);
- spin_lock_irq(&pps_idr_lock);
- idr_remove(&pps_idr, pps->id);
- spin_unlock_irq(&pps_idr_lock);
-
- kfree(pps);
+ /* don't have to kfree(pps) here because it will be done on
+ * device destruction */
}
EXPORT_SYMBOL(pps_unregister_source);
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 9f7c2e8..79b4455 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -30,6 +30,7 @@
#include <linux/cdev.h>
#include <linux/poll.h>
#include <linux/pps_kernel.h>
+#include <linux/slab.h>
/*
* Local variables
@@ -38,6 +39,9 @@
static dev_t pps_devt;
static struct class *pps_class;
+static DEFINE_SPINLOCK(pps_idr_lock);
+static DEFINE_IDR(pps_idr);
+
/*
* Char device methods
*/
@@ -229,11 +233,48 @@ static const struct file_operations pps_cdev_fops = {
.release = pps_cdev_release,
};
+static void pps_device_destruct(struct device *dev)
+{
+ struct pps_device *pps = dev_get_drvdata(dev);
+
+ /* release id here to protect others from using it while it's
+ * still in use */
+ spin_lock_irq(&pps_idr_lock);
+ idr_remove(&pps_idr, pps->id);
+ spin_unlock_irq(&pps_idr_lock);
+
+ kfree(dev);
+ kfree(pps);
+}
+
int pps_register_cdev(struct pps_device *pps)
{
int err;
dev_t devt;
+ /* Get new ID for the new PPS source */
+ if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
+ return -ENOMEM;
+
+ /* Now really allocate the PPS source.
+ * After idr_get_new() calling the new source will be freely available
+ * into the kernel.
+ */
+ spin_lock_irq(&pps_idr_lock);
+ err = idr_get_new(&pps_idr, pps, &pps->id);
+ spin_unlock_irq(&pps_idr_lock);
+
+ if (err < 0)
+ return err;
+
+ pps->id &= MAX_ID_MASK;
+ if (pps->id >= PPS_MAX_SOURCES) {
+ pr_err("%s: too many PPS sources in the system\n",
+ pps->info.name);
+ err = -EBUSY;
+ goto free_idr;
+ }
+
devt = MKDEV(MAJOR(pps_devt), pps->id);
cdev_init(&pps->cdev, &pps_cdev_fops);
@@ -243,13 +284,15 @@ int pps_register_cdev(struct pps_device *pps)
if (err) {
pr_err("%s: failed to add char device %d:%d\n",
pps->info.name, MAJOR(pps_devt), pps->id);
- return err;
+ goto free_idr;
}
pps->dev = device_create(pps_class, pps->info.dev, devt, pps,
"pps%d", pps->id);
if (IS_ERR(pps->dev))
goto del_cdev;
+ pps->dev->release = pps_device_destruct;
+
pr_debug("source %s got cdev (%d:%d)\n", pps->info.name,
MAJOR(pps_devt), pps->id);
@@ -258,6 +301,11 @@ int pps_register_cdev(struct pps_device *pps)
del_cdev:
cdev_del(&pps->cdev);
+free_idr:
+ spin_lock_irq(&pps_idr_lock);
+ idr_remove(&pps_idr, pps->id);
+ spin_unlock_irq(&pps_idr_lock);
+
return err;
}
--
1.7.2.3
Now pps_idr_lock is never used in interrupt context so replace
spin_lock_irq/spin_unlock_irq with plain spin_lock/spin_unlock.
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/pps.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 79b4455..54964a5 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -239,9 +239,9 @@ static void pps_device_destruct(struct device *dev)
/* release id here to protect others from using it while it's
* still in use */
- spin_lock_irq(&pps_idr_lock);
+ spin_lock(&pps_idr_lock);
idr_remove(&pps_idr, pps->id);
- spin_unlock_irq(&pps_idr_lock);
+ spin_unlock(&pps_idr_lock);
kfree(dev);
kfree(pps);
@@ -260,9 +260,9 @@ int pps_register_cdev(struct pps_device *pps)
* After idr_get_new() calling the new source will be freely available
* into the kernel.
*/
- spin_lock_irq(&pps_idr_lock);
+ spin_lock(&pps_idr_lock);
err = idr_get_new(&pps_idr, pps, &pps->id);
- spin_unlock_irq(&pps_idr_lock);
+ spin_unlock(&pps_idr_lock);
if (err < 0)
return err;
@@ -302,9 +302,9 @@ del_cdev:
cdev_del(&pps->cdev);
free_idr:
- spin_lock_irq(&pps_idr_lock);
+ spin_lock(&pps_idr_lock);
idr_remove(&pps_idr, pps->id);
- spin_unlock_irq(&pps_idr_lock);
+ spin_unlock(&pps_idr_lock);
return err;
}
--
1.7.2.3
Remove the code that gatheres timestamp in pps_tty_dcd_change() in case
passed ts parameter is NULL because it never happens in the current
code. Fix comments as well.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/serial/tty.txt | 2 +-
drivers/pps/clients/pps-ldisc.c | 8 --------
include/linux/tty_ldisc.h | 2 +-
3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
index 7c90050..540db41 100644
--- a/Documentation/serial/tty.txt
+++ b/Documentation/serial/tty.txt
@@ -107,7 +107,7 @@ write_wakeup() - May be called at any point between open and close.
dcd_change() - Report to the tty line the current DCD pin status
changes and the relative timestamp. The timestamp
- can be NULL.
+ cannot be NULL.
Driver Access
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 199a83b..19b3477 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -32,14 +32,6 @@ static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
struct pps_event_time *ts)
{
struct pps_device *pps = (struct pps_device *)tty->disc_data;
- struct pps_event_time __ts;
-
- /* First of all we get the time stamp... */
- pps_get_ts(&__ts);
-
- /* Does caller give us a timestamp? */
- if (!ts) /* No. Do it ourself! */
- ts = &__ts;
BUG_ON(pps == NULL);
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 763e061..ff7dc08 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -104,7 +104,7 @@
* struct pps_event_time *ts)
*
* Tells the discipline that the DCD pin has changed its status and
- * the relative timestamp. Pointer ts can be NULL.
+ * the relative timestamp. Pointer ts cannot be NULL.
*/
#include <linux/fs.h>
--
1.7.2.3
MONOTONIC_RAW clock timestamps are ideally suited for frequency
calculation and also fit well into the original NTP hardpps design. Now
phase and frequency can be adjusted separately: the former based on
REALTIME clock and the latter based on MONOTONIC_RAW clock.
A new function getnstime_raw_and_real is added to timekeeping subsystem
to capture both timestamps at the same time and atomically.
Acked-by: John Stultz <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
include/linux/pps_kernel.h | 3 ++-
include/linux/time.h | 2 ++
kernel/time/timekeeping.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletions(-)
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 1aedf50..6ae8898 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -47,6 +47,7 @@ struct pps_source_info {
};
struct pps_event_time {
+ struct timespec ts_raw;
struct timespec ts_real;
};
@@ -99,7 +100,7 @@ static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
static inline void pps_get_ts(struct pps_event_time *ts)
{
- getnstimeofday(&ts->ts_real);
+ getnstime_raw_and_real(&ts->ts_raw, &ts->ts_real);
}
#endif /* LINUX_PPS_KERNEL_H */
diff --git a/include/linux/time.h b/include/linux/time.h
index 9f15ac7..1e6d3b5 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -158,6 +158,8 @@ extern unsigned int alarm_setitimer(unsigned int seconds);
extern int do_getitimer(int which, struct itimerval *value);
extern void getnstimeofday(struct timespec *tv);
extern void getrawmonotonic(struct timespec *ts);
+extern void getnstime_raw_and_real(struct timespec *ts_raw,
+ struct timespec *ts_real);
extern void getboottime(struct timespec *ts);
extern void monotonic_to_bootbased(struct timespec *ts);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49010d8..947c7dc 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -285,6 +285,44 @@ void ktime_get_ts(struct timespec *ts)
EXPORT_SYMBOL_GPL(ktime_get_ts);
/**
+ * getnstime_raw_and_real - Returns both the time of day an raw
+ * monotonic time in a timespec format
+ * @ts_mono_raw: pointer to the timespec to be set to raw
+ * monotonic time
+ * @ts_real: pointer to the timespec to be set to the time
+ * of day
+ */
+void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
+{
+ unsigned long seq;
+ s64 nsecs_raw, nsecs_real;
+
+ WARN_ON_ONCE(timekeeping_suspended);
+
+ do {
+ u32 arch_offset;
+
+ seq = read_seqbegin(&xtime_lock);
+
+ *ts_raw = raw_time;
+ *ts_real = xtime;
+
+ nsecs_raw = timekeeping_get_ns_raw();
+ nsecs_real = timekeeping_get_ns();
+
+ /* If arch requires, add in gettimeoffset() */
+ arch_offset = arch_gettimeoffset();
+ nsecs_raw += arch_offset;
+ nsecs_real += arch_offset;
+
+ } while (read_seqretry(&xtime_lock, seq));
+
+ timespec_add_ns(ts_raw, nsecs_raw);
+ timespec_add_ns(ts_real, nsecs_real);
+}
+EXPORT_SYMBOL(getnstime_raw_and_real);
+
+/**
* do_gettimeofday - Returns the time of day in a timeval
* @tv: pointer to the timeval to be set
*
--
1.7.2.3
There was a possibility that uart_handle_dcd_change() could obtain a
reference to ldisc while running in parallel with tty_set_ldisc() on
different CPU but call dcd_change() operation after tty_ldisc_close()
which is incorrect.
Treat this situation specially by locking the whole
uart_handle_dcd_change() with spinlock and adding a "barrier" to
tty_ldisc_halt() which ensures that there are no active ldisc
references in uart_handle_dcd_change() after tty_ldisc_halt().
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/char/tty_io.c | 1 +
drivers/char/tty_ldisc.c | 7 +++++++
include/linux/serial_core.h | 18 ++++++++++++------
include/linux/tty.h | 1 +
4 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 613c852..18576d4 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2816,6 +2816,7 @@ void initialize_tty_struct(struct tty_struct *tty,
mutex_init(&tty->echo_lock);
spin_lock_init(&tty->read_lock);
spin_lock_init(&tty->ctrl_lock);
+ spin_lock_init(&tty->dcd_change_lock);
INIT_LIST_HEAD(&tty->tty_files);
INIT_WORK(&tty->SAK_work, do_SAK_work);
diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
index 412f977..27fadb0 100644
--- a/drivers/char/tty_ldisc.c
+++ b/drivers/char/tty_ldisc.c
@@ -522,11 +522,18 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
* You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex)
* in order to make sure any currently executing ldisc work is also
* flushed.
+ *
+ * dcd_change() doesn't use workqueues so it needs a special
+ * "barrier", which ensures that there are no active ldisc references
+ * in dcd_change().
*/
static int tty_ldisc_halt(struct tty_struct *tty)
{
+ spin_lock_irq(&tty->dcd_change_lock);
clear_bit(TTY_LDISC, &tty->flags);
+ spin_unlock_irq(&tty->dcd_change_lock);
+
return cancel_delayed_work_sync(&tty->buf.work);
}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 55c8192..62835b6 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -508,11 +508,15 @@ static inline int uart_handle_break(struct uart_port *port)
static inline void
uart_handle_dcd_change(struct uart_port *uport, unsigned int status)
{
- struct uart_state *state = uport->state;
- struct tty_port *port = &state->port;
- struct tty_ldisc *ld = tty_ldisc_ref(port->tty);
+ struct tty_port *port = &uport->state->port;
+ struct tty_struct *tty = port->tty;
struct pps_event_time ts;
+ struct tty_ldisc *ld;
+ unsigned long flags;
+ spin_lock_irqsave(&tty->dcd_change_lock, flags);
+
+ ld = tty_ldisc_ref(tty);
if (ld && ld->ops->dcd_change)
pps_get_ts(&ts);
@@ -525,14 +529,16 @@ uart_handle_dcd_change(struct uart_port *uport, unsigned int status)
if (port->flags & ASYNC_CHECK_CD) {
if (status)
wake_up_interruptible(&port->open_wait);
- else if (port->tty)
- tty_hangup(port->tty);
+ else if (tty)
+ tty_hangup(tty);
}
if (ld && ld->ops->dcd_change)
- ld->ops->dcd_change(port->tty, status, &ts);
+ ld->ops->dcd_change(tty, status, &ts);
if (ld)
tty_ldisc_deref(ld);
+
+ spin_unlock_irqrestore(&tty->dcd_change_lock, flags);
}
/**
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 67d64e6..506fe1c 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -327,6 +327,7 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+ spinlock_t dcd_change_lock;
};
/* Each of a tty's open files has private_data pointing to tty_file_private */
--
1.7.2.3
Here are some very trivial fixes combined:
* add macro definitions to protect header file from including several times
* remove declaration for an unexistent array
* fix typos
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 2 +-
include/linux/pps_kernel.h | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 1aa02db..55f3961 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -324,7 +324,7 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
captured = ~0;
}
- /* Wake up iif captured somthing */
+ /* Wake up if captured something */
if (captured) {
pps->go = ~0;
wake_up_interruptible(&pps->queue);
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index e0a193f..c930d11 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -18,6 +18,9 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#ifndef LINUX_PPS_KERNEL_H
+#define LINUX_PPS_KERNEL_H
+
#include <linux/pps.h>
#include <linux/cdev.h>
@@ -71,7 +74,6 @@ struct pps_device {
extern spinlock_t pps_idr_lock;
extern struct idr pps_idr;
-extern struct timespec pps_irq_ts[];
extern struct device_attribute pps_attrs[];
@@ -87,3 +89,6 @@ extern void pps_unregister_source(int source);
extern int pps_register_cdev(struct pps_device *pps);
extern void pps_unregister_cdev(struct pps_device *pps);
extern void pps_event(int source, struct pps_ktime *ts, int event, void *data);
+
+#endif /* LINUX_PPS_KERNEL_H */
+
--
1.7.2.3
Add a helper function to gather timestamps. This way clients don't have
to duplicate it.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/clients/pps-ktimer.c | 9 ++-------
drivers/pps/clients/pps-ldisc.c | 18 ++++++------------
drivers/pps/kapi.c | 19 ++++++++++++-------
include/linux/pps_kernel.h | 19 ++++++++++++++++++-
include/linux/serial_core.h | 5 +++--
include/linux/tty_ldisc.h | 5 +++--
6 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index e7ef5b8..e1bdd8b 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -40,18 +40,13 @@ static struct timer_list ktimer;
static void pps_ktimer_event(unsigned long ptr)
{
- struct timespec __ts;
- struct pps_ktime ts;
+ struct pps_event_time ts;
/* First of all we get the time stamp... */
- getnstimeofday(&__ts);
+ pps_get_ts(&ts);
pr_info("PPS event at %lu\n", jiffies);
- /* ... and translate it to PPS time data struct */
- ts.sec = __ts.tv_sec;
- ts.nsec = __ts.tv_nsec;
-
pps_event(source, &ts, PPS_CAPTUREASSERT, NULL);
mod_timer(&ktimer, jiffies + HZ);
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 8e1932d..20fc9f7 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -27,26 +27,20 @@
#define PPS_TTY_MAGIC 0x0001
static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
- struct timespec *ts)
+ struct pps_event_time *ts)
{
int id = (long)tty->disc_data;
- struct timespec __ts;
- struct pps_ktime pps_ts;
+ struct pps_event_time __ts;
/* First of all we get the time stamp... */
- getnstimeofday(&__ts);
+ pps_get_ts(&__ts);
/* Does caller give us a timestamp? */
- if (ts) { /* Yes. Let's use it! */
- pps_ts.sec = ts->tv_sec;
- pps_ts.nsec = ts->tv_nsec;
- } else { /* No. Do it ourself! */
- pps_ts.sec = __ts.tv_sec;
- pps_ts.nsec = __ts.tv_nsec;
- }
+ if (!ts) /* No. Do it ourself! */
+ ts = &__ts;
/* Now do the PPS event report */
- pps_event(id, &pps_ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
+ pps_event(id, ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
NULL);
pr_debug("PPS %s at %lu on source #%d\n",
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 3f89f5eb..b431d83 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -268,11 +268,12 @@ EXPORT_SYMBOL(pps_unregister_source);
* pps->info.echo(source, event, data);
*/
-void pps_event(int source, struct pps_ktime *ts, int event, void *data)
+void pps_event(int source, struct pps_event_time *ts, int event, void *data)
{
struct pps_device *pps;
unsigned long flags;
int captured = 0;
+ struct pps_ktime ts_real;
if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
@@ -284,8 +285,10 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
if (!pps)
return;
- pr_debug("PPS event on source %d at %llu.%06u\n",
- pps->id, (unsigned long long) ts->sec, ts->nsec);
+ pr_debug("PPS event on source %d at %ld.%09ld\n",
+ pps->id, ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
+
+ timespec_to_pps_ktime(&ts_real, ts->ts_real);
spin_lock_irqsave(&pps->lock, flags);
@@ -299,10 +302,11 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
(pps->params.mode & PPS_CAPTUREASSERT)) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETASSERT)
- pps_add_offset(ts, &pps->params.assert_off_tu);
+ pps_add_offset(&ts_real,
+ &pps->params.assert_off_tu);
/* Save the time stamp */
- pps->assert_tu = *ts;
+ pps->assert_tu = ts_real;
pps->assert_sequence++;
pr_debug("capture assert seq #%u for source %d\n",
pps->assert_sequence, source);
@@ -313,10 +317,11 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
(pps->params.mode & PPS_CAPTURECLEAR)) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETCLEAR)
- pps_add_offset(ts, &pps->params.clear_off_tu);
+ pps_add_offset(&ts_real,
+ &pps->params.clear_off_tu);
/* Save the time stamp */
- pps->clear_tu = *ts;
+ pps->clear_tu = ts_real;
pps->clear_sequence++;
pr_debug("capture clear seq #%u for source %d\n",
pps->clear_sequence, source);
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index c3aed4b..32aa676 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -43,6 +43,10 @@ struct pps_source_info {
struct device *dev;
};
+struct pps_event_time {
+ struct timespec ts_real;
+};
+
/* The main struct */
struct pps_device {
struct pps_source_info info; /* PSS source info */
@@ -88,7 +92,20 @@ extern int pps_register_source(struct pps_source_info *info,
extern void pps_unregister_source(int source);
extern int pps_register_cdev(struct pps_device *pps);
extern void pps_unregister_cdev(struct pps_device *pps);
-extern void pps_event(int source, struct pps_ktime *ts, int event, void *data);
+extern void pps_event(int source, struct pps_event_time *ts, int event,
+ void *data);
+
+static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
+ struct timespec ts)
+{
+ kt->sec = ts.tv_sec;
+ kt->nsec = ts.tv_nsec;
+}
+
+static inline void pps_get_ts(struct pps_event_time *ts)
+{
+ getnstimeofday(&ts->ts_real);
+}
#endif /* LINUX_PPS_KERNEL_H */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 563e234..55c8192 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -206,6 +206,7 @@
#include <linux/tty.h>
#include <linux/mutex.h>
#include <linux/sysrq.h>
+#include <linux/pps_kernel.h>
struct uart_port;
struct serial_struct;
@@ -510,10 +511,10 @@ uart_handle_dcd_change(struct uart_port *uport, unsigned int status)
struct uart_state *state = uport->state;
struct tty_port *port = &state->port;
struct tty_ldisc *ld = tty_ldisc_ref(port->tty);
- struct timespec ts;
+ struct pps_event_time ts;
if (ld && ld->ops->dcd_change)
- getnstimeofday(&ts);
+ pps_get_ts(&ts);
uport->icount.dcd++;
#ifdef CONFIG_HARD_PPS
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 526d66f..763e061 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -101,7 +101,7 @@
* any pending driver I/O is completed.
*
* void (*dcd_change)(struct tty_struct *tty, unsigned int status,
- * struct timespec *ts)
+ * struct pps_event_time *ts)
*
* Tells the discipline that the DCD pin has changed its status and
* the relative timestamp. Pointer ts can be NULL.
@@ -109,6 +109,7 @@
#include <linux/fs.h>
#include <linux/wait.h>
+#include <linux/pps_kernel.h>
struct tty_ldisc_ops {
int magic;
@@ -143,7 +144,7 @@ struct tty_ldisc_ops {
char *fp, int count);
void (*write_wakeup)(struct tty_struct *);
void (*dcd_change)(struct tty_struct *, unsigned int,
- struct timespec *);
+ struct pps_event_time *);
struct module *owner;
--
1.7.2.3
Add parallel port PPS client. It uses a standard method for capturing
timestamps for assert edge transitions: getting a timestamp soon after
an interrupt has happened. This is not a very precise source of time
information due to interrupt handling delays. However, timestamps for
clear edge transitions are much more precise because the interrupt
handler continuously polls hardware port until the transition is done.
Hardware port operations require only about 1us so the maximum error
should not exceed this value. This was my primary goal when developing
this client.
Clear edge capture could be disabled using clear_wait parameter.
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/clients/Kconfig | 7 +
drivers/pps/clients/Makefile | 1 +
drivers/pps/clients/pps_parport.c | 247 +++++++++++++++++++++++++++++++++++++
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 drivers/pps/clients/pps_parport.c
diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
index 4e801bd..8520a7f 100644
--- a/drivers/pps/clients/Kconfig
+++ b/drivers/pps/clients/Kconfig
@@ -22,4 +22,11 @@ config PPS_CLIENT_LDISC
If you say yes here you get support for a PPS source connected
with the CD (Carrier Detect) pin of your serial port.
+config PPS_CLIENT_PARPORT
+ tristate "Parallel port PPS client"
+ depends on PPS && PARPORT
+ help
+ If you say yes here you get support for a PPS source connected
+ with the interrupt pin of your parallel port.
+
endif
diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
index 812c9b1..42517da 100644
--- a/drivers/pps/clients/Makefile
+++ b/drivers/pps/clients/Makefile
@@ -4,6 +4,7 @@
obj-$(CONFIG_PPS_CLIENT_KTIMER) += pps-ktimer.o
obj-$(CONFIG_PPS_CLIENT_LDISC) += pps-ldisc.o
+obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
ifeq ($(CONFIG_PPS_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
new file mode 100644
index 0000000..501d440
--- /dev/null
+++ b/drivers/pps/clients/pps_parport.c
@@ -0,0 +1,247 @@
+/*
+ * pps_parport.c -- kernel parallel port PPS client
+ *
+ *
+ * Copyright (C) 2009 Alexander Gordeev <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+/*
+ * TODO:
+ * implement echo over SEL pin
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/irqnr.h>
+#include <linux/time.h>
+#include <linux/parport.h>
+#include <linux/pps_kernel.h>
+
+#define DRVDESC "parallel port PPS client"
+
+/* module parameters */
+
+#define CLEAR_WAIT_MAX 100
+#define CLEAR_WAIT_MAX_ERRORS 5
+
+static unsigned int clear_wait = 100;
+MODULE_PARM_DESC(clear_wait,
+ "Maximum number of port reads when polling for signal clear,"
+ " zero turns clear edge capture off entirely");
+module_param(clear_wait, uint, 0);
+
+
+/* internal per port structure */
+struct pps_client_pp {
+ struct pardevice *pardev; /* parport device */
+ struct pps_device *pps; /* PPS device */
+ unsigned int cw; /* port clear timeout */
+ unsigned int cw_err; /* number of timeouts */
+};
+
+static inline int signal_is_set(struct parport *port)
+{
+ return (port->ops->read_status(port) & PARPORT_STATUS_ACK) != 0;
+}
+
+/* parport interrupt handler */
+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;
+ unsigned int i;
+ unsigned long flags;
+
+ /* first of all we get the time stamp... */
+ pps_get_ts(&ts_assert);
+
+ if (dev->cw == 0)
+ /* clear edge capture disabled */
+ goto out_assert;
+
+ /* try capture the clear edge */
+ local_irq_save(flags);
+ /* 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");
+ goto out_assert;
+ }
+
+ /* poll the port until the signal is unset */
+ for (i = dev->cw; i; i--)
+ if (!signal_is_set(port)) {
+ pps_get_ts(&ts_clear);
+ local_irq_restore(flags);
+ dev->cw_err = 0;
+ goto out_both;
+ }
+ local_irq_restore(flags);
+
+ /* timeout */
+ dev->cw_err++;
+ if (dev->cw_err >= CLEAR_WAIT_MAX_ERRORS) {
+ dev_err(dev->pps->dev, "disabled clear edge capture after %d"
+ " timeouts\n", dev->cw_err);
+ dev->cw = 0;
+ dev->cw_err = 0;
+ }
+
+out_assert:
+ /* fire assert event */
+ pps_event(dev->pps, &ts_assert,
+ PPS_CAPTUREASSERT, NULL);
+ return;
+
+out_both:
+ /* fire assert event */
+ pps_event(dev->pps, &ts_assert,
+ PPS_CAPTUREASSERT, NULL);
+ /* fire clear event */
+ pps_event(dev->pps, &ts_clear,
+ PPS_CAPTURECLEAR, NULL);
+ return;
+}
+
+/* the PPS echo function */
+static void pps_echo(struct pps_device *pps, int event, void *data)
+{
+ dev_info(pps->dev, "echo %s %s\n",
+ event & PPS_CAPTUREASSERT ? "assert" : "",
+ event & PPS_CAPTURECLEAR ? "clear" : "");
+}
+
+static void parport_attach(struct parport *port)
+{
+ struct pps_client_pp *device;
+ struct pps_source_info info = {
+ .name = KBUILD_MODNAME,
+ .path = "",
+ .mode = PPS_CAPTUREBOTH | \
+ PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
+ PPS_ECHOASSERT | PPS_ECHOCLEAR | \
+ PPS_CANWAIT | PPS_TSFMT_TSPEC,
+ .echo = pps_echo,
+ .owner = THIS_MODULE,
+ .dev = NULL
+ };
+
+ device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
+ if (!device) {
+ pr_err("memory allocation failed, not attaching\n");
+ return;
+ }
+
+ device->pardev = parport_register_device(port, KBUILD_MODNAME,
+ NULL, NULL, parport_irq, 0, device);
+ if (!device->pardev) {
+ pr_err("couldn't register with %s\n", port->name);
+ goto err_free;
+ }
+
+ if (parport_claim_or_block(device->pardev) < 0) {
+ pr_err("couldn't claim %s\n", port->name);
+ goto err_unregister_dev;
+ }
+
+ device->pps = pps_register_source(&info,
+ PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
+ if (device->pps == NULL) {
+ pr_err("couldn't register PPS source\n");
+ goto err_release_dev;
+ }
+
+ device->cw = clear_wait;
+
+ port->ops->enable_irq(port);
+
+ pr_info("attached to %s\n", port->name);
+
+ return;
+
+err_release_dev:
+ parport_release(device->pardev);
+err_unregister_dev:
+ parport_unregister_device(device->pardev);
+err_free:
+ kfree(device);
+}
+
+static void parport_detach(struct parport *port)
+{
+ struct pardevice *pardev = port->cad;
+ struct pps_client_pp *device;
+
+ /* FIXME: oooh, this is ugly! */
+ if (strcmp(pardev->name, KBUILD_MODNAME))
+ /* not our port */
+ return;
+
+ device = pardev->private;
+
+ port->ops->disable_irq(port);
+ pps_unregister_source(device->pps);
+ parport_release(pardev);
+ parport_unregister_device(pardev);
+ kfree(device);
+}
+
+static struct parport_driver pps_parport_driver = {
+ .name = KBUILD_MODNAME,
+ .attach = parport_attach,
+ .detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_parport_init(void)
+{
+ int ret;
+
+ pr_info(DRVDESC "\n");
+
+ if (clear_wait > CLEAR_WAIT_MAX) {
+ pr_err("clear_wait value should be not greater"
+ " then %d\n", CLEAR_WAIT_MAX);
+ return -EINVAL;
+ }
+
+ ret = parport_register_driver(&pps_parport_driver);
+ if (ret) {
+ pr_err("unable to register with parport\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit pps_parport_exit(void)
+{
+ parport_unregister_driver(&pps_parport_driver);
+}
+
+module_init(pps_parport_init);
+module_exit(pps_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <[email protected]>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
--
1.7.2.3
This way less overhead is involved when running production kernel.
If you want to debug a pps client module please define DEBUG to enable
the checks.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index c42d3cb..3e8eb3f 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -166,10 +166,8 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
int captured = 0;
struct pps_ktime ts_real;
- if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
- dev_err(pps->dev, "unknown event (%x)\n", event);
- return;
- }
+ /* check event type */
+ BUG_ON((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0);
dev_dbg(pps->dev, "PPS event at %ld.%09ld\n",
ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
--
1.7.2.3
Move variable declarations where they are used in pps_cdev_ioctl.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/pps.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index ca5183b..c76afb9 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -61,8 +61,6 @@ static long pps_cdev_ioctl(struct file *file,
{
struct pps_device *pps = file->private_data;
struct pps_kparams params;
- struct pps_fdata fdata;
- unsigned long ticks;
void __user *uarg = (void __user *) arg;
int __user *iuarg = (int __user *) arg;
int err;
@@ -136,7 +134,9 @@ static long pps_cdev_ioctl(struct file *file,
break;
- case PPS_FETCH:
+ case PPS_FETCH: {
+ struct pps_fdata fdata;
+
pr_debug("PPS_FETCH: source %d\n", pps->id);
err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
@@ -149,6 +149,8 @@ static long pps_cdev_ioctl(struct file *file,
if (fdata.timeout.flags & PPS_TIME_INVALID)
err = wait_event_interruptible(pps->queue, pps->go);
else {
+ unsigned long ticks;
+
pr_debug("timeout %lld.%09d\n",
(long long) fdata.timeout.sec,
fdata.timeout.nsec);
@@ -185,7 +187,7 @@ static long pps_cdev_ioctl(struct file *file,
return -EFAULT;
break;
-
+ }
default:
return -ENOTTY;
break;
--
1.7.2.3
Using device index as a pointer needs some unnecessary work to be done
every time the pointer is needed (in irq handler for example).
Using a direct pointer is much more easy (and safe as well).
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/clients/pps-ktimer.c | 30 ++++-----
drivers/pps/clients/pps-ldisc.c | 36 ++++++-----
drivers/pps/kapi.c | 124 +++++++++-----------------------------
drivers/pps/pps.c | 22 ++-----
include/linux/pps_kernel.h | 23 +++----
5 files changed, 75 insertions(+), 160 deletions(-)
diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index e1bdd8b..966ead1 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -31,7 +31,7 @@
* Global variables
*/
-static int source;
+static struct pps_device *pps;
static struct timer_list ktimer;
/*
@@ -47,7 +47,7 @@ static void pps_ktimer_event(unsigned long ptr)
pr_info("PPS event at %lu\n", jiffies);
- pps_event(source, &ts, PPS_CAPTUREASSERT, NULL);
+ pps_event(pps, &ts, PPS_CAPTUREASSERT, NULL);
mod_timer(&ktimer, jiffies + HZ);
}
@@ -56,12 +56,11 @@ static void pps_ktimer_event(unsigned long ptr)
* The echo function
*/
-static void pps_ktimer_echo(int source, int event, void *data)
+static void pps_ktimer_echo(struct pps_device *pps, int event, void *data)
{
- pr_info("echo %s %s for source %d\n",
+ dev_info(pps->dev, "echo %s %s\n",
event & PPS_CAPTUREASSERT ? "assert" : "",
- event & PPS_CAPTURECLEAR ? "clear" : "",
- source);
+ event & PPS_CAPTURECLEAR ? "clear" : "");
}
/*
@@ -84,30 +83,27 @@ static struct pps_source_info pps_ktimer_info = {
static void __exit pps_ktimer_exit(void)
{
- del_timer_sync(&ktimer);
- pps_unregister_source(source);
+ dev_info(pps->dev, "ktimer PPS source unregistered\n");
- pr_info("ktimer PPS source unregistered\n");
+ del_timer_sync(&ktimer);
+ pps_unregister_source(pps);
}
static int __init pps_ktimer_init(void)
{
- int ret;
-
- ret = pps_register_source(&pps_ktimer_info,
+ pps = pps_register_source(&pps_ktimer_info,
PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
- if (ret < 0) {
+ if (pps == NULL) {
printk(KERN_ERR "cannot register ktimer source\n");
- return ret;
+ return -ENOMEM;
}
- source = ret;
setup_timer(&ktimer, pps_ktimer_event, 0);
mod_timer(&ktimer, jiffies + HZ);
- pr_info("ktimer PPS source registered at %d\n", source);
+ dev_info(pps->dev, "ktimer PPS source registered\n");
- return 0;
+ return 0;
}
module_init(pps_ktimer_init);
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 20fc9f7..f0ba65e 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -29,7 +29,7 @@
static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
struct pps_event_time *ts)
{
- int id = (long)tty->disc_data;
+ struct pps_device *pps = (struct pps_device *)tty->disc_data;
struct pps_event_time __ts;
/* First of all we get the time stamp... */
@@ -39,12 +39,14 @@ static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
if (!ts) /* No. Do it ourself! */
ts = &__ts;
+ BUG_ON(pps == NULL);
+
/* Now do the PPS event report */
- pps_event(id, ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
- NULL);
+ pps_event(pps, ts, status ? PPS_CAPTUREASSERT :
+ PPS_CAPTURECLEAR, NULL);
- pr_debug("PPS %s at %lu on source #%d\n",
- status ? "assert" : "clear", jiffies, id);
+ dev_dbg(pps->dev, "PPS %s at %lu\n",
+ status ? "assert" : "clear", jiffies);
}
static int (*alias_n_tty_open)(struct tty_struct *tty);
@@ -54,7 +56,7 @@ static int pps_tty_open(struct tty_struct *tty)
struct pps_source_info info;
struct tty_driver *drv = tty->driver;
int index = tty->index + drv->name_base;
- int ret;
+ struct pps_device *pps;
info.owner = THIS_MODULE;
info.dev = NULL;
@@ -64,20 +66,19 @@ static int pps_tty_open(struct tty_struct *tty)
PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
PPS_CANWAIT | PPS_TSFMT_TSPEC;
- ret = pps_register_source(&info, PPS_CAPTUREBOTH | \
+ pps = pps_register_source(&info, PPS_CAPTUREBOTH | \
PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
- if (ret < 0) {
+ if (pps == NULL) {
pr_err("cannot register PPS source \"%s\"\n", info.path);
- return ret;
+ return -ENOMEM;
}
- tty->disc_data = (void *)(long)ret;
+ tty->disc_data = pps;
/* Should open N_TTY ldisc too */
- ret = alias_n_tty_open(tty);
- if (ret < 0)
- pps_unregister_source((long)tty->disc_data);
+ if (alias_n_tty_open(tty) < 0)
+ pps_unregister_source(pps);
- pr_info("PPS source #%d \"%s\" added\n", ret, info.path);
+ dev_info(pps->dev, "source \"%s\" added\n", info.path);
return 0;
}
@@ -86,12 +87,13 @@ static void (*alias_n_tty_close)(struct tty_struct *tty);
static void pps_tty_close(struct tty_struct *tty)
{
- int id = (long)tty->disc_data;
+ struct pps_device *pps = (struct pps_device *)tty->disc_data;
- pps_unregister_source(id);
alias_n_tty_close(tty);
- pr_info("PPS source #%d removed\n", id);
+ tty->disc_data = NULL;
+ dev_info(pps->dev, "removed\n");
+ pps_unregister_source(pps);
}
static struct tty_ldisc_ops pps_ldisc_ops;
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index b431d83..98d4012 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -32,11 +32,11 @@
#include <linux/slab.h>
/*
- * Global variables
+ * Local variables
*/
-DEFINE_SPINLOCK(pps_idr_lock);
-DEFINE_IDR(pps_idr);
+static DEFINE_SPINLOCK(pps_idr_lock);
+static DEFINE_IDR(pps_idr);
/*
* Local functions
@@ -60,60 +60,6 @@ static void pps_add_offset(struct pps_ktime *ts, struct pps_ktime *offset)
* Exported functions
*/
-/* pps_get_source - find a PPS source
- * @source: the PPS source ID.
- *
- * This function is used to find an already registered PPS source into the
- * system.
- *
- * The function returns NULL if found nothing, otherwise it returns a pointer
- * to the PPS source data struct (the refcounter is incremented by 1).
- */
-
-struct pps_device *pps_get_source(int source)
-{
- struct pps_device *pps;
- unsigned long flags;
-
- spin_lock_irqsave(&pps_idr_lock, flags);
-
- pps = idr_find(&pps_idr, source);
- if (pps != NULL)
- atomic_inc(&pps->usage);
-
- spin_unlock_irqrestore(&pps_idr_lock, flags);
-
- return pps;
-}
-
-/* pps_put_source - free the PPS source data
- * @pps: a pointer to the PPS source.
- *
- * This function is used to free a PPS data struct if its refcount is 0.
- */
-
-void pps_put_source(struct pps_device *pps)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&pps_idr_lock, flags);
- BUG_ON(atomic_read(&pps->usage) == 0);
-
- if (!atomic_dec_and_test(&pps->usage)) {
- pps = NULL;
- goto exit;
- }
-
- /* No more reference to the PPS source. We can safely remove the
- * PPS data struct.
- */
- idr_remove(&pps_idr, pps->id);
-
-exit:
- spin_unlock_irqrestore(&pps_idr_lock, flags);
- kfree(pps);
-}
-
/* pps_register_source - add a PPS source in the system
* @info: the PPS info struct
* @default_params: the default PPS parameters of the new source
@@ -122,10 +68,11 @@ exit:
* source is described by info's fields and it will have, as default PPS
* parameters, the ones specified into default_params.
*
- * The function returns, in case of success, the PPS source ID.
+ * The function returns, in case of success, the PPS device. Otherwise NULL.
*/
-int pps_register_source(struct pps_source_info *info, int default_params)
+struct pps_device *pps_register_source(struct pps_source_info *info,
+ int default_params)
{
struct pps_device *pps;
int id;
@@ -168,7 +115,6 @@ int pps_register_source(struct pps_source_info *info, int default_params)
init_waitqueue_head(&pps->queue);
spin_lock_init(&pps->lock);
- atomic_set(&pps->usage, 1);
/* Get new ID for the new PPS source */
if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
@@ -211,7 +157,7 @@ int pps_register_source(struct pps_source_info *info, int default_params)
pr_info("new PPS source %s at ID %d\n", info->name, id);
- return id;
+ return pps;
free_idr:
spin_lock_irq(&pps_idr_lock);
@@ -224,38 +170,33 @@ kfree_pps:
pps_register_source_exit:
printk(KERN_ERR "pps: %s: unable to register source\n", info->name);
- return err;
+ return NULL;
}
EXPORT_SYMBOL(pps_register_source);
/* pps_unregister_source - remove a PPS source from the system
- * @source: the PPS source ID
+ * @pps: the PPS source
*
* This function is used to remove a previously registered PPS source from
* the system.
*/
-void pps_unregister_source(int source)
+void pps_unregister_source(struct pps_device *pps)
{
- struct pps_device *pps;
+ unsigned int id = pps->id;
- spin_lock_irq(&pps_idr_lock);
- pps = idr_find(&pps_idr, source);
+ pps_unregister_cdev(pps);
- if (!pps) {
- BUG();
- spin_unlock_irq(&pps_idr_lock);
- return;
- }
+ spin_lock_irq(&pps_idr_lock);
+ idr_remove(&pps_idr, pps->id);
spin_unlock_irq(&pps_idr_lock);
- pps_unregister_cdev(pps);
- pps_put_source(pps);
+ kfree(pps);
}
EXPORT_SYMBOL(pps_unregister_source);
/* pps_event - register a PPS event into the system
- * @source: the PPS source ID
+ * @pps: the PPS device
* @ts: the event timestamp
* @event: the event type
* @data: userdef pointer
@@ -263,30 +204,24 @@ EXPORT_SYMBOL(pps_unregister_source);
* This function is used by each PPS client in order to register a new
* PPS event into the system (it's usually called inside an IRQ handler).
*
- * If an echo function is associated with the PPS source it will be called
+ * If an echo function is associated with the PPS device it will be called
* as:
- * pps->info.echo(source, event, data);
+ * pps->info.echo(pps, event, data);
*/
-
-void pps_event(int source, struct pps_event_time *ts, int event, void *data)
+void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
+ void *data)
{
- struct pps_device *pps;
unsigned long flags;
int captured = 0;
struct pps_ktime ts_real;
if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
- printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
- event, source);
+ dev_err(pps->dev, "unknown event (%x)\n", event);
return;
}
- pps = pps_get_source(source);
- if (!pps)
- return;
-
- pr_debug("PPS event on source %d at %ld.%09ld\n",
- pps->id, ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
+ dev_dbg(pps->dev, "PPS event at %ld.%09ld\n",
+ ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
timespec_to_pps_ktime(&ts_real, ts->ts_real);
@@ -294,7 +229,7 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
/* Must call the echo function? */
if ((pps->params.mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR)))
- pps->info.echo(source, event, data);
+ pps->info.echo(pps, event, data);
/* Check the event */
pps->current_mode = pps->params.mode;
@@ -308,8 +243,8 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
/* Save the time stamp */
pps->assert_tu = ts_real;
pps->assert_sequence++;
- pr_debug("capture assert seq #%u for source %d\n",
- pps->assert_sequence, source);
+ dev_dbg(pps->dev, "capture assert seq #%u\n",
+ pps->assert_sequence);
captured = ~0;
}
@@ -323,8 +258,8 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
/* Save the time stamp */
pps->clear_tu = ts_real;
pps->clear_sequence++;
- pr_debug("capture clear seq #%u for source %d\n",
- pps->clear_sequence, source);
+ dev_dbg(pps->dev, "capture clear seq #%u\n",
+ pps->clear_sequence);
captured = ~0;
}
@@ -338,8 +273,5 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
}
spin_unlock_irqrestore(&pps->lock, flags);
-
- /* Now we can release the PPS source for (possible) deregistration */
- pps_put_source(pps);
}
EXPORT_SYMBOL(pps_event);
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index dc7e66c..1922f27 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -204,12 +204,6 @@ static int pps_cdev_open(struct inode *inode, struct file *file)
{
struct pps_device *pps = container_of(inode->i_cdev,
struct pps_device, cdev);
- int found;
-
- found = pps_get_source(pps->id) != 0;
- if (!found)
- return -ENODEV;
-
file->private_data = pps;
return 0;
@@ -217,11 +211,6 @@ static int pps_cdev_open(struct inode *inode, struct file *file)
static int pps_cdev_release(struct inode *inode, struct file *file)
{
- struct pps_device *pps = file->private_data;
-
- /* Free the PPS source and wake up (possible) deregistration */
- pps_put_source(pps);
-
return 0;
}
@@ -242,22 +231,23 @@ static const struct file_operations pps_cdev_fops = {
int pps_register_cdev(struct pps_device *pps)
{
int err;
+ dev_t devt;
+
+ devt = MKDEV(MAJOR(pps_devt), pps->id);
- pps->devno = MKDEV(MAJOR(pps_devt), pps->id);
cdev_init(&pps->cdev, &pps_cdev_fops);
pps->cdev.owner = pps->info.owner;
- err = cdev_add(&pps->cdev, pps->devno, 1);
+ err = cdev_add(&pps->cdev, devt, 1);
if (err) {
printk(KERN_ERR "pps: %s: failed to add char device %d:%d\n",
pps->info.name, MAJOR(pps_devt), pps->id);
return err;
}
- pps->dev = device_create(pps_class, pps->info.dev, pps->devno, NULL,
+ pps->dev = device_create(pps_class, pps->info.dev, devt, pps,
"pps%d", pps->id);
if (IS_ERR(pps->dev))
goto del_cdev;
- dev_set_drvdata(pps->dev, pps);
pr_debug("source %s got cdev (%d:%d)\n", pps->info.name,
MAJOR(pps_devt), pps->id);
@@ -272,7 +262,7 @@ del_cdev:
void pps_unregister_cdev(struct pps_device *pps)
{
- device_destroy(pps_class, pps->devno);
+ device_destroy(pps_class, pps->dev->devt);
cdev_del(&pps->cdev);
}
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 32aa676..1aedf50 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -31,13 +31,16 @@
* Global defines
*/
+struct pps_device;
+
/* The specific PPS source info */
struct pps_source_info {
char name[PPS_MAX_NAME_LEN]; /* simbolic name */
char path[PPS_MAX_NAME_LEN]; /* path of connected device */
int mode; /* PPS's allowed mode */
- void (*echo)(int source, int event, void *data); /* PPS echo function */
+ void (*echo)(struct pps_device *pps,
+ int event, void *data); /* PPS echo function */
struct module *owner;
struct device *dev;
@@ -65,35 +68,27 @@ struct pps_device {
unsigned int id; /* PPS source unique ID */
struct cdev cdev;
struct device *dev;
- int devno;
struct fasync_struct *async_queue; /* fasync method */
spinlock_t lock;
-
- atomic_t usage; /* usage count */
};
/*
* Global variables
*/
-extern spinlock_t pps_idr_lock;
-extern struct idr pps_idr;
-
extern struct device_attribute pps_attrs[];
/*
* Exported functions
*/
-struct pps_device *pps_get_source(int source);
-extern void pps_put_source(struct pps_device *pps);
-extern int pps_register_source(struct pps_source_info *info,
- int default_params);
-extern void pps_unregister_source(int source);
+extern struct pps_device *pps_register_source(
+ struct pps_source_info *info, int default_params);
+extern void pps_unregister_source(struct pps_device *pps);
extern int pps_register_cdev(struct pps_device *pps);
extern void pps_unregister_cdev(struct pps_device *pps);
-extern void pps_event(int source, struct pps_event_time *ts, int event,
- void *data);
+extern void pps_event(struct pps_device *pps,
+ struct pps_event_time *ts, int event, void *data);
static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
struct timespec ts)
--
1.7.2.3
There was a race in PPS_FETCH ioctl handler when several processes want
to obtain PPS data simultaneously using sleeping PPS_FETCH. They all
sleep most of the time in the system call.
With the old approach when the first process waiting on the pps queue
is waken up it makes new system call right away and zeroes pps->go. So
other processes continue to sleep. This is a clear race condition
because of the global 'go' variable.
With the new approach pps->last_ev holds some value increasing at each
PPS event. PPS_FETCH ioctl handler saves current value to the local
variable at the very beginning so it can safely check that there is a
new event by just comparing both variables.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 4 ++--
drivers/pps/pps.c | 10 +++++++---
include/linux/pps_kernel.h | 2 +-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 55f3961..3f89f5eb 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -326,8 +326,8 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
/* Wake up if captured something */
if (captured) {
- pps->go = ~0;
- wake_up_interruptible(&pps->queue);
+ pps->last_ev++;
+ wake_up_interruptible_all(&pps->queue);
kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
}
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index c76afb9..dc7e66c 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -136,6 +136,7 @@ static long pps_cdev_ioctl(struct file *file,
case PPS_FETCH: {
struct pps_fdata fdata;
+ unsigned int ev;
pr_debug("PPS_FETCH: source %d\n", pps->id);
@@ -143,11 +144,12 @@ static long pps_cdev_ioctl(struct file *file,
if (err)
return -EFAULT;
- pps->go = 0;
+ ev = pps->last_ev;
/* Manage the timeout */
if (fdata.timeout.flags & PPS_TIME_INVALID)
- err = wait_event_interruptible(pps->queue, pps->go);
+ err = wait_event_interruptible(pps->queue,
+ ev != pps->last_ev);
else {
unsigned long ticks;
@@ -159,7 +161,9 @@ static long pps_cdev_ioctl(struct file *file,
if (ticks != 0) {
err = wait_event_interruptible_timeout(
- pps->queue, pps->go, ticks);
+ pps->queue,
+ ev != pps->last_ev,
+ ticks);
if (err == 0)
return -ETIMEDOUT;
}
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index c930d11..c3aed4b 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -55,7 +55,7 @@ struct pps_device {
struct pps_ktime clear_tu;
int current_mode; /* PPS mode at event time */
- int go; /* PPS event is arrived? */
+ unsigned int last_ev; /* last PPS event id */
wait_queue_head_t queue; /* PPS event queue */
unsigned int id; /* PPS source unique ID */
--
1.7.2.3
Since we now have direct pointers to struct pps_device everywhere it's
easy to use dev_* functions to print messages instead of plain printks.
Where dev_* cannot be used printks are converted to pr_*.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/clients/pps-ktimer.c | 5 +++--
drivers/pps/clients/pps-ldisc.c | 2 ++
drivers/pps/kapi.c | 15 ++++++++-------
drivers/pps/pps.c | 25 +++++++++++++------------
4 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index 966ead1..2728469 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -19,6 +19,7 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
#include <linux/module.h>
@@ -45,7 +46,7 @@ static void pps_ktimer_event(unsigned long ptr)
/* First of all we get the time stamp... */
pps_get_ts(&ts);
- pr_info("PPS event at %lu\n", jiffies);
+ dev_info(pps->dev, "PPS event at %lu\n", jiffies);
pps_event(pps, &ts, PPS_CAPTUREASSERT, NULL);
@@ -94,7 +95,7 @@ static int __init pps_ktimer_init(void)
pps = pps_register_source(&pps_ktimer_info,
PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
if (pps == NULL) {
- printk(KERN_ERR "cannot register ktimer source\n");
+ pr_err("cannot register PPS source\n");
return -ENOMEM;
}
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index f0ba65e..199a83b 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -19,6 +19,8 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/serial_core.h>
#include <linux/tty.h>
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 98d4012..e8847c1 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -19,6 +19,7 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
#include <linux/module.h>
@@ -80,20 +81,20 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
/* Sanity checks */
if ((info->mode & default_params) != default_params) {
- printk(KERN_ERR "pps: %s: unsupported default parameters\n",
+ pr_err("%s: unsupported default parameters\n",
info->name);
err = -EINVAL;
goto pps_register_source_exit;
}
if ((info->mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR)) != 0 &&
info->echo == NULL) {
- printk(KERN_ERR "pps: %s: echo function is not defined\n",
+ pr_err("%s: echo function is not defined\n",
info->name);
err = -EINVAL;
goto pps_register_source_exit;
}
if ((info->mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
- printk(KERN_ERR "pps: %s: unspecified time format\n",
+ pr_err("%s: unspecified time format\n",
info->name);
err = -EINVAL;
goto pps_register_source_exit;
@@ -138,7 +139,7 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
if (id >= PPS_MAX_SOURCES) {
spin_unlock_irq(&pps_idr_lock);
- printk(KERN_ERR "pps: %s: too many PPS sources in the system\n",
+ pr_err("%s: too many PPS sources in the system\n",
info->name);
err = -EBUSY;
goto free_idr;
@@ -150,12 +151,12 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
/* Create the char device */
err = pps_register_cdev(pps);
if (err < 0) {
- printk(KERN_ERR "pps: %s: unable to create char device\n",
+ pr_err("%s: unable to create char device\n",
info->name);
goto free_idr;
}
- pr_info("new PPS source %s at ID %d\n", info->name, id);
+ dev_info(pps->dev, "new PPS source %s\n", info->name);
return pps;
@@ -168,7 +169,7 @@ kfree_pps:
kfree(pps);
pps_register_source_exit:
- printk(KERN_ERR "pps: %s: unable to register source\n", info->name);
+ pr_err("%s: unable to register source\n", info->name);
return NULL;
}
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 1922f27..9f7c2e8 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -19,6 +19,7 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
#include <linux/module.h>
@@ -67,7 +68,7 @@ static long pps_cdev_ioctl(struct file *file,
switch (cmd) {
case PPS_GETPARAMS:
- pr_debug("PPS_GETPARAMS: source %d\n", pps->id);
+ dev_dbg(pps->dev, "PPS_GETPARAMS\n");
spin_lock_irq(&pps->lock);
@@ -83,7 +84,7 @@ static long pps_cdev_ioctl(struct file *file,
break;
case PPS_SETPARAMS:
- pr_debug("PPS_SETPARAMS: source %d\n", pps->id);
+ dev_dbg(pps->dev, "PPS_SETPARAMS\n");
/* Check the capabilities */
if (!capable(CAP_SYS_TIME))
@@ -93,14 +94,14 @@ static long pps_cdev_ioctl(struct file *file,
if (err)
return -EFAULT;
if (!(params.mode & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR))) {
- pr_debug("capture mode unspecified (%x)\n",
+ dev_dbg(pps->dev, "capture mode unspecified (%x)\n",
params.mode);
return -EINVAL;
}
/* Check for supported capabilities */
if ((params.mode & ~pps->info.mode) != 0) {
- pr_debug("unsupported capabilities (%x)\n",
+ dev_dbg(pps->dev, "unsupported capabilities (%x)\n",
params.mode);
return -EINVAL;
}
@@ -113,7 +114,7 @@ static long pps_cdev_ioctl(struct file *file,
/* Restore the read only parameters */
if ((params.mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
/* section 3.3 of RFC 2783 interpreted */
- pr_debug("time format unspecified (%x)\n",
+ dev_dbg(pps->dev, "time format unspecified (%x)\n",
params.mode);
pps->params.mode |= PPS_TSFMT_TSPEC;
}
@@ -126,7 +127,7 @@ static long pps_cdev_ioctl(struct file *file,
break;
case PPS_GETCAP:
- pr_debug("PPS_GETCAP: source %d\n", pps->id);
+ dev_dbg(pps->dev, "PPS_GETCAP\n");
err = put_user(pps->info.mode, iuarg);
if (err)
@@ -138,7 +139,7 @@ static long pps_cdev_ioctl(struct file *file,
struct pps_fdata fdata;
unsigned int ev;
- pr_debug("PPS_FETCH: source %d\n", pps->id);
+ dev_dbg(pps->dev, "PPS_FETCH\n");
err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
if (err)
@@ -153,7 +154,7 @@ static long pps_cdev_ioctl(struct file *file,
else {
unsigned long ticks;
- pr_debug("timeout %lld.%09d\n",
+ dev_dbg(pps->dev, "timeout %lld.%09d\n",
(long long) fdata.timeout.sec,
fdata.timeout.nsec);
ticks = fdata.timeout.sec * HZ;
@@ -171,7 +172,7 @@ static long pps_cdev_ioctl(struct file *file,
/* Check for pending signals */
if (err == -ERESTARTSYS) {
- pr_debug("pending signal caught\n");
+ dev_dbg(pps->dev, "pending signal caught\n");
return -EINTR;
}
@@ -240,7 +241,7 @@ int pps_register_cdev(struct pps_device *pps)
err = cdev_add(&pps->cdev, devt, 1);
if (err) {
- printk(KERN_ERR "pps: %s: failed to add char device %d:%d\n",
+ pr_err("%s: failed to add char device %d:%d\n",
pps->info.name, MAJOR(pps_devt), pps->id);
return err;
}
@@ -282,14 +283,14 @@ static int __init pps_init(void)
pps_class = class_create(THIS_MODULE, "pps");
if (!pps_class) {
- printk(KERN_ERR "pps: failed to allocate class\n");
+ pr_err("failed to allocate class\n");
return -ENOMEM;
}
pps_class->dev_attrs = pps_attrs;
err = alloc_chrdev_region(&pps_devt, 0, PPS_MAX_SOURCES, "pps");
if (err < 0) {
- printk(KERN_ERR "pps: failed to allocate char device region\n");
+ pr_err("failed to allocate char device region\n");
goto remove_class;
}
--
1.7.2.3
Add PPS signal generator which utilizes STROBE pin of a parallel port to
send PPS signals. It uses parport abstraction layer and hrtimers to
precisely control the signal.
Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/pps/pps.txt | 46 +++++
drivers/pps/Kconfig | 2 +
drivers/pps/Makefile | 2 +-
drivers/pps/generators/Kconfig | 17 ++
drivers/pps/generators/Makefile | 9 +
drivers/pps/generators/pps_gen_parport.c | 275 ++++++++++++++++++++++++++++++
6 files changed, 350 insertions(+), 1 deletions(-)
create mode 100644 drivers/pps/generators/Kconfig
create mode 100644 drivers/pps/generators/Makefile
create mode 100644 drivers/pps/generators/pps_gen_parport.c
diff --git a/Documentation/pps/pps.txt b/Documentation/pps/pps.txt
index 125f4ab..d35dcdd 100644
--- a/Documentation/pps/pps.txt
+++ b/Documentation/pps/pps.txt
@@ -170,3 +170,49 @@ and the run ppstest as follow:
Please, note that to compile userland programs you need the file timepps.h
(see Documentation/pps/).
+
+
+Generators
+----------
+
+Sometimes one needs to be able not only to catch PPS signals but to produce
+them also. For example, running a distributed simulation, which requires
+computers' clock to be synchronized very tightly. One way to do this is to
+invent some complicated hardware solutions but it may be neither necessary
+nor affordable. The cheap way is to load a PPS generator on one of the
+computers (master) and PPS clients on others (slaves), and use very simple
+cables to deliver signals using parallel ports, for example.
+
+Parallel port cable pinout:
+pin name master slave
+1 STROBE *------ *
+2 D0 * | *
+3 D1 * | *
+4 D2 * | *
+5 D3 * | *
+6 D4 * | *
+7 D5 * | *
+8 D6 * | *
+9 D7 * | *
+10 ACK * ------*
+11 BUSY * *
+12 PE * *
+13 SEL * *
+14 AUTOFD * *
+15 ERROR * *
+16 INIT * *
+17 SELIN * *
+18-25 GND *-----------*
+
+Please note that parallel port interrupt occurs only on high->low transition,
+so it is used for PPS assert edge. PPS clear edge can be determined only
+using polling in the interrupt handler which actually can be done way more
+precisely because interrupt handling delays can be quite big and random. So
+current parport PPS generator implementation (pps_gen_parport module) is
+geared towards using the clear edge for time synchronization.
+
+Clear edge polling is done with disabled interrupts so it's better to select
+delay between assert and clear edge as small as possible to reduce system
+latencies. But if it is too small slave won't be able to capture clear edge
+transition. The default of 30us should be good enough in most situations.
+The delay can be selected using 'delay' pps_gen_parport module parameter.
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 4823e47..79bda60 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -40,4 +40,6 @@ config NTP_PPS
source drivers/pps/clients/Kconfig
+source drivers/pps/generators/Kconfig
+
endmenu
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index 98960dd..1906eb70 100644
--- a/drivers/pps/Makefile
+++ b/drivers/pps/Makefile
@@ -4,6 +4,6 @@
pps_core-y := pps.o kapi.o sysfs.o
obj-$(CONFIG_PPS) := pps_core.o
-obj-y += clients/
+obj-y += clients/ generators/
ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
new file mode 100644
index 0000000..5fbd614
--- /dev/null
+++ b/drivers/pps/generators/Kconfig
@@ -0,0 +1,17 @@
+#
+# PPS generators configuration
+#
+
+if PPS
+
+comment "PPS generators support"
+
+config PPS_GENERATOR_PARPORT
+ tristate "Parallel port PPS signal generator"
+ depends on PARPORT != n && GENERIC_TIME
+ 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
+ parport abstraction layer and hrtimers to precisely control the signal.
+
+endif
diff --git a/drivers/pps/generators/Makefile b/drivers/pps/generators/Makefile
new file mode 100644
index 0000000..303304a
--- /dev/null
+++ b/drivers/pps/generators/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for PPS generators.
+#
+
+obj-$(CONFIG_PPS_GENERATOR_PARPORT) += pps_gen_parport.o
+
+ifeq ($(CONFIG_PPS_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
diff --git a/drivers/pps/generators/pps_gen_parport.c b/drivers/pps/generators/pps_gen_parport.c
new file mode 100644
index 0000000..aa66803
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_parport.c
@@ -0,0 +1,275 @@
+/*
+ * pps_gen_parport.c -- kernel parallel port PPS signal generator
+ *
+ *
+ * Copyright (C) 2009 Alexander Gordeev <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+/*
+ * TODO:
+ * fix issues when realtime clock is adjusted in a leap
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/time.h>
+#include <linux/hrtimer.h>
+#include <linux/parport.h>
+
+#define DRVDESC "parallel port PPS signal generator"
+
+#define SIGNAL 0
+#define NO_SIGNAL PARPORT_CONTROL_STROBE
+
+/* module parameters */
+
+#define SEND_DELAY_MAX 100000
+
+static unsigned int send_delay = 30000;
+MODULE_PARM_DESC(delay,
+ "Delay between setting and dropping the signal (ns)");
+module_param_named(delay, send_delay, uint, 0);
+
+
+#define SAFETY_INTERVAL 3000 /* set the hrtimer earlier for safety (ns) */
+
+/* internal per port structure */
+struct pps_generator_pp {
+ struct pardevice *pardev; /* parport device */
+ struct hrtimer timer;
+ long port_write_time; /* calibrated port write time (ns) */
+};
+
+static struct pps_generator_pp device = {
+ .pardev = NULL,
+};
+
+static int attached;
+
+/* calibrated time between a hrtimer event and the reaction */
+static long hrtimer_error = SAFETY_INTERVAL;
+
+/* the kernel hrtimer event */
+static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
+{
+ struct timespec expire_time, ts1, ts2, ts3, dts;
+ struct pps_generator_pp *dev;
+ struct parport *port;
+ long lim, delta;
+ unsigned long flags;
+
+ /* NB: approx time with blocked interrupts =
+ send_delay + 3 * SAFETY_INTERVAL */
+ local_irq_save(flags);
+
+ /* first of all we get the time stamp... */
+ getnstimeofday(&ts1);
+ expire_time = ktime_to_timespec(timer->_expires);
+ dev = container_of(timer, struct pps_generator_pp, timer);
+ lim = NSEC_PER_SEC - send_delay - dev->port_write_time;
+
+ /* check if we are late */
+ if (expire_time.tv_sec != ts1.tv_sec || ts1.tv_nsec > lim) {
+ local_irq_restore(flags);
+ pr_err("we are late this time %ld.%09ld\n",
+ ts1.tv_sec, ts1.tv_nsec);
+ goto done;
+ }
+
+ /* busy loop until the time is right for an assert edge */
+ do {
+ getnstimeofday(&ts2);
+ } while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
+
+ /* set the signal */
+ port = dev->pardev->port;
+ port->ops->write_control(port, SIGNAL);
+
+ /* busy loop until the time is right for a clear edge */
+ lim = NSEC_PER_SEC - dev->port_write_time;
+ do {
+ getnstimeofday(&ts2);
+ } while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
+
+ /* unset the signal */
+ port->ops->write_control(port, NO_SIGNAL);
+
+ getnstimeofday(&ts3);
+
+ local_irq_restore(flags);
+
+ /* update calibrated port write time */
+ dts = timespec_sub(ts3, ts2);
+ dev->port_write_time =
+ (dev->port_write_time + timespec_to_ns(&dts)) >> 1;
+
+done:
+ /* update calibrated hrtimer error */
+ dts = timespec_sub(ts1, expire_time);
+ delta = timespec_to_ns(&dts);
+ /* If the new error value is bigger then the old, use the new
+ * value, if not then slowly move towards the new value. This
+ * way it should be safe in bad conditions and efficient in
+ * good conditions.
+ */
+ if (delta >= hrtimer_error)
+ hrtimer_error = delta;
+ else
+ hrtimer_error = (3 * hrtimer_error + delta) >> 2;
+
+ /* update the hrtimer expire time */
+ hrtimer_set_expires(timer,
+ ktime_set(expire_time.tv_sec + 1,
+ NSEC_PER_SEC - (send_delay +
+ dev->port_write_time + SAFETY_INTERVAL +
+ 2 * hrtimer_error)));
+
+ return HRTIMER_RESTART;
+}
+
+/* calibrate port write time */
+#define PORT_NTESTS_SHIFT 5
+static void calibrate_port(struct pps_generator_pp *dev)
+{
+ struct parport *port = dev->pardev->port;
+ int i;
+ long acc = 0;
+
+ for (i = 0; i < (1 << PORT_NTESTS_SHIFT); i++) {
+ struct timespec a, b;
+ unsigned long irq_flags;
+
+ local_irq_save(irq_flags);
+ getnstimeofday(&a);
+ port->ops->write_control(port, NO_SIGNAL);
+ getnstimeofday(&b);
+ local_irq_restore(irq_flags);
+
+ b = timespec_sub(b, a);
+ acc += timespec_to_ns(&b);
+ }
+
+ dev->port_write_time = acc >> PORT_NTESTS_SHIFT;
+ pr_info("port write takes %ldns\n", dev->port_write_time);
+}
+
+static inline ktime_t next_intr_time(struct pps_generator_pp *dev)
+{
+ struct timespec ts;
+
+ getnstimeofday(&ts);
+
+ return ktime_set(ts.tv_sec +
+ ((ts.tv_nsec > 990 * NSEC_PER_MSEC) ? 1 : 0),
+ NSEC_PER_SEC - (send_delay +
+ dev->port_write_time + 3 * SAFETY_INTERVAL));
+}
+
+static void parport_attach(struct parport *port)
+{
+ if (attached) {
+ /* we already have a port */
+ return;
+ }
+
+ device.pardev = parport_register_device(port, KBUILD_MODNAME,
+ NULL, NULL, NULL, 0, &device);
+ if (!device.pardev) {
+ pr_err("couldn't register with %s\n", port->name);
+ return;
+ }
+
+ if (parport_claim_or_block(device.pardev) < 0) {
+ pr_err("couldn't claim %s\n", port->name);
+ goto err_unregister_dev;
+ }
+
+ pr_info("attached to %s\n", port->name);
+ attached = 1;
+
+ calibrate_port(&device);
+
+ hrtimer_init(&device.timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+ device.timer.function = hrtimer_event;
+#ifdef CONFIG_PREEMPT_RT
+ /* hrtimer interrupt will run in the interrupt context with this */
+ device.timer.irqsafe = 1;
+#endif
+
+ hrtimer_start(&device.timer, next_intr_time(&device), HRTIMER_MODE_ABS);
+
+ return;
+
+err_unregister_dev:
+ parport_unregister_device(device.pardev);
+}
+
+static void parport_detach(struct parport *port)
+{
+ if (port->cad != device.pardev)
+ return; /* not our port */
+
+ hrtimer_cancel(&device.timer);
+ parport_release(device.pardev);
+ parport_unregister_device(device.pardev);
+}
+
+static struct parport_driver pps_gen_parport_driver = {
+ .name = KBUILD_MODNAME,
+ .attach = parport_attach,
+ .detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_gen_parport_init(void)
+{
+ int ret;
+
+ pr_info(DRVDESC "\n");
+
+ if (send_delay > SEND_DELAY_MAX) {
+ pr_err("delay value should be not greater"
+ " then %d\n", SEND_DELAY_MAX);
+ return -EINVAL;
+ }
+
+ ret = parport_register_driver(&pps_gen_parport_driver);
+ if (ret) {
+ pr_err("unable to register with parport\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit pps_gen_parport_exit(void)
+{
+ parport_unregister_driver(&pps_gen_parport_driver);
+ pr_info("hrtimer avg error is %ldns\n", hrtimer_error);
+}
+
+module_init(pps_gen_parport_init);
+module_exit(pps_gen_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <[email protected]>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
--
1.7.2.3
Bitwise conjunction is distributive so we can simplify some conditions.
Acked-by: Rodolfo Giometti <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 3e8eb3f..ebf3374 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -182,8 +182,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
/* Check the event */
pps->current_mode = pps->params.mode;
- if ((event & PPS_CAPTUREASSERT) &
- (pps->params.mode & PPS_CAPTUREASSERT)) {
+ if (event & pps->params.mode & PPS_CAPTUREASSERT) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETASSERT)
pps_add_offset(&ts_real,
@@ -197,8 +196,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
captured = ~0;
}
- if ((event & PPS_CAPTURECLEAR) &
- (pps->params.mode & PPS_CAPTURECLEAR)) {
+ if (event & pps->params.mode & PPS_CAPTURECLEAR) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETCLEAR)
pps_add_offset(&ts_real,
--
1.7.2.3
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.
Acked-by: John Stultz <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/Kconfig | 7 +
include/linux/timex.h | 1 +
kernel/time/ntp.c | 425 +++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 418 insertions(+), 15 deletions(-)
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 1afe4e0..33c9126 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -30,6 +30,13 @@ config PPS_DEBUG
messages to the system log. Select this if you are having a
problem with PPS support and want to see more of what is going on.
+config NTP_PPS
+ bool "PPS kernel consumer support"
+ depends on PPS
+ help
+ This option adds support for direct in-kernel time
+ syncronization using an external PPS signal.
+
source drivers/pps/clients/Kconfig
endmenu
diff --git a/include/linux/timex.h b/include/linux/timex.h
index 32d852f..d23999f 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -268,6 +268,7 @@ extern u64 tick_length;
extern void second_overflow(void);
extern void update_ntp_one_tick(void);
extern int do_adjtimex(struct timex *);
+extern void hardpps(const struct timespec *, const struct timespec *);
int read_current_timer(unsigned long *timer_val);
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index c631168..d721ba7 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -14,6 +14,7 @@
#include <linux/timex.h>
#include <linux/time.h>
#include <linux/mm.h>
+#include <linux/module.h>
/*
* NTP timekeeping variables:
@@ -74,6 +75,162 @@ static long time_adjust;
/* constant (boot-param configurable) NTP tick adjustment (upscaled) */
static s64 ntp_tick_adj;
+#ifdef CONFIG_NTP_PPS
+
+/*
+ * The following variables are used when a pulse-per-second (PPS) signal
+ * is available. They establish the engineering parameters of the clock
+ * discipline loop when controlled by the PPS signal.
+ */
+#define PPS_VALID 10 /* PPS signal watchdog max (s) */
+#define PPS_POPCORN 4 /* popcorn spike threshold (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
+ increase pps_shift or consecutive bad
+ intervals to decrease it */
+#define PPS_MAXWANDER 100000 /* max PPS freq wander (ns/s) */
+
+static int pps_valid; /* signal watchdog counter */
+static long pps_tf[3]; /* phase median filter */
+static long pps_jitter; /* current jitter (ns) */
+static struct timespec pps_fbase; /* beginning of the last freq interval */
+static int pps_shift; /* current interval duration (s) (shift) */
+static int pps_intcnt; /* interval counter */
+static s64 pps_freq; /* frequency offset (scaled ns/s) */
+static long pps_stabil; /* current stability (scaled ns/s) */
+
+/*
+ * PPS signal quality monitors
+ */
+static long pps_calcnt; /* calibration intervals */
+static long pps_jitcnt; /* jitter limit exceeded */
+static long pps_stbcnt; /* stability limit exceeded */
+static long pps_errcnt; /* calibration errors */
+
+
+/* PPS kernel consumer compensates the whole phase error immediately.
+ * Otherwise, reduce the offset by a fixed factor times the time constant.
+ */
+static inline s64 ntp_offset_chunk(s64 offset)
+{
+ if (time_status & STA_PPSTIME && time_status & STA_PPSSIGNAL)
+ return offset;
+ else
+ return shift_right(offset, SHIFT_PLL + time_constant);
+}
+
+static inline void pps_reset_freq_interval(void)
+{
+ /* the PPS calibration interval may end
+ surprisingly early */
+ pps_shift = PPS_INTMIN;
+ pps_intcnt = 0;
+}
+
+/**
+ * pps_clear - Clears the PPS state variables
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_clear(void)
+{
+ pps_reset_freq_interval();
+ pps_tf[0] = 0;
+ pps_tf[1] = 0;
+ pps_tf[2] = 0;
+ pps_fbase.tv_sec = pps_fbase.tv_nsec = 0;
+ pps_freq = 0;
+}
+
+/* Decrease pps_valid to indicate that another second has passed since
+ * the last PPS signal. When it reaches 0, indicate that PPS signal is
+ * missing.
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_dec_valid(void)
+{
+ if (pps_valid > 0)
+ pps_valid--;
+ else {
+ time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
+ STA_PPSWANDER | STA_PPSERROR);
+ pps_clear();
+ }
+}
+
+static inline void pps_set_freq(s64 freq)
+{
+ pps_freq = freq;
+}
+
+static inline int is_error_status(int status)
+{
+ return (time_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))
+ /* PPS jitter exceeded when
+ * PPS time synchronization requested */
+ || ((time_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)));
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+ txc->ppsfreq = shift_right((pps_freq >> PPM_SCALE_INV_SHIFT) *
+ PPM_SCALE_INV, NTP_SCALE_SHIFT);
+ txc->jitter = pps_jitter;
+ if (!(time_status & STA_NANO))
+ txc->jitter /= NSEC_PER_USEC;
+ txc->shift = pps_shift;
+ txc->stabil = pps_stabil;
+ txc->jitcnt = pps_jitcnt;
+ txc->calcnt = pps_calcnt;
+ txc->errcnt = pps_errcnt;
+ txc->stbcnt = pps_stbcnt;
+}
+
+#else /* !CONFIG_NTP_PPS */
+
+static inline s64 ntp_offset_chunk(s64 offset)
+{
+ return shift_right(offset, SHIFT_PLL + time_constant);
+}
+
+static inline void pps_reset_freq_interval(void) {}
+static inline void pps_clear(void) {}
+static inline void pps_dec_valid(void) {}
+static inline void pps_set_freq(s64 freq) {}
+
+static inline int is_error_status(int status)
+{
+ return status & (STA_UNSYNC|STA_CLOCKERR);
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+ /* PPS is not implemented, so these are zero */
+ txc->ppsfreq = 0;
+ txc->jitter = 0;
+ txc->shift = 0;
+ txc->stabil = 0;
+ txc->jitcnt = 0;
+ txc->calcnt = 0;
+ txc->errcnt = 0;
+ txc->stbcnt = 0;
+}
+
+#endif /* CONFIG_NTP_PPS */
+
/*
* NTP methods:
*/
@@ -177,6 +334,9 @@ void ntp_clear(void)
tick_length = tick_length_base;
time_offset = 0;
+
+ /* Clear PPS state variables */
+ pps_clear();
}
/*
@@ -242,16 +402,16 @@ void second_overflow(void)
time_status |= STA_UNSYNC;
}
- /*
- * Compute the phase adjustment for the next second. The offset is
- * reduced by a fixed factor times the time constant.
- */
+ /* Compute the phase adjustment for the next second */
tick_length = tick_length_base;
- delta = shift_right(time_offset, SHIFT_PLL + time_constant);
+ delta = ntp_offset_chunk(time_offset);
time_offset -= delta;
tick_length += delta;
+ /* Check PPS signal */
+ pps_dec_valid();
+
if (!time_adjust)
return;
@@ -361,6 +521,8 @@ static inline void process_adj_status(struct timex *txc, struct timespec *ts)
if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) {
time_state = TIME_OK;
time_status = STA_UNSYNC;
+ /* restart PPS frequency calibration */
+ pps_reset_freq_interval();
}
/*
@@ -410,6 +572,8 @@ static inline void process_adjtimex_modes(struct timex *txc, struct timespec *ts
time_freq = txc->freq * PPM_SCALE;
time_freq = min(time_freq, MAXFREQ_SCALED);
time_freq = max(time_freq, -MAXFREQ_SCALED);
+ /* update pps_freq */
+ pps_set_freq(time_freq);
}
if (txc->modes & ADJ_MAXERROR)
@@ -500,7 +664,8 @@ int do_adjtimex(struct timex *txc)
}
result = time_state; /* mostly `TIME_OK' */
- if (time_status & (STA_UNSYNC|STA_CLOCKERR))
+ /* check for errors */
+ if (is_error_status(time_status))
result = TIME_ERROR;
txc->freq = shift_right((time_freq >> PPM_SCALE_INV_SHIFT) *
@@ -514,15 +679,8 @@ int do_adjtimex(struct timex *txc)
txc->tick = tick_usec;
txc->tai = time_tai;
- /* PPS is not implemented, so these are zero */
- txc->ppsfreq = 0;
- txc->jitter = 0;
- txc->shift = 0;
- txc->stabil = 0;
- txc->jitcnt = 0;
- txc->calcnt = 0;
- txc->errcnt = 0;
- txc->stbcnt = 0;
+ /* fill PPS status fields */
+ pps_fill_timex(txc);
write_sequnlock_irq(&xtime_lock);
@@ -536,6 +694,243 @@ int do_adjtimex(struct timex *txc)
return result;
}
+#ifdef CONFIG_NTP_PPS
+
+/* actually struct pps_normtime is good old struct timespec, but it is
+ * semantically different (and it is the reason why it was invented):
+ * pps_normtime.nsec has a range of ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ]
+ * while timespec.tv_nsec has a range of [0, NSEC_PER_SEC) */
+struct pps_normtime {
+ __kernel_time_t sec; /* seconds */
+ long nsec; /* nanoseconds */
+};
+
+/* 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;
+}
+
+/* get current phase correction and jitter */
+static inline long pps_phase_filter_get(long *jitter)
+{
+ *jitter = pps_tf[0] - pps_tf[1];
+ if (*jitter < 0)
+ *jitter = -*jitter;
+
+ /* TODO: test various filters */
+ return pps_tf[0];
+}
+
+/* add the sample to the phase filter */
+static inline void pps_phase_filter_add(long err)
+{
+ pps_tf[2] = pps_tf[1];
+ pps_tf[1] = pps_tf[0];
+ pps_tf[0] = err;
+}
+
+/* decrease frequency calibration interval length.
+ * It is halved after four consecutive unstable intervals.
+ */
+static inline void pps_dec_freq_interval(void)
+{
+ if (--pps_intcnt <= -PPS_INTCOUNT) {
+ pps_intcnt = -PPS_INTCOUNT;
+ if (pps_shift > PPS_INTMIN) {
+ pps_shift--;
+ pps_intcnt = 0;
+ }
+ }
+}
+
+/* increase frequency calibration interval length.
+ * It is doubled after four consecutive stable intervals.
+ */
+static inline void pps_inc_freq_interval(void)
+{
+ if (++pps_intcnt >= PPS_INTCOUNT) {
+ pps_intcnt = PPS_INTCOUNT;
+ if (pps_shift < PPS_INTMAX) {
+ pps_shift++;
+ pps_intcnt = 0;
+ }
+ }
+}
+
+/* update clock frequency based on MONOTONIC_RAW clock PPS signal
+ * timestamps
+ *
+ * At the end of the calibration interval the difference between the
+ * first and last MONOTONIC_RAW clock timestamps divided by the length
+ * of the interval becomes the frequency update. If the interval was
+ * too long, the data are discarded.
+ * Returns the difference between old and new frequency values.
+ */
+static long hardpps_update_freq(struct pps_normtime freq_norm)
+{
+ long delta, delta_mod;
+ s64 ftemp;
+
+ /* check if the frequency interval was too long */
+ if (freq_norm.sec > (2 << pps_shift)) {
+ time_status |= STA_PPSERROR;
+ pps_errcnt++;
+ pps_dec_freq_interval();
+ pr_err("hardpps: PPSERROR: interval too long - %ld s\n",
+ freq_norm.sec);
+ return 0;
+ }
+
+ /* here the raw frequency offset and wander (stability) is
+ * calculated. If the wander is less than the wander threshold
+ * the interval is increased; otherwise it is decreased.
+ */
+ ftemp = div_s64(((s64)(-freq_norm.nsec)) << NTP_SCALE_SHIFT,
+ freq_norm.sec);
+ delta = shift_right(ftemp - pps_freq, NTP_SCALE_SHIFT);
+ pps_freq = ftemp;
+ if (delta > PPS_MAXWANDER || delta < -PPS_MAXWANDER) {
+ pr_warning("hardpps: PPSWANDER: change=%ld\n", delta);
+ time_status |= STA_PPSWANDER;
+ pps_stbcnt++;
+ pps_dec_freq_interval();
+ } else { /* good sample */
+ pps_inc_freq_interval();
+ }
+
+ /* the stability metric is calculated as the average of recent
+ * 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) <<
+ (NTP_SCALE_SHIFT - SHIFT_USEC),
+ NSEC_PER_USEC) - pps_stabil) >> PPS_INTMIN;
+
+ /* if enabled, the system clock frequency is updated */
+ if ((time_status & STA_PPSFREQ) != 0 &&
+ (time_status & STA_FREQHOLD) == 0) {
+ time_freq = pps_freq;
+ ntp_update_frequency();
+ }
+
+ return delta;
+}
+
+/* correct REALTIME clock phase error against PPS signal */
+static void hardpps_update_phase(long error)
+{
+ long correction = -error;
+ long jitter;
+
+ /* add the sample to the median filter */
+ pps_phase_filter_add(correction);
+ correction = pps_phase_filter_get(&jitter);
+
+ /* Nominal jitter is due to PPS signal noise. If it exceeds the
+ * threshold, the sample is discarded; otherwise, if so enabled,
+ * the time offset is updated.
+ */
+ if (jitter > (pps_jitter << PPS_POPCORN)) {
+ pr_warning("hardpps: PPSJITTER: jitter=%ld, limit=%ld\n",
+ jitter, (pps_jitter << PPS_POPCORN));
+ time_status |= STA_PPSJITTER;
+ pps_jitcnt++;
+ } else if (time_status & STA_PPSTIME) {
+ /* correct the time using the phase offset */
+ time_offset = div_s64(((s64)correction) << NTP_SCALE_SHIFT,
+ NTP_INTERVAL_FREQ);
+ /* cancel running adjtime() */
+ time_adjust = 0;
+ }
+ /* update jitter */
+ pps_jitter += (jitter - pps_jitter) >> PPS_INTMIN;
+}
+
+/*
+ * hardpps() - discipline CPU clock oscillator to external PPS signal
+ *
+ * This routine is called at each PPS signal arrival in order to
+ * discipline the CPU clock oscillator to the PPS signal. It takes two
+ * parameters: REALTIME and MONOTONIC_RAW clock timestamps. The former
+ * is used to correct clock phase error and the latter is used to
+ * correct the frequency.
+ *
+ * This code is based on David Mills's reference nanokernel
+ * implementation. It was mostly rewritten but keeps the same idea.
+ */
+void hardpps(const struct timespec *phase_ts, const struct timespec *raw_ts)
+{
+ struct pps_normtime pts_norm, freq_norm;
+ unsigned long flags;
+
+ pts_norm = pps_normalize_ts(*phase_ts);
+
+ write_seqlock_irqsave(&xtime_lock, flags);
+
+ /* clear the error bits, they will be set again if needed */
+ time_status &= ~(STA_PPSJITTER | STA_PPSWANDER | STA_PPSERROR);
+
+ /* indicate signal presence */
+ time_status |= STA_PPSSIGNAL;
+ pps_valid = PPS_VALID;
+
+ /* when called for the first time,
+ * just start the frequency interval */
+ if (unlikely(pps_fbase.tv_sec == 0)) {
+ pps_fbase = *raw_ts;
+ write_sequnlock_irqrestore(&xtime_lock, flags);
+ return;
+ }
+
+ /* ok, now we have a base for frequency calculation */
+ freq_norm = pps_normalize_ts(timespec_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)) {
+ time_status |= STA_PPSJITTER;
+ /* restart the frequency calibration interval */
+ pps_fbase = *raw_ts;
+ write_sequnlock_irqrestore(&xtime_lock, flags);
+ pr_err("hardpps: PPSJITTER: bad pulse\n");
+ return;
+ }
+
+ /* signal is ok */
+
+ /* check if the current frequency interval is finished */
+ if (freq_norm.sec >= (1 << pps_shift)) {
+ pps_calcnt++;
+ /* restart the frequency calibration interval */
+ pps_fbase = *raw_ts;
+ hardpps_update_freq(freq_norm);
+ }
+
+ hardpps_update_phase(pts_norm.nsec);
+
+ write_sequnlock_irqrestore(&xtime_lock, flags);
+}
+EXPORT_SYMBOL(hardpps);
+
+#endif /* CONFIG_NTP_PPS */
+
static int __init ntp_tick_adj_setup(char *str)
{
ntp_tick_adj = simple_strtol(str, NULL, 0);
--
1.7.2.3
On Wed, 24 Nov 2010 19:15:43 +0300
Alexander Gordeev <[email protected]> wrote:
> There was a possibility that uart_handle_dcd_change() could obtain a
> reference to ldisc while running in parallel with tty_set_ldisc() on
> different CPU but call dcd_change() operation after tty_ldisc_close()
> which is incorrect.
How can this occur ?
> + spin_lock_irqsave(&tty->dcd_change_lock, flags);
> +
> + ld = tty_ldisc_ref(tty);
What is the expecting lock ordering rule here ?
I don't see why this patch is needed. You've got an ldisc ref from
tty_ldisc_ref, until you drop that ldisc ref you are fine. If for some
reason that is not the case then there is a bug in the ldisc code.
Alan
On Wed, 24 Nov 2010, Alexander Gordeev wrote:
> /**
> + * getnstime_raw_and_real - Returns both the time of day an raw
> + * monotonic time in a timespec format
IIRC then kerneldoc does not handle multiline comments for the
function name
> + * @ts_mono_raw: pointer to the timespec to be set to raw
> + * monotonic time
> + * @ts_real: pointer to the timespec to be set to the time
> + * of day
> + */
> +void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
> +{
> + unsigned long seq;
> + s64 nsecs_raw, nsecs_real;
> +
> + WARN_ON_ONCE(timekeeping_suspended);
Shouldn't this just return an error code instead of reading some
stale or random value ?
> + do {
> + u32 arch_offset;
> +
> + seq = read_seqbegin(&xtime_lock);
> +
> + *ts_raw = raw_time;
> + *ts_real = xtime;
> +
> + nsecs_raw = timekeeping_get_ns_raw();
> + nsecs_real = timekeeping_get_ns();
> +
> + /* If arch requires, add in gettimeoffset() */
> + arch_offset = arch_gettimeoffset();
> + nsecs_raw += arch_offset;
> + nsecs_real += arch_offset;
> +
> + } while (read_seqretry(&xtime_lock, seq));
> +
> + timespec_add_ns(ts_raw, nsecs_raw);
> + timespec_add_ns(ts_real, nsecs_real);
> +}
> +EXPORT_SYMBOL(getnstime_raw_and_real);
Otherwise, I'm fine with that.
Thanks,
tglx
В Wed, 24 Nov 2010 16:43:29 +0000
Alan Cox <[email protected]> пишет:
> On Wed, 24 Nov 2010 19:15:43 +0300
> Alexander Gordeev <[email protected]> wrote:
>
> > There was a possibility that uart_handle_dcd_change() could obtain a
> > reference to ldisc while running in parallel with tty_set_ldisc() on
> > different CPU but call dcd_change() operation after
> > tty_ldisc_close() which is incorrect.
>
> How can this occur ?
>
>
> > + spin_lock_irqsave(&tty->dcd_change_lock, flags);
> > +
> > + ld = tty_ldisc_ref(tty);
>
> What is the expecting lock ordering rule here ?
>
>
>
> I don't see why this patch is needed. You've got an ldisc ref from
> tty_ldisc_ref, until you drop that ldisc ref you are fine. If for some
> reason that is not the case then there is a bug in the ldisc code.
Yes, indeed, it's a bug. Please consider the following example:
CPU1 CPU2
=========================================================
uart_handle_dcd_change() { tty_set_ldisc() {
ld = tty_ldisc_ref(...) ...
... tty_ldisc_halt(...)
... ...
... tty_ldisc_close(...)
if (ld && ld->ops->dcd_change) ...
ld->ops->dcd_change(...); ...
... tty_ldisc_open(...)
} }
I think that semantically ldisc ops should never be called before open
or after close. This situation is possible because tty_ldisc_halt() only
ensures that no more references are taken. This is ok for everything
except dcd_change() because it cleans up workqueue and doesn't accept
any more data. dcd_change() is a different story because it doesn't use
workqueues.
I think tty code is exactly the right place to fix this bug; this is
what my patch is for.
--
Alexander
> Yes, indeed, it's a bug. Please consider the following example:
>
> CPU1 CPU2
> =========================================================
> uart_handle_dcd_change() { tty_set_ldisc() {
> ld = tty_ldisc_ref(...) ...
[We have a reference]
> ... tty_ldisc_halt(...)
[Should block]
> I think tty code is exactly the right place to fix this bug; this is
> what my patch is for.
More special case magic on top of the current crap isn't the right fix
here, tty_ldisc_halt needs to wait for the references to hit zero.
Alan
В Wed, 24 Nov 2010 17:49:44 +0100 (CET)
Thomas Gleixner <[email protected]> пишет:
> On Wed, 24 Nov 2010, Alexander Gordeev wrote:
> > /**
> > + * getnstime_raw_and_real - Returns both the time of day an raw
> > + * monotonic time in a timespec format
>
> IIRC then kerneldoc does not handle multiline comments for the
> function name
Ah, I see, fixed that.
> > + * @ts_mono_raw: pointer to the timespec to be set to raw
> > + * monotonic time
> > + * @ts_real: pointer to the timespec to be set to the time
> > + * of day
> > + */
> > +void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
> > +{
> > + unsigned long seq;
> > + s64 nsecs_raw, nsecs_real;
> > +
> > + WARN_ON_ONCE(timekeeping_suspended);
>
> Shouldn't this just return an error code instead of reading some
> stale or random value ?
Sorry, I don't know if it should, just copied it from getnstimofday().
BTW what is the bad value for timespec or you mean returning int
instead of void? I think this function should be handled the same way as
getnstimeofday() and no one currently expects that the latter can return
some bad value.
> > + do {
> > + u32 arch_offset;
> > +
> > + seq = read_seqbegin(&xtime_lock);
> > +
> > + *ts_raw = raw_time;
> > + *ts_real = xtime;
> > +
> > + nsecs_raw = timekeeping_get_ns_raw();
> > + nsecs_real = timekeeping_get_ns();
> > +
> > + /* If arch requires, add in gettimeoffset() */
> > + arch_offset = arch_gettimeoffset();
> > + nsecs_raw += arch_offset;
> > + nsecs_real += arch_offset;
> > +
> > + } while (read_seqretry(&xtime_lock, seq));
> > +
> > + timespec_add_ns(ts_raw, nsecs_raw);
> > + timespec_add_ns(ts_real, nsecs_real);
> > +}
> > +EXPORT_SYMBOL(getnstime_raw_and_real);
>
> Otherwise, I'm fine with that.
>
> Thanks,
>
> tglx
--
Alexander
В Wed, 24 Nov 2010 22:36:00 +0000
Alan Cox <[email protected]> пишет:
> > Yes, indeed, it's a bug. Please consider the following example:
> >
> > CPU1 CPU2
> > =========================================================
> > uart_handle_dcd_change() { tty_set_ldisc() {
> > ld = tty_ldisc_ref(...) ...
>
> [We have a reference]
> > ... tty_ldisc_halt(...)
> [Should block]
>
> > I think tty code is exactly the right place to fix this bug; this is
> > what my patch is for.
>
> More special case magic on top of the current crap isn't the right fix
> here, tty_ldisc_halt needs to wait for the references to hit zero.
Didn't know that current design is crap. :)
Ok, I think I'll add a new waitqueue and a new bit (TTY_LDISC_NOREF)
that halt will wait for. Is it good?
--
Alexander
> Didn't know that current design is crap. :)
It's based on the old big kernel lock and it has lots of horrors when
used from interrupt paths as a result. A big chunk of it can in time I
think move to just having a per tty ldisc lock that covers all changes
etc, but we aren't there yet.
> Ok, I think I'll add a new waitqueue and a new bit (TTY_LDISC_NOREF)
We've got the wait queue already
My first guess would be something like
static struct tty_ldisc *tty_ldisc_irqget(struct tty_struct *tty)
{
unsigned long flags;
struct tty_ldisc *ld;
spin_lock_irqsave(&tty_ldisc_lock, flags);
ld = NULL;
if (test_bit(TTY_LDISC, &tty->flags)) {
ld = get_ldisc(tty->ldisc);
WARN_ON(test_and_set_bit(TTY_LDISC_IRQ, &tty->flags));
}
spin_unlock_irqrestore(&tty_ldisc_lock, flags);
return ld;
}
static struct tty_ldisc *tty_ldisc_irqput(struct tty_ldisc *ld)
{
clear_bit(TTY_LDISC_IRQ, &tty->flags);
tty_ldisc_put(ld);
}
(and I think you can then wait on tty_ldisc_idle for the flag to go clear
after clearing TTY_LDISC)
It might be better to make it a count of course in case there are later
multiple users of this ?
Alan
Hi,
Very sorry for the long delay.
В Thu, 25 Nov 2010 14:03:54 +0000
Alan Cox <[email protected]> пишет:
> > Didn't know that current design is crap. :)
>
> It's based on the old big kernel lock and it has lots of horrors when
> used from interrupt paths as a result. A big chunk of it can in time I
> think move to just having a per tty ldisc lock that covers all changes
> etc, but we aren't there yet.
>
> > Ok, I think I'll add a new waitqueue and a new bit (TTY_LDISC_NOREF)
>
> We've got the wait queue already
>
> My first guess would be something like
>
> static struct tty_ldisc *tty_ldisc_irqget(struct tty_struct *tty)
> {
> unsigned long flags;
> struct tty_ldisc *ld;
>
> spin_lock_irqsave(&tty_ldisc_lock, flags);
> ld = NULL;
> if (test_bit(TTY_LDISC, &tty->flags)) {
> ld = get_ldisc(tty->ldisc);
> WARN_ON(test_and_set_bit(TTY_LDISC_IRQ, &tty->flags));
> }
> spin_unlock_irqrestore(&tty_ldisc_lock, flags);
> return ld;
> }
>
> static struct tty_ldisc *tty_ldisc_irqput(struct tty_ldisc *ld)
> {
> clear_bit(TTY_LDISC_IRQ, &tty->flags);
> tty_ldisc_put(ld);
> }
>
> (and I think you can then wait on tty_ldisc_idle for the flag to go clear
> after clearing TTY_LDISC)
>
> It might be better to make it a count of course in case there are later
> multiple users of this ?
I've just checked the code from v2.6.37 release candidates and found
that the problem is already completely fixed there with the revoking of
tty_ldisc_idle and tty_ldisc_wait_idle() in 100eeae2. :)
Sorry for the fuzz. The discussed patched is not needed any more and
will be dropped.
--
Alexander
On Wed, Nov 24, 2010 at 07:15:44PM +0300, Alexander Gordeev wrote:
> Using device index as a pointer needs some unnecessary work to be done
> every time the pointer is needed (in irq handler for example).
> Using a direct pointer is much more easy (and safe as well).
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Wed, Nov 24, 2010 at 07:15:46PM +0300, Alexander Gordeev wrote:
> Since now idr is only used to manage char device id's and not used in
> kernel API anymore it should be moved to pps.c. This also makes it
> possible to release id only at actual device freeing so nobody can
> register a pps device with the same id while our device is not freed
> yet.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Wed, Nov 24, 2010 at 07:15:47PM +0300, Alexander Gordeev wrote:
> Now pps_idr_lock is never used in interrupt context so replace
> spin_lock_irq/spin_unlock_irq with plain spin_lock/spin_unlock.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Wed, Nov 24, 2010 at 07:15:53PM +0300, Alexander Gordeev wrote:
> Add an optional feature of PPSAPI, kernel consumer support, which uses
> the added hardpps() function.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Wed, Nov 24, 2010 at 07:15:54PM +0300, Alexander Gordeev wrote:
> Add parallel port PPS client. It uses a standard method for capturing
> timestamps for assert edge transitions: getting a timestamp soon after
> an interrupt has happened. This is not a very precise source of time
> information due to interrupt handling delays. However, timestamps for
> clear edge transitions are much more precise because the interrupt
> handler continuously polls hardware port until the transition is done.
> Hardware port operations require only about 1us so the maximum error
> should not exceed this value. This was my primary goal when developing
> this client.
> Clear edge capture could be disabled using clear_wait parameter.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
On Wed, Nov 24, 2010 at 07:15:55PM +0300, Alexander Gordeev wrote:
> Add PPS signal generator which utilizes STROBE pin of a parallel port to
> send PPS signals. It uses parport abstraction layer and hrtimers to
> precisely control the signal.
>
> Signed-off-by: Alexander Gordeev <[email protected]>
Acked-by: Rodolfo Giometti <[email protected]>
--
GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it
В Thu, 16 Dec 2010 16:52:38 +0100
Rodolfo Giometti <[email protected]> пишет:
> On Wed, Nov 24, 2010 at 07:15:44PM +0300, Alexander Gordeev wrote:
> > Using device index as a pointer needs some unnecessary work to be done
> > every time the pointer is needed (in irq handler for example).
> > Using a direct pointer is much more easy (and safe as well).
> >
> > Signed-off-by: Alexander Gordeev <[email protected]>
>
> Acked-by: Rodolfo Giometti <[email protected]>
Thanks, Rodolfo!
I'll resend the patchset now with all your acks and other changes.
--
Alexander