[REVIEW PATCH 00/12] em28xx: ioctl fixes/clean-ups

[REVIEW PATCH 00/12] em28xx: ioctl fixes/clean-ups

[Frank Schäfer]

This patch series addresses some issues with the ioctl code of the em28xx driver:
Patches 1-5 and 11 fix and clean up the enabling/disabling of the ioctls 
depending on the device type and equipment.
Patches 6-10 remove some obsolete/useless code.
Patch 12 improves the VBI support detection and device node registration.


Frank Schäfer (12):
  em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_QUERYSTD,
    VIDIOC_G/S_STD
  em28xx: disable tuner related ioctls for video and VBI devices
    without tuner
  em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_G_AUDIO and
    VIDIOC_S_AUDIO
  em28xx: use v4l2_disable_ioctl() to disable ioctl VIDIOC_S_PARM
  em28xx: disable ioctl VIDIOC_S_PARM for VBI devices
  em28xx: make ioctls VIDIOC_G/S_PARM working for VBI devices
  em28xx: remove ioctl VIDIOC_CROPCAP
  em28xx: get rid of duplicate function vidioc_s_fmt_vbi_cap()
  em28xx: VIDIOC_G_TUNER: remove unneeded setting of tuner type
  em28xx: remove obsolete device state checks from the ioctl functions
  em28xx: make ioctl VIDIOC_DBG_G_CHIP_IDENT available for radio
    devices
  em28xx: do not claim VBI support if the device is a camera

 drivers/media/usb/em28xx/em28xx-core.c  |    5 ++
 drivers/media/usb/em28xx/em28xx-video.c |  147 +++++++------------------------
 2 Dateien geändert, 35 Zeilen hinzugefügt(+), 117 Zeilen entfernt(-)

-- 
1.7.10.4

[REVIEW PATCH 01/12] em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_QUERYSTD, VIDIOC_G/S_STD

[Frank Schäfer]

Instead of checking the device type and returning -ENOTTY inside the ioctl
functions, use v4l2_disable_ioctl() to disable the ioctls VIDIOC_QUERYSTD,
VIDIOC_G_STD and VIDIOC_S_STD if the device is a camera.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   13 +++++++------
 1 Datei geändert, 7 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 2eabf2a..7f1f37c 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -959,8 +959,6 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct em28xx      *dev = fh->dev;
 	int                rc;
 
-	if (dev->board.is_webcam)
-		return -ENOTTY;
 	rc = check_dev(dev);
 	if (rc < 0)
 		return rc;
@@ -976,8 +974,6 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
 	struct em28xx      *dev = fh->dev;
 	int                rc;
 
-	if (dev->board.is_webcam)
-		return -ENOTTY;
 	rc = check_dev(dev);
 	if (rc < 0)
 		return rc;
@@ -994,8 +990,6 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct v4l2_format f;
 	int                rc;
 
-	if (dev->board.is_webcam)
-		return -ENOTTY;
 	if (*norm == dev->norm)
 		return 0;
 	rc = check_dev(dev);
@@ -1899,6 +1893,13 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 	dev->vdev->queue = &dev->vb_vidq;
 	dev->vdev->queue->lock = &dev->vb_queue_lock;
 
+	/* disable inapplicable ioctls */
+	if (dev->board.is_webcam) {
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_QUERYSTD);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_STD);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_STD);
+	}
+
 	/* register v4l2 video video_device */
 	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
 				       video_nr[dev->devno]);
-- 
1.7.10.4

[REVIEW PATCH 02/12] em28xx: disable tuner related ioctls for video and VBI devices without tuner

[Frank Schäfer]

Disable the ioctls VIDIOC_G_TUNER, VIDIOC_S_TUNER, VIDIOC_G_FREQUENCY and
VIDIOC_S_FREQUENCY for video and VBI devices without tuner.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   14 ++++++++++++++
 1 Datei geändert, 14 Zeilen hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 7f1f37c..dd2e31c 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1899,6 +1899,12 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_STD);
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_STD);
 	}
