[PATCH 0/4] via-camera/ov7670: various fixes/improvements

[PATCH 0/4] via-camera/ov7670: various fixes/improvements

[Hans Verkuil]

The main purpose of this series was to make via-camera use struct v4l2_fh,
since it was one of the few drivers that didn't use that struct.

While testing I found a bunch of other issues with this driver and the
ov7670 driver, which are also fixed.

The next step will be to convert this driver to use vb2, but that's
still work in progress.

This series has been tested on an OLPC 1.5 laptop.

Regards,

	Hans

Hans Verkuil (4):
  via-camera: call viafb_pm_unregister in remove()
  via-camera: use struct v4l2_fh
  ov7670: don't return ENOTTY if SUBDEV_API is not set
  via-camera: fix v4l2-compliance fails

 drivers/media/i2c/ov7670.c          |  6 +-
 drivers/media/platform/via-camera.c | 94 +++++++++++++++++------------
 2 files changed, 58 insertions(+), 42 deletions(-)

-- 
2.20.1

[PATCH 1/4] via-camera: call viafb_pm_unregister in remove()

[Hans Verkuil]

The power management hooks were never unregistered, which caused a
crash when unloading the module.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/via-camera.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c
index 24d5759501a5..f9016c2ee70d 100644
--- a/drivers/media/platform/via-camera.c
+++ b/drivers/media/platform/via-camera.c
@@ -1464,6 +1464,9 @@ static int viacam_remove(struct platform_device *pdev)
 
 	video_unregister_device(&cam->vdev);
 	v4l2_device_unregister(&cam->v4l2_dev);
+#ifdef CONFIG_PM
+	viafb_pm_unregister(&viacam_pm_hooks);
+#endif
 	free_irq(viadev->pdev->irq, cam);
 	via_sensor_power_release(cam);
 	v4l2_ctrl_handler_free(&cam->ctrl_handler);
-- 
2.20.1

[PATCH 2/4] via-camera: use struct v4l2_fh

[Hans Verkuil]

