| Message ID | 20191111203835.2260382-3-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | y2038 safety in v4l2 | expand |
On 11/11/19 9:38 PM, Arnd Bergmann wrote: > As a preparation for adding 64-bit time_t support in the uapi, > change the drivers to no longer care about the format of the > timestamp field in struct v4l2_buffer. > > The v4l2_timeval_to_ns() function is no longer needed in the > kernel after this, but there may be userspace code relying on > it because it is part of the uapi header. There is indeed userspace code that relies on this. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 ++-- > drivers/media/pci/meye/meye.c | 4 ++-- > drivers/media/usb/cpia2/cpia2_v4l.c | 4 ++-- > drivers/media/usb/stkwebcam/stk-webcam.c | 2 +- > drivers/media/usb/usbvision/usbvision-video.c | 4 ++-- > drivers/media/v4l2-core/videobuf-core.c | 4 ++-- > include/linux/videodev2.h | 17 ++++++++++++++++- > include/trace/events/v4l2.h | 2 +- > include/uapi/linux/videodev2.h | 2 ++ > 9 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 5a9ba3846f0a..9ec710878db6 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -143,7 +143,7 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb) > * and the timecode field and flag if needed. > */ > if (q->copy_timestamp) > - vb->timestamp = v4l2_timeval_to_ns(&b->timestamp); > + vb->timestamp = v4l2_buffer_get_timestamp(b); > vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE; > if (b->flags & V4L2_BUF_FLAG_TIMECODE) > vbuf->timecode = b->timecode; > @@ -476,7 +476,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) > > b->flags = vbuf->flags; > b->field = vbuf->field; > - b->timestamp = ns_to_timeval(vb->timestamp); > + v4l2_buffer_set_timestamp(b, vb->timestamp); > b->timecode = vbuf->timecode; > b->sequence = vbuf->sequence; > b->reserved2 = 0; > diff --git a/drivers/media/pci/meye/meye.c b/drivers/media/pci/meye/meye.c > index 0e61c81356ef..3a4c29bc0ba5 100644 > --- a/drivers/media/pci/meye/meye.c > +++ b/drivers/media/pci/meye/meye.c > @@ -1266,7 +1266,7 @@ static int vidioc_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf) > buf->flags |= V4L2_BUF_FLAG_DONE; > > buf->field = V4L2_FIELD_NONE; > - buf->timestamp = ns_to_timeval(meye.grab_buffer[index].ts); > + v4l2_buffer_set_timestamp(buf, meye.grab_buffer[index].ts); > buf->sequence = meye.grab_buffer[index].sequence; > buf->memory = V4L2_MEMORY_MMAP; > buf->m.offset = index * gbufsize; > @@ -1332,7 +1332,7 @@ static int vidioc_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) > buf->bytesused = meye.grab_buffer[reqnr].size; > buf->flags = V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > buf->field = V4L2_FIELD_NONE; > - buf->timestamp = ns_to_timeval(meye.grab_buffer[reqnr].ts); > + v4l2_buffer_set_timestamp(buf, meye.grab_buffer[reqnr].ts); > buf->sequence = meye.grab_buffer[reqnr].sequence; > buf->memory = V4L2_MEMORY_MMAP; > buf->m.offset = reqnr * gbufsize; > diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c > index 626264a56517..9d3d05125d7b 100644 > --- a/drivers/media/usb/cpia2/cpia2_v4l.c > +++ b/drivers/media/usb/cpia2/cpia2_v4l.c > @@ -800,7 +800,7 @@ static int cpia2_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf) > break; > case FRAME_READY: > buf->bytesused = cam->buffers[buf->index].length; > - buf->timestamp = ns_to_timeval(cam->buffers[buf->index].ts); > + v4l2_buffer_set_timestamp(buf, cam->buffers[buf->index].ts); > buf->sequence = cam->buffers[buf->index].seq; > buf->flags = V4L2_BUF_FLAG_DONE; > break; > @@ -907,7 +907,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) > buf->flags = V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_DONE > | V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > buf->field = V4L2_FIELD_NONE; > - buf->timestamp = ns_to_timeval(cam->buffers[buf->index].ts); > + v4l2_buffer_set_timestamp(buf, cam->buffers[buf->index].ts); > buf->sequence = cam->buffers[buf->index].seq; > buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer; > buf->length = cam->frame_size; > diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c > index 21f90a887485..b22501f76b78 100644 > --- a/drivers/media/usb/stkwebcam/stk-webcam.c > +++ b/drivers/media/usb/stkwebcam/stk-webcam.c > @@ -1125,7 +1125,7 @@ static int stk_vidioc_dqbuf(struct file *filp, > sbuf->v4lbuf.flags &= ~V4L2_BUF_FLAG_QUEUED; > sbuf->v4lbuf.flags |= V4L2_BUF_FLAG_DONE; > sbuf->v4lbuf.sequence = ++dev->sequence; > - sbuf->v4lbuf.timestamp = ns_to_timeval(ktime_get_ns()); > + v4l2_buffer_set_timestamp(&sbuf->v4lbuf, ktime_get_ns()); > > *buf = sbuf->v4lbuf; > return 0; > diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c > index cdc66adda755..15a423c5deb7 100644 > --- a/drivers/media/usb/usbvision/usbvision-video.c > +++ b/drivers/media/usb/usbvision/usbvision-video.c > @@ -687,7 +687,7 @@ static int vidioc_querybuf(struct file *file, > vb->length = usbvision->curwidth * > usbvision->curheight * > usbvision->palette.bytes_per_pixel; > - vb->timestamp = ns_to_timeval(usbvision->frame[vb->index].ts); > + v4l2_buffer_set_timestamp(vb, usbvision->frame[vb->index].ts); > vb->sequence = usbvision->frame[vb->index].sequence; > return 0; > } > @@ -756,7 +756,7 @@ static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *vb) > V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; > vb->index = f->index; > vb->sequence = f->sequence; > - vb->timestamp = ns_to_timeval(f->ts); > + v4l2_buffer_set_timestamp(vb, f->ts); > vb->field = V4L2_FIELD_NONE; > vb->bytesused = f->scanlength; > > diff --git a/drivers/media/v4l2-core/videobuf-core.c b/drivers/media/v4l2-core/videobuf-core.c > index 939fc11cf080..ab650371c151 100644 > --- a/drivers/media/v4l2-core/videobuf-core.c > +++ b/drivers/media/v4l2-core/videobuf-core.c > @@ -364,7 +364,7 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b, > } > > b->field = vb->field; > - b->timestamp = ns_to_timeval(vb->ts); > + v4l2_buffer_set_timestamp(b, vb->ts); > b->bytesused = vb->size; > b->sequence = vb->field_count >> 1; > } > @@ -578,7 +578,7 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b) > || q->type == V4L2_BUF_TYPE_SDR_OUTPUT) { > buf->size = b->bytesused; > buf->field = b->field; > - buf->ts = v4l2_timeval_to_ns(&b->timestamp); > + buf->ts = v4l2_buffer_get_timestamp(b); > } > break; > case V4L2_MEMORY_USERPTR: > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > index 16c0ed6c50a7..4086036e37d5 100644 > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -56,7 +56,22 @@ > #ifndef __LINUX_VIDEODEV2_H > #define __LINUX_VIDEODEV2_H > > -#include <linux/time.h> /* need struct timeval */ > +#include <linux/time.h> > #include <uapi/linux/videodev2.h> > > +static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) > +{ > + return buf->timestamp.tv_sec * NSEC_PER_SEC + > + (u32)buf->timestamp.tv_usec * NSEC_PER_USEC; Why the (u32) cast? > +} > + > +static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf, > + u64 timestamp) > +{ > + struct timespec64 ts = ns_to_timespec64(timestamp); > + > + buf->timestamp.tv_sec = ts.tv_sec; > + buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > +} > + This does not belong in the public header. This is kernel specific, so media/v4l2-common.h would be a good place. Regards, Hans > #endif /* __LINUX_VIDEODEV2_H */ > diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h > index 83860de120e3..248bc09bfc99 100644 > --- a/include/trace/events/v4l2.h > +++ b/include/trace/events/v4l2.h > @@ -130,7 +130,7 @@ DECLARE_EVENT_CLASS(v4l2_event_class, > __entry->bytesused = buf->bytesused; > __entry->flags = buf->flags; > __entry->field = buf->field; > - __entry->timestamp = timeval_to_ns(&buf->timestamp); > + __entry->timestamp = v4l2_buffer_get_timestamp(buf); > __entry->timecode_type = buf->timecode.type; > __entry->timecode_flags = buf->timecode.flags; > __entry->timecode_frames = buf->timecode.frames; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 530638dffd93..74d3d522f3db 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -1010,6 +1010,7 @@ struct v4l2_buffer { > }; > }; > > +#ifndef __KERNEL__ > /** > * v4l2_timeval_to_ns - Convert timeval to nanoseconds > * @ts: pointer to the timeval variable to be converted > @@ -1021,6 +1022,7 @@ static inline __u64 v4l2_timeval_to_ns(const struct timeval *tv) > { > return (__u64)tv->tv_sec * 1000000000ULL + tv->tv_usec * 1000; > } > +#endif > > /* Flags for 'flags' field */ > /* Buffer is mapped (flag) */ >
On Mon, Nov 25, 2019 at 4:52 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 11/11/19 9:38 PM, Arnd Bergmann wrote: > > As a preparation for adding 64-bit time_t support in the uapi, > > change the drivers to no longer care about the format of the > > timestamp field in struct v4l2_buffer. > > > > The v4l2_timeval_to_ns() function is no longer needed in the > > kernel after this, but there may be userspace code relying on > > it because it is part of the uapi header. > > There is indeed userspace code that relies on this. Ok, good to know. I rephrased the changelog text as The v4l2_timeval_to_ns() function is no longer needed in the kernel after this, but there is userspace code relying on it to be part of the uapi header. > > > > +static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) > > +{ > > + return buf->timestamp.tv_sec * NSEC_PER_SEC + > > + (u32)buf->timestamp.tv_usec * NSEC_PER_USEC; > > Why the (u32) cast? Simple question, long answer: on 32-bit architectures, the tv_usec member may be 32-bit wide plus padding in user space when interpreted as a regular 'struct timeval', but the kernel implementation now sees it as a 64-bit member, with half of it being possibly uninitialized user space data. The 32-bit cast avoids that uninitialized data and ensures user space passing garbage in the upper half gets ignored, as it has to be on 32-bit user space. On 64-bit native user space, the tv_usec field is always 64 bit wide, so this is a change in behavior for denormalized timeval data with tv_usec > U32_MAX, but the current behavior does not appear worth preserving either. The correct way would probably be to return an error for tv_usec >USEC_PER_SEC, but as the code never did that, this would risk a regression for user space that relies on passing invalid timestamps without getting an error. > > +static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf, > > + u64 timestamp) > > +{ > > + struct timespec64 ts = ns_to_timespec64(timestamp); > > + > > + buf->timestamp.tv_sec = ts.tv_sec; > > + buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > > +} > > + > > This does not belong in the public header. This is kernel specific, Note: this is not the uapi header but the in-kernel one. > so media/v4l2-common.h would be a good place. Ok, sounds good. I wasn't sure where to put it, and ended up with include/linux/videodev2.h as the best replacement for include/uapi/linux/videodev2.h, changed it to include/media/v4l2-common.h now. Arnd
On 11/26/19 12:34 PM, Arnd Bergmann wrote: > On Mon, Nov 25, 2019 at 4:52 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> On 11/11/19 9:38 PM, Arnd Bergmann wrote: >>> As a preparation for adding 64-bit time_t support in the uapi, >>> change the drivers to no longer care about the format of the >>> timestamp field in struct v4l2_buffer. >>> >>> The v4l2_timeval_to_ns() function is no longer needed in the >>> kernel after this, but there may be userspace code relying on >>> it because it is part of the uapi header. >> >> There is indeed userspace code that relies on this. > > Ok, good to know. I rephrased the changelog text as > > The v4l2_timeval_to_ns() function is no longer needed in the > kernel after this, but there is userspace code relying on > it to be part of the uapi header. > >>> >>> +static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) >>> +{ >>> + return buf->timestamp.tv_sec * NSEC_PER_SEC + >>> + (u32)buf->timestamp.tv_usec * NSEC_PER_USEC; >> >> Why the (u32) cast? > > Simple question, long answer: > > on 32-bit architectures, the tv_usec member may be 32-bit wide plus > padding in user space when interpreted as a regular 'struct timeval', > but the kernel implementation now sees it as a 64-bit member, > with half of it being possibly uninitialized user space data. > > The 32-bit cast avoids that uninitialized data and ensures user space > passing garbage in the upper half gets ignored, as it has to be on 32-bit > user space. But that's only valid for little endian 32 bit systems, right? Is this only an issue for x86 platforms? > > On 64-bit native user space, the tv_usec field is always 64 bit wide, > so this is a change in behavior for denormalized timeval data > with tv_usec > U32_MAX, but the current behavior does not appear > worth preserving either. > > The correct way would probably be to return an error for > tv_usec >USEC_PER_SEC, but as the code never did that, this > would risk a regression for user space that relies on passing > invalid timestamps without getting an error. This long answer needs to be added to a comment to that function. Because otherwise someone will come along later and remove that seemingly unnecessary cast. It's OK if it is a long comment, it's a non-trivial reason. > >>> +static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf, >>> + u64 timestamp) >>> +{ >>> + struct timespec64 ts = ns_to_timespec64(timestamp); >>> + >>> + buf->timestamp.tv_sec = ts.tv_sec; >>> + buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC; >>> +} >>> + >> >> This does not belong in the public header. This is kernel specific, > > Note: this is not the uapi header but the in-kernel one. Ah, I missed that. > >> so media/v4l2-common.h would be a good place. > > Ok, sounds good. I wasn't sure where to put it, and ended up > with include/linux/videodev2.h as the best replacement for > include/uapi/linux/videodev2.h, changed it to > include/media/v4l2-common.h now. Never use include/linux/videodev2.h. It's just a wrapper around the uapi header and should not contain any 'real' code. It's also why I missed that you modified that header since we never touch it. Regards, Hans > > Arnd >
On Tue, Nov 26, 2019 at 12:43 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > On 11/26/19 12:34 PM, Arnd Bergmann wrote: > > On Mon, Nov 25, 2019 at 4:52 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > >>> > >>> +static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) > >>> +{ > >>> + return buf->timestamp.tv_sec * NSEC_PER_SEC + > >>> + (u32)buf->timestamp.tv_usec * NSEC_PER_USEC; > >> > >> Why the (u32) cast? > > > > Simple question, long answer: > > > > on 32-bit architectures, the tv_usec member may be 32-bit wide plus > > padding in user space when interpreted as a regular 'struct timeval', > > but the kernel implementation now sees it as a 64-bit member, > > with half of it being possibly uninitialized user space data. > > > > The 32-bit cast avoids that uninitialized data and ensures user space > > passing garbage in the upper half gets ignored, as it has to be on 32-bit > > user space. > > But that's only valid for little endian 32 bit systems, right? > Is this only an issue for x86 platforms? Uninitialized data is an issue on all 32-bit architectures. The layout of the new timeval is such that the low 32 bits of tv_sec are in the same place on both 32-bit and 64-bit architectures of the same endianess, but if an application initializes the fields individually without a memset before it, it may still pass invalid data. > > On 64-bit native user space, the tv_usec field is always 64 bit wide, > > so this is a change in behavior for denormalized timeval data > > with tv_usec > U32_MAX, but the current behavior does not appear > > worth preserving either. > > > > The correct way would probably be to return an error for > > tv_usec >USEC_PER_SEC, but as the code never did that, this > > would risk a regression for user space that relies on passing > > invalid timestamps without getting an error. > > This long answer needs to be added to a comment to that function. > Because otherwise someone will come along later and remove that > seemingly unnecessary cast. > > It's OK if it is a long comment, it's a non-trivial reason. Added this comment now: /* * When the timestamp comes from 32-bit user space, there may be * uninitialized data in tv_usec, so cast it to u32. * Otherwise allow invalid input for backwards compatibility. */ Let me know if you prefer a more elaborate version. > >> so media/v4l2-common.h would be a good place. > > > > Ok, sounds good. I wasn't sure where to put it, and ended up > > with include/linux/videodev2.h as the best replacement for > > include/uapi/linux/videodev2.h, changed it to > > include/media/v4l2-common.h now. > > Never use include/linux/videodev2.h. It's just a wrapper around > the uapi header and should not contain any 'real' code. > > It's also why I missed that you modified that header since we never > touch it. Ok, got it. I now tried to remove this file completely, hoping that the include <linux/time.h> is no longer needed after my series, but it seems we still need it. Arnd
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 5a9ba3846f0a..9ec710878db6 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -143,7 +143,7 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb) * and the timecode field and flag if needed. */ if (q->copy_timestamp) - vb->timestamp = v4l2_timeval_to_ns(&b->timestamp); + vb->timestamp = v4l2_buffer_get_timestamp(b); vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE; if (b->flags & V4L2_BUF_FLAG_TIMECODE) vbuf->timecode = b->timecode; @@ -476,7 +476,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) b->flags = vbuf->flags; b->field = vbuf->field; - b->timestamp = ns_to_timeval(vb->timestamp); + v4l2_buffer_set_timestamp(b, vb->timestamp); b->timecode = vbuf->timecode; b->sequence = vbuf->sequence; b->reserved2 = 0; diff --git a/drivers/media/pci/meye/meye.c b/drivers/media/pci/meye/meye.c index 0e61c81356ef..3a4c29bc0ba5 100644 --- a/drivers/media/pci/meye/meye.c +++ b/drivers/media/pci/meye/meye.c @@ -1266,7 +1266,7 @@ static int vidioc_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf) buf->flags |= V4L2_BUF_FLAG_DONE; buf->field = V4L2_FIELD_NONE; - buf->timestamp = ns_to_timeval(meye.grab_buffer[index].ts); + v4l2_buffer_set_timestamp(buf, meye.grab_buffer[index].ts); buf->sequence = meye.grab_buffer[index].sequence; buf->memory = V4L2_MEMORY_MMAP; buf->m.offset = index * gbufsize; @@ -1332,7 +1332,7 @@ static int vidioc_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) buf->bytesused = meye.grab_buffer[reqnr].size; buf->flags = V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; buf->field = V4L2_FIELD_NONE; - buf->timestamp = ns_to_timeval(meye.grab_buffer[reqnr].ts); + v4l2_buffer_set_timestamp(buf, meye.grab_buffer[reqnr].ts); buf->sequence = meye.grab_buffer[reqnr].sequence; buf->memory = V4L2_MEMORY_MMAP; buf->m.offset = reqnr * gbufsize; diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c index 626264a56517..9d3d05125d7b 100644 --- a/drivers/media/usb/cpia2/cpia2_v4l.c +++ b/drivers/media/usb/cpia2/cpia2_v4l.c @@ -800,7 +800,7 @@ static int cpia2_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf) break; case FRAME_READY: buf->bytesused = cam->buffers[buf->index].length; - buf->timestamp = ns_to_timeval(cam->buffers[buf->index].ts); + v4l2_buffer_set_timestamp(buf, cam->buffers[buf->index].ts); buf->sequence = cam->buffers[buf->index].seq; buf->flags = V4L2_BUF_FLAG_DONE; break; @@ -907,7 +907,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) buf->flags = V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; buf->field = V4L2_FIELD_NONE; - buf->timestamp = ns_to_timeval(cam->buffers[buf->index].ts); + v4l2_buffer_set_timestamp(buf, cam->buffers[buf->index].ts); buf->sequence = cam->buffers[buf->index].seq; buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer; buf->length = cam->frame_size; diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c index 21f90a887485..b22501f76b78 100644 --- a/drivers/media/usb/stkwebcam/stk-webcam.c +++ b/drivers/media/usb/stkwebcam/stk-webcam.c @@ -1125,7 +1125,7 @@ static int stk_vidioc_dqbuf(struct file *filp, sbuf->v4lbuf.flags &= ~V4L2_BUF_FLAG_QUEUED; sbuf->v4lbuf.flags |= V4L2_BUF_FLAG_DONE; sbuf->v4lbuf.sequence = ++dev->sequence; - sbuf->v4lbuf.timestamp = ns_to_timeval(ktime_get_ns()); + v4l2_buffer_set_timestamp(&sbuf->v4lbuf, ktime_get_ns()); *buf = sbuf->v4lbuf; return 0; diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c index cdc66adda755..15a423c5deb7 100644 --- a/drivers/media/usb/usbvision/usbvision-video.c +++ b/drivers/media/usb/usbvision/usbvision-video.c @@ -687,7 +687,7 @@ static int vidioc_querybuf(struct file *file, vb->length = usbvision->curwidth * usbvision->curheight * usbvision->palette.bytes_per_pixel; - vb->timestamp = ns_to_timeval(usbvision->frame[vb->index].ts); + v4l2_buffer_set_timestamp(vb, usbvision->frame[vb->index].ts); vb->sequence = usbvision->frame[vb->index].sequence; return 0; } @@ -756,7 +756,7 @@ static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *vb) V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; vb->index = f->index; vb->sequence = f->sequence; - vb->timestamp = ns_to_timeval(f->ts); + v4l2_buffer_set_timestamp(vb, f->ts); vb->field = V4L2_FIELD_NONE; vb->bytesused = f->scanlength; diff --git a/drivers/media/v4l2-core/videobuf-core.c b/drivers/media/v4l2-core/videobuf-core.c index 939fc11cf080..ab650371c151 100644 --- a/drivers/media/v4l2-core/videobuf-core.c +++ b/drivers/media/v4l2-core/videobuf-core.c @@ -364,7 +364,7 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b, } b->field = vb->field; - b->timestamp = ns_to_timeval(vb->ts); + v4l2_buffer_set_timestamp(b, vb->ts); b->bytesused = vb->size; b->sequence = vb->field_count >> 1; } @@ -578,7 +578,7 @@ int videobuf_qbuf(struct videobuf_queue *q, struct v4l2_buffer *b) || q->type == V4L2_BUF_TYPE_SDR_OUTPUT) { buf->size = b->bytesused; buf->field = b->field; - buf->ts = v4l2_timeval_to_ns(&b->timestamp); + buf->ts = v4l2_buffer_get_timestamp(b); } break; case V4L2_MEMORY_USERPTR: diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 16c0ed6c50a7..4086036e37d5 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -56,7 +56,22 @@ #ifndef __LINUX_VIDEODEV2_H #define __LINUX_VIDEODEV2_H -#include <linux/time.h> /* need struct timeval */ +#include <linux/time.h> #include <uapi/linux/videodev2.h> +static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf) +{ + return buf->timestamp.tv_sec * NSEC_PER_SEC + + (u32)buf->timestamp.tv_usec * NSEC_PER_USEC; +} + +static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf, + u64 timestamp) +{ + struct timespec64 ts = ns_to_timespec64(timestamp); + + buf->timestamp.tv_sec = ts.tv_sec; + buf->timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC; +} + #endif /* __LINUX_VIDEODEV2_H */ diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h index 83860de120e3..248bc09bfc99 100644 --- a/include/trace/events/v4l2.h +++ b/include/trace/events/v4l2.h @@ -130,7 +130,7 @@ DECLARE_EVENT_CLASS(v4l2_event_class, __entry->bytesused = buf->bytesused; __entry->flags = buf->flags; __entry->field = buf->field; - __entry->timestamp = timeval_to_ns(&buf->timestamp); + __entry->timestamp = v4l2_buffer_get_timestamp(buf); __entry->timecode_type = buf->timecode.type; __entry->timecode_flags = buf->timecode.flags; __entry->timecode_frames = buf->timecode.frames; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 530638dffd93..74d3d522f3db 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1010,6 +1010,7 @@ struct v4l2_buffer { }; }; +#ifndef __KERNEL__ /** * v4l2_timeval_to_ns - Convert timeval to nanoseconds * @ts: pointer to the timeval variable to be converted @@ -1021,6 +1022,7 @@ static inline __u64 v4l2_timeval_to_ns(const struct timeval *tv) { return (__u64)tv->tv_sec * 1000000000ULL + tv->tv_usec * 1000; } +#endif /* Flags for 'flags' field */ /* Buffer is mapped (flag) */
As a preparation for adding 64-bit time_t support in the uapi, change the drivers to no longer care about the format of the timestamp field in struct v4l2_buffer. The v4l2_timeval_to_ns() function is no longer needed in the kernel after this, but there may be userspace code relying on it because it is part of the uapi header. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 ++-- drivers/media/pci/meye/meye.c | 4 ++-- drivers/media/usb/cpia2/cpia2_v4l.c | 4 ++-- drivers/media/usb/stkwebcam/stk-webcam.c | 2 +- drivers/media/usb/usbvision/usbvision-video.c | 4 ++-- drivers/media/v4l2-core/videobuf-core.c | 4 ++-- include/linux/videodev2.h | 17 ++++++++++++++++- include/trace/events/v4l2.h | 2 +- include/uapi/linux/videodev2.h | 2 ++ 9 files changed, 30 insertions(+), 13 deletions(-)