Libraw1394 at present requires an async write and read to get the cycle time. It would be nice if it was read directly from the kernel. Secondly, there is currently no function that reads the cycle time and system time simultaneously so that they are synchronized. It would be great if such a function existed so we could discover the exact receive time of an ISO packet and express it as a system time with microsecond accuracy.
Proposed patch posted by Pieter at http://thread.gmane.org/gmane.linux.kernel.firewire.devel/8934
Created attachment 10268 [details] cycle timer read extension for raw1394
Created attachment 10269 [details] cycle timer read extension for libraw1394
Created attachment 10270 [details] cycle timer read extension for raw1394 Update 2: - Use 1000000ULL instead of USEC_PER_SEC, - include asm/system.h instead of linux/irqflags.h for local_irq_save and local_irq_restore.
Created attachment 10271 [details] cycle timer read extension for raw1394 revert struct name to Pieter's original version
Created attachment 10377 [details] cycle timer read extension for libraw1394 Update of the libraw1394 part of the patch: - comments, - fully copied the kernel's ieee1394-ioctl.h, - slightly shrunk main.c::raw1394_read_cycle_timer There remains a difference between libraw1394's kernel-raw1394.h and the kernel's raw1394.h: --- src/kernel-raw1394.h 2007-02-11 14:16:35.000000000 +0100 +++ ../../linux-2.6.20/drivers/ieee1394/raw1394.h 2007-02-11 01:23:05.000000000 +0100 @@ -27,6 +27,7 @@ #define RAW1394_REQ_GET_ROM 203 #define RAW1394_REQ_UPDATE_ROM 204 #define RAW1394_REQ_ECHO 205 +#define RAW1394_REQ_MODIFY_ROM 206 #define RAW1394_REQ_ARM_REGISTER 300 #define RAW1394_REQ_ARM_UNREGISTER 301 @@ -104,18 +105,18 @@ __u8 extended_transaction_code; __u32 generation; __u16 buffer_length; - __u8 *buffer; + __u8 __user *buffer; } *arm_request_t; typedef struct arm_response { __s32 response_code; __u16 buffer_length; - __u8 *buffer; + __u8 __user *buffer; } *arm_response_t; typedef struct arm_request_response { - struct arm_request *request; - struct arm_response *response; + struct arm_request __user *request; + struct arm_response __user *response; } *arm_request_response_t; /* rawiso API */ @@ -135,7 +136,7 @@ /* argument for RAW1394_ISO_RECV/XMIT_PACKETS ioctls */ struct raw1394_iso_packets { __u32 n_packets; - struct raw1394_iso_packet_info *infos; + struct raw1394_iso_packet_info __user *infos; }; struct raw1394_iso_config {
How about changing main.c::raw1394_read_cycle_timer to: int raw1394_read_cycle_timer(raw1394handle_t handle, u_int32_t *cycle_timer u_int64_t *local_time) { int err; struct _raw1394_cycle_timer ctr = { 0 }; err = ioctl(handle->fd, RAW1394_IOC_GET_CYCLE_TIMER, &ctr); if (!err) { *cycle_timer = ctr.cycle_timer; *local_time = ctr.local_time; } return err; } The benefit of doing so would be that we would get rid of libraw1394's struct raw1394_cycle_timer which duplicates the kernel's struct _raw1394_cycle_timer. (Of course we would rename the latter to struct raw1394_cycle_timer then.)
Created attachment 10378 [details] cycle timer read extension for libraw1394, API changed as proposed in the previous comment
Created attachment 10379 [details] cycle timer read extension for libraw1394 inserted a dropped comment again
I tested out the latest versions of the patches, and they work for me. However, I wonder if we should should bump the raw1394 protocol version. Without it and a version mismatch between raw1394 and libraw1394, one gets a EINVAL when calling raw1394_read_cycle_timer. By bumping the protocol version and a small change in libraw1394, one can get an EPROTO on raw1394_new_handle. What do you think?
OK, let's change RAW1394_KERNELAPI_VERSION to 5. The additional check in libraw1394 is minimal and gives application clients a better return value. I will commit this tonight to git and push to Linus on Saturday.
I have second thoughts on this version bump. A simple version bump in kernel raw1394 will cause released libraw1394 versions to succeed and use protocol version 3. This causes special handling resulting in libraw1394 raw1394_get_irm_id() returning unknown values, thereby breaking raw1394_bandwidth_modify(), raw1394_channel_modify(), and app calls. With some additional change to kernel raw1394 we can remove backwards compatibilty. However, I think it would suck for users to have libraw1394 and its apps break when they upgrade to kernel 2.4.21 because there is nothing like shared library versioning to help reinforce dependencies.
The feature was merged in mainline and is available since 2.6.21-rc1.