On 11/26/19 4:17 PM, Arnd Bergmann wrote:
> On Tue, Nov 26, 2019 at 3:15 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>>
>> Then use that in the struct v4l2_buffer definition:
>>
>> struct v4l2_buffer {
>> ...
>> #ifdef __KERNEL__
>> struct __kernel_v4l2_timeval timestamp;
>> #else
>> struct timeval timestamp;
>> #endif
>>
>> That keeps struct v4l2_buffer fairly clean. And it also makes it
>> possible to have a bit more extensive documentation for the
>> struct __kernel_v4l2_timeval without polluting the actual struct
>> v4l2_buffer definition.
>
> Yes, good idea. I've added this version now:
>
> #ifdef __KERNEL__
> /*
> * This corresponds to the user space version of timeval
> * for 64-bit time_t. sparc64 is different from everyone
> * else, using the microseconds in the wrong half of the
> * second 64-bit word.
> */
> struct __kernel_v4l2_timeval {
> long long tv_sec;
> #if defined(__sparc__) && defined(__arch64__)
> int tv_usec;
> int __pad;
> #else
> long long tv_usec;
> #endif
> };
> #endif
>
> I briefly considered using #else #define __kernel_v4l2_timeval timeval
> to avoid the second #ifdef, but went back to your version again
> for clarify.
>
>> The videodev2.h header is something users of the API look at a
>> lot and having this really ugly kernel timestamp in there is
>> not acceptably IMHO. But splitting it off should work.
>
> Do you also mean moving it into a separate header file, or
> just outside of struct v4l2_buffer? Since it's hidden in #ifdef
> __KERNEL__, it could be moved to media/ioctl.h or elsewhere.
I've thought about that, but that risks having to change drivers
since they would now have to include another header to get the
right timeval definition. In the end I don't think it is worth the
effort.
I think it is best to define __kernel_v4l2_timeval just before
the struct v4l2_requestbuffers definition rather than before the
struct v4l2_buffer. That way it doesn't interfere with the
userspace structs for the buffer API.
Regards,
Hans
>
> Arnd
>
