linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] [media] y2038 conversion for subsystem
@ 2015-09-17 21:19 Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout Arnd Bergmann
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

This is my second attempt to convert subsystem-wide code in v4l
for y2038 changes, removing uses of time_t in common files
and adding support for user space that defines time_t as 64 bit.

Based on the initial feedback from Hans Verkuil, I've changed the
ioctl handling to remain 100% compatible with existing headers,
which also makes it more likely that existing source code can
compile without changes.

This comes at a noticeable expense of adding complexity to
the v4l2-ioctl.c file, as we now have to handle two versions
of each ioctl command that passes a time_t in its arguments.

I have not added support for the new binary layout of v4l2_timeval
to v4l2-compat-ioctl32 yet, that is something I can do when the
basic approach has been agreed on.

	Arnd

Arnd Bergmann (9):
  [media] dvb: use ktime_t for internal timeout
  [media] dvb: remove unused systime() function
  [media] dvb: don't use 'time_t' in event ioctl
  [media] exynos4-is: use monotonic timestamps as advertized
  [media] make VIDIOC_DQEVENT work with 64-bit time_t
  [media] use v4l2_get_timestamp where possible
  [media] v4l2: introduce v4l2_timeval
  [media] handle 64-bit time_t in v4l2_buffer
  [media] omap3isp: support 64-bit version of omap3isp_stat_data

 drivers/media/dvb-core/demux.h                   |   2 +-
 drivers/media/dvb-core/dmxdev.c                  |   2 +-
 drivers/media/dvb-core/dvb_demux.c               |  17 +-
 drivers/media/dvb-core/dvb_demux.h               |   4 +-
 drivers/media/dvb-core/dvb_net.c                 |   2 +-
 drivers/media/dvb-frontends/dibx000_common.c     |  10 -
 drivers/media/dvb-frontends/dibx000_common.h     |   2 -
 drivers/media/pci/bt8xx/bttv-driver.c            |   7 +-
 drivers/media/pci/cx18/cx18-mailbox.c            |   2 +-
 drivers/media/pci/meye/meye.h                    |   2 +-
 drivers/media/pci/zoran/zoran.h                  |   2 +-
 drivers/media/platform/coda/coda.h               |   2 +-
 drivers/media/platform/exynos4-is/fimc-capture.c |   8 +-
 drivers/media/platform/exynos4-is/fimc-lite.c    |   7 +-
 drivers/media/platform/omap/omap_vout.c          |   4 +-
 drivers/media/platform/omap3isp/isph3a_aewb.c    |   2 +
 drivers/media/platform/omap3isp/isph3a_af.c      |   2 +
 drivers/media/platform/omap3isp/isphist.c        |   2 +
 drivers/media/platform/omap3isp/ispstat.c        |  20 +-
 drivers/media/platform/omap3isp/ispstat.h        |   4 +-
 drivers/media/platform/s3c-camif/camif-capture.c |   8 +-
 drivers/media/platform/vim2m.c                   |   2 +-
 drivers/media/platform/vivid/vivid-ctrls.c       |   2 +-
 drivers/media/usb/cpia2/cpia2.h                  |   2 +-
 drivers/media/usb/cpia2/cpia2_v4l.c              |   2 +-
 drivers/media/usb/gspca/gspca.c                  |   6 +-
 drivers/media/usb/usbvision/usbvision.h          |   2 +-
 drivers/media/v4l2-core/v4l2-common.c            |   6 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c    |  35 ----
 drivers/media/v4l2-core/v4l2-dev.c               |   1 +
 drivers/media/v4l2-core/v4l2-event.c             |  35 +++-
 drivers/media/v4l2-core/v4l2-ioctl.c             | 227 ++++++++++++++++++++---
 drivers/media/v4l2-core/v4l2-subdev.c            |   6 +
 drivers/staging/media/omap4iss/iss_video.c       |   5 +-
 include/media/v4l2-common.h                      |   2 +-
 include/media/v4l2-event.h                       |   2 +
 include/media/videobuf-core.h                    |   2 +-
 include/trace/events/v4l2.h                      |  12 +-
 include/uapi/linux/dvb/video.h                   |   3 +-
 include/uapi/linux/omap3isp.h                    |  19 ++
 include/uapi/linux/videodev2.h                   |  78 ++++++++
 41 files changed, 415 insertions(+), 145 deletions(-)

-- 
2.1.0.rc2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 2/9] [media] dvb: remove unused systime() function Arnd Bergmann
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

The dvb demuxer code uses a 'struct timespec' to pass a timeout
as absolute time. This will cause problems on 32-bit architectures
in 2038 when time_t overflows, and it is racy with a concurrent
settimeofday() call.

This patch changes the code to use ktime_get() instead, using
the monotonic time base to avoid both the race and the overflow.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/dvb-core/demux.h     |  2 +-
 drivers/media/dvb-core/dmxdev.c    |  2 +-
 drivers/media/dvb-core/dvb_demux.c | 17 ++++++-----------
 drivers/media/dvb-core/dvb_demux.h |  4 ++--
 drivers/media/dvb-core/dvb_net.c   |  2 +-
 5 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
index 833191bcd810..d8e2b1213bef 100644
--- a/drivers/media/dvb-core/demux.h
+++ b/drivers/media/dvb-core/demux.h
@@ -92,7 +92,7 @@ struct dmx_ts_feed {
 		    int type,
 		    enum dmx_ts_pes pes_type,
 		    size_t circular_buffer_size,
-		    struct timespec timeout);
+		    ktime_t timeout);
 	int (*start_filtering) (struct dmx_ts_feed* feed);
 	int (*stop_filtering) (struct dmx_ts_feed* feed);
 };
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index d0e3f9d85f34..0d20b379eeec 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -558,7 +558,7 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
 				 struct dmxdev_filter *filter,
 				 struct dmxdev_feed *feed)
 {
-	struct timespec timeout = { 0 };
+	ktime_t timeout = ktime_set(0, 0);
 	struct dmx_pes_filter_params *para = &filter->params.pes;
 	dmx_output_t otype;
 	int ret;
diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c
index 6c7ff0cdcd32..f9f676112d30 100644
--- a/drivers/media/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb-core/dvb_demux.c
@@ -399,28 +399,23 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf)
 	int dvr_done = 0;
 
 	if (dvb_demux_speedcheck) {
-		struct timespec cur_time, delta_time;
+		ktime_t cur_time;
 		u64 speed_bytes, speed_timedelta;
 
 		demux->speed_pkts_cnt++;
 
 		/* show speed every SPEED_PKTS_INTERVAL packets */
 		if (!(demux->speed_pkts_cnt % SPEED_PKTS_INTERVAL)) {
-			cur_time = current_kernel_time();
+			cur_time = ktime_get();
 
-			if (demux->speed_last_time.tv_sec != 0 &&
-					demux->speed_last_time.tv_nsec != 0) {
-				delta_time = timespec_sub(cur_time,
-						demux->speed_last_time);
+			if (ktime_to_ns(demux->speed_last_time) != 0) {
 				speed_bytes = (u64)demux->speed_pkts_cnt
 					* 188 * 8;
 				/* convert to 1024 basis */
 				speed_bytes = 1000 * div64_u64(speed_bytes,
 						1024);
-				speed_timedelta =
-					(u64)timespec_to_ns(&delta_time);
-				speed_timedelta = div64_u64(speed_timedelta,
-						1000000); /* nsec -> usec */
+				speed_timedelta = ktime_ms_delta(cur_time,
+							demux->speed_last_time);
 				printk(KERN_INFO "TS speed %llu Kbits/sec \n",
 						div64_u64(speed_bytes,
 							speed_timedelta));
@@ -667,7 +662,7 @@ out:
 
 static int dmx_ts_feed_set(struct dmx_ts_feed *ts_feed, u16 pid, int ts_type,
 			   enum dmx_ts_pes pes_type,
-			   size_t circular_buffer_size, struct timespec timeout)
+			   size_t circular_buffer_size, ktime_t timeout)
 {
 	struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
 	struct dvb_demux *demux = feed->demux;
diff --git a/drivers/media/dvb-core/dvb_demux.h b/drivers/media/dvb-core/dvb_demux.h
index ae7fc33c3231..5ed3cab4ad28 100644
--- a/drivers/media/dvb-core/dvb_demux.h
+++ b/drivers/media/dvb-core/dvb_demux.h
@@ -83,7 +83,7 @@ struct dvb_demux_feed {
 	u8 *buffer;
 	int buffer_size;
 
-	struct timespec timeout;
+	ktime_t timeout;
 	struct dvb_demux_filter *filter;
 
 	int ts_type;
@@ -134,7 +134,7 @@ struct dvb_demux {
 
 	uint8_t *cnt_storage; /* for TS continuity check */
 
-	struct timespec speed_last_time; /* for TS speed check */
+	ktime_t speed_last_time; /* for TS speed check */
 	uint32_t speed_pkts_cnt; /* for TS speed check */
 };
 
diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index b81e026edab3..df3ba15c007e 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -998,7 +998,7 @@ static int dvb_net_feed_start(struct net_device *dev)
 		netdev_dbg(dev, "start filtering\n");
 		priv->secfeed->start_filtering(priv->secfeed);
 	} else if (priv->feedtype == DVB_NET_FEEDTYPE_ULE) {
-		struct timespec timeout = { 0, 10000000 }; // 10 msec
+		ktime_t timeout = ns_to_ktime(10 * NSEC_PER_MSEC);
 
 		/* we have payloads encapsulated in TS */
 		netdev_dbg(dev, "alloc tsfeed\n");
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/9] [media] dvb: remove unused systime() function
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 3/9] [media] dvb: don't use 'time_t' in event ioctl Arnd Bergmann
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

The systime function uses struct timespec, which we want to stop
using in the kernel because it overflows in 2038. Fortunately,
this use in dibx000_common is in a function that is never called,
so we can just remove it.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/dvb-frontends/dibx000_common.c | 10 ----------
 drivers/media/dvb-frontends/dibx000_common.h |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/media/dvb-frontends/dibx000_common.c b/drivers/media/dvb-frontends/dibx000_common.c
index 43be7238311e..79cb007b401f 100644
--- a/drivers/media/dvb-frontends/dibx000_common.c
+++ b/drivers/media/dvb-frontends/dibx000_common.c
@@ -500,16 +500,6 @@ void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst)
 }
 EXPORT_SYMBOL(dibx000_exit_i2c_master);
 
-
-u32 systime(void)
-{
-	struct timespec t;
-
-	t = current_kernel_time();
-	return (t.tv_sec * 10000) + (t.tv_nsec / 100000);
-}
-EXPORT_SYMBOL(systime);
-
 MODULE_AUTHOR("Patrick Boettcher <pboettcher@dibcom.fr>");
 MODULE_DESCRIPTION("Common function the DiBcom demodulator family");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/dibx000_common.h b/drivers/media/dvb-frontends/dibx000_common.h
index b538e0555c95..61f4152f24ee 100644
--- a/drivers/media/dvb-frontends/dibx000_common.h
+++ b/drivers/media/dvb-frontends/dibx000_common.h
@@ -47,8 +47,6 @@ extern void dibx000_exit_i2c_master(struct dibx000_i2c_master *mst);
 extern void dibx000_reset_i2c_master(struct dibx000_i2c_master *mst);
 extern int dibx000_i2c_set_speed(struct i2c_adapter *i2c_adap, u16 speed);
 
-extern u32 systime(void);
-
 #define BAND_LBAND 0x01
 #define BAND_UHF   0x02
 #define BAND_VHF   0x04
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/9] [media] dvb: don't use 'time_t' in event ioctl
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 2/9] [media] dvb: remove unused systime() function Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 4/9] [media] exynos4-is: use monotonic timestamps as advertized Arnd Bergmann
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

'struct video_event' is used for the VIDEO_GET_EVENT ioctl, implemented
by drivers/media/pci/ivtv/ivtv-ioctl.c and
drivers/media/pci/ttpci/av7110_av.c. The structure contains a 'time_t',
which will be redefined in the future to be 64-bit wide, causing an
incompatible ABI change for this ioctl.

As it turns out, neither of the drivers currently sets the timestamp
field, and it is presumably useless anyway because of the limited
resolutions (no sub-second times). This means we can simply change
the structure definition to use a 'long' instead of 'time_t' and
remain compatible with all existing user space binaries when time_t
gets changed.

If anybody ever starts using this field, they have to make sure not
to use 1970 based seconds in there, as those overflow in 2038.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/uapi/linux/dvb/video.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
index d3d14a59d2d5..6c7f9298d7c2 100644
--- a/include/uapi/linux/dvb/video.h
+++ b/include/uapi/linux/dvb/video.h
@@ -135,7 +135,8 @@ struct video_event {
 #define VIDEO_EVENT_FRAME_RATE_CHANGED	2
 #define VIDEO_EVENT_DECODER_STOPPED 	3
 #define VIDEO_EVENT_VSYNC 		4
-	__kernel_time_t timestamp;
+	/* unused, make sure to use atomic time for y2038 if it ever gets used */
+	long timestamp;
 	union {
 		video_size_t size;
 		unsigned int frame_rate;	/* in frames per 1000sec */
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/9] [media] exynos4-is: use monotonic timestamps as advertized
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (2 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 3/9] [media] dvb: don't use 'time_t' in event ioctl Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 5/9] [media] make VIDIOC_DQEVENT work with 64-bit time_t Arnd Bergmann
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

The exynos4 fimc capture driver claims to use monotonic
timestamps but calls ktime_get_real_ts(). This is both
an incorrect API use, and a bad idea because of the y2038
problem and the fact that the wall clock time is not reliable
for timestamps across suspend or settimeofday().

This changes the driver to use the normal v4l2_get_timestamp()
function like all other drivers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/platform/exynos4-is/fimc-capture.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index cfebf292e15a..776ea6d78d03 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -183,8 +183,6 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
 	struct v4l2_subdev *csis = p->subdevs[IDX_CSIS];
 	struct fimc_frame *f = &cap->ctx->d_frame;
 	struct fimc_vid_buffer *v_buf;
-	struct timeval *tv;
-	struct timespec ts;
 
 	if (test_and_clear_bit(ST_CAPT_SHUT, &fimc->state)) {
 		wake_up(&fimc->irq_queue);
@@ -193,13 +191,9 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
 
 	if (!list_empty(&cap->active_buf_q) &&
 	    test_bit(ST_CAPT_RUN, &fimc->state) && deq_buf) {
-		ktime_get_real_ts(&ts);
-
 		v_buf = fimc_active_queue_pop(cap);
 
-		tv = &v_buf->vb.v4l2_buf.timestamp;
-		tv->tv_sec = ts.tv_sec;
-		tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+		v4l2_get_timestamp(&v_buf->vb.v4l2_buf.timestamp);
 		v_buf->vb.v4l2_buf.sequence = cap->frame_count++;
 
 		vb2_buffer_done(&v_buf->vb, VB2_BUF_STATE_DONE);
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 5/9] [media] make VIDIOC_DQEVENT work with 64-bit time_t
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (3 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 4/9] [media] exynos4-is: use monotonic timestamps as advertized Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 6/9] [media] use v4l2_get_timestamp where possible Arnd Bergmann
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

The v4l2 event queue uses a 'struct timespec' to pass monotonic
timestamps. This is not a problem by itself, but it breaks when user
space redefines timespec to use 'long long' on 32-bit systems.

This is the second approach on fixing the problem, by changing
the kernel to internally use 64-bit members for the timestamps
in struct v4l2_event, and letting user space use either version.

Unfortunately, we cannot use timespec64 here, because that does
not have a well-defined ABI yet, and differs from the 64-bit
version of 'timespec'. Using a pair of __s64 members makes sure
the structure is well-defined and contains no padding that might
leak kernel stack data.

We now need the handling for VIDIOC_DQEVENT32 on both 32-bit
and 64-bit architectures, so the handling for that is moved from
v4l2-compat-ioctl32.c to v4l2-ioctl.c and v4l2-subdev.c.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 35 ------------------
 drivers/media/v4l2-core/v4l2-event.c          | 35 +++++++++++++++---
 drivers/media/v4l2-core/v4l2-ioctl.c          | 53 ++++++++++++++++++++-------
 drivers/media/v4l2-core/v4l2-subdev.c         |  6 +++
 include/media/v4l2-event.h                    |  2 +
 include/uapi/linux/videodev2.h                | 31 ++++++++++++++++
 6 files changed, 107 insertions(+), 55 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index af635430524e..9ffe7302206e 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -735,32 +735,6 @@ static int put_v4l2_ext_controls32(struct v4l2_ext_controls *kp, struct v4l2_ext
 	return 0;
 }
 
-struct v4l2_event32 {
-	__u32				type;
-	union {
-		__u8			data[64];
-	} u;
-	__u32				pending;
-	__u32				sequence;
-	struct compat_timespec		timestamp;
-	__u32				id;
-	__u32				reserved[8];
-};
-
-static int put_v4l2_event32(struct v4l2_event *kp, struct v4l2_event32 __user *up)
-{
-	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_event32)) ||
-		put_user(kp->type, &up->type) ||
-		copy_to_user(&up->u, &kp->u, sizeof(kp->u)) ||
-		put_user(kp->pending, &up->pending) ||
-		put_user(kp->sequence, &up->sequence) ||
-		compat_put_timespec(&kp->timestamp, &up->timestamp) ||
-		put_user(kp->id, &up->id) ||
-		copy_to_user(up->reserved, kp->reserved, 8 * sizeof(__u32)))
-			return -EFAULT;
-	return 0;
-}
-
 struct v4l2_edid32 {
 	__u32 pad;
 	__u32 start_block;
@@ -814,7 +788,6 @@ static int put_v4l2_edid32(struct v4l2_edid *kp, struct v4l2_edid32 __user *up)
 #define VIDIOC_G_EXT_CTRLS32    _IOWR('V', 71, struct v4l2_ext_controls32)
 #define VIDIOC_S_EXT_CTRLS32    _IOWR('V', 72, struct v4l2_ext_controls32)
 #define VIDIOC_TRY_EXT_CTRLS32  _IOWR('V', 73, struct v4l2_ext_controls32)
-#define	VIDIOC_DQEVENT32	_IOR ('V', 89, struct v4l2_event32)
 #define VIDIOC_CREATE_BUFS32	_IOWR('V', 92, struct v4l2_create_buffers32)
 #define VIDIOC_PREPARE_BUF32	_IOWR('V', 93, struct v4l2_buffer32)
 
@@ -860,7 +833,6 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	case VIDIOC_G_EXT_CTRLS32: cmd = VIDIOC_G_EXT_CTRLS; break;
 	case VIDIOC_S_EXT_CTRLS32: cmd = VIDIOC_S_EXT_CTRLS; break;
 	case VIDIOC_TRY_EXT_CTRLS32: cmd = VIDIOC_TRY_EXT_CTRLS; break;
-	case VIDIOC_DQEVENT32: cmd = VIDIOC_DQEVENT; break;
 	case VIDIOC_OVERLAY32: cmd = VIDIOC_OVERLAY; break;
 	case VIDIOC_STREAMON32: cmd = VIDIOC_STREAMON; break;
 	case VIDIOC_STREAMOFF32: cmd = VIDIOC_STREAMOFF; break;
@@ -940,9 +912,6 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		err = get_v4l2_ext_controls32(&karg.v2ecs, up);
 		compatible_arg = 0;
 		break;
-	case VIDIOC_DQEVENT:
-		compatible_arg = 0;
-		break;
 	}
 	if (err)
 		return err;
