On Monday 26 October 2015 18:16:54 Deepa Dinamani wrote:
timeval is deprecated and not y2038 safe. Its size also changes according to 32 bit/ 64 bit compilation. Replace it with 32 and 64 bit versions of its individual fields, giving two ioctls with different code values. The two ioctls are necessary to maintain the 32 bit and 64 bit userspace compatibility with a 64/32 bit kernel.
Change unsigned to __u32 types for a definitive userspace interface. This is in accordance with the psABI that the unsigned type is always 32 bits.
Also use motonic timer instead of real time to ensure positive delta values.
Refactor usbtest_ioctl for readability to isolate the handling of the testing timing measurement.
The official testusb userspace tool can be changed in a separate patch to reflect the __u32 changes as well. It can use the usbtest_param_32 struct, since 32 bit seconds is long enough for test durations.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Sorry for taking so long to reply. This looks really good, great work!
I have just one small comment, please address this and then send the patch to the respective maintainers. Unfortunately, we are in a merge window now, so it might not get picked up right away.
- getrawmonotonic64(&start);
- retval = usbtest_do_ioctl(intf, param_32);
- if (retval)
goto free_mutex;- getrawmonotonic64(&end);
- duration = timespec64_sub(end, start);
- temp.duration_sec = duration.tv_sec;
- temp.duration_usec = duration.tv_nsec/1000;
I would use ktime_get_ts64() instead of getrawmonotonic64(): We normally don't want the 'raw' time in drivers but the normal monotonic time that ktime_get_* uses.
One more small style comment: I would use NSECS_PER_USEC instead of the open-coded '1000'.
Arnd