+	if (dev->tuner_type == TUNER_ABSENT) {
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_TUNER);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_TUNER);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_FREQUENCY);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_FREQUENCY);
+	}
 
 	/* register v4l2 video video_device */
 	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
@@ -1917,6 +1923,14 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		dev->vbi_dev->queue = &dev->vb_vbiq;
 		dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
 
+		/* disable inapplicable ioctls */
+		if (dev->tuner_type == TUNER_ABSENT) {
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_TUNER);
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_TUNER);
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_FREQUENCY);
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_FREQUENCY);
+		}
+
 		/* register v4l2 vbi video_device */
 		ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
 					    vbi_nr[dev->devno]);
-- 
1.7.10.4

[REVIEW PATCH 03/12] em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_G_AUDIO and VIDIOC_S_AUDIO

[Frank Schäfer]

Instead of checking the device type and returning -EINVAL inside the ioctl
functions, use v4l2_disable_ioctl() to disable the ioctls VIDIOC_G_AUDIO and
VIDIOC_S_AUDIO if the device doesn't support audio.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   15 ++++++++-------
 1 Datei geändert, 8 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index dd2e31c..378d8a1 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1130,9 +1130,6 @@ static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 	struct em28xx_fh   *fh    = priv;
 	struct em28xx      *dev   = fh->dev;
 
-	if (!dev->audio_mode.has_audio)
-		return -EINVAL;
-
 	switch (a->index) {
 	case EM28XX_AMUX_VIDEO:
 		strcpy(a->name, "Television");
@@ -1173,10 +1170,6 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 
-
-	if (!dev->audio_mode.has_audio)
-		return -EINVAL;
-
 	if (a->index >= MAX_EM28XX_INPUT)
 		return -EINVAL;
 	if (0 == INPUT(a->index)->type)
@@ -1905,6 +1898,10 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_FREQUENCY);
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_FREQUENCY);
 	}
+	if (!dev->audio_mode.has_audio) {
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_AUDIO);
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_AUDIO);
+	}
 
 	/* register v4l2 video video_device */
 	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
@@ -1930,6 +1927,10 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_FREQUENCY);
 			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_FREQUENCY);
 		}
+		if (!dev->audio_mode.has_audio) {
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_AUDIO);
+			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_AUDIO);
+		}
 
 		/* register v4l2 vbi video_device */
 		ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
-- 
1.7.10.4

[REVIEW PATCH 04/12] em28xx: use v4l2_disable_ioctl() to disable ioctl VIDIOC_S_PARM

[Frank Schäfer]

Instead of checking the device type and returning -ENOTTY inside the ioctl
function, use v4l2_disable_ioctl() to disable the ioctl VIDIOC_S_PARM if the
device is not a camera.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    5 ++---
 1 Datei geändert, 2 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 378d8a1..c76714d 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1044,9 +1044,6 @@ static int vidioc_s_parm(struct file *file, void *priv,
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 
-	if (!dev->board.is_webcam)
-		return -ENOTTY;
-
 	if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
@@ -1891,6 +1888,8 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_QUERYSTD);
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_STD);
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_STD);
+	} else {
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
 	}
 	if (dev->tuner_type == TUNER_ABSENT) {
 		v4l2_disable_ioctl(dev->vdev, VIDIOC_G_TUNER);
-- 
1.7.10.4

[REVIEW PATCH 05/12] em28xx: disable ioctl VIDIOC_S_PARM for VBI devices

[Frank Schäfer]

VIDIOC_S_PARM doesn't make sense for VBI device nodes, because we don't support
selecting the number of read buffers to use.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +
 1 Datei geändert, 1 Zeile hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index c76714d..d4dc5b2 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1920,6 +1920,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
 
 		/* disable inapplicable ioctls */
+		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
 		if (dev->tuner_type == TUNER_ABSENT) {
 			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_TUNER);
 			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_TUNER);
-- 
1.7.10.4

[REVIEW PATCH 06/12] em28xx: make ioctls VIDIOC_G/S_PARM working for VBI devices

[Frank Schäfer]