@@ -983,10 +952,6 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		err = put_v4l2_framebuffer32(&karg.v2fb, up);
 		break;
 
-	case VIDIOC_DQEVENT:
-		err = put_v4l2_event32(&karg.v2ev, up);
-		break;
-
 	case VIDIOC_G_EDID:
 	case VIDIOC_S_EDID:
 		err = put_v4l2_edid32(&karg.v2edid, up);
diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 8d3171c6bee8..149342490e91 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -92,6 +92,28 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
 }
 EXPORT_SYMBOL_GPL(v4l2_event_dequeue);
 
+int v4l2_event_dequeue32(struct v4l2_fh *fh, struct v4l2_event32 *event32,
+		       int nonblocking)
+{
+	struct v4l2_event event64;
+	int ret;
+
+	ret = v4l2_event_dequeue(fh, &event64, nonblocking);
+	if (ret)
+		return ret;
+
+	/* only the timestamp differs, so use memcpy to copy everything else */
+	memcpy(event32, &event64, offsetof(struct v4l2_event32, timestamp));
+	event32->timestamp.tv_sec = (long)event64.timestamp.tv_sec;
+	event32->timestamp.tv_nsec = (long)event64.timestamp.tv_nsec;
+	memcpy(&event32->id, &event64.id,
+	       sizeof(event32->id) + sizeof(event32->reserved));
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_event_dequeue32);
+
+
 /* Caller must hold fh->vdev->fh_lock! */
 static struct v4l2_subscribed_event *v4l2_event_subscribed(
 		struct v4l2_fh *fh, u32 type, u32 id)
@@ -108,7 +130,7 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
 }
 
 static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev,
-		const struct timespec *ts)
+		const struct timespec64 *ts)
 {
 	struct v4l2_subscribed_event *sev;
 	struct v4l2_kevent *kev;
@@ -156,7 +178,8 @@ static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *e
 	if (copy_payload)
 		kev->event.u = ev->u;
 	kev->event.id = ev->id;
-	kev->event.timestamp = *ts;
+	kev->event.timestamp.tv_sec = ts->tv_sec;
+	kev->event.timestamp.tv_nsec = ts->tv_nsec;
 	kev->event.sequence = fh->sequence;
 	sev->in_use++;
 	list_add_tail(&kev->list, &fh->available);
@@ -170,12 +193,12 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
 {
 	struct v4l2_fh *fh;
 	unsigned long flags;
-	struct timespec timestamp;
+	struct timespec64 timestamp;
 
 	if (vdev == NULL)
 		return;
 
-	ktime_get_ts(&timestamp);
+	ktime_get_ts64(&timestamp);
 
 	spin_lock_irqsave(&vdev->fh_lock, flags);
 
@@ -189,9 +212,9 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
 void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev)
 {
 	unsigned long flags;
-	struct timespec timestamp;
+	struct timespec64 timestamp;
 
-	ktime_get_ts(&timestamp);
+	ktime_get_ts64(&timestamp);
 
 	spin_lock_irqsave(&fh->vdev->fh_lock, flags);
 	__v4l2_event_queue_fh(fh, ev, &timestamp);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 4a384fc765b8..7aab18dd2ca5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -797,22 +797,18 @@ static void v4l_print_frmivalenum(const void *arg, bool write_only)
 	}
 }
 
-static void v4l_print_event(const void *arg, bool write_only)
+static void v4l_print_event_data(const void *arg, bool write_only, u32 type)
 {
-	const struct v4l2_event *p = arg;
-	const struct v4l2_event_ctrl *c;
+	const struct v4l2_event_vsync *vsync = arg;
+	const struct v4l2_event_ctrl *c = arg;
+	const struct v4l2_event_frame_sync *frame_sync = arg;
 
-	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, "
-		"timestamp=%lu.%9.9lu\n",
-			p->type, p->pending, p->sequence, p->id,
-			p->timestamp.tv_sec, p->timestamp.tv_nsec);
-	switch (p->type) {
+	switch (type) {
 	case V4L2_EVENT_VSYNC:
 		printk(KERN_DEBUG "field=%s\n",
-			prt_names(p->u.vsync.field, v4l2_field_names));
+			prt_names(vsync->field, v4l2_field_names));
 		break;
 	case V4L2_EVENT_CTRL:
-		c = &p->u.ctrl;
 		printk(KERN_DEBUG "changes=0x%x, type=%u, ",
 			c->changes, c->type);
 		if (c->type == V4L2_CTRL_TYPE_INTEGER64)
@@ -825,12 +821,34 @@ static void v4l_print_event(const void *arg, bool write_only)
 			c->step, c->default_value);
 		break;
 	case V4L2_EVENT_FRAME_SYNC:
-		pr_cont("frame_sequence=%u\n",
-			p->u.frame_sync.frame_sequence);
+		pr_cont("frame_sequence=%u\n", frame_sync->frame_sequence);
 		break;
 	}
 }
 
+static void v4l_print_event32(const void *arg, bool write_only)
+{
+	const struct v4l2_event32 *p = arg;
+	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, "
+			"timestamp=%d.%9.9u\n",
+			p->type, p->pending, p->sequence, p->id,
+			p->timestamp.tv_sec, p->timestamp.tv_nsec);
+
+	return v4l_print_event_data(&p->u, write_only, p->type);
+}
+
+static void v4l_print_event64(const void *arg, bool write_only)
+{
+	const struct v4l2_event *p = arg;
+
+	pr_cont("type=0x%x, pending=%u, sequence=%u, id=%u, "
+			"timestamp=%lld.%9.9llu\n",
+			p->type, p->pending, p->sequence, p->id,
+			p->timestamp.tv_sec, p->timestamp.tv_nsec);
+
+	return v4l_print_event_data(&p->u, write_only, p->type);
+}
+
 static void v4l_print_event_subscription(const void *arg, bool write_only)
 {
 	const struct v4l2_event_subscription *p = arg;
@@ -2235,12 +2253,18 @@ static int v4l_dbg_g_chip_info(const struct v4l2_ioctl_ops *ops,
 #endif
 }
 
-static int v4l_dqevent(const struct v4l2_ioctl_ops *ops,
+static int v4l_dqevent64(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
 	return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
 }
 
+static int v4l_dqevent32(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	return v4l2_event_dequeue32(fh, arg, file->f_flags & O_NONBLOCK);
+}
+
 static int v4l_subscribe_event(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
@@ -2449,7 +2473,8 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO_FNC(VIDIOC_S_HW_FREQ_SEEK, v4l_s_hw_freq_seek, v4l_print_hw_freq_seek, INFO_FL_PRIO),
 	IOCTL_INFO_STD(VIDIOC_S_DV_TIMINGS, vidioc_s_dv_timings, v4l_print_dv_timings, INFO_FL_PRIO),
 	IOCTL_INFO_STD(VIDIOC_G_DV_TIMINGS, vidioc_g_dv_timings, v4l_print_dv_timings, 0),
-	IOCTL_INFO_FNC(VIDIOC_DQEVENT, v4l_dqevent, v4l_print_event, 0),
+	IOCTL_INFO_FNC(VIDIOC_DQEVENT, v4l_dqevent64, v4l_print_event64, 0),
+	IOCTL_INFO_FNC(VIDIOC_DQEVENT32, v4l_dqevent32, v4l_print_event32, 0),
 	IOCTL_INFO_FNC(VIDIOC_SUBSCRIBE_EVENT, v4l_subscribe_event, v4l_print_event_subscription, 0),
 	IOCTL_INFO_FNC(VIDIOC_UNSUBSCRIBE_EVENT, v4l_unsubscribe_event, v4l_print_event_subscription, 0),
 	IOCTL_INFO_FNC(VIDIOC_CREATE_BUFS, v4l_create_bufs, v4l_print_create_buffers, INFO_FL_PRIO | INFO_FL_QUEUE),
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 83615b8fb46a..9b0cde143c6c 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -211,6 +211,12 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_TRY_EXT_CTRLS:
 		return v4l2_try_ext_ctrls(vfh->ctrl_handler, arg);
 
+	case VIDIOC_DQEVENT32:
+		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
+			return -ENOIOCTLCMD;
+
+		return v4l2_event_dequeue32(vfh, arg, file->f_flags & O_NONBLOCK);
+
 	case VIDIOC_DQEVENT:
 		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
 			return -ENOIOCTLCMD;
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index 9792f906423b..5e8ce27a0671 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -124,6 +124,8 @@ struct v4l2_subscribed_event {
 
 int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
 		       int nonblocking);
+int v4l2_event_dequeue32(struct v4l2_fh *fh, struct v4l2_event32 *event,
+		       int nonblocking);
 void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev);
 void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev);
 int v4l2_event_pending(struct v4l2_fh *fh);
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3228fbebcd63..3e2c497c31fd 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2082,10 +2082,40 @@ struct v4l2_event {
 	} u;
 	__u32				pending;
 	__u32				sequence;
+#ifdef __KERNEL__
+	struct {
+		__s64			tv_sec;
+		__s64			tv_nsec;
+	} timestamp;
+#else
 	struct timespec			timestamp;
+#endif
+	__u32				id;
+	__u32				reserved[8];
+};
+
+#ifdef __KERNEL__
+/*
+ * User space will see either 64-bit or 32-bit time_t, which changes
+ * the v4l2_event layout. Both are y2038 safe because the timestamps
+ * are in monotonic time, but the kernel has to handle both cases.
+ */
+struct v4l2_event32 {
+	__u32				type;
+	union {
+		__u8			data[64];
+	} u;
+	__u32				pending;
+	__u32				sequence;
+	struct {
+		__s32			tv_sec;
+		__s32			tv_nsec;
+	}				timestamp;
+
 	__u32				id;
 	__u32				reserved[8];
 };
+#endif
 
 #define V4L2_EVENT_SUB_FL_SEND_INITIAL		(1 << 0)
 #define V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK	(1 << 1)
@@ -2237,6 +2267,7 @@ struct v4l2_create_buffers {
 #define	VIDIOC_S_DV_TIMINGS	_IOWR('V', 87, struct v4l2_dv_timings)
 #define	VIDIOC_G_DV_TIMINGS	_IOWR('V', 88, struct v4l2_dv_timings)
 #define	VIDIOC_DQEVENT		 _IOR('V', 89, struct v4l2_event)
+#define	VIDIOC_DQEVENT32	 _IOR('V', 89, struct v4l2_event32)
 #define	VIDIOC_SUBSCRIBE_EVENT	 _IOW('V', 90, struct v4l2_event_subscription)
 #define	VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription)
 
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 6/9] [media] use v4l2_get_timestamp where possible
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (4 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 5/9] [media] make VIDIOC_DQEVENT work with 64-bit time_t Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

This is a preparation for a change to the type of v4l2 timestamps.
v4l2_get_timestamp() is a helper function that reads the monotonic
time and stores it into a 'struct timeval'. Multiple drivers implement
the same thing themselves for historic reasons.

Changing them all to use v4l2_get_timestamp() is more consistent
and reduces the amount of code duplication, and most importantly
simplifies the following changes.

