em28xx: ioctl fixes/clean-ups

PATCH Superseded Frank Schäfer 2 participants 16 messages Jan 25, 2013 – Jan 29, 2013

Subsystems (2)

Media (V4L/DVB) EM28XX VIDEO4LINUX driver

lore

Share & tools

lore

Frank Schäfer

author

2013-01-25 17:26 UTC #0 lore raw

cover em28xx: ioctl fixes/clean-ups

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(-)

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #1 lore raw

PATCH 1/12 em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_QUERYSTD, VIDIOC_G/S_STD

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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   13 +++++++------

1 Datei geändert, 7 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +7 -6

index 2eabf2a..7f1f37c 100644
@@ -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]);

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #2 lore raw

PATCH 2/12 em28xx: disable tuner related ioctls for video and VBI devices without tuner

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   14 ++++++++++++++

1 Datei geändert, 14 Zeilen hinzugefügt(+)

drivers/media/usb/em28xx/em28xx-video.c +14 -0

index 7f1f37c..dd2e31c 100644
@@ -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]);

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #3 lore raw

PATCH 3/12 em28xx: use v4l2_disable_ioctl() to disable ioctls VIDIOC_G_AUDIO and VIDIOC_S_AUDIO

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   15 ++++++++-------

1 Datei geändert, 8 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +8 -7

index dd2e31c..378d8a1 100644
@@ -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,

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #4 lore raw

PATCH 4/12 em28xx: use v4l2_disable_ioctl() to disable ioctl VIDIOC_S_PARM

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |    5 ++---

1 Datei geändert, 2 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +2 -3

index 378d8a1..c76714d 100644
@@ -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);

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #5 lore raw

PATCH 5/12 em28xx: disable ioctl VIDIOC_S_PARM for VBI devices

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +

1 Datei geändert, 1 Zeile hinzugefügt(+)

drivers/media/usb/em28xx/em28xx-video.c +1 -0

index c76714d..d4dc5b2 100644
@@ -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);

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #6 lore raw

PATCH 6/12 em28xx: make ioctls VIDIOC_G/S_PARM working for VBI devices

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |    6 ------

1 Datei geändert, 6 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +0 -6

index d4dc5b2..6172d59 100644
@@ -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);
 }

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #7 lore raw

PATCH 7/12 em28xx: remove ioctl VIDIOC_CROPCAP

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   21 ---------------------

1 Datei geändert, 21 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +0 ~~-21~~

index 6172d59..edd29ae 100644
@@ -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,

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:26 UTC #8 lore raw

PATCH 8/12 em28xx: get rid of duplicate function vidioc_s_fmt_vbi_cap()

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   31 +------------------------------

1 Datei geändert, 1 Zeile hinzugefügt(+), 30 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +1 ~~-30~~

index edd29ae..af3e70a 100644
@@ -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,

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:27 UTC #9 lore raw

PATCH 9/12 em28xx: VIDIOC_G_TUNER: remove unneeded setting of tuner type

In reply to Frank Schäfer

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

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |    1 -

1 Datei geändert, 1 Zeile entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +0 -1

index af3e70a..319d0ee 100644
@@ -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);

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:27 UTC #10 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |   43 -------------------------------

1 Datei geändert, 43 Zeilen entfernt(-)

drivers/media/usb/em28xx/em28xx-video.c +0 ~~-43~~

index 319d0ee..dd05cfb 100644
@@ -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;

Signature · 1 line

1.7.10.4

Frank Schäfer

author

2013-01-25 17:27 UTC #11 lore raw

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

In reply to Frank Schäfer

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +

1 Datei geändert, 1 Zeile hinzugefügt(+)

drivers/media/usb/em28xx/em28xx-video.c +1 -0

index dd05cfb..e97b095 100644
@@ -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
 };

Signature · 1 line

1.7.10.4

Hans Verkuil

2013-01-28 10:01 UTC #12 lore raw

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

Quoted 11 lines · On Fri January 25 2013 18:27:01 Frank Schäfer wrote:

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +
1 Datei geändert, 1 Zeile hinzugefügt(+)
drivers/media/usb/em28xx/em28xx-video.c +0 -0
index dd05cfb..e97b095 100644

drivers/media/usb/em28xx/em28xx-video.c static const struct v4l2_ioctl_ops radio_ioctl_ops = { · line 1701

Quoted 7 lines

drivers/media/usb/em28xx/em28xx-video.c +1 -0

@@ -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

Frank Schäfer

author

2013-01-29 17:37 UTC #13 lore raw

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

Am 28.01.2013 11:00, schrieb Hans Verkuil:

Quoted 6 lines · On Fri January 25 2013 18:27:01 Frank Schäfer wrote:

Quoted 5 lines
Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c |    1 +
1 Datei geändert, 1 Zeile hinzugefügt(+)

Quoted 14 lines

drivers/media/usb/em28xx/em28xx-video.c static const struct v4l2_ioctl_ops radio_ioctl_ops = { · line 1701
Quoted 13 lines
drivers/media/usb/em28xx/em28xx-video.c +1 -0
index dd05cfb..e97b095 100644
@@ -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

Quoted 3 lines

Regards,
Hans

Frank Schäfer

author

2013-01-25 17:27 UTC #14 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-core.c |    5 +++++

1 Datei geändert, 5 Zeilen hinzugefügt(+)

drivers/media/usb/em28xx/em28xx-core.c +5 -0

index f516a63..f743e09 100644
@@ -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;

Signature · 1 line

1.7.10.4

Hans Verkuil

Acked-by on the v1 series

2013-01-28 10:01 UTC #15 lore raw

In reply to Frank Schäfer

Quoted 25 lines · 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

Quoted 5 lines

 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 <[email protected]>

Regards,

Hans