Skip to contentSkip to conversation

some em28xx-v4l cleanup patches

PATCH In review Frank Schäfer 2 participants 7 messages Jul 25, 2014 – Sep 18, 2014

Subsystems (2)
Media (V4L/DVB) EM28XX VIDEO4LINUX driver
lore
Share & tools
lore

Frank Schäfer

author
#0 lore raw

cover some em28xx-v4l cleanup patches

This patch series consists of 4 cleanup patches for the em28xx-v4l module.

Frank Schäfer (4):
em28xx-v4l: simplify some pointers in em28xx_init_camera()
em28xx-v4l: get rid of struct em28xx_fh
em28xx-v4l: simplify em28xx_v4l2_open() by using v4l2_fh_open()
em28xx-v4l: get rid of field "users" in struct em28xx_v4l2

 drivers/media/usb/em28xx/em28xx-camera.c |   4 +-
 drivers/media/usb/em28xx/em28xx-video.c  | 113 ++++++++++++-------------------
 drivers/media/usb/em28xx/em28xx.h        |   8 ---
 3 files changed, 47 insertions(+), 78 deletions(-)
Signature · 1 line
1.8.4.5
author
#1 lore raw

PATCH 1/4 em28xx-v4l: simplify some pointers in em28xx_init_camera()

Pointer v4l2" can be used instead of "dev->v4l2, which saves some characters.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/em28xx/em28xx-camera.c +2 -2
index 12d4c03..6d2ea9a 100644
@@ -366,7 +366,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		v4l2->sensor_xtal = 4300000;
 		pdata.xtal = v4l2->sensor_xtal;
 		if (NULL ==
-		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
+		    v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap,
 					      &mt9v011_info, NULL)) {
 			ret = -ENODEV;
 			break;

@@ -423,7 +423,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		v4l2->sensor_yres = 480;
 
 		subdev =
-		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
+		     v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap,
 					       &ov2640_info, NULL);
 		if (NULL == subdev) {
 			ret = -ENODEV;
Signature · 1 line
1.8.4.5
author
#2 lore raw

PATCH 2/4 em28xx-v4l: get rid of struct em28xx_fh

In reply to Frank Schäfer

struct em28xx_fh isn't needed anymore because the only used field which is left is struct v4l2_fh fh. Use struct v4l2_fh directly and remvove struct em28xx_fh.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 83 ++++++++++++---------------------
 drivers/media/usb/em28xx/em28xx.h       |  7 ---
 2 files changed, 29 insertions(+), 61 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +29 -54
index 3f8b5aa..4eb4a6a 100644
@@ -1227,8 +1227,7 @@ static void scale_to_size(struct em28xx *dev,
 static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 					struct v4l2_format *f)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx         *dev = video_drvdata(file);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	f->fmt.pix.width = v4l2->width;

@@ -1261,8 +1260,7 @@ static struct em28xx_fmt *format_by_fourcc(unsigned int fourcc)
 static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 			struct v4l2_format *f)
 {
-	struct em28xx_fh      *fh    = priv;
-	struct em28xx         *dev   = fh->dev;
+	struct em28xx         *dev   = video_drvdata(file);
 	struct em28xx_v4l2    *v4l2  = dev->v4l2;
 	unsigned int          width  = f->fmt.pix.width;
 	unsigned int          height = f->fmt.pix.height;

@@ -1355,8 +1353,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	*norm = dev->v4l2->norm;
 

@@ -1365,8 +1362,7 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 
 static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm);
 

@@ -1375,8 +1371,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
 
 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 em28xx      *dev  = video_drvdata(file);
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct v4l2_format f;
 