If desired, this patch can easily be split up into one patch per
driver.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/pci/bt8xx/bttv-driver.c            | 5 +----
 drivers/media/pci/cx18/cx18-mailbox.c            | 2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c    | 7 +------
 drivers/media/platform/omap3isp/ispstat.c        | 5 ++---
 drivers/media/platform/omap3isp/ispstat.h        | 2 +-
 drivers/media/platform/s3c-camif/camif-capture.c | 8 +-------
 drivers/media/usb/gspca/gspca.c                  | 4 ++--
 drivers/media/v4l2-core/v4l2-dev.c               | 1 +
 drivers/staging/media/omap4iss/iss_video.c       | 5 +----
 9 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 3632958f2158..15a4ebc2844d 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3625,13 +3625,10 @@ static void
 bttv_irq_wakeup_vbi(struct bttv *btv, struct bttv_buffer *wakeup,
 		    unsigned int state)
 {
-	struct timeval ts;
-
 	if (NULL == wakeup)
 		return;
 
-	v4l2_get_timestamp(&ts);
-	wakeup->vb.ts = ts;
+	v4l2_get_timestamp(&wakeup->vb.ts);
 	wakeup->vb.field_count = btv->field_count;
 	wakeup->vb.state = state;
 	wake_up(&wakeup->vb.done);
diff --git a/drivers/media/pci/cx18/cx18-mailbox.c b/drivers/media/pci/cx18/cx18-mailbox.c
index eabf00c6351b..1f8aa9a749a1 100644
--- a/drivers/media/pci/cx18/cx18-mailbox.c
+++ b/drivers/media/pci/cx18/cx18-mailbox.c
@@ -202,7 +202,7 @@ static void cx18_mdl_send_to_videobuf(struct cx18_stream *s,
 	}
 
 	if (dispatch) {
-		vb_buf->vb.ts = ktime_to_timeval(ktime_get());
+		v4l2_get_timestamp(&vb_buf->vb.ts);
 		list_del(&vb_buf->vb.queue);
 		vb_buf->vb.state = VIDEOBUF_DONE;
 		wake_up(&vb_buf->vb.done);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index ca6261a86a5f..459bc65b545d 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -254,8 +254,6 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
 	struct fimc_lite *fimc = priv;
 	struct flite_buffer *vbuf;
 	unsigned long flags;
-	struct timeval *tv;
-	struct timespec ts;
 	u32 intsrc;
 
 	spin_lock_irqsave(&fimc->slock, flags);
@@ -294,10 +292,7 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
 	    test_bit(ST_FLITE_RUN, &fimc->state) &&
 	    !list_empty(&fimc->active_buf_q)) {
 		vbuf = fimc_lite_active_queue_pop(fimc);
-		ktime_get_ts(&ts);
-		tv = &vbuf->vb.v4l2_buf.timestamp;
-		tv->tv_sec = ts.tv_sec;
-		tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+		v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
 		vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
 		flite_hw_mask_dma_buffer(fimc, vbuf->index);
 		vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 20434e83e801..94d4c295d3d0 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -235,7 +235,7 @@ static int isp_stat_buf_queue(struct ispstat *stat)
 	if (!stat->active_buf)
 		return STAT_NO_BUF;
 
-	ktime_get_ts(&stat->active_buf->ts);
+	v4l2_get_timestamp(&stat->active_buf->ts);
 
 	stat->active_buf->buf_size = stat->buf_size;
 	if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
@@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
 		return PTR_ERR(buf);
 	}
 
-	data->ts.tv_sec = buf->ts.tv_sec;
-	data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC;
+	data->ts = buf->ts;
 	data->config_counter = buf->config_counter;
 	data->frame_number = buf->frame_number;
 	data->buf_size = buf->buf_size;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index b79380d83fcf..6d9b0244f320 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
 	struct sg_table sgt;
 	void *virt_addr;
 	dma_addr_t dma_addr;
-	struct timespec ts;
+	struct timeval ts;
 	u32 buf_size;
 	u32 frame_number;
 	u16 config_counter;
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 76e6289a5612..edf70725ecf3 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -328,23 +328,17 @@ irqreturn_t s3c_camif_irq_handler(int irq, void *priv)
 	    !list_empty(&vp->active_buf_q)) {
 		unsigned int index;
 		struct camif_buffer *vbuf;
-		struct timeval *tv;
-		struct timespec ts;
 		/*
 		 * Get previous DMA write buffer index:
 		 * 0 => DMA buffer 0, 2;
 		 * 1 => DMA buffer 1, 3.
 		 */
 		index = (CISTATUS_FRAMECNT(status) + 2) & 1;
-
-		ktime_get_ts(&ts);
 		vbuf = camif_active_queue_peek(vp, index);
 
 		if (!WARN_ON(vbuf == NULL)) {
 			/* Dequeue a filled buffer */
-			tv = &vbuf->vb.v4l2_buf.timestamp;
-			tv->tv_sec = ts.tv_sec;
-			tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+			v4l2_get_timestamp(&vbuf->vb.v4l2_buf.timestamp);
 			vbuf->vb.v4l2_buf.sequence = vp->frame_sequence++;
 			vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
 
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index e54cee856a80..af5cd8213e8b 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -436,7 +436,7 @@ void gspca_frame_add(struct gspca_dev *gspca_dev,
 		}
 		j = gspca_dev->fr_queue[i];
 		frame = &gspca_dev->frame[j];
-		frame->v4l2_buf.timestamp = ktime_to_timeval(ktime_get());
+		v4l2_get_timestamp(&frame->v4l2_buf.timestamp);
 		frame->v4l2_buf.sequence = gspca_dev->sequence++;
 		gspca_dev->image = frame->data;
 		gspca_dev->image_len = 0;
@@ -1909,7 +1909,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
 	}
 
 	/* get a frame */
-	timestamp = ktime_to_timeval(ktime_get());
+	v4l2_get_timestamp(&timestamp);
 	timestamp.tv_sec--;
 	n = 2;
 	for (;;) {
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 71a1b93b0790..8faa50dc5b19 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -565,6 +565,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 #endif
 	/* yes, really vidioc_subscribe_event */
 	SET_VALID_IOCTL(ops, VIDIOC_DQEVENT, vidioc_subscribe_event);
+	SET_VALID_IOCTL(ops, VIDIOC_DQEVENT32, vidioc_subscribe_event);
 	SET_VALID_IOCTL(ops, VIDIOC_SUBSCRIBE_EVENT, vidioc_subscribe_event);
 	SET_VALID_IOCTL(ops, VIDIOC_UNSUBSCRIBE_EVENT, vidioc_unsubscribe_event);
 	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 40405d8710a6..485a90ce12df 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -420,7 +420,6 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
 	enum iss_pipeline_state state;
 	struct iss_buffer *buf;
 	unsigned long flags;
-	struct timespec ts;
 
 	spin_lock_irqsave(&video->qlock, flags);
 	if (WARN_ON(list_empty(&video->dmaqueue))) {
@@ -433,9 +432,7 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
 	list_del(&buf->list);
 	spin_unlock_irqrestore(&video->qlock, flags);
 
-	ktime_get_ts(&ts);
-	buf->vb.v4l2_buf.timestamp.tv_sec = ts.tv_sec;
-	buf->vb.v4l2_buf.timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+	v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
 
 	/* Do frame number propagation only if this is the output video node.
 	 * Frame number either comes from the CSI receivers or it gets
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (5 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 6/9] [media] use v4l2_get_timestamp where possible Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-18  8:05   ` Hans Verkuil
  2015-09-17 21:19 ` [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann
  2015-09-17 21:19 ` [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data Arnd Bergmann
  8 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

The v4l2 API uses a 'struct timeval' to communicate time stamps to user
space. This is broken on 32-bit architectures as soon as we have a C library
that defines time_t as 64 bit, which then changes the structure layout of
struct v4l2_buffer.

Since existing user space source code relies on the type to be 'struct
timeva' and we want to preserve compile-time compatibility when moving
to a new libc, we cannot make user-visible changes to the header file.

In this patch, we change the type of the timestamp to 'struct v4l2_timeval'
as a preparation for a follow-up that adds API compatiblity for both
32-bit and 64-bit time_t. This patch should have no impact on generated
code in either user space or kernel.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/pci/bt8xx/bttv-driver.c      |  2 +-
 drivers/media/pci/meye/meye.h              |  2 +-
 drivers/media/pci/zoran/zoran.h            |  2 +-
 drivers/media/platform/coda/coda.h         |  2 +-
 drivers/media/platform/omap/omap_vout.c    |  4 ++--
 drivers/media/platform/omap3isp/ispstat.c  |  3 ++-
 drivers/media/platform/omap3isp/ispstat.h  |  2 +-
 drivers/media/platform/vim2m.c             |  2 +-
 drivers/media/platform/vivid/vivid-ctrls.c |  2 +-
 drivers/media/usb/cpia2/cpia2.h            |  2 +-
 drivers/media/usb/cpia2/cpia2_v4l.c        |  2 +-
 drivers/media/usb/gspca/gspca.c            |  2 +-
 drivers/media/usb/usbvision/usbvision.h    |  2 +-
 drivers/media/v4l2-core/v4l2-common.c      |  6 +++---
 include/media/v4l2-common.h                |  2 +-
 include/media/videobuf-core.h              |  2 +-
 include/trace/events/v4l2.h                | 12 ++++++++++--
 include/uapi/linux/videodev2.h             | 17 +++++++++++++++++
 18 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 15a4ebc2844d..b0ed8d799c14 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3585,7 +3585,7 @@ static void
 bttv_irq_wakeup_video(struct bttv *btv, struct bttv_buffer_set *wakeup,
 		      struct bttv_buffer_set *curr, unsigned int state)
 {
-	struct timeval ts;
+	struct v4l2_timeval ts;
 
 	v4l2_get_timestamp(&ts);
 
diff --git a/drivers/media/pci/meye/meye.h b/drivers/media/pci/meye/meye.h
index 751be5e533c7..a06aa5ba9abc 100644
--- a/drivers/media/pci/meye/meye.h
+++ b/drivers/media/pci/meye/meye.h
@@ -281,7 +281,7 @@
 struct meye_grab_buffer {
 	int state;			/* state of buffer */
 	unsigned long size;		/* size of jpg frame */
-	struct timeval timestamp;	/* timestamp */
+	struct v4l2_timeval timestamp;	/* timestamp */
 	unsigned long sequence;		/* sequence number */
 };
 
diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
index 4e7db8939c2b..9a9f782cede9 100644
--- a/drivers/media/pci/zoran/zoran.h
+++ b/drivers/media/pci/zoran/zoran.h
@@ -39,7 +39,7 @@ struct zoran_sync {
 	unsigned long frame;	/* number of buffer that has been free'd */
 	unsigned long length;	/* number of code bytes in buffer (capture only) */
 	unsigned long seq;	/* frame sequence number */
-	struct timeval timestamp;	/* timestamp */
+	struct v4l2_timeval timestamp;	/* timestamp */
 };
 
 
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 59b2af9c7749..fa1e15a757cd 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -138,7 +138,7 @@ struct coda_buffer_meta {
 	struct list_head	list;
 	u32			sequence;
 	struct v4l2_timecode	timecode;
-	struct timeval		timestamp;
+	struct v4l2_timeval	timestamp;
 	u32			start;
 	u32			end;
 };
diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
index 70c28d19ea04..974a238a86fe 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -513,7 +513,7 @@ static int omapvid_apply_changes(struct omap_vout_device *vout)
 }
 
 static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
-		unsigned int irqstatus, struct timeval timevalue)
+		unsigned int irqstatus, struct v4l2_timeval timevalue)
 {
 	u32 fid;
 
@@ -557,7 +557,7 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
 	int ret, fid, mgr_id;
 	u32 addr, irq;
 	struct omap_overlay *ovl;
-	struct timeval timevalue;
+	struct v4l2_timeval timevalue;
 	struct omapvideo_info *ovid;
 	struct omap_dss_device *cur_display;
 	struct omap_vout_device *vout = (struct omap_vout_device *)arg;
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 94d4c295d3d0..fa96e330c563 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -496,7 +496,8 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
 		return PTR_ERR(buf);
 	}
 
