[LinuxPPS] [PATCH] PPS: Add a read() method
George Spelvin
linux at horizon.com
Mon Feb 9 15:16:21 CET 2009
Rodolfo Giometti <giometti at enneenne.com> wrote:
> On Mon, Feb 09, 2009 at 05:44:19AM -0500, George Spelvin wrote:
>> I'll happily propose one, but you objected to my ideas about the format.
>> Should I go ahead anyway (using my format which you don't like) and we'll
>> argue about the format later?
>
> Ok. I think this could be the right-thing(TM) to do. :)
Okay, here's a first draft. The locking is stolen from fs/sysfs/file.c,
because it appears that multiple threads can call a ->read method in
parallel. The same applies to unlocked_ioctl, AFAICT, so maybe we need
something there, too.
This is without my 12/11 API change patch, assuming that makes it easier
for you to review.
(I'm not entirely happy with the private data structure name, but that's
what came to me at the time. Changing that name is a lot of the clutter
in this patch.)
PPS: add a read(2) method
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 7a32920..032df0a 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -29,6 +29,7 @@
#include <linux/cdev.h>
#include <linux/poll.h>
#include <linux/pps.h>
+#include <linux/mutex.h>
/*
* Local variables
@@ -38,6 +39,16 @@ static dev_t pps_devt;
static struct class *pps_class;
#define PPS_ECHOBOTH (PPS_ECHOASSERT | PPS_ECHOCLEAR)
+struct pps_kdata {
+ struct mutex mutex;
+ char *buf; /* For device read output */
+ unsigned pos, len; /* Position within the buffer */
+ /* Following is just a copy of struct kinfo */
+ u32 assert_sequence, clear_sequence; /* Read pos */
+ struct pps_ktime assert_tu, clear_tu; /* Offsets */
+ int current_mode; /* current mode bits */
+};
+
/*
* Local functions
*/
@@ -69,13 +80,16 @@ static void pps_add_offset(struct pps_ktime *ts, struct pps_ktime const *offset)
ts->sec += offset->sec;
}
-static bool __pure
-pps_is_ready(struct pps_device const *pps, struct pps_kinfo const *info)
+static int __pure
+pps_is_ready(struct pps_device const *pps, struct pps_kdata const *data)
{
- return (info->current_mode & PPS_CAPTUREASSERT &&
- pps->assert_sequence != info->assert_sequence) ||
- (info->current_mode & PPS_CAPTURECLEAR &&
- pps->clear_sequence != info->clear_sequence);
+ int status = 0;
+
+ if (pps->assert_sequence != data->assert_sequence)
+ status |= PPS_CAPTUREASSERT;
+ if (pps->clear_sequence != data->clear_sequence)
+ status |= PPS_CAPTURECLEAR;
+ return status & data->current_mode;
}
/*
@@ -86,11 +100,11 @@ static unsigned int pps_cdev_poll(struct file *file, poll_table *wait)
{
struct pps_device *pps = container_of(file->f_dentry->d_inode->i_cdev,
struct pps_device, cdev);
- struct pps_kinfo const *info = file->private_data;
+ struct pps_kdata const *data = file->private_data;
poll_wait(file, &pps->queue, wait);
- return pps_is_ready(pps, info) ? POLLIN | POLLRDNORM : 0;
+ return pps_is_ready(pps, data) ? POLLIN | POLLRDNORM : 0;
}
/*
@@ -124,8 +138,8 @@ static int pps_cdev_fasync(int fd, struct file *file, int on)
{
struct pps_device *pps = container_of(file->f_dentry->d_inode->i_cdev,
struct pps_device, cdev);
- struct pps_kinfo const *info = file->private_data;
- int i, mode = on ? info->current_mode & PPS_CAPTUREBOTH : 0;
+ struct pps_kdata const *data = file->private_data;
+ int i, mode = on ? data->current_mode & PPS_CAPTUREBOTH : 0;
int err, result = 0;
/* Remove from wrong list */
@@ -149,7 +163,7 @@ static long pps_cdev_ioctl(struct file *file,
{
struct pps_device *pps = container_of(file->f_dentry->d_inode->i_cdev,
struct pps_device, cdev);
- struct pps_kinfo *info = file->private_data;
+ struct pps_kdata *data = file->private_data;
void __user *uarg = (void __user *) arg;
struct pps_fdata __user *fuarg = uarg;
struct pps_kparams params;
@@ -169,9 +183,9 @@ static long pps_cdev_ioctl(struct file *file,
pr_debug("PPS_GETPARAMS: source %d\n", pps->id);
/* Return current parameters */
- params.mode = info->current_mode & ~PPS_ECHOBOTH;
- params.assert_off_tu = info->assert_tu;
- params.clear_off_tu = info->clear_tu;
+ params.mode = data->current_mode & ~PPS_ECHOBOTH;
+ params.assert_off_tu = data->assert_tu;
+ params.clear_off_tu = data->clear_tu;
spin_lock(&pps->lock);
params.api_version = PPS_API_VERS;
@@ -226,9 +240,9 @@ static long pps_cdev_ioctl(struct file *file,
}
/* Save the new parameters in the local version */
- pps_norm_offset(&info->assert_tu, ¶ms.assert_off_tu);
- pps_norm_offset(&info->clear_tu, ¶ms.clear_off_tu);
- info->current_mode = params.mode & ~PPS_ECHOBOTH;
+ pps_norm_offset(&data->assert_tu, ¶ms.assert_off_tu);
+ pps_norm_offset(&data->clear_tu, ¶ms.clear_off_tu);
+ data->current_mode = params.mode & ~PPS_ECHOBOTH;
break;
@@ -254,7 +268,7 @@ static long pps_cdev_ioctl(struct file *file,
/* Manage the timeout */
if (fdata.timeout.flags & PPS_TIME_INVALID)
err = wait_event_interruptible(pps->queue,
- pps_is_ready(pps, info));
+ pps_is_ready(pps, data));
else {
pr_debug("timeout %lld.%09d\n",
(long long) fdata.timeout.sec,
@@ -265,7 +279,7 @@ static long pps_cdev_ioctl(struct file *file,
if (ticks != 0) {
err = wait_event_interruptible_timeout(
pps->queue,
- pps_is_ready(pps, info),
+ pps_is_ready(pps, data),
ticks);
/*
* RFC 2783 requires this error, although
@@ -283,20 +297,20 @@ static long pps_cdev_ioctl(struct file *file,
}
/* Return the fetched timestamp */
- fdata.info.assert_sequence = info->assert_sequence = seq1 =
+ fdata.info.assert_sequence = data->assert_sequence = seq1 =
pps->assert_sequence;
- fdata.info.clear_sequence = info->clear_sequence = seq2 =
+ fdata.info.clear_sequence = data->clear_sequence = seq2 =
pps->clear_sequence;
read_barrier_depends();
fdata.info.assert_tu = pps->assert_tu[seq1 % 4];
- if (info->current_mode & PPS_OFFSETASSERT)
- pps_add_offset(&fdata.info.assert_tu, &info->assert_tu);
+ if (data->current_mode & PPS_OFFSETASSERT)
+ pps_add_offset(&fdata.info.assert_tu, &data->assert_tu);
fdata.info.clear_tu = pps->clear_tu[seq2 % 4];
- if (info->current_mode & PPS_OFFSETCLEAR)
- pps_add_offset(&fdata.info.clear_tu, &info->clear_tu);
- fdata.info.current_mode = info->current_mode;
+ if (data->current_mode & PPS_OFFSETCLEAR)
+ pps_add_offset(&fdata.info.clear_tu, &data->clear_tu);
+ fdata.info.current_mode = data->current_mode;
err = copy_to_user(&fuarg->info, &fdata.info, sizeof fdata.info);
if (err)
@@ -317,19 +331,20 @@ 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;
- struct pps_kinfo *info;
+ struct pps_kdata *data;
found = pps_get_source(pps->id) != 0; /* Bump refcount */
if (!found)
return -ENODEV;
- info = kzalloc(sizeof *info, GFP_KERNEL);
- if (!info) {
+ data = kzalloc(sizeof *data, GFP_KERNEL);
+ if (!data) {
pps_put_source(pps);
return -ENOMEM;
}
- info->current_mode = pps->default_mode;
+ mutex_init(&data->mutex);
+ data->current_mode = pps->default_mode;
/* Offsets remain zero */
- file->private_data = info;
+ file->private_data = data;
return 0;
}
@@ -338,20 +353,119 @@ static int pps_cdev_release(struct inode *inode, struct file *file)
{
struct pps_device *pps = container_of(inode->i_cdev,
struct pps_device, cdev);
-
- kfree(file->private_data);
+ struct pps_kdata const *data = file->private_data;
+ kfree(data->buf);
+ kfree(data);
/* Free the PPS source and wake up (possible) deregistration */
pps_put_source(pps);
return 0;
}
+#define PPS_LINE_SIZE 49
+static ssize_t
+pps_cdev_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
+{
+ struct pps_device *pps = container_of(file->f_dentry->d_inode->i_cdev,
+ struct pps_device, cdev);
+ struct pps_kdata *data = file->private_data;
+ ssize_t n = 0;
+ int err = -EAGAIN;
+
+ /* Concurrent calls to this function *are* possible */
+ mutex_lock(&data->mutex);
+
+ /* Ensure there's space. */
+ if (!data->buf) {
+ data->buf = kmalloc(PPS_LINE_SIZE, GFP_KERNEL);
+ if (unlikely(!data->buf)) {
+ err = -ENOMEM;
+ goto done;
+ }
+ }
+
+ while (nbytes) {
+ int ready;
+ struct pps_ktime tu[2];
+ u32 seq[2];
+ bool which;
+
+ /* Copy out available data */
+ if (data->pos < data->len) {
+ unsigned count = data->len - data->pos;
+ if (count > nbytes)
+ count = (unsigned)nbytes;
+ if (copy_to_user(buf, data->buf + data->pos, count)) {
+ err = -EFAULT;
+ break;
+ }
+ nbytes -= count;
+ buf += count;
+ data->pos += count;
+ n += count;
+ continue;
+ }
+
+ /* Wait until new data is available */
+ if (n || file->f_flags & O_NONBLOCK) {
+ ready = pps_is_ready(pps, data);
+ if (!ready)
+ break;
+ } else {
+ err = wait_event_interruptible(pps->queue,
+ (ready = pps_is_ready(pps, data)));
+ if (err)
+ break;
+ }
+
+ /* Fetch timestamps */
+ data->assert_sequence = seq[0] = pps->assert_sequence;
+ data->clear_sequence = seq[1] = pps->clear_sequence;
+ rmb();
+ tu[0] = pps->assert_tu[seq[0] % 4];
+ tu[1] = pps->clear_tu[seq[1] % 4];
+
+ /* See which to print */
+ switch (ready) {
+ default:
+ BUG();
+ goto done;
+ case PPS_CAPTUREASSERT:
+ which = 0;
+ break;
+ case PPS_CAPTURECLEAR:
+ which = 1;
+ break;
+ case PPS_CAPTUREBOTH:
+ /* Select the earlier timestamp */
+ if (tu[0].sec != tu[1].sec)
+ which = tu[0].sec > tu[1].sec;
+ else
+ which = tu[0].nsec > tu[1].nsec;
+ break;
+ }
+
+ /* At most 6+1+10+1+20+1+9+1 = 49 bytes */
+ data->len = sprintf(data->buf, "%s %u %lld.%09d\n",
+ which ? "clear" : "assert",
+ (unsigned)seq[which],
+ (long long)tu[which].sec, tu[which].nsec);
+ data->pos = 0;
+ BUG_ON(data->len > PPS_LINE_SIZE);
+ }
+done:
+ mutex_unlock(&data->mutex);
+ return n ? n : err;
+}
+
+
/*
* Char device stuff
*/
static const struct file_operations pps_cdev_fops = {
.owner = THIS_MODULE,
+ .read = pps_cdev_read,
.llseek = no_llseek,
.poll = pps_cdev_poll,
.fasync = pps_cdev_fasync,
More information about the LinuxPPS
mailing list