Modern V4L2 drivers should use struct v4l2_fh to represent a filehandle.
This driver was one of the few that didn't use it.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/via-camera.c | 47 +++++++++++++++++------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c
index f9016c2ee70d..49e51feebc7d 100644
--- a/drivers/media/platform/via-camera.c
+++ b/drivers/media/platform/via-camera.c
@@ -660,19 +660,22 @@ static const struct videobuf_queue_ops viacam_vb_ops = {
 static int viacam_open(struct file *filp)
 {
 	struct via_camera *cam = video_drvdata(filp);
+	int ret;
 
-	filp->private_data = cam;
 	/*
 	 * Note the new user.  If this is the first one, we'll also
 	 * need to power up the sensor.
 	 */
 	mutex_lock(&cam->lock);
-	if (cam->users == 0) {
-		int ret = viafb_request_dma();
+	ret = v4l2_fh_open(filp);
+	if (ret)
+		goto out;
+	if (v4l2_fh_is_singular_file(filp)) {
+		ret = viafb_request_dma();
 
 		if (ret) {
-			mutex_unlock(&cam->lock);
-			return ret;
+			v4l2_fh_release(filp);
+			goto out;
 		}
 		via_sensor_power_up(cam);
 		set_bit(CF_CONFIG_NEEDED, &cam->flags);
@@ -685,16 +688,19 @@ static int viacam_open(struct file *filp)
 				sizeof(struct videobuf_buffer), cam, NULL);
 	}
 	(cam->users)++;
+out:
 	mutex_unlock(&cam->lock);
-	return 0;
+	return ret;
 }
 
 static int viacam_release(struct file *filp)
 {
 	struct via_camera *cam = video_drvdata(filp);
+	bool last_open;
 
 	mutex_lock(&cam->lock);
 	(cam->users)--;
+	last_open = v4l2_fh_is_singular_file(filp);
 	/*
 	 * If the "owner" is closing, shut down any ongoing
 	 * operations.
@@ -714,11 +720,12 @@ static int viacam_release(struct file *filp)
 	/*
 	 * Last one out needs to turn out the lights.
 	 */
-	if (cam->users == 0) {
+	if (last_open) {
 		videobuf_mmap_free(&cam->vb_queue);
 		via_sensor_power_down(cam);
 		viafb_release_dma();
 	}
+	v4l2_fh_release(filp);
 	mutex_unlock(&cam->lock);
 	return 0;
 }
@@ -927,7 +934,7 @@ static int viacam_do_try_fmt(struct via_camera *cam,
 static int viacam_try_fmt_vid_cap(struct file *filp, void *priv,
 		struct v4l2_format *fmt)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	struct v4l2_format sfmt;
 	int ret;
 
@@ -941,7 +948,7 @@ static int viacam_try_fmt_vid_cap(struct file *filp, void *priv,
 static int viacam_g_fmt_vid_cap(struct file *filp, void *priv,
 		struct v4l2_format *fmt)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 
 	mutex_lock(&cam->lock);
 	fmt->fmt.pix = cam->user_format;
@@ -952,7 +959,7 @@ static int viacam_g_fmt_vid_cap(struct file *filp, void *priv,
 static int viacam_s_fmt_vid_cap(struct file *filp, void *priv,
 		struct v4l2_format *fmt)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	int ret;
 	struct v4l2_format sfmt;
 	struct via_format *f = via_find_format(fmt->fmt.pix.pixelformat);
@@ -1004,7 +1011,7 @@ static int viacam_querycap(struct file *filp, void *priv,
 static int viacam_reqbufs(struct file *filp, void *priv,
 		struct v4l2_requestbuffers *rb)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 
 	return videobuf_reqbufs(&cam->vb_queue, rb);
 }
@@ -1012,28 +1019,28 @@ static int viacam_reqbufs(struct file *filp, void *priv,
 static int viacam_querybuf(struct file *filp, void *priv,
 		struct v4l2_buffer *buf)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 
 	return videobuf_querybuf(&cam->vb_queue, buf);
 }
 
 static int viacam_qbuf(struct file *filp, void *priv, struct v4l2_buffer *buf)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 
 	return videobuf_qbuf(&cam->vb_queue, buf);
 }
 
 static int viacam_dqbuf(struct file *filp, void *priv, struct v4l2_buffer *buf)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 
 	return videobuf_dqbuf(&cam->vb_queue, buf, filp->f_flags & O_NONBLOCK);
 }
 
 static int viacam_streamon(struct file *filp, void *priv, enum v4l2_buf_type t)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	int ret = 0;
 
 	if (t != V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1084,7 +1091,7 @@ static int viacam_streamon(struct file *filp, void *priv, enum v4l2_buf_type t)
 
 static int viacam_streamoff(struct file *filp, void *priv, enum v4l2_buf_type t)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	int ret;
 
 	if (t != V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -1113,7 +1120,7 @@ static int viacam_streamoff(struct file *filp, void *priv, enum v4l2_buf_type t)
 static int viacam_g_parm(struct file *filp, void *priv,
 		struct v4l2_streamparm *parm)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	int ret;
 
 	mutex_lock(&cam->lock);
@@ -1126,7 +1133,7 @@ static int viacam_g_parm(struct file *filp, void *priv,
 static int viacam_s_parm(struct file *filp, void *priv,
 		struct v4l2_streamparm *parm)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	int ret;
 
 	mutex_lock(&cam->lock);
@@ -1153,7 +1160,7 @@ static int viacam_enum_framesizes(struct file *filp, void *priv,
 static int viacam_enum_frameintervals(struct file *filp, void *priv,
 		struct v4l2_frmivalenum *interval)
 {
-	struct via_camera *cam = priv;
+	struct via_camera *cam = video_drvdata(filp);
 	struct v4l2_subdev_frame_interval_enum fie = {
 		.index = interval->index,
 		.code = cam->mbus_code,
@@ -1427,10 +1434,10 @@ static int viacam_probe(struct platform_device *pdev)
 	 */
 	cam->vdev = viacam_v4l_template;
 	cam->vdev.v4l2_dev = &cam->v4l2_dev;
+	video_set_drvdata(&cam->vdev, cam);
 	ret = video_register_device(&cam->vdev, VFL_TYPE_GRABBER, -1);
 	if (ret)
 		goto out_irq;
-	video_set_drvdata(&cam->vdev, cam);
 
 #ifdef CONFIG_PM
 	/*
-- 
2.20.1

Re: [PATCH 2/4] via-camera: use struct v4l2_fh

[Ezequiel Garcia]

On Wed, 2019-07-17 at 11:03 +0200, Hans Verkuil wrote:
> Modern V4L2 drivers should use struct v4l2_fh to represent a filehandle.
> This driver was one of the few that didn't use it.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>

> ---
>  drivers/media/platform/via-camera.c | 47 +++++++++++++++++------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c
> index f9016c2ee70d..49e51feebc7d 100644
> --- a/drivers/media/platform/via-camera.c
> +++ b/drivers/media/platform/via-camera.c
> @@ -660,19 +660,22 @@ static const struct videobuf_queue_ops viacam_vb_ops = {
>  static int viacam_open(struct file *filp)
>  {
>  	struct via_camera *cam = video_drvdata(filp);
> +	int ret;
>  
> -	filp->private_data = cam;
>  	/*
>  	 * Note the new user.  If this is the first one, we'll also
>  	 * need to power up the sensor.
>  	 */
>  	mutex_lock(&cam->lock);
> -	if (cam->users == 0) {
> -		int ret = viafb_request_dma();
> +	ret = v4l2_fh_open(filp);
> +	if (ret)
> +		goto out;
> +	if (v4l2_fh_is_singular_file(filp)) {
> +		ret = viafb_request_dma();
>  
>  		if (ret) {
> -			mutex_unlock(&cam->lock);
> -			return ret;
> +			v4l2_fh_release(filp);
> +			goto out;
>  		}
>  		via_sensor_power_up(cam);
>  		set_bit(CF_CONFIG_NEEDED, &cam->flags);
> @@ -685,16 +688,19 @@ static int viacam_open(struct file *filp)
>  				sizeof(struct videobuf_buffer), cam, NULL);
>  	}
>  	(cam->users)++;
> +out:
>  	mutex_unlock(&cam->lock);
> -	return 0;
> +	return ret;
>  }
>  
>  static int viacam_release(struct file *filp)
>  {
>  	struct via_camera *cam = video_drvdata(filp);
> +	bool last_open;
>  
>  	mutex_lock(&cam->lock);
>  	(cam->users)--;
> +	last_open = v4l2_fh_is_singular_file(filp);
>  	/*
>  	 * If the "owner" is closing, shut down any ongoing
>  	 * operations.
> @@ -714,11 +720,12 @@ static int viacam_release(struct file *filp)
>  	/*
>  	 * Last one out needs to turn out the lights.
>  	 */
> -	if (cam->users == 0) {
> +	if (last_open) {
>  		videobuf_mmap_free(&cam->vb_queue);
>  		via_sensor_power_down(cam);
>  		viafb_release_dma();
>  	}
> +	v4l2_fh_release(filp);
>  	mutex_unlock(&cam->lock);
>  	return 0;
>  }
> @@ -927,7 +934,7 @@ static int viacam_do_try_fmt(struct via_camera *cam,
>  static int viacam_try_fmt_vid_cap(struct file *filp, void *priv,
>  		struct v4l2_format *fmt)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	struct v4l2_format sfmt;
>  	int ret;
>  
> @@ -941,7 +948,7 @@ static int viacam_try_fmt_vid_cap(struct file *filp, void *priv,
>  static int viacam_g_fmt_vid_cap(struct file *filp, void *priv,
>  		struct v4l2_format *fmt)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  
>  	mutex_lock(&cam->lock);
>  	fmt->fmt.pix = cam->user_format;
> @@ -952,7 +959,7 @@ static int viacam_g_fmt_vid_cap(struct file *filp, void *priv,
>  static int viacam_s_fmt_vid_cap(struct file *filp, void *priv,
>  		struct v4l2_format *fmt)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	int ret;
>  	struct v4l2_format sfmt;
>  	struct via_format *f = via_find_format(fmt->fmt.pix.pixelformat);
> @@ -1004,7 +1011,7 @@ static int viacam_querycap(struct file *filp, void *priv,
>  static int viacam_reqbufs(struct file *filp, void *priv,
>  		struct v4l2_requestbuffers *rb)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  
>  	return videobuf_reqbufs(&cam->vb_queue, rb);
>  }
> @@ -1012,28 +1019,28 @@ static int viacam_reqbufs(struct file *filp, void *priv,
>  static int viacam_querybuf(struct file *filp, void *priv,
>  		struct v4l2_buffer *buf)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  
>  	return videobuf_querybuf(&cam->vb_queue, buf);
>  }
>  
>  static int viacam_qbuf(struct file *filp, void *priv, struct v4l2_buffer *buf)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  
>  	return videobuf_qbuf(&cam->vb_queue, buf);
>  }
>  
>  static int viacam_dqbuf(struct file *filp, void *priv, struct v4l2_buffer *buf)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  
>  	return videobuf_dqbuf(&cam->vb_queue, buf, filp->f_flags & O_NONBLOCK);
>  }
>  
>  static int viacam_streamon(struct file *filp, void *priv, enum v4l2_buf_type t)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	int ret = 0;
>  
>  	if (t != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1084,7 +1091,7 @@ static int viacam_streamon(struct file *filp, void *priv, enum v4l2_buf_type t)
>  
>  static int viacam_streamoff(struct file *filp, void *priv, enum v4l2_buf_type t)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	int ret;
>  
>  	if (t != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1113,7 +1120,7 @@ static int viacam_streamoff(struct file *filp, void *priv, enum v4l2_buf_type t)
>  static int viacam_g_parm(struct file *filp, void *priv,
>  		struct v4l2_streamparm *parm)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	int ret;
>  
>  	mutex_lock(&cam->lock);
> @@ -1126,7 +1133,7 @@ static int viacam_g_parm(struct file *filp, void *priv,
>  static int viacam_s_parm(struct file *filp, void *priv,
>  		struct v4l2_streamparm *parm)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	int ret;
>  
>  	mutex_lock(&cam->lock);
> @@ -1153,7 +1160,7 @@ static int viacam_enum_framesizes(struct file *filp, void *priv,
>  static int viacam_enum_frameintervals(struct file *filp, void *priv,
>  		struct v4l2_frmivalenum *interval)
>  {
> -	struct via_camera *cam = priv;
> +	struct via_camera *cam = video_drvdata(filp);
>  	struct v4l2_subdev_frame_interval_enum fie = {
>  		.index = interval->index,
>  		.code = cam->mbus_code,
> @@ -1427,10 +1434,10 @@ static int viacam_probe(struct platform_device *pdev)
>  	 */
>  	cam->vdev = viacam_v4l_template;
>  	cam->vdev.v4l2_dev = &cam->v4l2_dev;
> +	video_set_drvdata(&cam->vdev, cam);
>  	ret = video_register_device(&cam->vdev, VFL_TYPE_GRABBER, -1);
>  	if (ret)
>  		goto out_irq;
> -	video_set_drvdata(&cam->vdev, cam);
>  
>  #ifdef CONFIG_PM
>  	/*

[PATCH 3/4] ov7670: don't return ENOTTY if SUBDEV_API is not set

[Hans Verkuil]

If CONFIG_VIDEO_V4L2_SUBDEV_API is not set, then it is still possible
to call set_fmt for V4L2_SUBDEV_FORMAT_TRY, the result is just not
stored. So return 0 instead of -ENOTTY.

Calling get_fmt with V4L2_SUBDEV_FORMAT_TRY should return -EINVAL
instead of -ENOTTY, after all the get_fmt functionality is still
present, just not supported for TRY.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/ov7670.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index a70a6ff7b36e..53e238e98d11 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1030,10 +1030,8 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
 #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
 		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
 		*mbus_fmt = format->format;
-		return 0;
-#else
-		return -ENOTTY;
 #endif
+		return 0;
 	}
 
 	ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
@@ -1116,7 +1114,7 @@ static int ov7670_get_fmt(struct v4l2_subdev *sd,
 		format->format = *mbus_fmt;
 		return 0;
 #else
-		return -ENOTTY;
+		return -EINVAL;
 #endif
 	} else {
 		format->format = info->format;
-- 
2.20.1

[PATCH 4/4] via-camera: fix v4l2-compliance fails

[Hans Verkuil]

The patch fixes various v4l2-compliance failures:

- missing support for control events
- support of s/g_std even though this doesn't apply to a webcam
- missing colorspace reporting
- bus_info wasn't filled in by VIDIOC_QUERYCAP
- parm.capture.readbuffers was overridden with wrong value
- viacam_enum_framesizes/intervals didn't check the arguments

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/via-camera.c | 44 +++++++++++++++++------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/via-camera.c b/drivers/media/platform/via-camera.c
index 49e51feebc7d..70da662b9241 100644
--- a/drivers/media/platform/via-camera.c
+++ b/drivers/media/platform/via-camera.c
@@ -18,6 +18,7 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-image-sizes.h>
 #include <media/i2c/ov7670.h>
 #include <media/videobuf-dma-sg.h>
@@ -780,7 +781,7 @@ static __poll_t viacam_poll(struct file *filp, struct poll_table_struct *pt)
 {
 	struct via_camera *cam = video_drvdata(filp);
 
-	return videobuf_poll_stream(filp, &cam->vb_queue, pt);
+	return v4l2_ctrl_poll(filp, pt) | videobuf_poll_stream(filp, &cam->vb_queue, pt);
 }
 
 
@@ -818,7 +819,6 @@ static int viacam_enum_input(struct file *filp, void *priv,
 		return -EINVAL;
 
 	input->type = V4L2_INPUT_TYPE_CAMERA;
-	input->std = V4L2_STD_ALL; /* Not sure what should go here */
 	strscpy(input->name, "Camera", sizeof(input->name));
 	return 0;
 }
@@ -836,17 +836,6 @@ static int viacam_s_input(struct file *filp, void *priv, unsigned int i)
 	return 0;
 }
 
-static int viacam_s_std(struct file *filp, void *priv, v4l2_std_id std)
-{
-	return 0;
-}
-
-static int viacam_g_std(struct file *filp, void *priv, v4l2_std_id *std)
-{
-	*std = V4L2_STD_NTSC_M;
-	return 0;
-}
-
 /*
  * Video format stuff.	Here is our default format until
  * user space messes with things.
@@ -858,6 +847,7 @@ static const struct v4l2_pix_format viacam_def_pix_format = {
 	.field		= V4L2_FIELD_NONE,
 	.bytesperline	= VGA_WIDTH * 2,
 	.sizeimage	= VGA_WIDTH * VGA_HEIGHT * 2,
+	.colorspace	= V4L2_COLORSPACE_SRGB,
 };
 
 static const u32 via_def_mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
@@ -904,6 +894,10 @@ static void viacam_fmt_post(struct v4l2_pix_format *userfmt,
 	userfmt->field = sensorfmt->field;
 	userfmt->bytesperline = 2 * userfmt->width;
 	userfmt->sizeimage = userfmt->bytesperline * userfmt->height;
+	userfmt->colorspace = sensorfmt->colorspace;
+	userfmt->ycbcr_enc = sensorfmt->ycbcr_enc;
+	userfmt->quantization = sensorfmt->quantization;
+	userfmt->xfer_func = sensorfmt->xfer_func;
 }
 
 
@@ -999,6 +993,7 @@ static int viacam_querycap(struct file *filp, void *priv,
 {
 	strscpy(cap->driver, "via-camera", sizeof(cap->driver));
 	strscpy(cap->card, "via-camera", sizeof(cap->card));
+	strscpy(cap->bus_info, "platform:via-camera", sizeof(cap->bus_info));
 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
@@ -1126,7 +1121,6 @@ static int viacam_g_parm(struct file *filp, void *priv,
 	mutex_lock(&cam->lock);
 	ret = v4l2_g_parm_cap(video_devdata(filp), cam->sensor, parm);
 	mutex_unlock(&cam->lock);
-	parm->parm.capture.readbuffers = cam->n_cap_bufs;
 	return ret;
 }
 
@@ -1139,15 +1133,21 @@ static int viacam_s_parm(struct file *filp, void *priv,
 	mutex_lock(&cam->lock);
 	ret = v4l2_s_parm_cap(video_devdata(filp), cam->sensor, parm);
 	mutex_unlock(&cam->lock);
-	parm->parm.capture.readbuffers = cam->n_cap_bufs;
 	return ret;
 }
 
 static int viacam_enum_framesizes(struct file *filp, void *priv,
 		struct v4l2_frmsizeenum *sizes)
 {
+	unsigned int i;
+
 	if (sizes->index != 0)
 		return -EINVAL;
+	for (i = 0; i < N_VIA_FMTS; i++)
+		if (sizes->pixel_format == via_formats[i].pixelformat)
+			break;
+	if (i >= N_VIA_FMTS)
+		return -EINVAL;
 	sizes->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
 	sizes->stepwise.min_width = QCIF_WIDTH;
 	sizes->stepwise.min_height = QCIF_HEIGHT;
@@ -1168,8 +1168,17 @@ static int viacam_enum_frameintervals(struct file *filp, void *priv,
 		.height = cam->sensor_format.height,
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
+	unsigned int i;
 	int ret;
 
+	for (i = 0; i < N_VIA_FMTS; i++)
+		if (interval->pixel_format == via_formats[i].pixelformat)
+			break;
+	if (i >= N_VIA_FMTS)
+		return -EINVAL;
+	if (interval->width < QCIF_WIDTH || interval->width > VGA_WIDTH ||
+	    interval->height < QCIF_HEIGHT || interval->height > VGA_HEIGHT)
+		return -EINVAL;
 	mutex_lock(&cam->lock);
 	ret = sensor_call(cam, pad, enum_frame_interval, NULL, &fie);
 	mutex_unlock(&cam->lock);
@@ -1186,8 +1195,6 @@ static const struct v4l2_ioctl_ops viacam_ioctl_ops = {
 	.vidioc_enum_input	= viacam_enum_input,
 	.vidioc_g_input		= viacam_g_input,
 	.vidioc_s_input		= viacam_s_input,
-	.vidioc_s_std		= viacam_s_std,
-	.vidioc_g_std		= viacam_g_std,
 	.vidioc_enum_fmt_vid_cap = viacam_enum_fmt_vid_cap,
 	.vidioc_try_fmt_vid_cap = viacam_try_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap	= viacam_g_fmt_vid_cap,
@@ -1203,6 +1210,8 @@ static const struct v4l2_ioctl_ops viacam_ioctl_ops = {
 	.vidioc_s_parm		= viacam_s_parm,
 	.vidioc_enum_framesizes = viacam_enum_framesizes,
 	.vidioc_enum_frameintervals = viacam_enum_frameintervals,
+	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
 };
 
 /*----------------------------------------------------------------------------*/
@@ -1274,7 +1283,6 @@ static struct viafb_pm_hooks viacam_pm_hooks = {
 static const struct video_device viacam_v4l_template = {
 	.name		= "via-camera",
 	.minor		= -1,
-	.tvnorms	= V4L2_STD_NTSC_M,
 	.fops		= &viacam_fops,
 	.ioctl_ops	= &viacam_ioctl_ops,
 	.release	= video_device_release_empty, /* Check this */
-- 
2.20.1