-	data->ts = buf->ts;
+	data->ts.tv_sec = buf->ts.tv_sec;
+	data->ts.tv_usec = buf->ts.tv_usec;
 	data->config_counter = buf->config_counter;
 	data->frame_number = buf->frame_number;
 	data->buf_size = buf->buf_size;
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index 6d9b0244f320..7b4f136567a3 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -39,7 +39,7 @@ struct ispstat_buffer {
 	struct sg_table sgt;
 	void *virt_addr;
 	dma_addr_t dma_addr;
-	struct timeval ts;
+	struct v4l2_timeval ts;
 	u32 buf_size;
 	u32 frame_number;
 	u16 config_counter;
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 295fde5fdb75..df5daac6d099 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
 	in_vb->v4l2_buf.sequence = q_data->sequence++;
 	memcpy(&out_vb->v4l2_buf.timestamp,
 			&in_vb->v4l2_buf.timestamp,
-			sizeof(struct timeval));
+			sizeof(struct v4l2_timeval));
 	if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
 		memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
 			sizeof(struct v4l2_timecode));
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index 339c8b7e53c8..065f3d6c2409 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -935,7 +935,7 @@ static const struct v4l2_ctrl_config vivid_ctrl_has_scaler_out = {
 static int vivid_streaming_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vivid_dev *dev = container_of(ctrl->handler, struct vivid_dev, ctrl_hdl_streaming);
-	struct timeval tv;
+	struct v4l2_timeval tv;
 
 	switch (ctrl->id) {
 	case VIVID_CID_DQBUF_ERROR:
diff --git a/drivers/media/usb/cpia2/cpia2.h b/drivers/media/usb/cpia2/cpia2.h
index cdef677d57ec..3e7c523784e7 100644
--- a/drivers/media/usb/cpia2/cpia2.h
+++ b/drivers/media/usb/cpia2/cpia2.h
@@ -354,7 +354,7 @@ struct cpia2_sbuf {
 };
 
 struct framebuf {
-	struct timeval timestamp;
+	struct v4l2_timeval timestamp;
 	unsigned long seq;
 	int num;
 	int length;
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea8344547..ce50d7ef4da7 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -892,7 +892,7 @@ static int find_earliest_filled_buffer(struct camera_data *cam)
 				found = i;
 			} else {
 				/* find which buffer is earlier */
-				struct timeval *tv1, *tv2;
+				struct v4l2_timeval *tv1, *tv2;
 				tv1 = &cam->buffers[i].timestamp;
 				tv2 = &cam->buffers[found].timestamp;
 				if(tv1->tv_sec < tv2->tv_sec ||
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index af5cd8213e8b..c977937da9c4 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1898,7 +1898,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
 	struct gspca_dev *gspca_dev = video_drvdata(file);
 	struct gspca_frame *frame;
 	struct v4l2_buffer v4l2_buf;
-	struct timeval timestamp;
+	struct v4l2_timeval timestamp;
 	int n, ret, ret2;
 
 	PDEBUG(D_FRAM, "read (%zd)", count);
diff --git a/drivers/media/usb/usbvision/usbvision.h b/drivers/media/usb/usbvision/usbvision.h
index 4f2e4fde38f2..512de31613df 100644
--- a/drivers/media/usb/usbvision/usbvision.h
+++ b/drivers/media/usb/usbvision/usbvision.h
@@ -320,7 +320,7 @@ struct usbvision_frame {
 	long bytes_read;				/* amount of scanlength that has been read from data */
 	struct usbvision_v4l2_format_st v4l2_format;	/* format the user needs*/
 	int v4l2_linesize;				/* bytes for one videoline*/
-	struct timeval timestamp;
+	struct v4l2_timeval timestamp;
 	int sequence;					/* How many video frames we send to user */
 };
 
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 5b808500e7e7..589fab615f21 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -396,11 +396,11 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
 }
 EXPORT_SYMBOL_GPL(v4l2_find_nearest_format);
 
-void v4l2_get_timestamp(struct timeval *tv)
+void v4l2_get_timestamp(struct v4l2_timeval *tv)
 {
-	struct timespec ts;
+	struct timespec64 ts;
 
-	ktime_get_ts(&ts);
+	ktime_get_ts64(&ts);
 	tv->tv_sec = ts.tv_sec;
 	tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 1cc0c5ba16b3..6e77d889c3f8 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -187,6 +187,6 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
 		const struct v4l2_discrete_probe *probe,
 		s32 width, s32 height);
 
-void v4l2_get_timestamp(struct timeval *tv);
+void v4l2_get_timestamp(struct v4l2_timeval *tv);
 
 #endif /* V4L2_COMMON_H_ */
diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
index d760aa73ebbb..6381338a9588 100644
--- a/include/media/videobuf-core.h
+++ b/include/media/videobuf-core.h
@@ -80,7 +80,7 @@ struct videobuf_buffer {
 	struct list_head        queue;
 	wait_queue_head_t       done;
 	unsigned int            field_count;
-	struct timeval          ts;
+	struct v4l2_timeval     ts;
 
 	/* Memory type */
 	enum v4l2_memory        memory;
diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index dbf017bfddd9..a88be48dc473 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -6,6 +6,14 @@
 
 #include <linux/tracepoint.h>
 
+#ifndef v4l2_timeval_to_ns
+#define v4l2_timeval_to_ns v4l2_timeval_to_ns
+static inline u64 v4l2_timeval_to_ns(struct v4l2_timeval *tv)
+{
+	return (u64)tv->tv_sec * NSEC_PER_SEC + tv->tv_usec * NSEC_PER_USEC;
+}
+#endif
+
 /* Enums require being exported to userspace, for user tool parsing */
 #undef EM
 #undef EMe
@@ -126,7 +134,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_timeval_to_ns(&buf->timestamp);
 		__entry->timecode_type = buf->timecode.type;
 		__entry->timecode_flags = buf->timecode.flags;
 		__entry->timecode_frames = buf->timecode.frames;
@@ -211,7 +219,7 @@ DECLARE_EVENT_CLASS(vb2_event_class,
 		__entry->bytesused = vb->v4l2_planes[0].bytesused;
 		__entry->flags = vb->v4l2_buf.flags;
 		__entry->field = vb->v4l2_buf.field;
-		__entry->timestamp = timeval_to_ns(&vb->v4l2_buf.timestamp);
+		__entry->timestamp = v4l2_timeval_to_ns(&vb->v4l2_buf.timestamp);
 		__entry->timecode_type = vb->v4l2_buf.timecode.type;
 		__entry->timecode_flags = vb->v4l2_buf.timecode.flags;
 		__entry->timecode_frames = vb->v4l2_buf.timecode.frames;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3e2c497c31fd..450b3249ba30 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -803,6 +803,19 @@ struct v4l2_plane {
 	__u32			reserved[11];
 };
 
+#ifdef __KERNEL__
+/*
+ * This is used for the in-kernel version of v4l2_buffer, as we are
+ * migrating away from using time_t based structures in the kernel.
+ * User space might see this defined either using 32-bit or 64-bit
+ * time_t, so we have to convert it when accessing user data.
+ */
+struct v4l2_timeval {
+	long tv_sec;
+	long tv_usec;
+};
+#endif
+
 /**
  * struct v4l2_buffer - video buffer info
  * @index:	id number of the buffer
@@ -839,7 +852,11 @@ struct v4l2_buffer {
 	__u32			bytesused;
 	__u32			flags;
 	__u32			field;
+#ifdef __KERNEL__
+	struct v4l2_timeval	timestamp;
+#else
 	struct timeval		timestamp;
+#endif
 	struct v4l2_timecode	timecode;
 	__u32			sequence;
 
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (6 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-09-18  7:18   ` Hans Verkuil
  2015-09-17 21:19 ` [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data Arnd Bergmann
  8 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

This is the final change to enable user space with 64-bit time_t
in the core v4l2 code.

Four ioctls are affected here: VIDIOC_QUERYBUF, VIDIOC_QBUF,
VIDIOC_DQBUF, and VIDIOC_PREPARE_BUF. All of them pass a
struct v4l2_buffer, which can either contain a 32-bit time_t
or a 64-bit time on 32-bit architectures.

In this patch, we redefine the in-kernel version of this structure
to use the 64-bit __s64 type through the use of v4l2_timeval.
This means the normal ioctl handling now assumes 64-bit time_t
and so we have to add support for 32-bit time_t in new handlers.

This is somewhat similar to the 32-bit compat handling on 64-bit
architectures, but those also have to modify other fields of
the structure. For now, I leave that compat code alone, as it
handles the normal case (32-bit compat_time_t) correctly, this
has to be done as a follow-up.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 174 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/videodev2.h       |  34 ++++++-
 2 files changed, 199 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 7aab18dd2ca5..137475c28e8f 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -438,15 +438,14 @@ static void v4l_print_buffer(const void *arg, bool write_only)
 	const struct v4l2_timecode *tc = &p->timecode;
 	const struct v4l2_plane *plane;
 	int i;
+	/* y2038 safe because of monotonic time */
+	unsigned int seconds = p->timestamp.tv_sec;
 
-	pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, "
+	pr_cont("%02d:%02d:%02d.%08ld index=%d, type=%s, "
 		"flags=0x%08x, field=%s, sequence=%d, memory=%s",
-			p->timestamp.tv_sec / 3600,
-			(int)(p->timestamp.tv_sec / 60) % 60,
-			(int)(p->timestamp.tv_sec % 60),
-			(long)p->timestamp.tv_usec,
-			p->index,
-			prt_names(p->type, v4l2_type_names),
+			seconds / 3600, (seconds / 60) % 60,
+			seconds % 60, (long)p->timestamp.tv_usec,
+			p->index, prt_names(p->type, v4l2_type_names),
 			p->flags, prt_names(p->field, v4l2_field_names),
 			p->sequence, prt_names(p->memory, v4l2_memory_names));
 
@@ -1846,6 +1845,123 @@ static int v4l_prepare_buf(const struct v4l2_ioctl_ops *ops,
 	return ret ? ret : ops->vidioc_prepare_buf(file, fh, b);
 }
 
+#ifndef CONFIG_64BIT
+static void v4l_get_buffer_time32(struct v4l2_buffer *to,
+				  const struct v4l2_buffer_time32 *from)
+{
+	to->index		= from->index;
+	to->type		= from->type;
+	to->bytesused		= from->bytesused;
+	to->flags		= from->flags;
+	to->field		= from->field;
+	to->timestamp.tv_sec	= from->timestamp.tv_sec;
+	to->timestamp.tv_usec	= from->timestamp.tv_usec;
+	to->timecode		= from->timecode;
+	to->sequence		= from->sequence;
+	to->memory		= from->memory;
+	to->m.offset		= from->m.offset;
+	to->length		= from->length;
+	to->reserved2		= from->reserved2;
+	to->reserved		= from->reserved;
+}
+
+static void v4l_put_buffer_time32(struct v4l2_buffer_time32 *to,
+				  const struct v4l2_buffer *from)
+{
+	to->index		= from->index;
+	to->type		= from->type;
+	to->bytesused		= from->bytesused;
+	to->flags		= from->flags;
+	to->field		= from->field;
+	to->timestamp.tv_sec	= from->timestamp.tv_sec;
+	to->timestamp.tv_usec	= from->timestamp.tv_usec;
+	to->timecode		= from->timecode;
+	to->sequence		= from->sequence;
+	to->memory		= from->memory;
+	to->m.offset		= from->m.offset;
+	to->length		= from->length;
+	to->reserved2		= from->reserved2;
+	to->reserved		= from->reserved;
+}
+
+static int v4l_querybuf_time32(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	struct v4l2_buffer buffer;
+	int ret;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	ret = v4l_querybuf(ops, file, fh, &buffer);
+	v4l_put_buffer_time32(arg, &buffer);
+
+	return ret;
+}
+
+static int v4l_qbuf_time32(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	struct v4l2_buffer buffer;
+	int ret;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	ret = v4l_qbuf(ops, file, fh, &buffer);
+	v4l_put_buffer_time32(arg, &buffer);
+
+	return ret;
+}
+
+static int v4l_dqbuf_time32(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	struct v4l2_buffer buffer;
+	int ret;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	ret = v4l_dqbuf(ops, file, fh, &buffer);
+	v4l_put_buffer_time32(arg, &buffer);
+
+	return ret;
+}
+
+static int v4l_prepare_buf_time32(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{
+	struct v4l2_buffer buffer;
+	int ret;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	ret = v4l_prepare_buf(ops, file, fh, &buffer);
+
+	return ret;
+}
+
+static void v4l_print_buffer_time32(const void *arg, bool write_only)
+{
+	struct v4l2_buffer buffer;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	v4l_print_buffer(&buffer, write_only);
+}
+
+#ifdef CONFIG_TRACEPOINTS
+static void trace_v4l2_dqbuf_time32(int minor, const struct v4l2_buffer_time32 *arg)
+{
+	struct v4l2_buffer buffer;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	trace_v4l2_dqbuf(minor, &buffer);
+}
+
+static void trace_v4l2_qbuf_time32(int minor, const struct v4l2_buffer_time32 *arg)
+{
+	struct v4l2_buffer buffer;
+
+	v4l_get_buffer_time32(&buffer, arg);
+	trace_v4l2_qbuf(minor, &buffer);
+}
+#endif
+#endif
+
 static int v4l_g_parm(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
@@ -2408,12 +2524,21 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO_FNC(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
 	IOCTL_INFO_FNC(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
 	IOCTL_INFO_FNC(VIDIOC_QUERYBUF, v4l_querybuf, v4l_print_buffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
+#ifndef CONFIG_64BIT
+	IOCTL_INFO_FNC(VIDIOC_QUERYBUF_TIME32, v4l_querybuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
+#endif
 	IOCTL_INFO_STD(VIDIOC_G_FBUF, vidioc_g_fbuf, v4l_print_framebuffer, 0),
 	IOCTL_INFO_STD(VIDIOC_S_FBUF, vidioc_s_fbuf, v4l_print_framebuffer, INFO_FL_PRIO),
 	IOCTL_INFO_FNC(VIDIOC_OVERLAY, v4l_overlay, v4l_print_u32, INFO_FL_PRIO),
 	IOCTL_INFO_FNC(VIDIOC_QBUF, v4l_qbuf, v4l_print_buffer, INFO_FL_QUEUE),
+#ifndef CONFIG_64BIT
+	IOCTL_INFO_FNC(VIDIOC_QBUF_TIME32, v4l_qbuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
+#endif
 	IOCTL_INFO_STD(VIDIOC_EXPBUF, vidioc_expbuf, v4l_print_exportbuffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_exportbuffer, flags)),
 	IOCTL_INFO_FNC(VIDIOC_DQBUF, v4l_dqbuf, v4l_print_buffer, INFO_FL_QUEUE),
+#ifndef CONFIG_64BIT
+	IOCTL_INFO_FNC(VIDIOC_DQBUF_TIME32, v4l_dqbuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
+#endif
 	IOCTL_INFO_FNC(VIDIOC_STREAMON, v4l_streamon, v4l_print_buftype, INFO_FL_PRIO | INFO_FL_QUEUE),
 	IOCTL_INFO_FNC(VIDIOC_STREAMOFF, v4l_streamoff, v4l_print_buftype, INFO_FL_PRIO | INFO_FL_QUEUE),
 	IOCTL_INFO_FNC(VIDIOC_G_PARM, v4l_g_parm, v4l_print_streamparm, INFO_FL_CLEAR(v4l2_streamparm, type)),
@@ -2479,6 +2604,9 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO_FNC(VIDIOC_UNSUBSCRIBE_EVENT, v4l_unsubscribe_event, v4l_print_event_subscription, 0),
 	IOCTL_INFO_FNC(VIDIOC_CREATE_BUFS, v4l_create_bufs, v4l_print_create_buffers, INFO_FL_PRIO | INFO_FL_QUEUE),
 	IOCTL_INFO_FNC(VIDIOC_PREPARE_BUF, v4l_prepare_buf, v4l_print_buffer, INFO_FL_QUEUE),
+#ifndef CONFIG_64BIT
+	IOCTL_INFO_FNC(VIDIOC_PREPARE_BUF_TIME32, v4l_prepare_buf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
+#endif
 	IOCTL_INFO_STD(VIDIOC_ENUM_DV_TIMINGS, vidioc_enum_dv_timings, v4l_print_enum_dv_timings, 0),
 	IOCTL_INFO_STD(VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings, v4l_print_dv_timings, 0),
 	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
@@ -2608,6 +2736,12 @@ done:
 		    (cmd == VIDIOC_QBUF || cmd == VIDIOC_DQBUF))
 			return ret;
 
+#ifndef CONFIG_64BIT
+		if (!(dev_debug & V4L2_DEV_DEBUG_STREAMING) &&
+		    (cmd == VIDIOC_QBUF_TIME32 || cmd == VIDIOC_DQBUF_TIME32))
+			return ret;
+#endif
+
 		v4l_printk_ioctl(video_device_node_name(vfd), cmd);
 		if (ret < 0)
 			pr_cont(": error %ld", ret);
@@ -2630,6 +2764,26 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
 	int ret = 0;
 
 	switch (cmd) {
+#ifndef CONFIG_64BIT
+	case VIDIOC_PREPARE_BUF_TIME32:
+	case VIDIOC_QUERYBUF_TIME32:
+	case VIDIOC_QBUF_TIME32:
+	case VIDIOC_DQBUF_TIME32: {
+		struct v4l2_buffer_time32 *buf = parg;
+
+		if (V4L2_TYPE_IS_MULTIPLANAR(buf->type) && buf->length > 0) {
+			if (buf->length > VIDEO_MAX_PLANES) {
+				ret = -EINVAL;
+				break;
+			}
+			*user_ptr = (void __user *)buf->m.planes;
+			*kernel_ptr = (void **)&buf->m.planes;
+			*array_size = sizeof(struct v4l2_plane) * buf->length;
+			ret = 1;
+		}
+		break;
+	}
+#endif
 	case VIDIOC_PREPARE_BUF:
 	case VIDIOC_QUERYBUF:
 	case VIDIOC_QBUF:
@@ -2774,6 +2928,12 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 			trace_v4l2_dqbuf(video_devdata(file)->minor, parg);
 		else if (cmd == VIDIOC_QBUF)
 			trace_v4l2_qbuf(video_devdata(file)->minor, parg);
+#ifndef CONFIG_64BIT
+		else if (cmd == VIDIOC_DQBUF_TIME32)
+			trace_v4l2_dqbuf_time32(video_devdata(file)->minor, parg);
+		else if (cmd == VIDIOC_QBUF_TIME32)
+			trace_v4l2_qbuf_time32(video_devdata(file)->minor, parg);
+#endif
 	}
 
 	if (has_array_args) {
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 450b3249ba30..0d3688a97ab8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -811,8 +811,8 @@ struct v4l2_plane {
  * time_t, so we have to convert it when accessing user data.
  */
 struct v4l2_timeval {
-	long tv_sec;
-	long tv_usec;
+	__s64 tv_sec;
+	__s64 tv_usec;
 };
 #endif
 
@@ -873,6 +873,32 @@ struct v4l2_buffer {
 	__u32			reserved;
 };
 
+struct v4l2_buffer_time32 {
+	__u32			index;
+	__u32			type;
+	__u32			bytesused;
+	__u32			flags;
+	__u32			field;
+	struct {
+		__s32		tv_sec;
+		__s32		tv_usec;
+	}			timestamp;
+	struct v4l2_timecode	timecode;
+	__u32			sequence;
+
+	/* memory location */
+	__u32			memory;
+	union {
+		__u32           offset;
+		unsigned long   userptr;
+		struct v4l2_plane *planes;
+		__s32		fd;
+	} m;
+	__u32			length;
+	__u32			reserved2;
+	__u32			reserved;
+};
+
 /*  Flags for 'flags' field */
 /* Buffer is mapped (flag) */
 #define V4L2_BUF_FLAG_MAPPED			0x00000001
@@ -2216,12 +2242,15 @@ struct v4l2_create_buffers {
 #define VIDIOC_S_FMT		_IOWR('V',  5, struct v4l2_format)
 #define VIDIOC_REQBUFS		_IOWR('V',  8, struct v4l2_requestbuffers)
 #define VIDIOC_QUERYBUF		_IOWR('V',  9, struct v4l2_buffer)
+#define VIDIOC_QUERYBUF_TIME32	_IOWR('V',  9, struct v4l2_buffer_time32)
 #define VIDIOC_G_FBUF		 _IOR('V', 10, struct v4l2_framebuffer)
 #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
 #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
 #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
+#define VIDIOC_QBUF_TIME32	_IOWR('V', 15, struct v4l2_buffer_time32)
 #define VIDIOC_EXPBUF		_IOWR('V', 16, struct v4l2_exportbuffer)
 #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
+#define VIDIOC_DQBUF_TIME32	_IOWR('V', 17, struct v4l2_buffer_time32)
 #define VIDIOC_STREAMON		 _IOW('V', 18, int)
 #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
 #define VIDIOC_G_PARM		_IOWR('V', 21, struct v4l2_streamparm)
@@ -2292,6 +2321,7 @@ struct v4l2_create_buffers {
    versions */
 #define VIDIOC_CREATE_BUFS	_IOWR('V', 92, struct v4l2_create_buffers)
 #define VIDIOC_PREPARE_BUF	_IOWR('V', 93, struct v4l2_buffer)
+#define VIDIOC_PREPARE_BUF_TIME32 _IOWR('V', 93, struct v4l2_buffer_time32)
 
 /* Experimental selection API */
 #define VIDIOC_G_SELECTION	_IOWR('V', 94, struct v4l2_selection)
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
  2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
                   ` (7 preceding siblings ...)
  2015-09-17 21:19 ` [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann
@ 2015-09-17 21:19 ` Arnd Bergmann
  2015-11-09 20:09   ` Laurent Pinchart
  8 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-17 21:19 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api,
	linux-samsung-soc, Hans Verkuil, Arnd Bergmann

C libraries with 64-bit time_t use an incompatible format for
struct omap3isp_stat_data. This changes the kernel code to
support either version, by moving over the normal handling
to the 64-bit variant, and adding compatiblity code to handle
the old binary format with the existing ioctl command code.

Fortunately, the command code includes the size of the structure,
so the difference gets handled automatically.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/platform/omap3isp/isph3a_aewb.c |  2 ++
 drivers/media/platform/omap3isp/isph3a_af.c   |  2 ++
 drivers/media/platform/omap3isp/isphist.c     |  2 ++
 drivers/media/platform/omap3isp/ispstat.c     | 18 ++++++++++++++++--
 drivers/media/platform/omap3isp/ispstat.h     |  2 ++
 include/uapi/linux/omap3isp.h                 | 19 +++++++++++++++++++
 6 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c b/drivers/media/platform/omap3isp/isph3a_aewb.c
index ccaf92f39236..2d567725608f 100644
--- a/drivers/media/platform/omap3isp/isph3a_aewb.c
+++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
@@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 		return omap3isp_stat_config(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_REQ:
 		return omap3isp_stat_request_statistics(stat, arg);
+	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+		return omap3isp_stat_request_statistics_time32(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_EN: {
 		unsigned long *en = arg;
 		return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c
index 92937f7eecef..2ac02371318b 100644
--- a/drivers/media/platform/omap3isp/isph3a_af.c
+++ b/drivers/media/platform/omap3isp/isph3a_af.c
@@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 		return omap3isp_stat_config(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_REQ:
 		return omap3isp_stat_request_statistics(stat, arg);
+	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+		return omap3isp_stat_request_statistics_time32(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_EN: {
 		int *en = arg;
 		return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/isphist.c b/drivers/media/platform/omap3isp/isphist.c
index 7138b043a4aa..669b97b079ee 100644
--- a/drivers/media/platform/omap3isp/isphist.c
+++ b/drivers/media/platform/omap3isp/isphist.c
@@ -436,6 +436,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 		return omap3isp_stat_config(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_REQ:
 		return omap3isp_stat_request_statistics(stat, arg);
+	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
+		return omap3isp_stat_request_statistics_time32(stat, arg);
 	case VIDIOC_OMAP3ISP_STAT_EN: {
 		int *en = arg;
 		return omap3isp_stat_enable(stat, !!*en);
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index fa96e330c563..3d70332422b5 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
 		return PTR_ERR(buf);
 	}
 
-	data->ts.tv_sec = buf->ts.tv_sec;
-	data->ts.tv_usec = buf->ts.tv_usec;
+	data->ts = buf->ts;
 	data->config_counter = buf->config_counter;
 	data->frame_number = buf->frame_number;
 	data->buf_size = buf->buf_size;
@@ -509,6 +508,21 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
 	return 0;
 }
 
+int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
+					struct omap3isp_stat_data_time32 *data)
+{
+	struct omap3isp_stat_data data64;
+	int ret;
+
+	ret = omap3isp_stat_request_statistics(stat, &data64);
+
+	data->ts.tv_sec = data64.ts.tv_sec;
+	data->ts.tv_usec = data64.ts.tv_usec;
+	memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts));
+
+	return ret;
+}
+
 /*
  * omap3isp_stat_config - Receives new statistic engine configuration.
  * @new_conf: Pointer to config structure.
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
index 7b4f136567a3..b19ea6c8f733 100644
--- a/drivers/media/platform/omap3isp/ispstat.h
+++ b/drivers/media/platform/omap3isp/ispstat.h
@@ -130,6 +130,8 @@ struct ispstat_generic_config {
 int omap3isp_stat_config(struct ispstat *stat, void *new_conf);
 int omap3isp_stat_request_statistics(struct ispstat *stat,
 				     struct omap3isp_stat_data *data);
+int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
+				     struct omap3isp_stat_data_time32 *data);
 int omap3isp_stat_init(struct ispstat *stat, const char *name,
 		       const struct v4l2_subdev_ops *sd_ops);
 void omap3isp_stat_cleanup(struct ispstat *stat);
diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
index c090cf9249bb..4bff66aefca5 100644
--- a/include/uapi/linux/omap3isp.h
+++ b/include/uapi/linux/omap3isp.h
@@ -54,6 +54,8 @@
 	_IOWR('V', BASE_VIDIOC_PRIVATE + 5, struct omap3isp_h3a_af_config)
 #define VIDIOC_OMAP3ISP_STAT_REQ \
 	_IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data)
+#define VIDIOC_OMAP3ISP_STAT_REQ_TIME32 \
+	_IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data_time32)
 #define VIDIOC_OMAP3ISP_STAT_EN \
 	_IOWR('V', BASE_VIDIOC_PRIVATE + 7, unsigned long)
 
@@ -164,7 +166,11 @@ struct omap3isp_h3a_aewb_config {
  * @config_counter: Number of the configuration associated with the data.
  */
 struct omap3isp_stat_data {
+#ifdef __KERNEL__
+	struct v4l2_timeval ts;
+#else
 	struct timeval ts;
+#endif
 	void __user *buf;
 	__u32 buf_size;
 	__u16 frame_number;
@@ -172,6 +178,19 @@ struct omap3isp_stat_data {
 	__u16 config_counter;
 };
 
+#ifdef __KERNEL__
+struct omap3isp_stat_data_time32 {
+	struct {
+		__s32	tv_sec;
+		__s32	tv_usec;
+	} ts;
+	__u32 buf;
+	__u32 buf_size;
+	__u16 frame_number;
+	__u16 cur_frame;
+	__u16 config_counter;
+};
+#endif
 
 /* Histogram related structs */
 
-- 
2.1.0.rc2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer
  2015-09-17 21:19 ` [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann
@ 2015-09-18  7:18   ` Hans Verkuil
  2015-09-18  9:26     ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18  7:18 UTC (permalink / raw)
  To: Arnd Bergmann, linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc

Hi Arnd,

Thanks once again for working on this! Unfortunately, this approach won't
work, see my comments below.

BTW, I would expect to see compile errors when compiling for 32 bit. Did
you try that?

On 09/17/2015 11:19 PM, Arnd Bergmann wrote:
> This is the final change to enable user space with 64-bit time_t
> in the core v4l2 code.
> 
> Four ioctls are affected here: VIDIOC_QUERYBUF, VIDIOC_QBUF,
> VIDIOC_DQBUF, and VIDIOC_PREPARE_BUF. All of them pass a
> struct v4l2_buffer, which can either contain a 32-bit time_t
> or a 64-bit time on 32-bit architectures.
> 
> In this patch, we redefine the in-kernel version of this structure
> to use the 64-bit __s64 type through the use of v4l2_timeval.
> This means the normal ioctl handling now assumes 64-bit time_t
> and so we have to add support for 32-bit time_t in new handlers.
> 
> This is somewhat similar to the 32-bit compat handling on 64-bit
> architectures, but those also have to modify other fields of
> the structure. For now, I leave that compat code alone, as it
> handles the normal case (32-bit compat_time_t) correctly, this
> has to be done as a follow-up.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 174 +++++++++++++++++++++++++++++++++--
>  include/uapi/linux/videodev2.h       |  34 ++++++-
>  2 files changed, 199 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 7aab18dd2ca5..137475c28e8f 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -438,15 +438,14 @@ static void v4l_print_buffer(const void *arg, bool write_only)
>  	const struct v4l2_timecode *tc = &p->timecode;
>  	const struct v4l2_plane *plane;
>  	int i;
> +	/* y2038 safe because of monotonic time */
> +	unsigned int seconds = p->timestamp.tv_sec;
>  
> -	pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, "
> +	pr_cont("%02d:%02d:%02d.%08ld index=%d, type=%s, "
>  		"flags=0x%08x, field=%s, sequence=%d, memory=%s",
> -			p->timestamp.tv_sec / 3600,
> -			(int)(p->timestamp.tv_sec / 60) % 60,
> -			(int)(p->timestamp.tv_sec % 60),
> -			(long)p->timestamp.tv_usec,
> -			p->index,
> -			prt_names(p->type, v4l2_type_names),
> +			seconds / 3600, (seconds / 60) % 60,
> +			seconds % 60, (long)p->timestamp.tv_usec,
> +			p->index, prt_names(p->type, v4l2_type_names),
>  			p->flags, prt_names(p->field, v4l2_field_names),
>  			p->sequence, prt_names(p->memory, v4l2_memory_names));
>  
> @@ -1846,6 +1845,123 @@ static int v4l_prepare_buf(const struct v4l2_ioctl_ops *ops,
>  	return ret ? ret : ops->vidioc_prepare_buf(file, fh, b);
>  }
>  
> +#ifndef CONFIG_64BIT
> +static void v4l_get_buffer_time32(struct v4l2_buffer *to,
> +				  const struct v4l2_buffer_time32 *from)
> +{
> +	to->index		= from->index;
> +	to->type		= from->type;
> +	to->bytesused		= from->bytesused;
> +	to->flags		= from->flags;
> +	to->field		= from->field;
> +	to->timestamp.tv_sec	= from->timestamp.tv_sec;
> +	to->timestamp.tv_usec	= from->timestamp.tv_usec;
> +	to->timecode		= from->timecode;
> +	to->sequence		= from->sequence;
> +	to->memory		= from->memory;
> +	to->m.offset		= from->m.offset;
> +	to->length		= from->length;
> +	to->reserved2		= from->reserved2;
> +	to->reserved		= from->reserved;
> +}
> +
> +static void v4l_put_buffer_time32(struct v4l2_buffer_time32 *to,
> +				  const struct v4l2_buffer *from)
> +{
> +	to->index		= from->index;
> +	to->type		= from->type;
> +	to->bytesused		= from->bytesused;
> +	to->flags		= from->flags;
> +	to->field		= from->field;
> +	to->timestamp.tv_sec	= from->timestamp.tv_sec;
> +	to->timestamp.tv_usec	= from->timestamp.tv_usec;
> +	to->timecode		= from->timecode;
> +	to->sequence		= from->sequence;
> +	to->memory		= from->memory;
> +	to->m.offset		= from->m.offset;
> +	to->length		= from->length;
> +	to->reserved2		= from->reserved2;
> +	to->reserved		= from->reserved;
> +}

Is there a reason why you didn't use memcpy like you did for VIDIOC_DQEVENT (path 5/9)?
I would prefer that over these explicit assignments.

> +
> +static int v4l_querybuf_time32(const struct v4l2_ioctl_ops *ops,
> +				struct file *file, void *fh, void *arg)
> +{
> +	struct v4l2_buffer buffer;
> +	int ret;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	ret = v4l_querybuf(ops, file, fh, &buffer);
> +	v4l_put_buffer_time32(arg, &buffer);
> +
> +	return ret;
> +}
> +
> +static int v4l_qbuf_time32(const struct v4l2_ioctl_ops *ops,
> +				struct file *file, void *fh, void *arg)
> +{
> +	struct v4l2_buffer buffer;
> +	int ret;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	ret = v4l_qbuf(ops, file, fh, &buffer);
> +	v4l_put_buffer_time32(arg, &buffer);
> +
> +	return ret;
> +}
> +
> +static int v4l_dqbuf_time32(const struct v4l2_ioctl_ops *ops,
> +				struct file *file, void *fh, void *arg)
> +{
> +	struct v4l2_buffer buffer;
> +	int ret;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	ret = v4l_dqbuf(ops, file, fh, &buffer);
> +	v4l_put_buffer_time32(arg, &buffer);
> +
> +	return ret;
> +}
> +
> +static int v4l_prepare_buf_time32(const struct v4l2_ioctl_ops *ops,
> +				struct file *file, void *fh, void *arg)
> +{
> +	struct v4l2_buffer buffer;
> +	int ret;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	ret = v4l_prepare_buf(ops, file, fh, &buffer);
> +
> +	return ret;
> +}
> +
> +static void v4l_print_buffer_time32(const void *arg, bool write_only)
> +{
> +	struct v4l2_buffer buffer;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	v4l_print_buffer(&buffer, write_only);
> +}
> +
> +#ifdef CONFIG_TRACEPOINTS
> +static void trace_v4l2_dqbuf_time32(int minor, const struct v4l2_buffer_time32 *arg)
> +{
> +	struct v4l2_buffer buffer;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	trace_v4l2_dqbuf(minor, &buffer);
> +}
> +
> +static void trace_v4l2_qbuf_time32(int minor, const struct v4l2_buffer_time32 *arg)
> +{
> +	struct v4l2_buffer buffer;
> +
> +	v4l_get_buffer_time32(&buffer, arg);
> +	trace_v4l2_qbuf(minor, &buffer);
> +}
> +#endif
> +#endif
> +
>  static int v4l_g_parm(const struct v4l2_ioctl_ops *ops,
>  				struct file *file, void *fh, void *arg)
>  {
> @@ -2408,12 +2524,21 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>  	IOCTL_INFO_FNC(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
>  	IOCTL_INFO_FNC(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
>  	IOCTL_INFO_FNC(VIDIOC_QUERYBUF, v4l_querybuf, v4l_print_buffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
> +#ifndef CONFIG_64BIT
> +	IOCTL_INFO_FNC(VIDIOC_QUERYBUF_TIME32, v4l_querybuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
> +#endif

This doesn't work. These IOCTL macros use the ioctl nr as the index in the array. Since
VIDIOC_QUERYBUF and VIDIOC_QUERYBUF_TIME32 have the same index, this will fail.

I think (not 100% certain, there may be better suggestions) that the conversion is best
done in video_usercopy(): just before func() is called have a switch for the TIME32
variants, convert to the regular variant, call func() and convert back.

My only concern here is that an additional v4l2_buffer struct (68 bytes) is needed on the
stack. I don't see any way around that, though.

>  	IOCTL_INFO_STD(VIDIOC_G_FBUF, vidioc_g_fbuf, v4l_print_framebuffer, 0),
>  	IOCTL_INFO_STD(VIDIOC_S_FBUF, vidioc_s_fbuf, v4l_print_framebuffer, INFO_FL_PRIO),
>  	IOCTL_INFO_FNC(VIDIOC_OVERLAY, v4l_overlay, v4l_print_u32, INFO_FL_PRIO),
>  	IOCTL_INFO_FNC(VIDIOC_QBUF, v4l_qbuf, v4l_print_buffer, INFO_FL_QUEUE),
> +#ifndef CONFIG_64BIT
> +	IOCTL_INFO_FNC(VIDIOC_QBUF_TIME32, v4l_qbuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
> +#endif
>  	IOCTL_INFO_STD(VIDIOC_EXPBUF, vidioc_expbuf, v4l_print_exportbuffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_exportbuffer, flags)),
>  	IOCTL_INFO_FNC(VIDIOC_DQBUF, v4l_dqbuf, v4l_print_buffer, INFO_FL_QUEUE),
> +#ifndef CONFIG_64BIT
> +	IOCTL_INFO_FNC(VIDIOC_DQBUF_TIME32, v4l_dqbuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
> +#endif
>  	IOCTL_INFO_FNC(VIDIOC_STREAMON, v4l_streamon, v4l_print_buftype, INFO_FL_PRIO | INFO_FL_QUEUE),
>  	IOCTL_INFO_FNC(VIDIOC_STREAMOFF, v4l_streamoff, v4l_print_buftype, INFO_FL_PRIO | INFO_FL_QUEUE),
>  	IOCTL_INFO_FNC(VIDIOC_G_PARM, v4l_g_parm, v4l_print_streamparm, INFO_FL_CLEAR(v4l2_streamparm, type)),
> @@ -2479,6 +2604,9 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>  	IOCTL_INFO_FNC(VIDIOC_UNSUBSCRIBE_EVENT, v4l_unsubscribe_event, v4l_print_event_subscription, 0),
>  	IOCTL_INFO_FNC(VIDIOC_CREATE_BUFS, v4l_create_bufs, v4l_print_create_buffers, INFO_FL_PRIO | INFO_FL_QUEUE),
>  	IOCTL_INFO_FNC(VIDIOC_PREPARE_BUF, v4l_prepare_buf, v4l_print_buffer, INFO_FL_QUEUE),
> +#ifndef CONFIG_64BIT
> +	IOCTL_INFO_FNC(VIDIOC_PREPARE_BUF_TIME32, v4l_prepare_buf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE),
> +#endif
>  	IOCTL_INFO_STD(VIDIOC_ENUM_DV_TIMINGS, vidioc_enum_dv_timings, v4l_print_enum_dv_timings, 0),
>  	IOCTL_INFO_STD(VIDIOC_QUERY_DV_TIMINGS, vidioc_query_dv_timings, v4l_print_dv_timings, 0),
>  	IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
> @@ -2608,6 +2736,12 @@ done:
>  		    (cmd == VIDIOC_QBUF || cmd == VIDIOC_DQBUF))
>  			return ret;
>  
> +#ifndef CONFIG_64BIT
> +		if (!(dev_debug & V4L2_DEV_DEBUG_STREAMING) &&
> +		    (cmd == VIDIOC_QBUF_TIME32 || cmd == VIDIOC_DQBUF_TIME32))
> +			return ret;
> +#endif
> +
>  		v4l_printk_ioctl(video_device_node_name(vfd), cmd);
>  		if (ret < 0)
>  			pr_cont(": error %ld", ret);
> @@ -2630,6 +2764,26 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
>  	int ret = 0;
>  
>  	switch (cmd) {
> +#ifndef CONFIG_64BIT
> +	case VIDIOC_PREPARE_BUF_TIME32:
> +	case VIDIOC_QUERYBUF_TIME32:
> +	case VIDIOC_QBUF_TIME32:
> +	case VIDIOC_DQBUF_TIME32: {
> +		struct v4l2_buffer_time32 *buf = parg;
> +
> +		if (V4L2_TYPE_IS_MULTIPLANAR(buf->type) && buf->length > 0) {
> +			if (buf->length > VIDEO_MAX_PLANES) {
> +				ret = -EINVAL;
> +				break;
> +			}
> +			*user_ptr = (void __user *)buf->m.planes;
> +			*kernel_ptr = (void **)&buf->m.planes;
> +			*array_size = sizeof(struct v4l2_plane) * buf->length;
> +			ret = 1;
> +		}
> +		break;
> +	}
> +#endif
>  	case VIDIOC_PREPARE_BUF:
>  	case VIDIOC_QUERYBUF:
>  	case VIDIOC_QBUF:
> @@ -2774,6 +2928,12 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>  			trace_v4l2_dqbuf(video_devdata(file)->minor, parg);
>  		else if (cmd == VIDIOC_QBUF)
>  			trace_v4l2_qbuf(video_devdata(file)->minor, parg);
> +#ifndef CONFIG_64BIT
> +		else if (cmd == VIDIOC_DQBUF_TIME32)
> +			trace_v4l2_dqbuf_time32(video_devdata(file)->minor, parg);
> +		else if (cmd == VIDIOC_QBUF_TIME32)
> +			trace_v4l2_qbuf_time32(video_devdata(file)->minor, parg);
> +#endif
>  	}
>  
>  	if (has_array_args) {
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 450b3249ba30..0d3688a97ab8 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -811,8 +811,8 @@ struct v4l2_plane {
>   * time_t, so we have to convert it when accessing user data.
>   */
>  struct v4l2_timeval {
> -	long tv_sec;
> -	long tv_usec;
> +	__s64 tv_sec;
> +	__s64 tv_usec;
>  };
>  #endif
>  
> @@ -873,6 +873,32 @@ struct v4l2_buffer {
>  	__u32			reserved;
>  };
>  
> +struct v4l2_buffer_time32 {
> +	__u32			index;
> +	__u32			type;
> +	__u32			bytesused;
> +	__u32			flags;
> +	__u32			field;
> +	struct {
> +		__s32		tv_sec;
> +		__s32		tv_usec;
> +	}			timestamp;
> +	struct v4l2_timecode	timecode;
> +	__u32			sequence;
> +
> +	/* memory location */
> +	__u32			memory;
> +	union {
> +		__u32           offset;
> +		unsigned long   userptr;
> +		struct v4l2_plane *planes;
> +		__s32		fd;
> +	} m;
> +	__u32			length;
> +	__u32			reserved2;
> +	__u32			reserved;
> +};

Should this be part of a public API at all? Userspace will never use this struct
or the TIME32 ioctls in the source code, right? This would be more appropriate for
v4l2-ioctl.h.

> +
>  /*  Flags for 'flags' field */
>  /* Buffer is mapped (flag) */
>  #define V4L2_BUF_FLAG_MAPPED			0x00000001
> @@ -2216,12 +2242,15 @@ struct v4l2_create_buffers {
>  #define VIDIOC_S_FMT		_IOWR('V',  5, struct v4l2_format)
>  #define VIDIOC_REQBUFS		_IOWR('V',  8, struct v4l2_requestbuffers)
>  #define VIDIOC_QUERYBUF		_IOWR('V',  9, struct v4l2_buffer)
> +#define VIDIOC_QUERYBUF_TIME32	_IOWR('V',  9, struct v4l2_buffer_time32)
>  #define VIDIOC_G_FBUF		 _IOR('V', 10, struct v4l2_framebuffer)
>  #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
>  #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
>  #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
> +#define VIDIOC_QBUF_TIME32	_IOWR('V', 15, struct v4l2_buffer_time32)
>  #define VIDIOC_EXPBUF		_IOWR('V', 16, struct v4l2_exportbuffer)
>  #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
> +#define VIDIOC_DQBUF_TIME32	_IOWR('V', 17, struct v4l2_buffer_time32)
>  #define VIDIOC_STREAMON		 _IOW('V', 18, int)
>  #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
>  #define VIDIOC_G_PARM		_IOWR('V', 21, struct v4l2_streamparm)
> @@ -2292,6 +2321,7 @@ struct v4l2_create_buffers {
>     versions */
>  #define VIDIOC_CREATE_BUFS	_IOWR('V', 92, struct v4l2_create_buffers)
>  #define VIDIOC_PREPARE_BUF	_IOWR('V', 93, struct v4l2_buffer)
> +#define VIDIOC_PREPARE_BUF_TIME32 _IOWR('V', 93, struct v4l2_buffer_time32)
>  
>  /* Experimental selection API */
>  #define VIDIOC_G_SELECTION	_IOWR('V', 94, struct v4l2_selection)
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-17 21:19 ` [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann
@ 2015-09-18  8:05   ` Hans Verkuil
  2015-09-18  9:09     ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18  8:05 UTC (permalink / raw)
  To: Arnd Bergmann, linux-media
  Cc: linux-kernel, y2038, Mauro Carvalho Chehab, linux-api, linux-samsung-soc

On 09/17/15 23:19, Arnd Bergmann wrote:
> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> space. This is broken on 32-bit architectures as soon as we have a C library
> that defines time_t as 64 bit, which then changes the structure layout of
> struct v4l2_buffer.
> 
> Since existing user space source code relies on the type to be 'struct
> timeva' and we want to preserve compile-time compatibility when moving

s/timeva/timeval/

> to a new libc, we cannot make user-visible changes to the header file.
> 
> In this patch, we change the type of the timestamp to 'struct v4l2_timeval'

Don't we need a kernel-wide timeval64? Rather than adding a v4l2-specific
struct?

> as a preparation for a follow-up that adds API compatiblity for both
> 32-bit and 64-bit time_t. This patch should have no impact on generated
> code in either user space or kernel.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/pci/bt8xx/bttv-driver.c      |  2 +-
>  drivers/media/pci/meye/meye.h              |  2 +-
>  drivers/media/pci/zoran/zoran.h            |  2 +-
>  drivers/media/platform/coda/coda.h         |  2 +-
>  drivers/media/platform/omap/omap_vout.c    |  4 ++--
>  drivers/media/platform/omap3isp/ispstat.c  |  3 ++-
>  drivers/media/platform/omap3isp/ispstat.h  |  2 +-
>  drivers/media/platform/vim2m.c             |  2 +-
>  drivers/media/platform/vivid/vivid-ctrls.c |  2 +-
>  drivers/media/usb/cpia2/cpia2.h            |  2 +-
>  drivers/media/usb/cpia2/cpia2_v4l.c        |  2 +-
>  drivers/media/usb/gspca/gspca.c            |  2 +-
>  drivers/media/usb/usbvision/usbvision.h    |  2 +-
>  drivers/media/v4l2-core/v4l2-common.c      |  6 +++---
>  include/media/v4l2-common.h                |  2 +-
>  include/media/videobuf-core.h              |  2 +-
>  include/trace/events/v4l2.h                | 12 ++++++++++--
>  include/uapi/linux/videodev2.h             | 17 +++++++++++++++++
>  18 files changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index 15a4ebc2844d..b0ed8d799c14 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -3585,7 +3585,7 @@ static void
>  bttv_irq_wakeup_video(struct bttv *btv, struct bttv_buffer_set *wakeup,
>  		      struct bttv_buffer_set *curr, unsigned int state)
>  {
> -	struct timeval ts;
> +	struct v4l2_timeval ts;
>  
>  	v4l2_get_timestamp(&ts);
>  
> diff --git a/drivers/media/pci/meye/meye.h b/drivers/media/pci/meye/meye.h
> index 751be5e533c7..a06aa5ba9abc 100644
> --- a/drivers/media/pci/meye/meye.h
> +++ b/drivers/media/pci/meye/meye.h
> @@ -281,7 +281,7 @@
>  struct meye_grab_buffer {
>  	int state;			/* state of buffer */
>  	unsigned long size;		/* size of jpg frame */
> -	struct timeval timestamp;	/* timestamp */
> +	struct v4l2_timeval timestamp;	/* timestamp */
>  	unsigned long sequence;		/* sequence number */
>  };
>  
> diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
> index 4e7db8939c2b..9a9f782cede9 100644
> --- a/drivers/media/pci/zoran/zoran.h
> +++ b/drivers/media/pci/zoran/zoran.h
> @@ -39,7 +39,7 @@ struct zoran_sync {
>  	unsigned long frame;	/* number of buffer that has been free'd */
>  	unsigned long length;	/* number of code bytes in buffer (capture only) */
>  	unsigned long seq;	/* frame sequence number */
> -	struct timeval timestamp;	/* timestamp */
> +	struct v4l2_timeval timestamp;	/* timestamp */
>  };
>  
>  
> diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
> index 59b2af9c7749..fa1e15a757cd 100644
> --- a/drivers/media/platform/coda/coda.h
> +++ b/drivers/media/platform/coda/coda.h
> @@ -138,7 +138,7 @@ struct coda_buffer_meta {
>  	struct list_head	list;
>  	u32			sequence;
>  	struct v4l2_timecode	timecode;
> -	struct timeval		timestamp;
> +	struct v4l2_timeval	timestamp;
>  	u32			start;
>  	u32			end;
>  };
> diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
> index 70c28d19ea04..974a238a86fe 100644
> --- a/drivers/media/platform/omap/omap_vout.c
> +++ b/drivers/media/platform/omap/omap_vout.c
> @@ -513,7 +513,7 @@ static int omapvid_apply_changes(struct omap_vout_device *vout)
>  }
>  
>  static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
> -		unsigned int irqstatus, struct timeval timevalue)
> +		unsigned int irqstatus, struct v4l2_timeval timevalue)
>  {
>  	u32 fid;
>  
> @@ -557,7 +557,7 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
>  	int ret, fid, mgr_id;
>  	u32 addr, irq;
>  	struct omap_overlay *ovl;
> -	struct timeval timevalue;
> +	struct v4l2_timeval timevalue;
>  	struct omapvideo_info *ovid;
>  	struct omap_dss_device *cur_display;
>  	struct omap_vout_device *vout = (struct omap_vout_device *)arg;
> diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
> index 94d4c295d3d0..fa96e330c563 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -496,7 +496,8 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
>  		return PTR_ERR(buf);
>  	}
>  
> -	data->ts = buf->ts;
> +	data->ts.tv_sec = buf->ts.tv_sec;
> +	data->ts.tv_usec = buf->ts.tv_usec;
>  	data->config_counter = buf->config_counter;
>  	data->frame_number = buf->frame_number;
>  	data->buf_size = buf->buf_size;
> diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h
> index 6d9b0244f320..7b4f136567a3 100644
> --- a/drivers/media/platform/omap3isp/ispstat.h
> +++ b/drivers/media/platform/omap3isp/ispstat.h
> @@ -39,7 +39,7 @@ struct ispstat_buffer {
>  	struct sg_table sgt;
>  	void *virt_addr;
>  	dma_addr_t dma_addr;
> -	struct timeval ts;
> +	struct v4l2_timeval ts;
>  	u32 buf_size;
>  	u32 frame_number;
>  	u16 config_counter;
> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> index 295fde5fdb75..df5daac6d099 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
>  	in_vb->v4l2_buf.sequence = q_data->sequence++;
>  	memcpy(&out_vb->v4l2_buf.timestamp,
>  			&in_vb->v4l2_buf.timestamp,
> -			sizeof(struct timeval));
> +			sizeof(struct v4l2_timeval));
>  	if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
>  		memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
>  			sizeof(struct v4l2_timecode));

See https://patchwork.linuxtv.org/patch/31405/

I'll merge that one for 4.4 very soon.

> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> index 339c8b7e53c8..065f3d6c2409 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -935,7 +935,7 @@ static const struct v4l2_ctrl_config vivid_ctrl_has_scaler_out = {
>  static int vivid_streaming_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct vivid_dev *dev = container_of(ctrl->handler, struct vivid_dev, ctrl_hdl_streaming);
> -	struct timeval tv;
> +	struct v4l2_timeval tv;
>  
>  	switch (ctrl->id) {
>  	case VIVID_CID_DQBUF_ERROR:
> diff --git a/drivers/media/usb/cpia2/cpia2.h b/drivers/media/usb/cpia2/cpia2.h
> index cdef677d57ec..3e7c523784e7 100644
> --- a/drivers/media/usb/cpia2/cpia2.h
> +++ b/drivers/media/usb/cpia2/cpia2.h
> @@ -354,7 +354,7 @@ struct cpia2_sbuf {
>  };
>  
>  struct framebuf {
> -	struct timeval timestamp;
> +	struct v4l2_timeval timestamp;
>  	unsigned long seq;
>  	int num;
>  	int length;
> diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
> index 9caea8344547..ce50d7ef4da7 100644
> --- a/drivers/media/usb/cpia2/cpia2_v4l.c
> +++ b/drivers/media/usb/cpia2/cpia2_v4l.c
> @@ -892,7 +892,7 @@ static int find_earliest_filled_buffer(struct camera_data *cam)
>  				found = i;
>  			} else {
>  				/* find which buffer is earlier */
> -				struct timeval *tv1, *tv2;
> +				struct v4l2_timeval *tv1, *tv2;
>  				tv1 = &cam->buffers[i].timestamp;
>  				tv2 = &cam->buffers[found].timestamp;
>  				if(tv1->tv_sec < tv2->tv_sec ||
> diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
> index af5cd8213e8b..c977937da9c4 100644
> --- a/drivers/media/usb/gspca/gspca.c
> +++ b/drivers/media/usb/gspca/gspca.c
> @@ -1898,7 +1898,7 @@ static ssize_t dev_read(struct file *file, char __user *data,
>  	struct gspca_dev *gspca_dev = video_drvdata(file);
>  	struct gspca_frame *frame;
>  	struct v4l2_buffer v4l2_buf;
> -	struct timeval timestamp;
> +	struct v4l2_timeval timestamp;
>  	int n, ret, ret2;
>  
>  	PDEBUG(D_FRAM, "read (%zd)", count);
> diff --git a/drivers/media/usb/usbvision/usbvision.h b/drivers/media/usb/usbvision/usbvision.h
> index 4f2e4fde38f2..512de31613df 100644
> --- a/drivers/media/usb/usbvision/usbvision.h
> +++ b/drivers/media/usb/usbvision/usbvision.h
> @@ -320,7 +320,7 @@ struct usbvision_frame {
>  	long bytes_read;				/* amount of scanlength that has been read from data */
>  	struct usbvision_v4l2_format_st v4l2_format;	/* format the user needs*/
>  	int v4l2_linesize;				/* bytes for one videoline*/
> -	struct timeval timestamp;
> +	struct v4l2_timeval timestamp;
>  	int sequence;					/* How many video frames we send to user */
>  };
>  
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 5b808500e7e7..589fab615f21 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -396,11 +396,11 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
>  }
>  EXPORT_SYMBOL_GPL(v4l2_find_nearest_format);
>  
> -void v4l2_get_timestamp(struct timeval *tv)
> +void v4l2_get_timestamp(struct v4l2_timeval *tv)
>  {
> -	struct timespec ts;
> +	struct timespec64 ts;
>  
> -	ktime_get_ts(&ts);
> +	ktime_get_ts64(&ts);
>  	tv->tv_sec = ts.tv_sec;
>  	tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  }
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 1cc0c5ba16b3..6e77d889c3f8 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -187,6 +187,6 @@ const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
>  		const struct v4l2_discrete_probe *probe,
>  		s32 width, s32 height);
>  
> -void v4l2_get_timestamp(struct timeval *tv);
> +void v4l2_get_timestamp(struct v4l2_timeval *tv);
>  
>  #endif /* V4L2_COMMON_H_ */
> diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
> index d760aa73ebbb..6381338a9588 100644
> --- a/include/media/videobuf-core.h
> +++ b/include/media/videobuf-core.h
> @@ -80,7 +80,7 @@ struct videobuf_buffer {
>  	struct list_head        queue;
>  	wait_queue_head_t       done;
>  	unsigned int            field_count;
> -	struct timeval          ts;
> +	struct v4l2_timeval     ts;
>  
>  	/* Memory type */
>  	enum v4l2_memory        memory;
> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
> index dbf017bfddd9..a88be48dc473 100644
> --- a/include/trace/events/v4l2.h
> +++ b/include/trace/events/v4l2.h
> @@ -6,6 +6,14 @@
>  
>  #include <linux/tracepoint.h>
>  
> +#ifndef v4l2_timeval_to_ns
> +#define v4l2_timeval_to_ns v4l2_timeval_to_ns
> +static inline u64 v4l2_timeval_to_ns(struct v4l2_timeval *tv)
> +{
> +	return (u64)tv->tv_sec * NSEC_PER_SEC + tv->tv_usec * NSEC_PER_USEC;
> +}
> +#endif
> +
>  /* Enums require being exported to userspace, for user tool parsing */
>  #undef EM
>  #undef EMe
> @@ -126,7 +134,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_timeval_to_ns(&buf->timestamp);
>  		__entry->timecode_type = buf->timecode.type;
>  		__entry->timecode_flags = buf->timecode.flags;
>  		__entry->timecode_frames = buf->timecode.frames;
> @@ -211,7 +219,7 @@ DECLARE_EVENT_CLASS(vb2_event_class,
>  		__entry->bytesused = vb->v4l2_planes[0].bytesused;
>  		__entry->flags = vb->v4l2_buf.flags;
>  		__entry->field = vb->v4l2_buf.field;
> -		__entry->timestamp = timeval_to_ns(&vb->v4l2_buf.timestamp);
> +		__entry->timestamp = v4l2_timeval_to_ns(&vb->v4l2_buf.timestamp);
>  		__entry->timecode_type = vb->v4l2_buf.timecode.type;
>  		__entry->timecode_flags = vb->v4l2_buf.timecode.flags;
>  		__entry->timecode_frames = vb->v4l2_buf.timecode.frames;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3e2c497c31fd..450b3249ba30 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -803,6 +803,19 @@ struct v4l2_plane {
>  	__u32			reserved[11];
>  };
>  
> +#ifdef __KERNEL__
> +/*
> + * This is used for the in-kernel version of v4l2_buffer, as we are
> + * migrating away from using time_t based structures in the kernel.
> + * User space might see this defined either using 32-bit or 64-bit
> + * time_t, so we have to convert it when accessing user data.
> + */
> +struct v4l2_timeval {
> +	long tv_sec;
> +	long tv_usec;
> +};
> +#endif
> +
>  /**
>   * struct v4l2_buffer - video buffer info
>   * @index:	id number of the buffer
> @@ -839,7 +852,11 @@ struct v4l2_buffer {
>  	__u32			bytesused;
>  	__u32			flags;
>  	__u32			field;
> +#ifdef __KERNEL__
> +	struct v4l2_timeval	timestamp;
> +#else
>  	struct timeval		timestamp;
> +#endif
>  	struct v4l2_timecode	timecode;
>  	__u32			sequence;
>  
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18  8:05   ` Hans Verkuil
@ 2015-09-18  9:09     ` Arnd Bergmann
  2015-09-18  9:27       ` Hans Verkuil
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-18  9:09 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On Friday 18 September 2015 10:05:06 Hans Verkuil wrote:
> On 09/17/15 23:19, Arnd Bergmann wrote:
> > The v4l2 API uses a 'struct timeval' to communicate time stamps to user
> > space. This is broken on 32-bit architectures as soon as we have a C library
> > that defines time_t as 64 bit, which then changes the structure layout of
> > struct v4l2_buffer.
> > 
> > Since existing user space source code relies on the type to be 'struct
> > timeva' and we want to preserve compile-time compatibility when moving
> 
> s/timeva/timeval/

Fixed

> > to a new libc, we cannot make user-visible changes to the header file.
> > 
> > In this patch, we change the type of the timestamp to 'struct v4l2_timeval'
> 
> Don't we need a kernel-wide timeval64? Rather than adding a v4l2-specific
> struct?

I still hope to avoid doing that. All in-kernel users should be changed to
use timespec64 or ktime_t, which are always more efficient and accurate.

For the system call interface, all timeval APIs are deprecated and have
replacements using timespec64 (e.g. clock_gettime() replaces gettimeofday).

Only a handful of ioctls pass timeval, and so far my impression is that
we are better off handling each one separately. The total amount of code
we need to add this way should be less than if we have to duplicate all
common code functions that today operate on timeval and can eventually
get removed.

> > diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> > index 295fde5fdb75..df5daac6d099 100644
> > --- a/drivers/media/platform/vim2m.c
> > +++ b/drivers/media/platform/vim2m.c
> > @@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
> >  	in_vb->v4l2_buf.sequence = q_data->sequence++;
> >  	memcpy(&out_vb->v4l2_buf.timestamp,
> >  			&in_vb->v4l2_buf.timestamp,
> > -			sizeof(struct timeval));
> > +			sizeof(struct v4l2_timeval));
> >  	if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
> >  		memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
> >  			sizeof(struct v4l2_timecode));
> 
> See https://patchwork.linuxtv.org/patch/31405/
> 
> I'll merge that one for 4.4 very soon.

Ok.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer
  2015-09-18  7:18   ` Hans Verkuil
@ 2015-09-18  9:26     ` Arnd Bergmann
  2015-09-18  9:49       ` Hans Verkuil
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-18  9:26 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On Friday 18 September 2015 09:18:45 Hans Verkuil wrote:
> Hi Arnd,
> 
> Thanks once again for working on this! Unfortunately, this approach won't
> work, see my comments below.
> 
> BTW, I would expect to see compile errors when compiling for 32 bit. Did
> you try that?

I only tested on 32-bit, both ARM and x86, but had a longer set of patches
applied below.

> > +static void v4l_put_buffer_time32(struct v4l2_buffer_time32 *to,
> > +				  const struct v4l2_buffer *from)
> > +{
> > +	to->index		= from->index;
> > +	to->type		= from->type;
> > +	to->bytesused		= from->bytesused;
> > +	to->flags		= from->flags;
> > +	to->field		= from->field;
> > +	to->timestamp.tv_sec	= from->timestamp.tv_sec;
> > +	to->timestamp.tv_usec	= from->timestamp.tv_usec;
> > +	to->timecode		= from->timecode;
> > +	to->sequence		= from->sequence;
> > +	to->memory		= from->memory;
> > +	to->m.offset		= from->m.offset;
> > +	to->length		= from->length;
> > +	to->reserved2		= from->reserved2;
> > +	to->reserved		= from->reserved;
> > +}
> 
> Is there a reason why you didn't use memcpy like you did for VIDIOC_DQEVENT (path 5/9)?
> I would prefer that over these explicit assignments.

No strong reason. I went back and forth a bit on the m.offset field that
is part of a union: In a previous version, I planned to move all the
compat handling here, and doing the conversion one field at a time would
make it easier to share the code between 32-bit and 64-bit kernels
handling old 32-bit user space. This version doesn't do that, so I can
use the memcpy approach instead.

> >  static int v4l_g_parm(const struct v4l2_ioctl_ops *ops,
> >  				struct file *file, void *fh, void *arg)
> >  {
> > @@ -2408,12 +2524,21 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
> >  	IOCTL_INFO_FNC(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
> >  	IOCTL_INFO_FNC(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
> >  	IOCTL_INFO_FNC(VIDIOC_QUERYBUF, v4l_querybuf, v4l_print_buffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
> > +#ifndef CONFIG_64BIT
> > +	IOCTL_INFO_FNC(VIDIOC_QUERYBUF_TIME32, v4l_querybuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
> > +#endif
> 
> This doesn't work. These IOCTL macros use the ioctl nr as the index in the array. Since
> VIDIOC_QUERYBUF and VIDIOC_QUERYBUF_TIME32 have the same index, this will fail.

Ah, I see. No idea why that did not cause a compile-time error. I got some
errors for duplicate 'case' values when the structures are the same size
(that's why we need the '#ifdef CONFIG_64BIT' in some places) but not here.

> I think (not 100% certain, there may be better suggestions) that the conversion is best
> done in video_usercopy(): just before func() is called have a switch for the TIME32
> variants, convert to the regular variant, call func() and convert back.

Does the handler have access to the _IOC_SIZE() value that was passed? If
it does, we could add a conditional inside of v4l_querybuf().
I did not see an easy way to do that though.

> My only concern here is that an additional v4l2_buffer struct (68 bytes) is needed on the
> stack. I don't see any way around that, though.

Agreed.

> > +struct v4l2_buffer_time32 {
> > +	__u32			index;
> > +	__u32			type;
> > +	__u32			bytesused;
> > +	__u32			flags;
> > +	__u32			field;
> > +	struct {
> > +		__s32		tv_sec;
> > +		__s32		tv_usec;
> > +	}			timestamp;
> > +	struct v4l2_timecode	timecode;
> > +	__u32			sequence;
> > +
> > +	/* memory location */
> > +	__u32			memory;
> > +	union {
> > +		__u32           offset;
> > +		unsigned long   userptr;
> > +		struct v4l2_plane *planes;
> > +		__s32		fd;
> > +	} m;
> > +	__u32			length;
> > +	__u32			reserved2;
> > +	__u32			reserved;
> > +};
> 
> Should this be part of a public API at all? Userspace will never use this struct
> or the TIME32 ioctls in the source code, right? This would be more appropriate for
> v4l2-ioctl.h.

Yes, makes sense. I think for the other structures I just enclosed them
inside #ifdef __KERNEL__ so they get stripped at 'make headers_install'
time, but I forgot to do that here.

My intention was to keep the structure close to the other one, so any
changes to them would be more likely to make it into both versions.

Let me know if you prefer to have an #ifdef added here, or if I should
move all three structures to v4l2-ioctl.h.

Thanks a lot for the review!

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18  9:09     ` Arnd Bergmann
@ 2015-09-18  9:27       ` Hans Verkuil
  2015-09-18  9:43         ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18  9:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On 09/18/15 11:09, Arnd Bergmann wrote:
> On Friday 18 September 2015 10:05:06 Hans Verkuil wrote:
>> On 09/17/15 23:19, Arnd Bergmann wrote:
>>> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
>>> space. This is broken on 32-bit architectures as soon as we have a C library
>>> that defines time_t as 64 bit, which then changes the structure layout of
>>> struct v4l2_buffer.
>>>
>>> Since existing user space source code relies on the type to be 'struct
>>> timeva' and we want to preserve compile-time compatibility when moving
>>
>> s/timeva/timeval/
> 
> Fixed
> 
>>> to a new libc, we cannot make user-visible changes to the header file.
>>>
>>> In this patch, we change the type of the timestamp to 'struct v4l2_timeval'
>>
>> Don't we need a kernel-wide timeval64? Rather than adding a v4l2-specific
>> struct?
> 
> I still hope to avoid doing that. All in-kernel users should be changed to
> use timespec64 or ktime_t, which are always more efficient and accurate.
> 
> For the system call interface, all timeval APIs are deprecated and have
> replacements using timespec64 (e.g. clock_gettime() replaces gettimeofday).
> 
> Only a handful of ioctls pass timeval, and so far my impression is that
> we are better off handling each one separately. The total amount of code
> we need to add this way should be less than if we have to duplicate all
> common code functions that today operate on timeval and can eventually
> get removed.

Ah, OK. Got it.

I think this is dependent on the upcoming media workshop next month. If we
decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
And the only place where we would need to convert it in the compat code
hidden in the v4l2 core (likely v4l2-ioctl.c).

I am not really keen on having v4l2_timeval in all these drivers. I would
have to check them anyway since I suspect that in several drivers the local
timeval variable can be avoided by rewriting that part of the driver.

Personally I am in favor of a redesigned v4l2_buffer: it's awkward to use
with multiplanar formats, there is cruft in there that can be removed (timecode),
and there is little space for additions (HW-specific timecodes, more buffer
meta data, etc).

We'll see.

Regards,

	Hans

> 
>>> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
>>> index 295fde5fdb75..df5daac6d099 100644
>>> --- a/drivers/media/platform/vim2m.c
>>> +++ b/drivers/media/platform/vim2m.c
>>> @@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
>>>  	in_vb->v4l2_buf.sequence = q_data->sequence++;
>>>  	memcpy(&out_vb->v4l2_buf.timestamp,
>>>  			&in_vb->v4l2_buf.timestamp,
>>> -			sizeof(struct timeval));
>>> +			sizeof(struct v4l2_timeval));
>>>  	if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
>>>  		memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
>>>  			sizeof(struct v4l2_timecode));
>>
>> See https://patchwork.linuxtv.org/patch/31405/
>>
>> I'll merge that one for 4.4 very soon.
> 
> Ok.
> 
> 	Arnd
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18  9:27       ` Hans Verkuil
@ 2015-09-18  9:43         ` Arnd Bergmann
  2015-09-18  9:52           ` Hans Verkuil
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-18  9:43 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On Friday 18 September 2015 11:27:40 Hans Verkuil wrote:
> Ah, OK. Got it.
> 
> I think this is dependent on the upcoming media workshop next month. If we
> decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
> And the only place where we would need to convert it in the compat code
> hidden in the v4l2 core (likely v4l2-ioctl.c).

Ah, I think I understood the idea now, I missed that earlier when you mention
the idea.

So what you are saying here is that you could come up with a new unambiguous
(using only __u32 and __u64 types and no pointers) format that gets exposed
to a new set of ioctls, and then change the handling of the existing three
formats (native 64-bit, traditional 32-bit, and 32-bit with 64-bit time_t)
so they get converted into the new format by the common ioctl handling code?

> I am not really keen on having v4l2_timeval in all these drivers. I would
> have to check them anyway since I suspect that in several drivers the local
> timeval variable can be avoided by rewriting that part of the driver.

I've tried to do that for all the drivers where I could find an easy solution
in patch 6/9, but I'm sure you can do it for a couple more.

We could also do a lightweight redesign and use 'timespec64' internally
in all the drivers and then convert that to 'timeval' or the 64-bit
format of that in the ioctl handler. This is also something I tried at
some point but then found it a bit more intuitive to leave the normal ioctl
path alone and have an explicit type.

> Personally I am in favor of a redesigned v4l2_buffer: it's awkward to use
> with multiplanar formats, there is cruft in there that can be removed (timecode),
> and there is little space for additions (HW-specific timecodes, more buffer
> meta data, etc).
> 
> We'll see.

Ok.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer
  2015-09-18  9:26     ` Arnd Bergmann
@ 2015-09-18  9:49       ` Hans Verkuil
  2015-09-18 10:14         ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18  9:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On 09/18/15 11:26, Arnd Bergmann wrote:
> On Friday 18 September 2015 09:18:45 Hans Verkuil wrote:
>> Hi Arnd,
>>
>> Thanks once again for working on this! Unfortunately, this approach won't
>> work, see my comments below.
>>
>> BTW, I would expect to see compile errors when compiling for 32 bit. Did
>> you try that?
> 
> I only tested on 32-bit, both ARM and x86, but had a longer set of patches
> applied below.
> 
>>> +static void v4l_put_buffer_time32(struct v4l2_buffer_time32 *to,
>>> +				  const struct v4l2_buffer *from)
>>> +{
>>> +	to->index		= from->index;
>>> +	to->type		= from->type;
>>> +	to->bytesused		= from->bytesused;
>>> +	to->flags		= from->flags;
>>> +	to->field		= from->field;
>>> +	to->timestamp.tv_sec	= from->timestamp.tv_sec;
>>> +	to->timestamp.tv_usec	= from->timestamp.tv_usec;
>>> +	to->timecode		= from->timecode;
>>> +	to->sequence		= from->sequence;
>>> +	to->memory		= from->memory;
>>> +	to->m.offset		= from->m.offset;
>>> +	to->length		= from->length;
>>> +	to->reserved2		= from->reserved2;
>>> +	to->reserved		= from->reserved;
>>> +}
>>
>> Is there a reason why you didn't use memcpy like you did for VIDIOC_DQEVENT (path 5/9)?
>> I would prefer that over these explicit assignments.
> 
> No strong reason. I went back and forth a bit on the m.offset field that
> is part of a union: In a previous version, I planned to move all the
> compat handling here, and doing the conversion one field at a time would
> make it easier to share the code between 32-bit and 64-bit kernels
> handling old 32-bit user space. This version doesn't do that, so I can
> use the memcpy approach instead.
> 
>>>  static int v4l_g_parm(const struct v4l2_ioctl_ops *ops,
>>>  				struct file *file, void *fh, void *arg)
>>>  {
>>> @@ -2408,12 +2524,21 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>>>  	IOCTL_INFO_FNC(VIDIOC_S_FMT, v4l_s_fmt, v4l_print_format, INFO_FL_PRIO),
>>>  	IOCTL_INFO_FNC(VIDIOC_REQBUFS, v4l_reqbufs, v4l_print_requestbuffers, INFO_FL_PRIO | INFO_FL_QUEUE),
>>>  	IOCTL_INFO_FNC(VIDIOC_QUERYBUF, v4l_querybuf, v4l_print_buffer, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
>>> +#ifndef CONFIG_64BIT
>>> +	IOCTL_INFO_FNC(VIDIOC_QUERYBUF_TIME32, v4l_querybuf_time32, v4l_print_buffer_time32, INFO_FL_QUEUE | INFO_FL_CLEAR(v4l2_buffer, length)),
>>> +#endif
>>
>> This doesn't work. These IOCTL macros use the ioctl nr as the index in the array. Since
>> VIDIOC_QUERYBUF and VIDIOC_QUERYBUF_TIME32 have the same index, this will fail.
> 
> Ah, I see. No idea why that did not cause a compile-time error. I got some
> errors for duplicate 'case' values when the structures are the same size
> (that's why we need the '#ifdef CONFIG_64BIT' in some places) but not here.
> 
>> I think (not 100% certain, there may be better suggestions) that the conversion is best
>> done in video_usercopy(): just before func() is called have a switch for the TIME32
>> variants, convert to the regular variant, call func() and convert back.
> 
> Does the handler have access to the _IOC_SIZE() value that was passed? If
> it does, we could add a conditional inside of v4l_querybuf().
> I did not see an easy way to do that though.

No, the v4l_ handlers don't have access to that. But I think it is much easier to
do the conversion at the first opportunity (video_usercopy), so no other code
needs to be modified. Easier to review and maintain that way.

>> My only concern here is that an additional v4l2_buffer struct (68 bytes) is needed on the
>> stack. I don't see any way around that, though.
> 
> Agreed.
> 
>>> +struct v4l2_buffer_time32 {
>>> +	__u32			index;
>>> +	__u32			type;
>>> +	__u32			bytesused;
>>> +	__u32			flags;
>>> +	__u32			field;
>>> +	struct {
>>> +		__s32		tv_sec;
>>> +		__s32		tv_usec;
>>> +	}			timestamp;
>>> +	struct v4l2_timecode	timecode;
>>> +	__u32			sequence;
>>> +
>>> +	/* memory location */
>>> +	__u32			memory;
>>> +	union {
>>> +		__u32           offset;
>>> +		unsigned long   userptr;
>>> +		struct v4l2_plane *planes;
>>> +		__s32		fd;
>>> +	} m;
>>> +	__u32			length;
>>> +	__u32			reserved2;
>>> +	__u32			reserved;
>>> +};
>>
>> Should this be part of a public API at all? Userspace will never use this struct
>> or the TIME32 ioctls in the source code, right? This would be more appropriate for
>> v4l2-ioctl.h.
> 
> Yes, makes sense. I think for the other structures I just enclosed them
> inside #ifdef __KERNEL__ so they get stripped at 'make headers_install'
> time, but I forgot to do that here.
> 
> My intention was to keep the structure close to the other one, so any
> changes to them would be more likely to make it into both versions.
> 
> Let me know if you prefer to have an #ifdef added here, or if I should
> move all three structures to v4l2-ioctl.h.

*If* the conversion takes place only in v4l2-ioctl.c, then it makes sense
have these structs + ioctls moved to v4l2-ioctl.h.

I noticed that v4l2-compat-ioctl32.c wasn't changed. Is that right? I have
unfortunately no time to double-check that, but it surprised me.

Regards,

	Hans

> Thanks a lot for the review!
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18  9:43         ` Arnd Bergmann
@ 2015-09-18  9:52           ` Hans Verkuil
  2015-09-18 10:08             ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18  9:52 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On 09/18/15 11:43, Arnd Bergmann wrote:
> On Friday 18 September 2015 11:27:40 Hans Verkuil wrote:
>> Ah, OK. Got it.
>>
>> I think this is dependent on the upcoming media workshop next month. If we
>> decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
>> And the only place where we would need to convert it in the compat code
>> hidden in the v4l2 core (likely v4l2-ioctl.c).
> 
> Ah, I think I understood the idea now, I missed that earlier when you mention
> the idea.
> 
> So what you are saying here is that you could come up with a new unambiguous
> (using only __u32 and __u64 types and no pointers) format that gets exposed
> to a new set of ioctls, and then change the handling of the existing three
> formats (native 64-bit, traditional 32-bit, and 32-bit with 64-bit time_t)
> so they get converted into the new format by the common ioctl handling code?

Right. Drivers only see the new struct, and only v4l2-ioctl.c (and possible
v4l2-compat-ioctl32.c) see the old ones.

BTW, I will probably pick up patches 4 and 6 for 4.4. That should help a bit.

Regards,

	Hans

>> I am not really keen on having v4l2_timeval in all these drivers. I would
>> have to check them anyway since I suspect that in several drivers the local
>> timeval variable can be avoided by rewriting that part of the driver.
> 
> I've tried to do that for all the drivers where I could find an easy solution
> in patch 6/9, but I'm sure you can do it for a couple more.
> 
> We could also do a lightweight redesign and use 'timespec64' internally
> in all the drivers and then convert that to 'timeval' or the 64-bit
> format of that in the ioctl handler. This is also something I tried at
> some point but then found it a bit more intuitive to leave the normal ioctl
> path alone and have an explicit type.
> 
>> Personally I am in favor of a redesigned v4l2_buffer: it's awkward to use
>> with multiplanar formats, there is cruft in there that can be removed (timecode),
>> and there is little space for additions (HW-specific timecodes, more buffer
>> meta data, etc).
>>
>> We'll see.
> 
> Ok.
> 
> 	Arnd
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18  9:52           ` Hans Verkuil
@ 2015-09-18 10:08             ` Arnd Bergmann
  2015-09-18 10:22               ` Hans Verkuil
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-18 10:08 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On Friday 18 September 2015 11:52:28 Hans Verkuil wrote:
> On 09/18/15 11:43, Arnd Bergmann wrote:
> > On Friday 18 September 2015 11:27:40 Hans Verkuil wrote:
> >> Ah, OK. Got it.
> >>
> >> I think this is dependent on the upcoming media workshop next month. If we
> >> decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
> >> And the only place where we would need to convert it in the compat code
> >> hidden in the v4l2 core (likely v4l2-ioctl.c).
> > 
> > Ah, I think I understood the idea now, I missed that earlier when you mention
> > the idea.
> > 
> > So what you are saying here is that you could come up with a new unambiguous
> > (using only __u32 and __u64 types and no pointers) format that gets exposed
> > to a new set of ioctls, and then change the handling of the existing three
> > formats (native 64-bit, traditional 32-bit, and 32-bit with 64-bit time_t)
> > so they get converted into the new format by the common ioctl handling code?
> 
> Right. Drivers only see the new struct, and only v4l2-ioctl.c (and possible
> v4l2-compat-ioctl32.c) see the old ones.
> 
> BTW, I will probably pick up patches 4 and 6 for 4.4. That should help a bit.

Ok, thanks!

I guess it's up to Mauro to pick up the first three patches?

As I don't see anything more to do for me here until you've had the
discussion about the new format, I'll move on to another subsystem now.
I have around 70 patches waiting to be submitted, plus the system call
series.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer
  2015-09-18  9:49       ` Hans Verkuil
@ 2015-09-18 10:14         ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-09-18 10:14 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On Friday 18 September 2015 11:49:50 Hans Verkuil wrote:
> *If* the conversion takes place only in v4l2-ioctl.c, then it makes sense
> have these structs + ioctls moved to v4l2-ioctl.h.

Ok.
 
> I noticed that v4l2-compat-ioctl32.c wasn't changed. Is that right? I have
> unfortunately no time to double-check that, but it surprised me.

Yes, I pointed that out in the cover letter as something that is still
left to be done. If we end up taking the current approach (plus the
change to the location of the conversion), the compat code would
basically be duplicated.

Alternatively, the video_usercopy() function could just be changed
to handle both 32-bit formats directly (including conversion of the
multiplane array) and then we can remove that compat handler for
all eight ioctls.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval
  2015-09-18 10:08             ` Arnd Bergmann
@ 2015-09-18 10:22               ` Hans Verkuil
  0 siblings, 0 replies; 24+ messages in thread
From: Hans Verkuil @ 2015-09-18 10:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc

On 09/18/15 12:08, Arnd Bergmann wrote:
> On Friday 18 September 2015 11:52:28 Hans Verkuil wrote:
>> On 09/18/15 11:43, Arnd Bergmann wrote:
>>> On Friday 18 September 2015 11:27:40 Hans Verkuil wrote:
>>>> Ah, OK. Got it.
>>>>
>>>> I think this is dependent on the upcoming media workshop next month. If we
>>>> decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
>>>> And the only place where we would need to convert it in the compat code
>>>> hidden in the v4l2 core (likely v4l2-ioctl.c).
>>>
>>> Ah, I think I understood the idea now, I missed that earlier when you mention
>>> the idea.
>>>
>>> So what you are saying here is that you could come up with a new unambiguous
>>> (using only __u32 and __u64 types and no pointers) format that gets exposed
>>> to a new set of ioctls, and then change the handling of the existing three
>>> formats (native 64-bit, traditional 32-bit, and 32-bit with 64-bit time_t)
>>> so they get converted into the new format by the common ioctl handling code?
>>
>> Right. Drivers only see the new struct, and only v4l2-ioctl.c (and possible
>> v4l2-compat-ioctl32.c) see the old ones.
>>
>> BTW, I will probably pick up patches 4 and 6 for 4.4. That should help a bit.
> 
> Ok, thanks!
> 
> I guess it's up to Mauro to pick up the first three patches?

Yes.

> As I don't see anything more to do for me here until you've had the
> discussion about the new format, I'll move on to another subsystem now.
> I have around 70 patches waiting to be submitted, plus the system call
> series.

Agreed.

Thanks again for working on this!

Regards,

	Hans

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
  2015-09-17 21:19 ` [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data Arnd Bergmann
@ 2015-11-09 20:09   ` Laurent Pinchart
  2015-11-09 20:30     ` [Y2038] " Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Laurent Pinchart @ 2015-11-09 20:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-media, linux-kernel, y2038, Mauro Carvalho Chehab,
	linux-api, linux-samsung-soc, Hans Verkuil

Hi Arnd,

Thank you for the patch.

On Thursday 17 September 2015 23:19:40 Arnd Bergmann wrote:
> C libraries with 64-bit time_t use an incompatible format for
> struct omap3isp_stat_data. This changes the kernel code to
> support either version, by moving over the normal handling
> to the 64-bit variant, and adding compatiblity code to handle
> the old binary format with the existing ioctl command code.
> 
> Fortunately, the command code includes the size of the structure,
> so the difference gets handled automatically.

We plan to design a new interface to handle statistics in V4L2. That API 
should support proper 64-bit timestamps out of the box, and will be 
implemented by the OMAP3 ISP driver. Userspace should then move to it. I 
wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl 
given that I expect it to have a handful of users at most.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/platform/omap3isp/isph3a_aewb.c |  2 ++
>  drivers/media/platform/omap3isp/isph3a_af.c   |  2 ++
>  drivers/media/platform/omap3isp/isphist.c     |  2 ++
>  drivers/media/platform/omap3isp/ispstat.c     | 18 ++++++++++++++++--
>  drivers/media/platform/omap3isp/ispstat.h     |  2 ++
>  include/uapi/linux/omap3isp.h                 | 19 +++++++++++++++++++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c
> b/drivers/media/platform/omap3isp/isph3a_aewb.c index
> ccaf92f39236..2d567725608f 100644
> --- a/drivers/media/platform/omap3isp/isph3a_aewb.c
> +++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
> @@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd,
> unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_REQ:
>  		return omap3isp_stat_request_statistics(stat, arg);
> +	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> +		return omap3isp_stat_request_statistics_time32(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_EN: {
>  		unsigned long *en = arg;
>  		return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/isph3a_af.c
> b/drivers/media/platform/omap3isp/isph3a_af.c index
> 92937f7eecef..2ac02371318b 100644
> --- a/drivers/media/platform/omap3isp/isph3a_af.c
> +++ b/drivers/media/platform/omap3isp/isph3a_af.c
> @@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd,
> unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_REQ:
>  		return omap3isp_stat_request_statistics(stat, arg);
> +	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> +		return omap3isp_stat_request_statistics_time32(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_EN: {
>  		int *en = arg;
>  		return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/isphist.c
> b/drivers/media/platform/omap3isp/isphist.c index
> 7138b043a4aa..669b97b079ee 100644
> --- a/drivers/media/platform/omap3isp/isphist.c
> +++ b/drivers/media/platform/omap3isp/isphist.c
> @@ -436,6 +436,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned
> int cmd, void *arg) return omap3isp_stat_config(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_REQ:
>  		return omap3isp_stat_request_statistics(stat, arg);
> +	case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
> +		return omap3isp_stat_request_statistics_time32(stat, arg);
>  	case VIDIOC_OMAP3ISP_STAT_EN: {
>  		int *en = arg;
>  		return omap3isp_stat_enable(stat, !!*en);
> diff --git a/drivers/media/platform/omap3isp/ispstat.c
> b/drivers/media/platform/omap3isp/ispstat.c index
> fa96e330c563..3d70332422b5 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -496,8 +496,7 @@ int omap3isp_stat_request_statistics(struct ispstat
> *stat, return PTR_ERR(buf);
>  	}
> 
> -	data->ts.tv_sec = buf->ts.tv_sec;
> -	data->ts.tv_usec = buf->ts.tv_usec;
> +	data->ts = buf->ts;
>  	data->config_counter = buf->config_counter;
>  	data->frame_number = buf->frame_number;
>  	data->buf_size = buf->buf_size;
> @@ -509,6 +508,21 @@ int omap3isp_stat_request_statistics(struct ispstat
> *stat, return 0;
>  }
> 
> +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
> +					struct omap3isp_stat_data_time32 *data)
> +{
> +	struct omap3isp_stat_data data64;
> +	int ret;
> +
> +	ret = omap3isp_stat_request_statistics(stat, &data64);
> +
> +	data->ts.tv_sec = data64.ts.tv_sec;
> +	data->ts.tv_usec = data64.ts.tv_usec;
> +	memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts));
> +
> +	return ret;
> +}
> +
>  /*
>   * omap3isp_stat_config - Receives new statistic engine configuration.
>   * @new_conf: Pointer to config structure.
> diff --git a/drivers/media/platform/omap3isp/ispstat.h
> b/drivers/media/platform/omap3isp/ispstat.h index
> 7b4f136567a3..b19ea6c8f733 100644
> --- a/drivers/media/platform/omap3isp/ispstat.h
> +++ b/drivers/media/platform/omap3isp/ispstat.h
> @@ -130,6 +130,8 @@ struct ispstat_generic_config {
>  int omap3isp_stat_config(struct ispstat *stat, void *new_conf);
>  int omap3isp_stat_request_statistics(struct ispstat *stat,
>  				     struct omap3isp_stat_data *data);
> +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
> +				     struct omap3isp_stat_data_time32 *data);
>  int omap3isp_stat_init(struct ispstat *stat, const char *name,
>  		       const struct v4l2_subdev_ops *sd_ops);
>  void omap3isp_stat_cleanup(struct ispstat *stat);
> diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
> index c090cf9249bb..4bff66aefca5 100644
> --- a/include/uapi/linux/omap3isp.h
> +++ b/include/uapi/linux/omap3isp.h
> @@ -54,6 +54,8 @@
>  	_IOWR('V', BASE_VIDIOC_PRIVATE + 5, struct omap3isp_h3a_af_config)
>  #define VIDIOC_OMAP3ISP_STAT_REQ \
>  	_IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data)
> +#define VIDIOC_OMAP3ISP_STAT_REQ_TIME32 \
> +	_IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data_time32)
>  #define VIDIOC_OMAP3ISP_STAT_EN \
>  	_IOWR('V', BASE_VIDIOC_PRIVATE + 7, unsigned long)
> 
> @@ -164,7 +166,11 @@ struct omap3isp_h3a_aewb_config {
>   * @config_counter: Number of the configuration associated with the data.
>   */
>  struct omap3isp_stat_data {
> +#ifdef __KERNEL__
> +	struct v4l2_timeval ts;
> +#else
>  	struct timeval ts;
> +#endif
>  	void __user *buf;
>  	__u32 buf_size;
>  	__u16 frame_number;
> @@ -172,6 +178,19 @@ struct omap3isp_stat_data {
>  	__u16 config_counter;
>  };
> 
> +#ifdef __KERNEL__
> +struct omap3isp_stat_data_time32 {
> +	struct {
> +		__s32	tv_sec;
> +		__s32	tv_usec;
> +	} ts;
> +	__u32 buf;
> +	__u32 buf_size;
> +	__u16 frame_number;
> +	__u16 cur_frame;
> +	__u16 config_counter;
> +};
> +#endif
> 
>  /* Histogram related structs */

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Y2038] [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
  2015-11-09 20:09   ` Laurent Pinchart
@ 2015-11-09 20:30     ` Arnd Bergmann
  2015-11-09 21:04       ` Laurent Pinchart
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-11-09 20:30 UTC (permalink / raw)
  To: y2038
  Cc: Laurent Pinchart, linux-samsung-soc, Mauro Carvalho Chehab,
	linux-api, linux-kernel, Hans Verkuil, linux-media

On Monday 09 November 2015 22:09:26 Laurent Pinchart wrote:
> Hi Arnd,
> 
> Thank you for the patch.
> 
> On Thursday 17 September 2015 23:19:40 Arnd Bergmann wrote:
> > C libraries with 64-bit time_t use an incompatible format for
> > struct omap3isp_stat_data. This changes the kernel code to
> > support either version, by moving over the normal handling
> > to the 64-bit variant, and adding compatiblity code to handle
> > the old binary format with the existing ioctl command code.
> > 
> > Fortunately, the command code includes the size of the structure,
> > so the difference gets handled automatically.
> 
> We plan to design a new interface to handle statistics in V4L2. That API 
> should support proper 64-bit timestamps out of the box, and will be 
> implemented by the OMAP3 ISP driver. Userspace should then move to it. I 
> wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl 
> given that I expect it to have a handful of users at most.

We still need to do something to the driver. The alternative would
be to make the existing ioctl command optional at kernel compile-time
so we can still build the driver once we remove the 'struct timeval'
definition. That patch would add slightly less complexity here
but also lose functionality.

As my patch here depends on the struct v4l2_timeval I introduced in
an earlier patch of the series, we will have to change it anyways,
but I'd prefer to keep the basic idea. Let's get back to this one
after the v4l_buffer replacement work is done.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Y2038] [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data
  2015-11-09 20:30     ` [Y2038] " Arnd Bergmann
@ 2015-11-09 21:04       ` Laurent Pinchart
  0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2015-11-09 21:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: y2038, linux-samsung-soc, Mauro Carvalho Chehab, linux-api,
	linux-kernel, Hans Verkuil, linux-media

Hi Arnd,

On Monday 09 November 2015 21:30:41 Arnd Bergmann wrote:
> On Monday 09 November 2015 22:09:26 Laurent Pinchart wrote:
> > On Thursday 17 September 2015 23:19:40 Arnd Bergmann wrote:
> > > C libraries with 64-bit time_t use an incompatible format for
> > > struct omap3isp_stat_data. This changes the kernel code to
> > > support either version, by moving over the normal handling
> > > to the 64-bit variant, and adding compatiblity code to handle
> > > the old binary format with the existing ioctl command code.
> > > 
> > > Fortunately, the command code includes the size of the structure,
> > > so the difference gets handled automatically.
> > 
> > We plan to design a new interface to handle statistics in V4L2. That API
> > should support proper 64-bit timestamps out of the box, and will be
> > implemented by the OMAP3 ISP driver. Userspace should then move to it. I
> > wonder if it's worth it to fix the existing VIDIOC_OMAP3ISP_STAT_REQ ioctl
> > given that I expect it to have a handful of users at most.
> 
> We still need to do something to the driver. The alternative would
> be to make the existing ioctl command optional at kernel compile-time
> so we can still build the driver once we remove the 'struct timeval'
> definition. That patch would add slightly less complexity here
> but also lose functionality.
> 
> As my patch here depends on the struct v4l2_timeval I introduced in
> an earlier patch of the series, we will have to change it anyways,
> but I'd prefer to keep the basic idea. Let's get back to this one
> after the v4l_buffer replacement work is done.

Fine with me. I'll mark the patch as "Obsoleted" in patchwork in the meantime.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-11-09 21:04 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-17 21:19 [PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 2/9] [media] dvb: remove unused systime() function Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 3/9] [media] dvb: don't use 'time_t' in event ioctl Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 4/9] [media] exynos4-is: use monotonic timestamps as advertized Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 5/9] [media] make VIDIOC_DQEVENT work with 64-bit time_t Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 6/9] [media] use v4l2_get_timestamp where possible Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann
2015-09-18  8:05   ` Hans Verkuil
2015-09-18  9:09     ` Arnd Bergmann
2015-09-18  9:27       ` Hans Verkuil
2015-09-18  9:43         ` Arnd Bergmann
2015-09-18  9:52           ` Hans Verkuil
2015-09-18 10:08             ` Arnd Bergmann
2015-09-18 10:22               ` Hans Verkuil
2015-09-17 21:19 ` [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann
2015-09-18  7:18   ` Hans Verkuil
2015-09-18  9:26     ` Arnd Bergmann
2015-09-18  9:49       ` Hans Verkuil
2015-09-18 10:14         ` Arnd Bergmann
2015-09-17 21:19 ` [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data Arnd Bergmann
2015-11-09 20:09   ` Laurent Pinchart
2015-11-09 20:30     ` [Y2038] " Arnd Bergmann
2015-11-09 21:04       ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).