With the current code V4L2_BUF_TYPE_VIDEO_CAPTURE is accepted only, but for VBI
devices only buffer type V4L2_BUF_TYPE_VBI_CAPTURE is used/valid.

Remove the buffer type check entirely, because this is already done by the v4l2-core.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    6 ------
 1 Datei geändert, 6 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index d4dc5b2..6172d59 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1024,9 +1024,6 @@ static int vidioc_g_parm(struct file *file, void *priv,
 	struct em28xx      *dev = fh->dev;
 	int rc = 0;
 
-	if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		return -EINVAL;
-
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
 	if (dev->board.is_webcam)
 		rc = v4l2_device_call_until_err(&dev->v4l2_dev, 0,
@@ -1044,9 +1041,6 @@ static int vidioc_s_parm(struct file *file, void *priv,
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 
-	if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		return -EINVAL;
-
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
 	return v4l2_device_call_until_err(&dev->v4l2_dev, 0, video, s_parm, p);
 }
-- 
1.7.10.4

[REVIEW PATCH 07/12] em28xx: remove ioctl VIDIOC_CROPCAP

[Frank Schäfer]

The em28xx driver doesn't support the VIDIOC_G_CROP and VIDIOC_S_CROP ioctls,
so VIDIOC_CROPCAP is useless and has the potential to confuse applications,
because it can be interpreted as indicator for cropping support.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   21 ---------------------
 1 Datei geändert, 21 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 6172d59..edd29ae 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1364,26 +1364,6 @@ static int vidioc_s_register(struct file *file, void *priv,
 #endif
 
 
-static int vidioc_cropcap(struct file *file, void *priv,
-					struct v4l2_cropcap *cc)
-{
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
-
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		return -EINVAL;
-
-	cc->bounds.left = 0;
-	cc->bounds.top = 0;
-	cc->bounds.width = dev->width;
-	cc->bounds.height = dev->height;
-	cc->defrect = cc->bounds;
-	cc->pixelaspect.numerator = 54;	/* 4:3 FIXME: remove magic numbers */
-	cc->pixelaspect.denominator = 59;
-
-	return 0;
-}
-
 static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *cap)
 {
@@ -1731,7 +1711,6 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_enum_framesizes     = vidioc_enum_framesizes,
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
-	.vidioc_cropcap             = vidioc_cropcap,
 
 	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
 	.vidioc_create_bufs         = vb2_ioctl_create_bufs,
-- 
1.7.10.4

[REVIEW PATCH 08/12] em28xx: get rid of duplicate function vidioc_s_fmt_vbi_cap()

[Frank Schäfer]

vidioc_s_fmt_vbi_cap() is a 100% duplicate of vidioc_g_fmt_vbi_cap() and
therefore can be removed.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   31 +------------------------------
 1 Datei geändert, 1 Zeile hinzugefügt(+), 30 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index edd29ae..af3e70a 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1480,35 +1480,6 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_s_fmt_vbi_cap(struct file *file, void *priv,
-				struct v4l2_format *format)
-{
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
-
-	format->fmt.vbi.samples_per_line = dev->vbi_width;
-	format->fmt.vbi.sample_format = V4L2_PIX_FMT_GREY;
-	format->fmt.vbi.offset = 0;
-	format->fmt.vbi.flags = 0;
-	format->fmt.vbi.sampling_rate = 6750000 * 4 / 2;
-	format->fmt.vbi.count[0] = dev->vbi_height;
-	format->fmt.vbi.count[1] = dev->vbi_height;
-	memset(format->fmt.vbi.reserved, 0, sizeof(format->fmt.vbi.reserved));
-
-	/* Varies by video standard (NTSC, PAL, etc.) */
-	if (dev->norm & V4L2_STD_525_60) {
-		/* NTSC */
-		format->fmt.vbi.start[0] = 10;
-		format->fmt.vbi.start[1] = 273;
-	} else if (dev->norm & V4L2_STD_625_50) {
-		/* PAL */
-		format->fmt.vbi.start[0] = 6;
-		format->fmt.vbi.start[1] = 318;
-	}
-
-	return 0;
-}
-
 /* ----------------------------------------------------------- */
 /* RADIO ESPECIFIC IOCTLS                                      */
 /* ----------------------------------------------------------- */
@@ -1707,7 +1678,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap       = vidioc_s_fmt_vid_cap,
 	.vidioc_g_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap     = vidioc_g_fmt_vbi_cap,
-	.vidioc_s_fmt_vbi_cap       = vidioc_s_fmt_vbi_cap,
+	.vidioc_s_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
 	.vidioc_enum_framesizes     = vidioc_enum_framesizes,
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
-- 
1.7.10.4

[REVIEW PATCH 09/12] em28xx: VIDIOC_G_TUNER: remove unneeded setting of tuner type

[Frank Schäfer]

The tuner type is set by the v4l2-core based on the device type.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    1 -
 1 Datei geändert, 1 Zeile entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index af3e70a..319d0ee 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1493,7 +1493,6 @@ static int radio_g_tuner(struct file *file, void *priv,
 		return -EINVAL;
 
 	strcpy(t->name, "Radio");
-	t->type = V4L2_TUNER_RADIO;
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
 
-- 
1.7.10.4

[REVIEW PATCH 10/12] em28xx: remove obsolete device state checks from the ioctl functions

[Frank Schäfer]

v4l2_device_disconnect() is called when the device is disconnected, so that the
v4l2-core rejects all ioctl calls.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   43 -------------------------------
 1 Datei geändert, 43 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 319d0ee..dd05cfb 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -799,15 +799,6 @@ const struct v4l2_ctrl_ops em28xx_ctrl_ops = {
 	.s_ctrl = em28xx_s_ctrl,
 };
 
-static int check_dev(struct em28xx *dev)
-{
-	if (dev->disconnected) {
-		em28xx_errdev("v4l2 ioctl: device not present\n");
-		return -ENODEV;
-	}
-	return 0;
-}
-
 static void get_scale(struct em28xx *dev,
 			unsigned int width, unsigned int height,
 			unsigned int *hscale, unsigned int *vscale)
@@ -957,11 +948,6 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 {
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
-	int                rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	*norm = dev->norm;
 
@@ -972,11 +958,6 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
 {
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
-	int                rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
 
@@ -988,13 +969,9 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 	struct v4l2_format f;
-	int                rc;
 
 	if (*norm == dev->norm)
 		return 0;
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	if (dev->streaming_users > 0)
 		return -EBUSY;
@@ -1101,11 +1078,6 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 {
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
-	int                rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	if (i >= MAX_EM28XX_INPUT)
 		return -EINVAL;
@@ -1180,11 +1152,6 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
-	int                   rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	if (0 != t->index)
 		return -EINVAL;
@@ -1200,11 +1167,6 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
-	int                   rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	if (0 != t->index)
 		return -EINVAL;
@@ -1231,11 +1193,6 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
-	int                   rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
 
 	if (0 != f->tuner)
 		return -EINVAL;
-- 
1.7.10.4

[REVIEW PATCH 11/12] em28xx: make ioctl VIDIOC_DBG_G_CHIP_IDENT available for radio devices

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +
 1 Datei geändert, 1 Zeile hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index dd05cfb..e97b095 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1695,6 +1695,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.vidioc_g_register    = vidioc_g_register,
 	.vidioc_s_register    = vidioc_s_register,
+	.vidioc_g_chip_ident  = vidioc_g_chip_ident,
 #endif
 };
 
-- 
1.7.10.4

[REVIEW PATCH 12/12] em28xx: do not claim VBI support if the device is a camera

[Frank Schäfer]

Avoids registering a VBI device and streaming in VBI-mode if the device is a
camera.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-core.c |    5 +++++
 1 Datei geändert, 5 Zeilen hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index f516a63..f743e09 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -681,6 +681,11 @@ int em28xx_vbi_supported(struct em28xx *dev)
 	if (disable_vbi == 1)
 		return 0;
 
+	if (dev->board.is_webcam)
+		return 0;
+
+	/* FIXME: check subdevices for VBI support */
+
 	if (dev->chip_id == CHIP_ID_EM2860 ||
 	    dev->chip_id == CHIP_ID_EM2883)
 		return 1;
-- 
1.7.10.4

Re: [REVIEW PATCH 11/12] em28xx: make ioctl VIDIOC_DBG_G_CHIP_IDENT available for radio devices

[Hans Verkuil]

On Fri January 25 2013 18:27:01 Frank Schäfer wrote:
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-video.c |    1 +
>  1 Datei geändert, 1 Zeile hinzugefügt(+)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index dd05cfb..e97b095 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -1695,6 +1695,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>  	.vidioc_g_register    = vidioc_g_register,
>  	.vidioc_s_register    = vidioc_s_register,
> +	.vidioc_g_chip_ident  = vidioc_g_chip_ident,
>  #endif
>  };
>  
> 

g_chip_ident can be moved out of ADV_DEBUG, both for video and radio devices.

Regards,

	Hans

Re: [REVIEW PATCH 00/12] em28xx: ioctl fixes/clean-ups

[Hans Verkuil]

On Fri January 25 2013 18:26:50 Frank Schäfer wrote:
> This patch series addresses some issues with the ioctl code of the em28xx driver:
> Patches 1-5 and 11 fix and clean up the enabling/disabling of the ioctls 
> depending on the device type and equipment.
> Patches 6-10 remove some obsolete/useless code.
> Patch 12 improves the VBI support detection and device node registration.
> 
> 
> Frank Schäfer (12):
>   em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_QUERYSTD,
>     VIDIOC_G/S_STD
>   em28xx: disable tuner related ioctls for video and VBI devices
>     without tuner
>   em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_G_AUDIO and
>     VIDIOC_S_AUDIO
>   em28xx: use v4l2_disable_ioctl() to disable ioctl VIDIOC_S_PARM
>   em28xx: disable ioctl VIDIOC_S_PARM for VBI devices
>   em28xx: make ioctls VIDIOC_G/S_PARM working for VBI devices
>   em28xx: remove ioctl VIDIOC_CROPCAP
>   em28xx: get rid of duplicate function vidioc_s_fmt_vbi_cap()
>   em28xx: VIDIOC_G_TUNER: remove unneeded setting of tuner type
>   em28xx: remove obsolete device state checks from the ioctl functions
>   em28xx: make ioctl VIDIOC_DBG_G_CHIP_IDENT available for radio
>     devices
>   em28xx: do not claim VBI support if the device is a camera
> 
>  drivers/media/usb/em28xx/em28xx-core.c  |    5 ++
>  drivers/media/usb/em28xx/em28xx-video.c |  147 +++++++------------------------
>  2 Dateien geändert, 35 Zeilen hinzugefügt(+), 117 Zeilen entfernt(-)
> 
> 

After fixing the small comment I made for patch 11/12 you can add my Acked-by
for this patch series.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [REVIEW PATCH 11/12] em28xx: make ioctl VIDIOC_DBG_G_CHIP_IDENT available for radio devices

[Frank Schäfer]

Am 28.01.2013 11:00, schrieb Hans Verkuil:
> On Fri January 25 2013 18:27:01 Frank Schäfer wrote:
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-video.c |    1 +
>>  1 Datei geändert, 1 Zeile hinzugefügt(+)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
>> index dd05cfb..e97b095 100644
>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>> @@ -1695,6 +1695,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
>>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>>  	.vidioc_g_register    = vidioc_g_register,
>>  	.vidioc_s_register    = vidioc_s_register,
>> +	.vidioc_g_chip_ident  = vidioc_g_chip_ident,
>>  #endif
>>  };
>>  
>>
> g_chip_ident can be moved out of ADV_DEBUG, both for video and radio devices.

Yes it can but I wasn't sure if we really should change it.
Anyway, with your vote for changing it, I will be glad to send a patch
(it's an additional change and I don't need to send a V2 of the series
then, which also makes life easier for Mauro).

Regards,
Frank

> Regards,
>
> 	Hans