Re: [RFC] cycle timer read extension for raw1394/libraw1394

Stefan Richter wrote:

(Cc lkml)

Pieter Palmers wrote to linux1394-devel:

Attached are two patches containing an extension to libraw1394 and the
kernel stack, implementing the simultaneous read of the isochronous
cycle timer and the system clock (in usecs). This implements what has
been requested before on the list.

There is one issue left, namely that I didn't disable preemption when
doing the timer reads. In order to have a reliable simultaneous read of
both values, I think IRQ's and preemption should be disabled while
reading them.


Thus?
	preempt_disable();
	local_irq_save(flags);

	read_cycle_timer;
	read_time_of_day;

	local_irq_restore(flags);
	preempt_enable();

in hpsb_read_cycle_timer.

My main issue was that I still had to figure out how to do this... I'm avery occasional kernel space programmer. Thanks for the hint, I'll doit like this.

I still have to check if I don't introduce a too long non-preemptiblepath though.

Kernel patch applies against 2.6.20-rc6.
libraw1394 patch applies against SVN.

Please comment.

Greets,

Pieter Palmers


------------------------------------------------------------------------

diff -u ../linux-2.6/drivers/ieee1394/ieee1394_core.c ieee1394-2.6.20/ieee1394_core.c
--- ../linux-2.6/drivers/ieee1394/ieee1394_core.c	2007-01-26 18:29:07.000000000 +0100
+++ ieee1394-2.6.20/ieee1394_core.c	2007-01-26 18:37:16.000000000 +0100
@@ -34,6 +34,8 @@
 #include <linux/suspend.h>
 #include <linux/kthread.h>

+#include <linux/time.h>

+
 #include <asm/byteorder.h>

#include "ieee1394_types.h"


No empty line necessary above #include <linux/time.h>.

@@ -940,6 +942,13 @@
 	}
 }
+void hpsb_read_cycle_timer(struct hpsb_host *host, u32 *ctr, u64 *local_time)
+{
+	struct timeval tv;


Empty line would be OK here.

+	*ctr = host->driver->devctl(host, GET_CYCLE_COUNTER, 0);
+	do_gettimeofday(&tv);
+	*local_time=tv.tv_sec*1000000+tv.tv_usec;
+}


Couldn't this overflow?
	*local_time = tv.tv_sec * 1000000L + tv.tv_usec;
or
	*local_time = tv.tv_sec * USEC_PER_SEC + tv.tv_usec;

Indeed. thx.

 static void abort_requests(struct hpsb_host *host)
 {
@@ -1209,6 +1218,7 @@
 EXPORT_SYMBOL(hpsb_read);
 EXPORT_SYMBOL(hpsb_write);
 EXPORT_SYMBOL(hpsb_packet_success);
+EXPORT_SYMBOL(hpsb_read_cycle_timer);

/** highlevel.c **/

 EXPORT_SYMBOL(hpsb_register_highlevel);
diff -u ../linux-2.6/drivers/ieee1394/ieee1394_core.h ieee1394-2.6.20/ieee1394_core.h
--- ../linux-2.6/drivers/ieee1394/ieee1394_core.h	2007-01-26 18:11:17.000000000 +0100
+++ ieee1394-2.6.20/ieee1394_core.h	2007-01-26 18:37:16.000000000 +0100
@@ -227,4 +227,8 @@
 extern struct class hpsb_host_class;
 extern struct class *hpsb_protocol_class;

+/* Reads the cycle timer, and the local time at which it was read.

+ */
+void hpsb_read_cycle_timer(struct hpsb_host *host, u32 *ctr, u64 *local_time);
+
 #endif /* _IEEE1394_CORE_H */


(This reminds me that many of ieee1394's exported symbols lack kerneldoc
comments.)

I'll change this one into proper kerneldoc format.


Alas I can't comment on the design & implementation of the userspace
ABI, that's out of my league.

diff -u ../linux-2.6/drivers/ieee1394/ieee1394-ioctl.h ieee1394-2.6.20/ieee1394-ioctl.h
--- ../linux-2.6/drivers/ieee1394/ieee1394-ioctl.h	2007-01-26 18:11:17.000000000 +0100
+++ ieee1394-2.6.20/ieee1394-ioctl.h	2007-01-26 18:40:04.000000000 +0100
@@ -100,5 +100,6 @@
 	_IO  ('#', 0x28)
 #define RAW1394_IOC_ISO_RECV_FLUSH		\
 	_IO  ('#', 0x29)
-
+#define RAW1394_IOC_GET_CYCLE_TIMER		\
+	_IOR ('#', 0x30, struct _raw1394_cycle_timer)
 #endif /* __IEEE1394_IOCTL_H */
diff -u ../linux-2.6/drivers/ieee1394/raw1394.c ieee1394-2.6.20/raw1394.c
--- ../linux-2.6/drivers/ieee1394/raw1394.c	2007-01-26 18:11:17.000000000 +0100
+++ ieee1394-2.6.20/raw1394.c	2007-01-26 18:37:16.000000000 +0100
@@ -2675,7 +2675,22 @@
 	return dma_region_mmap(&fi->iso_handle->data_buf, file, vma);
 }

-/* ioctl is only used for rawiso operations */