@@ -1408,8 +1403,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 static int vidioc_g_parm(struct file *file, void *priv,
 			 struct v4l2_streamparm *p)
 {
-	struct em28xx_fh   *fh   = priv;
-	struct em28xx      *dev  = fh->dev;
+	struct em28xx      *dev  = video_drvdata(file);
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	int rc = 0;
 

@@ -1427,8 +1421,7 @@ static int vidioc_g_parm(struct file *file, void *priv,
 static int vidioc_s_parm(struct file *file, void *priv,
 			 struct v4l2_streamparm *p)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
 	return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,

@@ -1450,8 +1443,7 @@ static const char *iname[] = {
 static int vidioc_enum_input(struct file *file, void *priv,
 				struct v4l2_input *i)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 	unsigned int       n;
 
 	n = i->index;

@@ -1479,8 +1471,7 @@ static int vidioc_enum_input(struct file *file, void *priv,
 
 static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	*i = dev->ctl_input;
 

@@ -1489,8 +1480,7 @@ static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 
 static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (i >= MAX_EM28XX_INPUT)
 		return -EINVAL;

@@ -1503,8 +1493,7 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 
 static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 {
-	struct em28xx_fh   *fh    = priv;
-	struct em28xx      *dev   = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	switch (a->index) {
 	case EM28XX_AMUX_VIDEO:

@@ -1543,8 +1532,7 @@ static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 
 static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio *a)
 {
-	struct em28xx_fh   *fh  = priv;
-	struct em28xx      *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (a->index >= MAX_EM28XX_INPUT)
 		return -EINVAL;

@@ -1563,8 +1551,7 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio
 static int vidioc_g_tuner(struct file *file, void *priv,
 				struct v4l2_tuner *t)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;

@@ -1578,8 +1565,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 static int vidioc_s_tuner(struct file *file, void *priv,
 				const struct v4l2_tuner *t)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;

@@ -1591,8 +1577,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 static int vidioc_g_frequency(struct file *file, void *priv,
 				struct v4l2_frequency *f)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx         *dev = video_drvdata(file);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	if (0 != f->tuner)

@@ -1606,8 +1591,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 				const struct v4l2_frequency *f)
 {
 	struct v4l2_frequency  new_freq = *f;
-	struct em28xx_fh          *fh   = priv;
-	struct em28xx             *dev  = fh->dev;
+	struct em28xx             *dev  = video_drvdata(file);
 	struct em28xx_v4l2        *v4l2 = dev->v4l2;
 
 	if (0 != f->tuner)

@@ -1624,8 +1608,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 static int vidioc_g_chip_info(struct file *file, void *priv,
 	       struct v4l2_dbg_chip_info *chip)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (chip->match.addr > 1)
 		return -EINVAL;

@@ -1652,8 +1635,7 @@ static int em28xx_reg_len(int reg)
 static int vidioc_g_register(struct file *file, void *priv,
 			     struct v4l2_dbg_register *reg)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 	int ret;
 
 	if (reg->match.addr > 1)

@@ -1693,8 +1675,7 @@ static int vidioc_g_register(struct file *file, void *priv,
 static int vidioc_s_register(struct file *file, void *priv,
 			     const struct v4l2_dbg_register *reg)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx *dev = video_drvdata(file);
 	__le16 buf;
 
 	if (reg->match.addr > 1)

@@ -1715,8 +1696,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 					struct v4l2_capability *cap)
 {
 	struct video_device   *vdev = video_devdata(file);
-	struct em28xx_fh      *fh   = priv;
-	struct em28xx         *dev  = fh->dev;
+	struct em28xx         *dev  = video_drvdata(file);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	strlcpy(cap->driver, "em28xx", sizeof(cap->driver));

@@ -1761,8 +1741,7 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 static int vidioc_enum_framesizes(struct file *file, void *priv,
 				  struct v4l2_frmsizeenum *fsize)
 {
-	struct em28xx_fh      *fh  = priv;
-	struct em28xx         *dev = fh->dev;
+	struct em28xx         *dev = video_drvdata(file);
 	struct em28xx_fmt     *fmt;
 	unsigned int	      maxw = norm_maxw(dev);
 	unsigned int	      maxh = norm_maxh(dev);

@@ -1806,8 +1785,7 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 				struct v4l2_format *format)
 {
-	struct em28xx_fh      *fh   = priv;
-	struct em28xx         *dev  = fh->dev;
+	struct em28xx         *dev  = video_drvdata(file);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	format->fmt.vbi.samples_per_line = v4l2->vbi_width;

@@ -1840,7 +1818,7 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 static int radio_g_tuner(struct file *file, void *priv,
 			 struct v4l2_tuner *t)
 {
-	struct em28xx *dev = ((struct em28xx_fh *)priv)->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (unlikely(t->index > 0))
 		return -EINVAL;

@@ -1855,7 +1833,7 @@ static int radio_g_tuner(struct file *file, void *priv,
 static int radio_s_tuner(struct file *file, void *priv,
 			 const struct v4l2_tuner *t)
 {
-	struct em28xx *dev = ((struct em28xx_fh *)priv)->dev;
+	struct em28xx *dev = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;

@@ -1890,7 +1868,7 @@ static int em28xx_v4l2_open(struct file *filp)
 	struct em28xx *dev = video_drvdata(filp);
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	enum v4l2_buf_type fh_type = 0;
-	struct em28xx_fh *fh;
+	struct v4l2_fh *fh;
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER:

@@ -1911,15 +1889,13 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;
-	fh = kzalloc(sizeof(struct em28xx_fh), GFP_KERNEL);
+	fh = kzalloc(sizeof(struct v4l2_fh), GFP_KERNEL);
 	if (!fh) {
 		em28xx_errdev("em28xx-video.c: Out of memory?!\n");
 		mutex_unlock(&dev->lock);
 		return -ENOMEM;
 	}
-	v4l2_fh_init(&fh->fh, vdev);
-	fh->dev = dev;
-	fh->type = fh_type;
+	v4l2_fh_init(fh, vdev);
 	filp->private_data = fh;
 
 	if (v4l2->users == 0) {

@@ -1945,7 +1921,7 @@ static int em28xx_v4l2_open(struct file *filp)
 	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
-	v4l2_fh_add(&fh->fh);
+	v4l2_fh_add(fh);
 
 	return 0;
 }

@@ -2046,8 +2022,7 @@ static int em28xx_v4l2_resume(struct em28xx *dev)
  */
 static int em28xx_v4l2_close(struct file *filp)
 {
-	struct em28xx_fh      *fh   = filp->private_data;
-	struct em28xx         *dev  = fh->dev;
+	struct em28xx         *dev  = video_drvdata(filp);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
drivers/media/usb/em28xx/em28xx.h +0 -7
index b4c837d..4360338 100644
@@ -576,13 +576,6 @@ struct em28xx_audio {
 
 struct em28xx;
 
-struct em28xx_fh {
-	struct v4l2_fh fh;
-	struct em28xx *dev;
-
-	enum v4l2_buf_type           type;
-};
-
 enum em28xx_i2c_algo_type {
 	EM28XX_I2C_ALGO_EM28XX = 0,
 	EM28XX_I2C_ALGO_EM2800,
Signature · 1 line
1.8.4.5
author
#3 lore raw

PATCH 3/4 em28xx-v4l: simplify em28xx_v4l2_open() by using v4l2_fh_open()

In reply to Frank Schäfer

Instead of calling

...
struct v4l2_fh *fh = kzalloc(sizeof(*fh), GFP_KERNEL); filp->private_data = fh;
v4l2_fh_init(fh, vdev);
v4l2_fh_add(fh);
...

simply use function v4l2_fh_open() which does all of these calls for us.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +7 -8
index 4eb4a6a..3a7ec3b 100644
@@ -1868,7 +1868,7 @@ static int em28xx_v4l2_open(struct file *filp)
 	struct em28xx *dev = video_drvdata(filp);
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	enum v4l2_buf_type fh_type = 0;
-	struct v4l2_fh *fh;
+	int ret;
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER:

@@ -1889,14 +1889,14 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;
-	fh = kzalloc(sizeof(struct v4l2_fh), GFP_KERNEL);
-	if (!fh) {
-		em28xx_errdev("em28xx-video.c: Out of memory?!\n");
+
+	ret = v4l2_fh_open(filp);
+	if (ret) {
+		em28xx_errdev("%s: v4l2_fh_open() returned error %d\n",
+			      __func__, ret);
 		mutex_unlock(&dev->lock);
-		return -ENOMEM;
+		return ret;
 	}
-	v4l2_fh_init(fh, vdev);
-	filp->private_data = fh;
 
 	if (v4l2->users == 0) {
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);

@@ -1921,7 +1921,6 @@ static int em28xx_v4l2_open(struct file *filp)
 	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
-	v4l2_fh_add(fh);
 
 	return 0;
 }
Signature · 1 line
1.8.4.5
author
#4 lore raw

PATCH 4/4 em28xx-v4l: get rid of field "users" in struct em28xx_v4l2

In reply to Frank Schäfer

Instead of counting the number of opened file handles, use function v4l2_fh_is_singular_file() in em28xx_v4l2_open() and em28xx_v4l2_close() to determine if the file handle is the first/last opened one.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 23 +++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h       |  1 -
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +13 -10
index 3a7ec3b..087ccf9 100644
@@ -1883,9 +1883,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		return -EINVAL;
 	}
 
-	em28xx_videodbg("open dev=%s type=%s users=%d\n",
-			video_device_node_name(vdev), v4l2_type_names[fh_type],
-			v4l2->users);
+	em28xx_videodbg("open dev=%s type=%s\n",
+			video_device_node_name(vdev), v4l2_type_names[fh_type]);
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;

@@ -1898,7 +1897,9 @@ static int em28xx_v4l2_open(struct file *filp)
 		return ret;
 	}
 
-	if (v4l2->users == 0) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("first opened filehandle, initializing device\n");
+
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
 
 		if (vdev->vfl_type != VFL_TYPE_RADIO)

@@ -1909,6 +1910,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		 * of some i2c devices
 		 */
 		em28xx_wake_i2c(dev);
+	} else {
+		em28xx_videodbg("further filehandles are already opened\n");
 	}
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO) {

@@ -1918,7 +1921,6 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	kref_get(&dev->ref);
 	kref_get(&v4l2->ref);
-	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
 

@@ -2025,12 +2027,11 @@ static int em28xx_v4l2_close(struct file *filp)
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
 
-	em28xx_videodbg("users=%d\n", v4l2->users);
-
-	vb2_fop_release(filp);
 	mutex_lock(&dev->lock);
 
-	if (v4l2->users == 1) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("last opened filehandle, shutting down device\n");
+
 		/* No sense to try to write to the device */
 		if (dev->disconnected)
 			goto exit;

@@ -2049,10 +2050,12 @@ static int em28xx_v4l2_close(struct file *filp)
 			em28xx_errdev("cannot change alternate number to "
 					"0 (error=%i)\n", errCode);
 		}
+	} else {
+		em28xx_videodbg("further opened filehandles left\n");
 	}
 
 exit:
-	v4l2->users--;
+	vb2_fop_release(filp);
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
 	mutex_unlock(&dev->lock);
 	kref_put(&dev->ref, em28xx_free_device);
drivers/media/usb/em28xx/em28xx.h +0 -1
index 4360338..84ef8ef 100644
@@ -524,7 +524,6 @@ struct em28xx_v4l2 {
 	int sensor_yres;
 	int sensor_xtal;
 
-	int users;		/* user count for exclusive use */
 	int streaming_users;    /* number of actively streaming users */
 
 	u32 frequency;		/* selected tuner frequency */
Signature · 1 line
1.8.4.5
#5 lore raw

Re: [PATCH 4/4] em28xx-v4l: get rid of field "users" in struct em28xx_v4l2

Hi Frank,

Quoted 3 lines · Frank Schäfer

Instead of counting the number of opened file handles, use function v4l2_fh_is_singular_file() in em28xx_v4l2_open() and em28xx_v4l2_close() to determine if the file handle is the first/last opened one.

This won't work: if you capture from both /dev/video and /dev/vbi and close one, then it stops all streaming.

I would just revert this patch completely.

There is currently no core support for detecting when all users of all devices registered by a driver have left, so you don't have really have an alternative but to use your old code.

Regards,

Hans

Quoted 94 lines
Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 23 +++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h       |  1 -
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +13 -10
index 3a7ec3b..087ccf9 100644
@@ -1883,9 +1883,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		return -EINVAL;
 	}
 
-	em28xx_videodbg("open dev=%s type=%s users=%d\n",
-			video_device_node_name(vdev), v4l2_type_names[fh_type],
-			v4l2->users);
+	em28xx_videodbg("open dev=%s type=%s\n",
+			video_device_node_name(vdev), v4l2_type_names[fh_type]);
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;

@@ -1898,7 +1897,9 @@ static int em28xx_v4l2_open(struct file *filp)
 		return ret;
 	}
 
-	if (v4l2->users == 0) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("first opened filehandle, initializing device\n");
+
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
 
 		if (vdev->vfl_type != VFL_TYPE_RADIO)

@@ -1909,6 +1910,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		 * of some i2c devices
 		 */
 		em28xx_wake_i2c(dev);
+	} else {
+		em28xx_videodbg("further filehandles are already opened\n");
 	}
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO) {

@@ -1918,7 +1921,6 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	kref_get(&dev->ref);
 	kref_get(&v4l2->ref);
-	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
 

@@ -2025,12 +2027,11 @@ static int em28xx_v4l2_close(struct file *filp)
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
 
-	em28xx_videodbg("users=%d\n", v4l2->users);
-
-	vb2_fop_release(filp);
 	mutex_lock(&dev->lock);
 
-	if (v4l2->users == 1) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("last opened filehandle, shutting down device\n");
+
 		/* No sense to try to write to the device */
 		if (dev->disconnected)
 			goto exit;

@@ -2049,10 +2050,12 @@ static int em28xx_v4l2_close(struct file *filp)
 			em28xx_errdev("cannot change alternate number to "
 					"0 (error=%i)\n", errCode);
 		}
+	} else {
+		em28xx_videodbg("further opened filehandles left\n");
 	}
 
 exit:
-	v4l2->users--;
+	vb2_fop_release(filp);
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
 	mutex_unlock(&dev->lock);
 	kref_put(&dev->ref, em28xx_free_device);
drivers/media/usb/em28xx/em28xx.h +0 -1
index 4360338..84ef8ef 100644
@@ -524,7 +524,6 @@ struct em28xx_v4l2 {
 	int sensor_yres;
 	int sensor_xtal;
 
-	int users;		/* user count for exclusive use */
 	int streaming_users;    /* number of actively streaming users */
 
 	u32 frequency;		/* selected tuner frequency */
author
#6 lore raw

Re: [PATCH 4/4] em28xx-v4l: get rid of field "users" in struct em28xx_v4l2

Hi Hans,

Am 18.09.2014 um 21:13 schrieb Hans Verkuil:

Quoted 8 lines

Hi Frank,

Quoted 3 lines · Frank Schäfer

Instead of counting the number of opened file handles, use function v4l2_fh_is_singular_file() in em28xx_v4l2_open() and em28xx_v4l2_close() to determine if the file handle is the first/last opened one.

This won't work: if you capture from both /dev/video and /dev/vbi and close one, then it stops all streaming.

You are 100% right, v4l2_fh_is_singular() is a per device _node_ check. How could I miss that ? :/

Quoted 5 lines

I would just revert this patch completely.

There is currently no core support for detecting when all users of all devices registered by a driver have left, so you don't have really have an alternative but to use your old code.

Indeed. I will send a revert patch.
Thanks for the hint !

Regards,
Frank

Quoted 98 lines

Regards,

Hans

Quoted 93 lines
Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 23 +++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h       |  1 -
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +13 -10
index 3a7ec3b..087ccf9 100644
@@ -1883,9 +1883,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		return -EINVAL;
 	}
 
-	em28xx_videodbg("open dev=%s type=%s users=%d\n",
-			video_device_node_name(vdev), v4l2_type_names[fh_type],
-			v4l2->users);
+	em28xx_videodbg("open dev=%s type=%s\n",
+			video_device_node_name(vdev), v4l2_type_names[fh_type]);
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;

@@ -1898,7 +1897,9 @@ static int em28xx_v4l2_open(struct file *filp)
 		return ret;
 	}
 
-	if (v4l2->users == 0) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("first opened filehandle, initializing device\n");
+
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
 
 		if (vdev->vfl_type != VFL_TYPE_RADIO)

@@ -1909,6 +1910,8 @@ static int em28xx_v4l2_open(struct file *filp)
 		 * of some i2c devices
 		 */
 		em28xx_wake_i2c(dev);
+	} else {
+		em28xx_videodbg("further filehandles are already opened\n");
 	}
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO) {

@@ -1918,7 +1921,6 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	kref_get(&dev->ref);
 	kref_get(&v4l2->ref);
-	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
 

@@ -2025,12 +2027,11 @@ static int em28xx_v4l2_close(struct file *filp)
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
 
-	em28xx_videodbg("users=%d\n", v4l2->users);
-
-	vb2_fop_release(filp);
 	mutex_lock(&dev->lock);
 
-	if (v4l2->users == 1) {
+	if (v4l2_fh_is_singular_file(filp)) {
+		em28xx_videodbg("last opened filehandle, shutting down device\n");
+
 		/* No sense to try to write to the device */
 		if (dev->disconnected)
 			goto exit;

@@ -2049,10 +2050,12 @@ static int em28xx_v4l2_close(struct file *filp)
 			em28xx_errdev("cannot change alternate number to "
 					"0 (error=%i)\n", errCode);
 		}
+	} else {
+		em28xx_videodbg("further opened filehandles left\n");
 	}
 
 exit:
-	v4l2->users--;
+	vb2_fop_release(filp);
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
 	mutex_unlock(&dev->lock);
 	kref_put(&dev->ref, em28xx_free_device);
drivers/media/usb/em28xx/em28xx.h +0 -1
index 4360338..84ef8ef 100644
@@ -524,7 +524,6 @@ struct em28xx_v4l2 {
 	int sensor_yres;
 	int sensor_xtal;
 
-	int users;		/* user count for exclusive use */
 	int streaming_users;    /* number of actively streaming users */
 
 	u32 frequency;		/* selected tuner frequency */