+/* read the Isochronous Cycle Counter along with the cpu time */
+static int raw1394_read_cycle_timer(struct file_info *fi, void __user * uaddr)
+{
+	struct _raw1394_cycle_timer ctr;
+	struct hpsb_host *host = fi->host;
+
+	hpsb_read_cycle_timer(host, &ctr.cycle_timer, &ctr.local_time);
+	if (copy_to_user(uaddr, &ctr, sizeof(ctr)))
+		return -EFAULT;
+
+	return 0;
+}
+
+/* ioctl is only used for rawiso operations,
+ * and to read the cycle timer
+ */
 static int raw1394_ioctl(struct inode *inode, struct file *file,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -2772,6 +2787,13 @@
 		break;
 	}

+ switch(cmd) {

+		case RAW1394_IOC_GET_CYCLE_TIMER:
+			return raw1394_read_cycle_timer(fi, argp);
+		default:
+		break;
+	}
+
 	return -EINVAL;
 }

diff -u ../linux-2.6/drivers/ieee1394/raw1394.h ieee1394-2.6.20/raw1394.h

--- ../linux-2.6/drivers/ieee1394/raw1394.h	2007-01-26 13:15:58.000000000 +0100
+++ ieee1394-2.6.20/raw1394.h	2007-01-26 18:37:16.000000000 +0100
@@ -178,4 +178,11 @@
 	__s16 xmit_cycle;
 };

+/* Simultanous Isochronous Cycle Timer read and local clock read */

+struct _raw1394_cycle_timer {
+	__u32 cycle_timer;
+
+	/* local time in microseconds */
+	__u64 local_time;
+};
 #endif /* IEEE1394_RAW1394_H */


------------------------------------------------------------------------

Index: src/ieee1394-ioctl.h
===================================================================
--- src/ieee1394-ioctl.h	(revision 168)
+++ src/ieee1394-ioctl.h	(working copy)
@@ -106,6 +106,6 @@
 	_IO  ('#', 0x28)
 #define RAW1394_IOC_ISO_RECV_FLUSH              \
 	_IO  ('#', 0x29)
-
-
+#define RAW1394_IOC_GET_CYCLE_TIMER		\
+	_IOR ('#', 0x30, struct _raw1394_cycle_timer)
 #endif /* __IEEE1394_IOCTL_H */
Index: src/raw1394.h
===================================================================
--- src/raw1394.h	(revision 168)
+++ src/raw1394.h	(working copy)
@@ -118,7 +118,14 @@
 	RAW1394_MODIFY_FREE
 };

+/* Simultanous Isochronous Cycle Timer read and local clock read */

+struct raw1394_cycle_timer {
+	u_int32_t cycle_timer;

+ /* local time in microseconds */

+	u_int64_t local_time;
+};
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -328,6 +335,18 @@
  **/
 void raw1394_iso_shutdown(raw1394handle_t handle);

+/**

+ * raw1394_get_cycle_timer - get the current value of the cycle timer
+ * @handle: libraw1394 handle
+ * @ctr: pointer to the target raw1394_cycle_timer struct
+ *
+ * Reads the cycle timer register, together with the system clock. It returns
+ * a reasonable estimate of the system time the cycle timer was read.
+ *
+ * Returns: the error code of the ioctl, or 0 if successful.
+ **/


The comment on accuracy could be dropped if preemption and IRQs are
disabled, right?

+int raw1394_read_cycle_timer(raw1394handle_t handle, struct raw1394_cycle_timer *ctr);
+
 typedef int raw1394_errcode_t;
 #define raw1394_make_errcode(ack, rcode) (((ack) << 16) | rcode)
 #define raw1394_internal_err(errcode) ((errcode) < 0)
Index: src/main.c
===================================================================
--- src/main.c	(revision 168)
+++ src/main.c	(working copy)
@@ -478,3 +478,15 @@
         return 0;
 }

+int raw1394_read_cycle_timer(raw1394handle_t handle, struct raw1394_cycle_timer *ctr)+{

+	int err;
+
+	err = ioctl(handle->fd, RAW1394_IOC_GET_CYCLE_TIMER, ctr);
+	if(err != 0) {
+		return err;
+	}
+
+    return 0;
+}
+
Index: src/kernel-raw1394.h
===================================================================
--- src/kernel-raw1394.h	(revision 168)
+++ src/kernel-raw1394.h	(working copy)
@@ -177,4 +177,11 @@
 	__s16 xmit_cycle;
 };

+/* Simultanous Isochronous Cycle Timer read and local clock read */

+struct _raw1394_cycle_timer {
+	__u32 cycle_timer;
+
+	/* local time in microseconds */
+	__u64 local_time;
+};
 #endif /* IEEE1394_RAW1394_H */


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Follow-Ups:
- Re: [RFC] cycle timer read extension for raw1394/libraw1394
  - From: Stefan Richter <[email protected]>

References:
- Re: [RFC] cycle timer read extension for raw1394/libraw1394
  - From: Stefan Richter <[email protected]>

Prev by Date: Re: [RFC] cycle timer read extension for raw1394/libraw1394
Next by Date: Re: [PATCH 49/59] sysctl: Move init_irq_proc into init/main where it belongs
Previous by thread: Re: [RFC] cycle timer read extension for raw1394/libraw1394
Next by thread: Re: [RFC] cycle timer read extension for raw1394/libraw1394
Index(es):
- Date
- Thread

[Index of Archives] [Kernel Newbies] [Netfilter] [Bugtraq] [Photo] [Stuff] [Gimp] [Yosemite News] [MIPS Linux] [ARM Linux] [Linux Security] [Linux RAID] [Video 4 Linux] [Linux for the blind] [Linux Resources]