[PATCH 00/19] em28xx: clean up the main device struct and move sub-module specific data to its own data structs

[PATCH 00/19] em28xx: clean up the main device struct and move sub-module specific data to its own data structs

[Frank Schäfer]

This patch series cleans up the main device struct of the em28xx driver.

Most of the patches (patches 3-16) are about moving the em28xx-v4l specific data
to it's own dynamically allocated data structure.
Patch 19 moves two em28xx-alsa specific fields to the em28xx_audio struct.
Patches 17 and 18 remove two fields which aren't needed.


Frank Schäfer (19):
  em28xx: move sub-module data structs to a common place in the main
    struct
  em28xx-video: simplify usage of the pointer to struct
    v4l2_ctrl_handler in em28xx_v4l2_init()
  em28xx: start moving em28xx-v4l specific data to its own struct
  em28xx: move struct v4l2_ctrl_handler ctrl_handler from struct em28xx
    to struct v4l2
  em28xx: move struct v4l2_clk *clk from struct em28xx to struct v4l2
  em28xx: move video_device structs from struct em28xx to struct v4l2
  em28xx: move videobuf2 related data from struct em28xx to struct v4l2
  em28xx: move v4l2 frame resolutions and scale data from struct em28xx
    to struct v4l2
  em28xx: move vinmode and vinctrl data from struct em28xx to struct
    v4l2
  em28xx: move TV norm from struct em28xx to struct v4l2
  em28xx: move struct em28xx_fmt *format from struct em28xx to struct
    v4l2
  em28xx: move progressive/interlaced fields from struct em28xx to
    struct v4l2
  em28xx: move sensor parameter fields from struct em28xx to struct v4l2
  em28xx: move capture state tracking fields from struct em28xx to
    struct v4l2
  em28xx: move v4l2 user counting fields from struct em28xx to struct
    v4l2
  em28xx: move tuner frequency field from struct em28xx to struct v4l2
  em28xx: remove field tda9887_conf from struct em28xx
  em28xx: remove field tuner_addr from struct em28xx
  em28xx: move fields wq_trigger and streaming_started from struct
    em28xx to struct em28xx_audio

 drivers/media/usb/em28xx/em28xx-audio.c  |  39 +-
 drivers/media/usb/em28xx/em28xx-camera.c |  51 +--
 drivers/media/usb/em28xx/em28xx-cards.c  |   9 -
 drivers/media/usb/em28xx/em28xx-vbi.c    |  10 +-
 drivers/media/usb/em28xx/em28xx-video.c  | 592 +++++++++++++++++--------------
 drivers/media/usb/em28xx/em28xx.h        | 120 ++++---
 6 files changed, 452 insertions(+), 369 deletions(-)

-- 
1.8.4.5

[PATCH 01/19] em28xx: move sub-module data structs to a common place in the main struct

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 4beb1fa..9a3c496 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -541,6 +541,11 @@ struct em28xx_i2c_bus {
 struct em28xx {
 	struct kref ref;
 
+	/* Sub-module data */
+	struct em28xx_dvb *dvb;
+	struct em28xx_audio adev;
+	struct em28xx_IR *ir;
+
 	/* generic device properties */
 	char name[30];		/* name (including minor) of the device */
 	int model;		/* index in the device_data struct */
@@ -576,8 +581,6 @@ struct em28xx {
 
 	struct em28xx_fmt *format;
 
-	struct em28xx_IR *ir;
-
 	/* Some older em28xx chips needs a waiting time after writing */
 	unsigned int wait_after_write;
 
@@ -623,8 +626,6 @@ struct em28xx {
 	unsigned long i2c_hash;	/* i2c devicelist hash -
 				   for boards with generic ID */
 
-	struct em28xx_audio adev;
-
 	/* capture state tracking */
 	int capture_type;
 	unsigned char top_field:1;
@@ -704,8 +705,6 @@ struct em28xx {
 	/* Snapshot button input device */
 	char snapshot_button_path[30];	/* path of the input dev */
 	struct input_dev *sbutton_input_dev;
-
-	struct em28xx_dvb *dvb;
 };
 
 #define kref_to_dev(d) container_of(d, struct em28xx, ref)
-- 
1.8.4.5

[PATCH 02/19] em28xx-video: simplify usage of the pointer to struct v4l2_ctrl_handler in em28xx_v4l2_init()

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 9df1826..45ad471 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2403,35 +2403,35 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* Add image controls */
 	/* NOTE: at this point, the subdevices are already registered, so bridge
 	 * controls are only added/enabled when no subdevice provides them */
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_CONTRAST))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_CONTRAST))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_CONTRAST,
 				  0, 0x1f, 1, CONTRAST_DEFAULT);
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_BRIGHTNESS))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_BRIGHTNESS))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_BRIGHTNESS,
 				  -0x80, 0x7f, 1, BRIGHTNESS_DEFAULT);
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_SATURATION))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_SATURATION))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_SATURATION,
 				  0, 0x1f, 1, SATURATION_DEFAULT);
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_BLUE_BALANCE))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_BLUE_BALANCE))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_BLUE_BALANCE,
 				  -0x30, 0x30, 1, BLUE_BALANCE_DEFAULT);
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_RED_BALANCE))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_RED_BALANCE))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_RED_BALANCE,
 				  -0x30, 0x30, 1, RED_BALANCE_DEFAULT);
-	if (NULL == v4l2_ctrl_find(&dev->ctrl_handler, V4L2_CID_SHARPNESS))
-		v4l2_ctrl_new_std(&dev->ctrl_handler, &em28xx_ctrl_ops,
+	if (NULL == v4l2_ctrl_find(hdl, V4L2_CID_SHARPNESS))
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
 				  V4L2_CID_SHARPNESS,
 				  0, 0x0f, 1, SHARPNESS_DEFAULT);
 
 	/* Reset image controls */
 	em28xx_colorlevels_set_default(dev);
-	v4l2_ctrl_handler_setup(&dev->ctrl_handler);
-	ret = dev->ctrl_handler.error;
+	v4l2_ctrl_handler_setup(hdl);
+	ret = hdl->error;
 	if (ret)
 		goto unregister_dev;
 
-- 
1.8.4.5

[PATCH 03/19] em28xx: start moving em28xx-v4l specific data to its own struct

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-camera.c |   4 +-
 drivers/media/usb/em28xx/em28xx-video.c  | 160 +++++++++++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h        |   8 +-
 3 files changed, 116 insertions(+), 56 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index 505e050..daebef3 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -365,7 +365,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		dev->sensor_xtal = 4300000;
 		pdata.xtal = dev->sensor_xtal;
 		if (NULL ==
-		    v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
+		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
 					      &mt9v011_info, NULL)) {
 			ret = -ENODEV;
 			break;
@@ -422,7 +422,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		dev->sensor_yres = 480;
 
 		subdev =
-		     v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
+		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
 					       &ov2640_info, NULL);
 		if (NULL == subdev) {
 			ret = -ENODEV;
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 45ad471..89947db 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -189,10 +189,11 @@ static int em28xx_vbi_supported(struct em28xx *dev)
  */
 static void em28xx_wake_i2c(struct em28xx *dev)
 {
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
+	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
+	v4l2_device_call_all(v4l2_dev, 0, core,  reset, 0);
+	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
 			INPUT(dev->ctl_input)->vmux, 0, 0);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
+	v4l2_device_call_all(v4l2_dev, 0, video, s_stream, 0);
 }
 
 static int em28xx_colorlevels_set_default(struct em28xx *dev)
@@ -952,7 +953,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 			f.type = V4L2_TUNER_RADIO;
 		else
 			f.type = V4L2_TUNER_ANALOG_TV;
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
+		v4l2_device_call_all(&dev->v4l2->v4l2_dev,
+				     0, tuner, s_frequency, &f);
 	}
 
 	dev->streaming_users++;
@@ -1083,6 +1085,7 @@ static int em28xx_vb2_setup(struct em28xx *dev)
 
 static void video_mux(struct em28xx *dev, int index)
 {
+	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
 	dev->ctl_input = index;
 	dev->ctl_ainput = INPUT(index)->amux;
 	dev->ctl_aoutput = INPUT(index)->aout;
@@ -1090,21 +1093,21 @@ static void video_mux(struct em28xx *dev, int index)
 	if (!dev->ctl_aoutput)
 		dev->ctl_aoutput = EM28XX_AOUT_MASTER;
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
+	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
 			INPUT(index)->vmux, 0, 0);
 
 	if (dev->board.has_msp34xx) {
 		if (dev->i2s_speed) {
-			v4l2_device_call_all(&dev->v4l2_dev, 0, audio,
+			v4l2_device_call_all(v4l2_dev, 0, audio,
 				s_i2s_clock_freq, dev->i2s_speed);
 		}
 		/* Note: this is msp3400 specific */
-		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
+		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
 			 dev->ctl_ainput, MSP_OUTPUT(MSP_SC_IN_DSP_SCART1), 0);
 	}
 
 	if (dev->board.adecoder != EM28XX_NOADECODER) {
-		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
+		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
 			dev->ctl_ainput, dev->ctl_aoutput, 0);
 	}
 
@@ -1344,7 +1347,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm);
 
 	return 0;
 }
@@ -1374,7 +1377,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
 
 	em28xx_resolution_set(dev);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, core, s_std, dev->norm);
 
 	return 0;
 }
@@ -1388,7 +1391,7 @@ static int vidioc_g_parm(struct file *file, void *priv,
 
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
 	if (dev->board.is_webcam)
-		rc = v4l2_device_call_until_err(&dev->v4l2_dev, 0,
+		rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
 						video, g_parm, p);
 	else
 		v4l2_video_std_frame_period(dev->norm,
@@ -1404,7 +1407,8 @@ static int vidioc_s_parm(struct file *file, void *priv,
 	struct em28xx      *dev = fh->dev;
 
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
-	return v4l2_device_call_until_err(&dev->v4l2_dev, 0, video, s_parm, p);
+	return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,
+					  0, video, s_parm, p);
 }
 
 static const char *iname[] = {
@@ -1543,7 +1547,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 
 	strcpy(t->name, "Tuner");
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
 	return 0;
 }
 
@@ -1556,7 +1560,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 	if (0 != t->index)
 		return -EINVAL;
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
 	return 0;
 }
 
@@ -1576,15 +1580,16 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 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 v4l2_frequency  new_freq = *f;
+	struct em28xx_fh          *fh   = priv;
+	struct em28xx             *dev  = fh->dev;
+	struct em28xx_v4l2        *v4l2 = dev->v4l2;
 
 	if (0 != f->tuner)
 		return -EINVAL;
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, f);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, &new_freq);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq);
 	dev->ctl_freq = new_freq.frequency;
 
 	return 0;
@@ -1602,7 +1607,8 @@ static int vidioc_g_chip_info(struct file *file, void *priv,
 	if (chip->match.addr == 1)
 		strlcpy(chip->name, "ac97", sizeof(chip->name));
 	else
-		strlcpy(chip->name, dev->v4l2_dev.name, sizeof(chip->name));
+		strlcpy(chip->name,
+			dev->v4l2->v4l2_dev.name, sizeof(chip->name));
 	return 0;
 }
 
@@ -1814,7 +1820,7 @@ static int radio_g_tuner(struct file *file, void *priv,
 
 	strcpy(t->name, "Radio");
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
 
 	return 0;
 }
@@ -1827,12 +1833,26 @@ static int radio_s_tuner(struct file *file, void *priv,
 	if (0 != t->index)
 		return -EINVAL;
 
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
+	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
 
 	return 0;
 }
 
 /*
+ * em28xx_free_v4l2() - Free struct em28xx_v4l2
+ *
+ * @ref: struct kref for struct em28xx_v4l2
+ *
+ * Called when all users of struct em28xx_v4l2 are gone
+ */
+void em28xx_free_v4l2(struct kref *ref)
+{
+	struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref);
+
+	kfree(v4l2);
+}
+
+/*
  * em28xx_v4l2_open()
  * inits the device and starts isoc transfer
  */
@@ -1840,6 +1860,7 @@ static int em28xx_v4l2_open(struct file *filp)
 {
 	struct video_device *vdev = video_devdata(filp);
 	struct em28xx *dev = video_drvdata(filp);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	enum v4l2_buf_type fh_type = 0;
 	struct em28xx_fh *fh;
 
@@ -1888,10 +1909,11 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO) {
 		em28xx_videodbg("video_open: setting radio device\n");
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_radio);
+		v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio);
 	}
 
 	kref_get(&dev->ref);
+	kref_get(&v4l2->ref);
 	dev->users++;
 
 	mutex_unlock(&dev->lock);
@@ -1907,6 +1929,8 @@ static int em28xx_v4l2_open(struct file *filp)
 */
 static int em28xx_v4l2_fini(struct em28xx *dev)
 {
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
+
 	if (dev->is_audio_only) {
 		/* Shouldn't initialize IR for this interface */
 		return 0;
@@ -1917,11 +1941,14 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		return 0;
 	}
 
+	if (v4l2 == NULL)
+		return 0;
+
 	em28xx_info("Closing video extension");
 
 	mutex_lock(&dev->lock);
 
-	v4l2_device_disconnect(&dev->v4l2_dev);
+	v4l2_device_disconnect(&v4l2->v4l2_dev);
 
 	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 
@@ -1942,14 +1969,17 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	}
 
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-	v4l2_device_unregister(&dev->v4l2_dev);
+	v4l2_device_unregister(&v4l2->v4l2_dev);
 
 	if (dev->clk) {
 		v4l2_clk_unregister_fixed(dev->clk);
 		dev->clk = NULL;
 	}
 
+	kref_put(&v4l2->ref, em28xx_free_v4l2);
+
 	mutex_unlock(&dev->lock);
+
 	kref_put(&dev->ref, em28xx_free_device);
 
 	return 0;
@@ -1988,8 +2018,9 @@ 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_fh      *fh   = filp->private_data;
+	struct em28xx         *dev  = fh->dev;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
 
 	em28xx_videodbg("users=%d\n", dev->users);
@@ -2003,7 +2034,7 @@ static int em28xx_v4l2_close(struct file *filp)
 			goto exit;
 
 		/* Save some power by putting tuner to sleep */
-		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
+		v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_power, 0);
 
 		/* do this before setting alternate! */
 		em28xx_set_mode(dev, EM28XX_SUSPEND);
@@ -2019,6 +2050,7 @@ static int em28xx_v4l2_close(struct file *filp)
 	}
 
 exit:
+	kref_put(&v4l2->ref, em28xx_free_v4l2);
 	dev->users--;
 	mutex_unlock(&dev->lock);
 	kref_put(&dev->ref, em28xx_free_device);
@@ -2162,7 +2194,7 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 		return NULL;
 
 	*vfd		= *template;
-	vfd->v4l2_dev	= &dev->v4l2_dev;
+	vfd->v4l2_dev	= &dev->v4l2->v4l2_dev;
 	vfd->debug	= video_debug;
 	vfd->lock	= &dev->lock;
 	set_bit(V4L2_FL_USE_FH_PRIO, &vfd->flags);
@@ -2178,6 +2210,7 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 
 static void em28xx_tuner_setup(struct em28xx *dev)
 {
+	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
 	struct tuner_setup           tun_setup;
 	struct v4l2_frequency        f;
 
@@ -2193,14 +2226,16 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 		tun_setup.type = dev->board.radio.type;
 		tun_setup.addr = dev->board.radio_addr;
 
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
+		v4l2_device_call_all(v4l2_dev,
+				     0, tuner, s_type_addr, &tun_setup);
 	}
 
 	if ((dev->tuner_type != TUNER_ABSENT) && (dev->tuner_type)) {
 		tun_setup.type   = dev->tuner_type;
 		tun_setup.addr   = dev->tuner_addr;
 
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
+		v4l2_device_call_all(v4l2_dev,
+				     0, tuner, s_type_addr, &tun_setup);
 	}
 
 	if (dev->tda9887_conf) {
@@ -2209,7 +2244,8 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 		tda9887_cfg.tuner = TUNER_TDA9887;
 		tda9887_cfg.priv = &dev->tda9887_conf;
 
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &tda9887_cfg);
+		v4l2_device_call_all(v4l2_dev,
+				     0, tuner, s_config, &tda9887_cfg);
 	}
 
 	if (dev->tuner_type == TUNER_XC2028) {
@@ -2224,7 +2260,7 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 		xc2028_cfg.tuner = TUNER_XC2028;
 		xc2028_cfg.priv  = &ctl;
 
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &xc2028_cfg);
+		v4l2_device_call_all(v4l2_dev, 0, tuner, s_config, &xc2028_cfg);
 	}
 
 	/* configure tuner */
@@ -2232,7 +2268,7 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 	f.type = V4L2_TUNER_ANALOG_TV;
 	f.frequency = 9076;     /* just a magic number */
 	dev->ctl_freq = f.frequency;
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
+	v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f);
 }
 
 static int em28xx_v4l2_init(struct em28xx *dev)
@@ -2241,6 +2277,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	int ret;
 	unsigned int maxw;
 	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
+	struct em28xx_v4l2 *v4l2;
 
 	if (dev->is_audio_only) {
 		/* Shouldn't initialize IR for this interface */
@@ -2256,14 +2293,23 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 
 	mutex_lock(&dev->lock);
 
-	ret = v4l2_device_register(&dev->udev->dev, &dev->v4l2_dev);
+	v4l2 = kzalloc(sizeof(struct em28xx_v4l2), GFP_KERNEL);
+	if (v4l2 == NULL) {
+		em28xx_info("em28xx_v4l: memory allocation failed\n");
+		mutex_unlock(&dev->lock);
+		return -ENOMEM;
+	}
+	kref_init(&v4l2->ref);
+	dev->v4l2 = v4l2;
+
+	ret = v4l2_device_register(&dev->udev->dev, &v4l2->v4l2_dev);
 	if (ret < 0) {
 		em28xx_errdev("Call to v4l2_device_register() failed!\n");
 		goto err;
 	}
 
 	v4l2_ctrl_handler_init(hdl, 8);
-	dev->v4l2_dev.ctrl_handler = hdl;
+	v4l2->v4l2_dev.ctrl_handler = hdl;
 
 	/*
 	 * Default format, used for tvp5150 or saa711x output formats
@@ -2275,20 +2321,24 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* request some modules */
 
 	if (dev->board.has_msp34xx)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"msp3400", 0, msp3400_addrs);
+		v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+				    &dev->i2c_adap[dev->def_i2c_bus],
+				    "msp3400", 0, msp3400_addrs);
 
 	if (dev->board.decoder == EM28XX_SAA711X)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"saa7115_auto", 0, saa711x_addrs);
+		v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+				    &dev->i2c_adap[dev->def_i2c_bus],
+				    "saa7115_auto", 0, saa711x_addrs);
 
 	if (dev->board.decoder == EM28XX_TVP5150)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"tvp5150", 0, tvp5150_addrs);
+		v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+				    &dev->i2c_adap[dev->def_i2c_bus],
+				    "tvp5150", 0, tvp5150_addrs);
 
 	if (dev->board.adecoder == EM28XX_TVAUDIO)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"tvaudio", dev->board.tvaudio_addr, NULL);
+		v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+				    &dev->i2c_adap[dev->def_i2c_bus],
+				    "tvaudio", dev->board.tvaudio_addr, NULL);
 
 	/* Initialize tuner and camera */
 
@@ -2296,11 +2346,12 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		int has_demod = (dev->tda9887_conf & TDA9887_PRESENT);
 
 		if (dev->board.radio.type)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-				"tuner", dev->board.radio_addr, NULL);
+			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+					  &dev->i2c_adap[dev->def_i2c_bus],
+					  "tuner", dev->board.radio_addr, NULL);
 
 		if (has_demod)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev,
+			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
 				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
 				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
 		if (dev->tuner_addr == 0) {
@@ -2308,15 +2359,16 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
 			struct v4l2_subdev *sd;
 
-			sd = v4l2_i2c_new_subdev(&dev->v4l2_dev,
+			sd = v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
 				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
 				0, v4l2_i2c_tuner_addrs(type));
 
 			if (sd)
 				dev->tuner_addr = v4l2_i2c_subdev_addr(sd);
 		} else {
-			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-				"tuner", dev->tuner_addr, NULL);
+			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
+					    &dev->i2c_adap[dev->def_i2c_bus],
+					    "tuner", dev->tuner_addr, NULL);
 		}
 	}
 
@@ -2372,7 +2424,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 
 	/* set default norm */
 	dev->norm = V4L2_STD_PAL;
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, dev->norm);
 	dev->interlaced = EM28XX_INTERLACED_DEFAULT;
 
 	/* Analog specific initialization */
@@ -2529,7 +2581,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 			    video_device_node_name(dev->vbi_dev));
 
 	/* Save some power by putting tuner to sleep */
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_power, 0);
 
 	/* initialize videobuf2 stuff */
 	em28xx_vb2_setup(dev);
@@ -2543,8 +2595,10 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 
 unregister_dev:
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-	v4l2_device_unregister(&dev->v4l2_dev);
+	v4l2_device_unregister(&v4l2->v4l2_dev);
 err:
+	dev->v4l2 = NULL;
+	kref_put(&v4l2->ref, em28xx_free_v4l2);
 	mutex_unlock(&dev->lock);
 	return ret;
 }
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 9a3c496..b18b968 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -497,6 +497,12 @@ struct em28xx_eeprom {
 #define EM28XX_RESOURCE_VIDEO 0x01
 #define EM28XX_RESOURCE_VBI   0x02
 
+struct em28xx_v4l2 {
+	struct kref ref;
+
+	struct v4l2_device v4l2_dev;
+};
+
 struct em28xx_audio {
 	char name[50];
 	unsigned num_urb;
@@ -542,6 +548,7 @@ struct em28xx {
 	struct kref ref;
 
 	/* Sub-module data */
+	struct em28xx_v4l2 *v4l2;
 	struct em28xx_dvb *dvb;
 	struct em28xx_audio adev;
 	struct em28xx_IR *ir;
@@ -559,7 +566,6 @@ struct em28xx {
 	unsigned int has_alsa_audio:1;
 	unsigned int is_audio_only:1;
 
-	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_clk *clk;
 	struct em28xx_board board;
-- 
1.8.4.5

[PATCH 04/19] em28xx: move struct v4l2_ctrl_handler ctrl_handler from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 13 +++++++++----
 drivers/media/usb/em28xx/em28xx.h       |  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 89947db..22acb0f 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1138,7 +1138,9 @@ static void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv)
 
 static int em28xx_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct em28xx *dev = container_of(ctrl->handler, struct em28xx, ctrl_handler);
+	struct em28xx_v4l2 *v4l2 =
+		  container_of(ctrl->handler, struct em28xx_v4l2, ctrl_handler);
+	struct em28xx *dev = v4l2->dev;
 	int ret = -EINVAL;
 
 	switch (ctrl->id) {
@@ -1849,6 +1851,7 @@ void em28xx_free_v4l2(struct kref *ref)
 {
 	struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref);
 
+	v4l2->dev->v4l2 = NULL;
 	kfree(v4l2);
 }
 
@@ -1968,7 +1971,7 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		video_unregister_device(dev->vdev);
 	}
 
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
 	v4l2_device_unregister(&v4l2->v4l2_dev);
 
 	if (dev->clk) {
@@ -2276,7 +2279,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	u8 val;
 	int ret;
 	unsigned int maxw;
-	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
+	struct v4l2_ctrl_handler *hdl;
 	struct em28xx_v4l2 *v4l2;
 
 	if (dev->is_audio_only) {
@@ -2300,6 +2303,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		return -ENOMEM;
 	}
 	kref_init(&v4l2->ref);
+	v4l2->dev = dev;
 	dev->v4l2 = v4l2;
 
 	ret = v4l2_device_register(&dev->udev->dev, &v4l2->v4l2_dev);
@@ -2308,6 +2312,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		goto err;
 	}
 
+	hdl = &v4l2->ctrl_handler;
 	v4l2_ctrl_handler_init(hdl, 8);
 	v4l2->v4l2_dev.ctrl_handler = hdl;
 
@@ -2594,7 +2599,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	return 0;
 
 unregister_dev:
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
 	v4l2_device_unregister(&v4l2->v4l2_dev);
 err:
 	dev->v4l2 = NULL;
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index b18b968..910c2d8 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -499,8 +499,10 @@ struct em28xx_eeprom {
 
 struct em28xx_v4l2 {
 	struct kref ref;
+	struct em28xx *dev;
 
 	struct v4l2_device v4l2_dev;
+	struct v4l2_ctrl_handler ctrl_handler;
 };
 
 struct em28xx_audio {
@@ -566,7 +568,6 @@ struct em28xx {
 	unsigned int has_alsa_audio:1;
 	unsigned int is_audio_only:1;
 
-	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_clk *clk;
 	struct em28xx_board board;
 
-- 
1.8.4.5

[PATCH 05/19] em28xx: move struct v4l2_clk *clk from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 11 ++++++-----
 drivers/media/usb/em28xx/em28xx-video.c  |  6 +++---
 drivers/media/usb/em28xx/em28xx.h        |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index daebef3..c2672b4 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -330,13 +330,14 @@ int em28xx_init_camera(struct em28xx *dev)
 	char clk_name[V4L2_SUBDEV_NAME_SIZE];
 	struct i2c_client *client = &dev->i2c_client[dev->def_i2c_bus];
 	struct i2c_adapter *adap = &dev->i2c_adap[dev->def_i2c_bus];
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	int ret = 0;
 
 	v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
 			  i2c_adapter_id(adap), client->addr);
-	dev->clk = v4l2_clk_register_fixed(clk_name, "mclk", -EINVAL);
-	if (IS_ERR(dev->clk))
-		return PTR_ERR(dev->clk);
+	v4l2->clk = v4l2_clk_register_fixed(clk_name, "mclk", -EINVAL);
+	if (IS_ERR(v4l2->clk))
+		return PTR_ERR(v4l2->clk);
 
 	switch (dev->em28xx_sensor) {
 	case EM28XX_MT9V011:
@@ -448,8 +449,8 @@ int em28xx_init_camera(struct em28xx *dev)
 	}
 
 	if (ret < 0) {
-		v4l2_clk_unregister_fixed(dev->clk);
-		dev->clk = NULL;
+		v4l2_clk_unregister_fixed(v4l2->clk);
+		v4l2->clk = NULL;
 	}
 
 	return ret;
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 22acb0f..4fb0053 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1974,9 +1974,9 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
 	v4l2_device_unregister(&v4l2->v4l2_dev);
 
-	if (dev->clk) {
-		v4l2_clk_unregister_fixed(dev->clk);
-		dev->clk = NULL;
+	if (v4l2->clk) {
+		v4l2_clk_unregister_fixed(v4l2->clk);
+		v4l2->clk = NULL;
 	}
 
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 910c2d8..a4d26bf 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -503,6 +503,7 @@ struct em28xx_v4l2 {
 
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
+	struct v4l2_clk *clk;
 };
 
 struct em28xx_audio {
@@ -568,7 +569,6 @@ struct em28xx {
 	unsigned int has_alsa_audio:1;
 	unsigned int is_audio_only:1;
 
-	struct v4l2_clk *clk;
 	struct em28xx_board board;
 
 	/* Webcam specific fields */
-- 
1.8.4.5

[PATCH 06/19] em28xx: move video_device structs from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 120 ++++++++++++++------------------
 drivers/media/usb/em28xx/em28xx.h       |   7 +-
 2 files changed, 56 insertions(+), 71 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 4fb0053..7252eef 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1447,7 +1447,7 @@ static int vidioc_enum_input(struct file *file, void *priv,
 		(EM28XX_VMUX_CABLE == INPUT(n)->type))
 		i->type = V4L2_INPUT_TYPE_TUNER;
 
-	i->std = dev->vdev->tvnorms;
+	i->std = dev->v4l2->vdev->tvnorms;
 	/* webcams do not have the STD API */
 	if (dev->board.is_webcam)
 		i->capabilities = 0;
@@ -1691,9 +1691,10 @@ static int vidioc_s_register(struct file *file, void *priv,
 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 video_device   *vdev = video_devdata(file);
+	struct em28xx_fh      *fh   = priv;
+	struct em28xx         *dev  = fh->dev;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	strlcpy(cap->driver, "em28xx", sizeof(cap->driver));
 	strlcpy(cap->card, em28xx_boards[dev->model].name, sizeof(cap->card));
@@ -1715,9 +1716,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
 
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
 		V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
-	if (dev->vbi_dev)
+	if (v4l2->vbi_dev)
 		cap->capabilities |= V4L2_CAP_VBI_CAPTURE;
-	if (dev->radio_dev)
+	if (v4l2->radio_dev)
 		cap->capabilities |= V4L2_CAP_RADIO;
 	return 0;
 }
@@ -1955,20 +1956,20 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 
 	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 
-	if (dev->radio_dev) {
+	if (v4l2->radio_dev) {
 		em28xx_info("V4L2 device %s deregistered\n",
-			    video_device_node_name(dev->radio_dev));
-		video_unregister_device(dev->radio_dev);
+			    video_device_node_name(v4l2->radio_dev));
+		video_unregister_device(v4l2->radio_dev);
 	}
-	if (dev->vbi_dev) {
+	if (v4l2->vbi_dev) {
 		em28xx_info("V4L2 device %s deregistered\n",
-			    video_device_node_name(dev->vbi_dev));
-		video_unregister_device(dev->vbi_dev);
+			    video_device_node_name(v4l2->vbi_dev));
+		video_unregister_device(v4l2->vbi_dev);
 	}
-	if (dev->vdev) {
+	if (v4l2->vdev) {
 		em28xx_info("V4L2 device %s deregistered\n",
-			    video_device_node_name(dev->vdev));
-		video_unregister_device(dev->vdev);
+			    video_device_node_name(v4l2->vdev));
+		video_unregister_device(v4l2->vdev);
 	}
 
 	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
@@ -2061,23 +2062,6 @@ exit:
 	return 0;
 }
 
-/*
- * em28xx_videodevice_release()
- * called when the last user of the video device exits and frees the memeory
- */
-static void em28xx_videodevice_release(struct video_device *vdev)
-{
-	struct em28xx *dev = video_get_drvdata(vdev);
-
-	video_device_release(vdev);
-	if (vdev == dev->vdev)
-		dev->vdev = NULL;
-	else if (vdev == dev->vbi_dev)
-		dev->vbi_dev = NULL;
-	else if (vdev == dev->radio_dev)
-		dev->radio_dev = NULL;
-}
-
 static const struct v4l2_file_operations em28xx_v4l_fops = {
 	.owner         = THIS_MODULE,
 	.open          = em28xx_v4l2_open,
@@ -2134,7 +2118,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 static const struct video_device em28xx_video_template = {
 	.fops		= &em28xx_v4l_fops,
 	.ioctl_ops	= &video_ioctl_ops,
-	.release	= em28xx_videodevice_release,
+	.release	= video_device_release,
 	.tvnorms	= V4L2_STD_ALL,
 };
 
@@ -2163,7 +2147,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 static struct video_device em28xx_radio_template = {
 	.fops		= &radio_fops,
 	.ioctl_ops	= &radio_ioctl_ops,
-	.release	= em28xx_videodevice_release,
+	.release	= video_device_release,
 };
 
 /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
@@ -2493,36 +2477,36 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		goto unregister_dev;
 
 	/* allocate and fill video video_device struct */
-	dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
-	if (!dev->vdev) {
+	v4l2->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
+	if (!v4l2->vdev) {
 		em28xx_errdev("cannot allocate video_device.\n");
 		ret = -ENODEV;
 		goto unregister_dev;
 	}
-	dev->vdev->queue = &dev->vb_vidq;
-	dev->vdev->queue->lock = &dev->vb_queue_lock;
+	v4l2->vdev->queue = &dev->vb_vidq;
+	v4l2->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);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD);
 	} else {
-		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM);
 	}
 	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);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY);
+		v4l2_disable_ioctl(v4l2->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);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO);
+		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO);
 	}
 
 	/* register v4l2 video video_device */
-	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
+	ret = video_register_device(v4l2->vdev, VFL_TYPE_GRABBER,
 				       video_nr[dev->devno]);
 	if (ret) {
 		em28xx_errdev("unable to register video device (error=%i).\n",
@@ -2532,27 +2516,27 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 
 	/* Allocate and fill vbi video_device struct */
 	if (em28xx_vbi_supported(dev) == 1) {
-		dev->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
+		v4l2->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
 						"vbi");
 
-		dev->vbi_dev->queue = &dev->vb_vbiq;
-		dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
+		v4l2->vbi_dev->queue = &dev->vb_vbiq;
+		v4l2->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
 
 		/* disable inapplicable ioctls */
-		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
+		v4l2_disable_ioctl(v4l2->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);
-			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_FREQUENCY);
-			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_FREQUENCY);
+			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_TUNER);
+			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_TUNER);
+			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_FREQUENCY);
+			v4l2_disable_ioctl(v4l2->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);
+			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_AUDIO);
+			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_AUDIO);
 		}
 
 		/* register v4l2 vbi video_device */
-		ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
+		ret = video_register_device(v4l2->vbi_dev, VFL_TYPE_VBI,
 					    vbi_nr[dev->devno]);
 		if (ret < 0) {
 			em28xx_errdev("unable to register vbi device\n");
@@ -2561,29 +2545,29 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	}
 
 	if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) {
-		dev->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
-						  "radio");
-		if (!dev->radio_dev) {
+		v4l2->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
+						   "radio");
+		if (!v4l2->radio_dev) {
 			em28xx_errdev("cannot allocate video_device.\n");
 			ret = -ENODEV;
 			goto unregister_dev;
 		}
-		ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO,
+		ret = video_register_device(v4l2->radio_dev, VFL_TYPE_RADIO,
 					    radio_nr[dev->devno]);
 		if (ret < 0) {
 			em28xx_errdev("can't register radio device\n");
 			goto unregister_dev;
 		}
 		em28xx_info("Registered radio device as %s\n",
-			    video_device_node_name(dev->radio_dev));
+			    video_device_node_name(v4l2->radio_dev));
 	}
 
 	em28xx_info("V4L2 video device registered as %s\n",
-		    video_device_node_name(dev->vdev));
+		    video_device_node_name(v4l2->vdev));
 
-	if (dev->vbi_dev)
+	if (v4l2->vbi_dev)
 		em28xx_info("V4L2 VBI device registered as %s\n",
-			    video_device_node_name(dev->vbi_dev));
+			    video_device_node_name(v4l2->vbi_dev));
 
 	/* Save some power by putting tuner to sleep */
 	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_power, 0);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index a4d26bf..88d0589 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -504,6 +504,10 @@ struct em28xx_v4l2 {
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_clk *clk;
+
+	struct video_device *vdev;
+	struct video_device *vbi_dev;
+	struct video_device *radio_dev;
 };
 
 struct em28xx_audio {
@@ -614,7 +618,6 @@ struct em28xx {
 	/* video for linux */
 	int users;		/* user count for exclusive use */
 	int streaming_users;    /* Number of actively streaming users */
-	struct video_device *vdev;	/* video for linux device struct */
 	v4l2_std_id norm;	/* selected tv norm */
 	int ctl_freq;		/* selected frequency */
 	unsigned int ctl_input;	/* selected input */
@@ -645,8 +648,6 @@ struct em28xx {
 	/* locks */
 	struct mutex lock;
 	struct mutex ctrl_urb_lock;	/* protects urb_buf */
-	struct video_device *vbi_dev;
-	struct video_device *radio_dev;
 
 	/* Videobuf2 */
 	struct vb2_queue vb_vidq;
-- 
1.8.4.5

[PATCH 07/19] em28xx: move videobuf2 related data from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |  2 --
 drivers/media/usb/em28xx/em28xx-video.c | 15 +++++++++------
 drivers/media/usb/em28xx/em28xx.h       | 12 ++++++------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index ff19833..a21cce1 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2991,8 +2991,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 	const char *chip_name = default_chip_name;
 
 	dev->udev = udev;
-	mutex_init(&dev->vb_queue_lock);
-	mutex_init(&dev->vb_vbi_queue_lock);
 	mutex_init(&dev->ctrl_urb_lock);
 	spin_lock_init(&dev->slock);
 
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 7252eef..301acef 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1049,9 +1049,10 @@ static int em28xx_vb2_setup(struct em28xx *dev)
 {
 	int rc;
 	struct vb2_queue *q;
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
 	/* Setup Videobuf2 for Video capture */
-	q = &dev->vb_vidq;
+	q = &v4l2->vb_vidq;
 	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
@@ -1065,7 +1066,7 @@ static int em28xx_vb2_setup(struct em28xx *dev)
 		return rc;
 
 	/* Setup Videobuf2 for VBI capture */
-	q = &dev->vb_vbiq;
+	q = &v4l2->vb_vbiq;
 	q->type = V4L2_BUF_TYPE_VBI_CAPTURE;
 	q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
@@ -2483,8 +2484,10 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		ret = -ENODEV;
 		goto unregister_dev;
 	}
-	v4l2->vdev->queue = &dev->vb_vidq;
-	v4l2->vdev->queue->lock = &dev->vb_queue_lock;
+	mutex_init(&v4l2->vb_queue_lock);
+	mutex_init(&v4l2->vb_vbi_queue_lock);
+	v4l2->vdev->queue = &v4l2->vb_vidq;
+	v4l2->vdev->queue->lock = &v4l2->vb_queue_lock;
 
 	/* disable inapplicable ioctls */
 	if (dev->board.is_webcam) {
@@ -2519,8 +2522,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		v4l2->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
 						"vbi");
 
-		v4l2->vbi_dev->queue = &dev->vb_vbiq;
-		v4l2->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
+		v4l2->vbi_dev->queue = &v4l2->vb_vbiq;
+		v4l2->vbi_dev->queue->lock = &v4l2->vb_vbi_queue_lock;
 
 		/* disable inapplicable ioctls */
 		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 88d0589..b02e8b1 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -508,6 +508,12 @@ struct em28xx_v4l2 {
 	struct video_device *vdev;
 	struct video_device *vbi_dev;
 	struct video_device *radio_dev;
+
+	/* Videobuf2 */
+	struct vb2_queue vb_vidq;
+	struct vb2_queue vb_vbiq;
+	struct mutex vb_queue_lock;
+	struct mutex vb_vbi_queue_lock;
 };
 
 struct em28xx_audio {
@@ -649,12 +655,6 @@ struct em28xx {
 	struct mutex lock;
 	struct mutex ctrl_urb_lock;	/* protects urb_buf */
 
-	/* Videobuf2 */
-	struct vb2_queue vb_vidq;
-	struct vb2_queue vb_vbiq;
-	struct mutex vb_queue_lock;
-	struct mutex vb_vbi_queue_lock;
-
 	/* resources in use */
 	unsigned int resources;
 
-- 
1.8.4.5

[PATCH 08/19] em28xx: move v4l2 frame resolutions and scale data from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-vbi.c   | 10 +++--
 drivers/media/usb/em28xx/em28xx-video.c | 80 +++++++++++++++++++--------------
 drivers/media/usb/em28xx/em28xx.h       | 16 ++++---
 3 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c b/drivers/media/usb/em28xx/em28xx-vbi.c
index db3d655..6d7f657 100644
--- a/drivers/media/usb/em28xx/em28xx-vbi.c
+++ b/drivers/media/usb/em28xx/em28xx-vbi.c
@@ -47,12 +47,13 @@ static int vbi_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 			   unsigned int sizes[], void *alloc_ctxs[])
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	unsigned long size;
 
 	if (fmt)
 		size = fmt->fmt.pix.sizeimage;
 	else
-		size = dev->vbi_width * dev->vbi_height * 2;
+		size = v4l2->vbi_width * v4l2->vbi_height * 2;
 
 	if (0 == *nbuffers)
 		*nbuffers = 32;
@@ -69,11 +70,12 @@ static int vbi_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 
 static int vbi_buffer_prepare(struct vb2_buffer *vb)
 {
-	struct em28xx        *dev = vb2_get_drv_priv(vb->vb2_queue);
-	struct em28xx_buffer *buf = container_of(vb, struct em28xx_buffer, vb);
+	struct em28xx        *dev  = vb2_get_drv_priv(vb->vb2_queue);
+	struct em28xx_v4l2   *v4l2 = dev->v4l2;
+	struct em28xx_buffer *buf  = container_of(vb, struct em28xx_buffer, vb);
 	unsigned long        size;
 
-	size = dev->vbi_width * dev->vbi_height * 2;
+	size = v4l2->vbi_width * v4l2->vbi_height * 2;
 
 	if (vb2_plane_size(vb, 0) < size) {
 		printk(KERN_INFO "%s data will not fit into plane (%lu < %lu)\n",
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 301acef..ecc4411 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -218,6 +218,7 @@ static int em28xx_set_outfmt(struct em28xx *dev)
 {
 	int ret;
 	u8 fmt, vinctrl;
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
 	fmt = dev->format->reg;
 	if (!dev->is_em25xx)
@@ -243,8 +244,8 @@ static int em28xx_set_outfmt(struct em28xx *dev)
 	if (em28xx_vbi_supported(dev) == 1) {
 		vinctrl |= EM28XX_VINCTRL_VBI_RAW;
 		em28xx_write_reg(dev, EM28XX_R34_VBI_START_H, 0x00);
-		em28xx_write_reg(dev, EM28XX_R36_VBI_WIDTH, dev->vbi_width/4);
-		em28xx_write_reg(dev, EM28XX_R37_VBI_HEIGHT, dev->vbi_height);
+		em28xx_write_reg(dev, EM28XX_R36_VBI_WIDTH, v4l2->vbi_width/4);
+		em28xx_write_reg(dev, EM28XX_R37_VBI_HEIGHT, v4l2->vbi_height);
 		if (dev->norm & V4L2_STD_525_60) {
 			/* NTSC */
 			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x09);
@@ -323,16 +324,16 @@ static int em28xx_scaler_set(struct em28xx *dev, u16 h, u16 v)
 /* FIXME: this only function read values from dev */
 static int em28xx_resolution_set(struct em28xx *dev)
 {
-	int width, height;
-	width = norm_maxw(dev);
-	height = norm_maxh(dev);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
+	int width = norm_maxw(dev);
+	int height = norm_maxh(dev);
 
 	/* Properly setup VBI */
-	dev->vbi_width = 720;
+	v4l2->vbi_width = 720;
 	if (dev->norm & V4L2_STD_525_60)
-		dev->vbi_height = 12;
+		v4l2->vbi_height = 12;
 	else
-		dev->vbi_height = 18;
+		v4l2->vbi_height = 18;
 
 	em28xx_set_outfmt(dev);
 
@@ -350,15 +351,16 @@ static int em28xx_resolution_set(struct em28xx *dev)
 	else
 		em28xx_capture_area_set(dev, 0, 0, width, height);
 
-	return em28xx_scaler_set(dev, dev->hscale, dev->vscale);
+	return em28xx_scaler_set(dev, v4l2->hscale, v4l2->vscale);
 }
 
 /* Set USB alternate setting for analog video */
 static int em28xx_set_alternate(struct em28xx *dev)
 {
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	int errCode;
 	int i;
-	unsigned int min_pkt_size = dev->width * 2 + 4;
+	unsigned int min_pkt_size = v4l2->width * 2 + 4;
 
 	/* NOTE: for isoc transfers, only alt settings > 0 are allowed
 		 bulk transfers seem to work only with alt=0 ! */
@@ -375,7 +377,7 @@ static int em28xx_set_alternate(struct em28xx *dev)
 	   the frame size should be increased, otherwise, only
 	   green screen will be received.
 	 */
-	if (dev->width * 2 * dev->height > 720 * 240 * 2)
+	if (v4l2->width * 2 * v4l2->height > 720 * 240 * 2)
 		min_pkt_size *= 2;
 
 	for (i = 0; i < dev->num_alt; i++) {
@@ -445,7 +447,7 @@ static void em28xx_copy_video(struct em28xx *dev,
 {
 	void *fieldstart, *startwrite, *startread;
 	int  linesdone, currlinedone, offset, lencopy, remain;
-	int bytesperline = dev->width << 1;
+	int bytesperline = dev->v4l2->width << 1;
 
 	if (buf->pos + len > buf->length)
 		len = buf->length - buf->pos;
@@ -531,7 +533,7 @@ static void em28xx_copy_vbi(struct em28xx *dev,
 	offset = buf->pos;
 	/* Make sure the bottom field populates the second half of the frame */
 	if (buf->top_field == 0)
-		offset += dev->vbi_width * dev->vbi_height;
+		offset += dev->v4l2->vbi_width * dev->v4l2->vbi_height;
 
 	memcpy(buf->vb_buf + offset, usb_buf, len);
 	buf->pos += len;
@@ -627,6 +629,7 @@ static inline void process_frame_data_em28xx(struct em28xx *dev,
 					     unsigned char *data_pkt,
 					     unsigned int  data_len)
 {
+	struct em28xx_v4l2      *v4l2 = dev->v4l2;
 	struct em28xx_buffer    *buf = dev->usb_ctl.vid_buf;
 	struct em28xx_buffer    *vbi_buf = dev->usb_ctl.vbi_buf;
 	struct em28xx_dmaqueue  *dma_q = &dev->vidq;
@@ -671,7 +674,7 @@ static inline void process_frame_data_em28xx(struct em28xx *dev,
 	}
 
 	if (dev->capture_type == 1) {
-		int vbi_size = dev->vbi_width * dev->vbi_height;
+		int vbi_size = v4l2->vbi_width * v4l2->vbi_height;
 		int vbi_data_len = ((dev->vbi_read + data_len) > vbi_size) ?
 				   (vbi_size - dev->vbi_read) : data_len;
 
@@ -865,12 +868,14 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 		       unsigned int sizes[], void *alloc_ctxs[])
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	unsigned long size;
 
 	if (fmt)
 		size = fmt->fmt.pix.sizeimage;
 	else
-		size = (dev->width * dev->height * dev->format->depth + 7) >> 3;
+		size =
+		     (v4l2->width * v4l2->height * dev->format->depth + 7) >> 3;
 
 	if (size == 0)
 		return -EINVAL;
@@ -888,12 +893,13 @@ static int
 buffer_prepare(struct vb2_buffer *vb)
 {
 	struct em28xx        *dev = vb2_get_drv_priv(vb->vb2_queue);
+	struct em28xx_v4l2   *v4l2 = dev->v4l2;
 	struct em28xx_buffer *buf = container_of(vb, struct em28xx_buffer, vb);
 	unsigned long size;
 
 	em28xx_videodbg("%s, field=%d\n", __func__, vb->v4l2_buf.field);
 
-	size = (dev->width * dev->height * dev->format->depth + 7) >> 3;
+	size = (v4l2->width * v4l2->height * dev->format->depth + 7) >> 3;
 
 	if (vb2_plane_size(vb, 0) < size) {
 		em28xx_videodbg("%s data will not fit into plane (%lu < %lu)\n",
@@ -1216,12 +1222,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
-	f->fmt.pix.width = dev->width;
-	f->fmt.pix.height = dev->height;
+	f->fmt.pix.width = v4l2->width;
+	f->fmt.pix.height = v4l2->height;
 	f->fmt.pix.pixelformat = dev->format->fourcc;
-	f->fmt.pix.bytesperline = (dev->width * dev->format->depth + 7) >> 3;
-	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline  * dev->height;
+	f->fmt.pix.bytesperline = (v4l2->width * dev->format->depth + 7) >> 3;
+	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * v4l2->height;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
 
 	/* FIXME: TOP? NONE? BOTTOM? ALTENATE? */
@@ -1304,17 +1311,19 @@ static int em28xx_set_video_format(struct em28xx *dev, unsigned int fourcc,
 				   unsigned width, unsigned height)
 {
 	struct em28xx_fmt     *fmt;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	fmt = format_by_fourcc(fourcc);
 	if (!fmt)
 		return -EINVAL;
 
 	dev->format = fmt;
-	dev->width  = width;
-	dev->height = height;
+	v4l2->width  = width;
+	v4l2->height = height;
 
 	/* set new image size */
-	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
+	size_to_scale(dev, v4l2->width, v4l2->height,
+			   &v4l2->hscale, &v4l2->vscale);
 
 	em28xx_resolution_set(dev);
 
@@ -1357,8 +1366,9 @@ 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_fh   *fh   = priv;
+	struct em28xx      *dev  = fh->dev;
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct v4l2_format f;
 
 	if (norm == dev->norm)
@@ -1375,12 +1385,13 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 	vidioc_try_fmt_vid_cap(file, priv, &f);
 
 	/* set new image size */
-	dev->width = f.fmt.pix.width;
-	dev->height = f.fmt.pix.height;
-	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
+	v4l2->width = f.fmt.pix.width;
+	v4l2->height = f.fmt.pix.height;
+	size_to_scale(dev, v4l2->width, v4l2->height,
+			   &v4l2->hscale, &v4l2->vscale);
 
 	em28xx_resolution_set(dev);
-	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, core, s_std, dev->norm);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, dev->norm);
 
 	return 0;
 }
@@ -1784,16 +1795,17 @@ 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_fh      *fh   = priv;
+	struct em28xx         *dev  = fh->dev;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
-	format->fmt.vbi.samples_per_line = dev->vbi_width;
+	format->fmt.vbi.samples_per_line = v4l2->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;
+	format->fmt.vbi.count[0] = v4l2->vbi_height;
+	format->fmt.vbi.count[1] = v4l2->vbi_height;
 	memset(format->fmt.vbi.reserved, 0, sizeof(format->fmt.vbi.reserved));
 
 	/* Varies by video standard (NTSC, PAL, etc.) */
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index b02e8b1..e029136 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -514,6 +514,14 @@ struct em28xx_v4l2 {
 	struct vb2_queue vb_vbiq;
 	struct mutex vb_queue_lock;
 	struct mutex vb_vbi_queue_lock;
+
+	/* Frame properties */
+	int width;		/* current frame width */
+	int height;		/* current frame height */
+	unsigned hscale;	/* horizontal scale factor (see datasheet) */
+	unsigned vscale;	/* vertical scale factor (see datasheet) */
+	unsigned int vbi_width;
+	unsigned int vbi_height; /* lines per field */
 };
 
 struct em28xx_audio {
@@ -631,11 +639,7 @@ struct em28xx {
 	unsigned int ctl_aoutput;/* selected audio output */
 	int mute;
 	int volume;
-	/* frame properties */
-	int width;		/* current frame width */
-	int height;		/* current frame height */
-	unsigned hscale;	/* horizontal scale factor (see datasheet) */
-	unsigned vscale;	/* vertical scale factor (see datasheet) */
+
 	int interlaced;		/* 1=interlace fileds, 0=just top fileds */
 
 	unsigned long hash;	/* eeprom hash - for boards with generic ID */
@@ -646,8 +650,6 @@ struct em28xx {
 	int capture_type;
 	unsigned char top_field:1;
 	int vbi_read;
-	unsigned int vbi_width;
-	unsigned int vbi_height; /* lines per field */
 
 	struct work_struct         request_module_wk;
 
-- 
1.8.4.5

[PATCH 09/19] em28xx: move vinmode and vinctrl data from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 16 ++++++++--------
 drivers/media/usb/em28xx/em28xx-video.c  | 10 +++++-----
 drivers/media/usb/em28xx/em28xx.h        |  6 +++---
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index c2672b4..3a88867 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -372,8 +372,8 @@ int em28xx_init_camera(struct em28xx *dev)
 			break;
 		}
 		/* probably means GRGB 16 bit bayer */
-		dev->vinmode = 0x0d;
-		dev->vinctl = 0x00;
+		v4l2->vinmode = 0x0d;
+		v4l2->vinctl = 0x00;
 
 		break;
 	}
@@ -384,8 +384,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		em28xx_initialize_mt9m001(dev);
 
 		/* probably means BGGR 16 bit bayer */
-		dev->vinmode = 0x0c;
-		dev->vinctl = 0x00;
+		v4l2->vinmode = 0x0c;
+		v4l2->vinctl = 0x00;
 
 		break;
 	case EM28XX_MT9M111:
@@ -396,8 +396,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
 		em28xx_initialize_mt9m111(dev);
 
-		dev->vinmode = 0x0a;
-		dev->vinctl = 0x00;
+		v4l2->vinmode = 0x0a;
+		v4l2->vinctl = 0x00;
 
 		break;
 	case EM28XX_OV2640:
@@ -438,8 +438,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		/* NOTE: for UXGA=1600x1200 switch to 12MHz */
 		dev->board.xclk = EM28XX_XCLK_FREQUENCY_24MHZ;
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
-		dev->vinmode = 0x08;
-		dev->vinctl = 0x00;
+		v4l2->vinmode = 0x08;
+		v4l2->vinctl = 0x00;
 
 		break;
 	}
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index ecc4411..0676aa4 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -236,11 +236,11 @@ static int em28xx_set_outfmt(struct em28xx *dev)
 	if (ret < 0)
 		return ret;
 
-	ret = em28xx_write_reg(dev, EM28XX_R10_VINMODE, dev->vinmode);
+	ret = em28xx_write_reg(dev, EM28XX_R10_VINMODE, v4l2->vinmode);
 	if (ret < 0)
 		return ret;
 
-	vinctrl = dev->vinctl;
+	vinctrl = v4l2->vinctl;
 	if (em28xx_vbi_supported(dev) == 1) {
 		vinctrl |= EM28XX_VINCTRL_VBI_RAW;
 		em28xx_write_reg(dev, EM28XX_R34_VBI_START_H, 0x00);
@@ -2316,9 +2316,9 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/*
 	 * Default format, used for tvp5150 or saa711x output formats
 	 */
-	dev->vinmode = 0x10;
-	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
-		       EM28XX_VINCTRL_CCIR656_ENABLE;
+	v4l2->vinmode = 0x10;
+	v4l2->vinctl  = EM28XX_VINCTRL_INTERLACED |
+			EM28XX_VINCTRL_CCIR656_ENABLE;
 
 	/* request some modules */
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index e029136..7ca0ff98 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -515,6 +515,9 @@ struct em28xx_v4l2 {
 	struct mutex vb_queue_lock;
 	struct mutex vb_vbi_queue_lock;
 
+	u8 vinmode;
+	u8 vinctl;
+
 	/* Frame properties */
 	int width;		/* current frame width */
 	int height;		/* current frame height */
@@ -597,9 +600,6 @@ struct em28xx {
 	/* Progressive (non-interlaced) mode */
 	int progressive;
 
-	/* Vinmode/Vinctl used at the driver */
-	int vinmode, vinctl;
-
 	/* Controls audio streaming */
 	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
 	atomic_t       stream_started;	/* stream should be running if true */
-- 
1.8.4.5

[PATCH 10/19] em28xx: move TV norm from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 35 ++++++++++++++++++---------------
 drivers/media/usb/em28xx/em28xx.h       |  3 ++-
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 0676aa4..821d182 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -155,13 +155,15 @@ static inline unsigned int norm_maxw(struct em28xx *dev)
 
 static inline unsigned int norm_maxh(struct em28xx *dev)
 {
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
+
 	if (dev->board.is_webcam)
 		return dev->sensor_yres;
 
 	if (dev->board.max_range_640_480)
 		return 480;
 
-	return (dev->norm & V4L2_STD_625_50) ? 576 : 480;
+	return (v4l2->norm & V4L2_STD_625_50) ? 576 : 480;
 }
 
 static int em28xx_vbi_supported(struct em28xx *dev)
@@ -246,10 +248,10 @@ static int em28xx_set_outfmt(struct em28xx *dev)
 		em28xx_write_reg(dev, EM28XX_R34_VBI_START_H, 0x00);
 		em28xx_write_reg(dev, EM28XX_R36_VBI_WIDTH, v4l2->vbi_width/4);
 		em28xx_write_reg(dev, EM28XX_R37_VBI_HEIGHT, v4l2->vbi_height);
-		if (dev->norm & V4L2_STD_525_60) {
+		if (v4l2->norm & V4L2_STD_525_60) {
 			/* NTSC */
 			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x09);
-		} else if (dev->norm & V4L2_STD_625_50) {
+		} else if (v4l2->norm & V4L2_STD_625_50) {
 			/* PAL */
 			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x07);
 		}
@@ -330,7 +332,7 @@ static int em28xx_resolution_set(struct em28xx *dev)
 
 	/* Properly setup VBI */
 	v4l2->vbi_width = 720;
-	if (dev->norm & V4L2_STD_525_60)
+	if (v4l2->norm & V4L2_STD_525_60)
 		v4l2->vbi_height = 12;
 	else
 		v4l2->vbi_height = 18;
@@ -1349,7 +1351,7 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct em28xx_fh   *fh  = priv;
 	struct em28xx      *dev = fh->dev;
 
-	*norm = dev->norm;
+	*norm = dev->v4l2->norm;
 
 	return 0;
 }
@@ -1371,13 +1373,13 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct v4l2_format f;
 
-	if (norm == dev->norm)
+	if (norm == v4l2->norm)
 		return 0;
 
 	if (dev->streaming_users > 0)
 		return -EBUSY;
 
-	dev->norm = norm;
+	v4l2->norm = norm;
 
 	/* Adjusts width/height, if needed */
 	f.fmt.pix.width = 720;
@@ -1391,7 +1393,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 			   &v4l2->hscale, &v4l2->vscale);
 
 	em28xx_resolution_set(dev);
-	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, dev->norm);
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, v4l2->norm);
 
 	return 0;
 }
@@ -1399,16 +1401,17 @@ 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_fh   *fh   = priv;
+	struct em28xx      *dev  = fh->dev;
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	int rc = 0;
 
 	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
 	if (dev->board.is_webcam)
-		rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
+		rc = v4l2_device_call_until_err(&v4l2->v4l2_dev, 0,
 						video, g_parm, p);
 	else
-		v4l2_video_std_frame_period(dev->norm,
+		v4l2_video_std_frame_period(v4l2->norm,
 						 &p->parm.capture.timeperframe);
 
 	return rc;
@@ -1809,11 +1812,11 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 	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) {
+	if (v4l2->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) {
+	} else if (v4l2->norm & V4L2_STD_625_50) {
 		/* PAL */
 		format->fmt.vbi.start[0] = 6;
 		format->fmt.vbi.start[1] = 318;
@@ -2425,8 +2428,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	}
 
 	/* set default norm */
-	dev->norm = V4L2_STD_PAL;
-	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, dev->norm);
+	v4l2->norm = V4L2_STD_PAL;
+	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, v4l2->norm);
 	dev->interlaced = EM28XX_INTERLACED_DEFAULT;
 
 	/* Analog specific initialization */
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 7ca0ff98..f0b3b8c 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -518,6 +518,8 @@ struct em28xx_v4l2 {
 	u8 vinmode;
 	u8 vinctl;
 
+	v4l2_std_id norm;	/* selected tv norm */
+
 	/* Frame properties */
 	int width;		/* current frame width */
 	int height;		/* current frame height */
@@ -632,7 +634,6 @@ struct em28xx {
 	/* video for linux */
 	int users;		/* user count for exclusive use */
 	int streaming_users;    /* Number of actively streaming users */
-	v4l2_std_id norm;	/* selected tv norm */
 	int ctl_freq;		/* selected frequency */
 	unsigned int ctl_input;	/* selected input */
 	unsigned int ctl_ainput;/* selected audio input */
-- 
1.8.4.5

[PATCH 11/19] em28xx: move struct em28xx_fmt *format from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 14 +++++++-------
 drivers/media/usb/em28xx/em28xx.h       |  3 +--
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 821d182..c316147 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -222,7 +222,7 @@ static int em28xx_set_outfmt(struct em28xx *dev)
 	u8 fmt, vinctrl;
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
-	fmt = dev->format->reg;
+	fmt = v4l2->format->reg;
 	if (!dev->is_em25xx)
 		fmt |= 0x20;
 	/*
@@ -877,7 +877,7 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
 		size = fmt->fmt.pix.sizeimage;
 	else
 		size =
-		     (v4l2->width * v4l2->height * dev->format->depth + 7) >> 3;
+		    (v4l2->width * v4l2->height * v4l2->format->depth + 7) >> 3;
 
 	if (size == 0)
 		return -EINVAL;
@@ -901,7 +901,7 @@ buffer_prepare(struct vb2_buffer *vb)
 
 	em28xx_videodbg("%s, field=%d\n", __func__, vb->v4l2_buf.field);
 
-	size = (v4l2->width * v4l2->height * dev->format->depth + 7) >> 3;
+	size = (v4l2->width * v4l2->height * v4l2->format->depth + 7) >> 3;
 
 	if (vb2_plane_size(vb, 0) < size) {
 		em28xx_videodbg("%s data will not fit into plane (%lu < %lu)\n",
@@ -1228,8 +1228,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 
 	f->fmt.pix.width = v4l2->width;
 	f->fmt.pix.height = v4l2->height;
-	f->fmt.pix.pixelformat = dev->format->fourcc;
-	f->fmt.pix.bytesperline = (v4l2->width * dev->format->depth + 7) >> 3;
+	f->fmt.pix.pixelformat = v4l2->format->fourcc;
+	f->fmt.pix.bytesperline = (v4l2->width * v4l2->format->depth + 7) >> 3;
 	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * v4l2->height;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
 
@@ -1319,7 +1319,7 @@ static int em28xx_set_video_format(struct em28xx *dev, unsigned int fourcc,
 	if (!fmt)
 		return -EINVAL;
 
-	dev->format = fmt;
+	v4l2->format = fmt;
 	v4l2->width  = width;
 	v4l2->height = height;
 
@@ -2433,7 +2433,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	dev->interlaced = EM28XX_INTERLACED_DEFAULT;
 
 	/* Analog specific initialization */
-	dev->format = &format[0];
+	v4l2->format = &format[0];
 
 	maxw = norm_maxw(dev);
 	/* MaxPacketSize for em2800 is too small to capture at full resolution
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index f0b3b8c..dd93a37 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -518,6 +518,7 @@ struct em28xx_v4l2 {
 	u8 vinmode;
 	u8 vinctl;
 
+	struct em28xx_fmt *format;
 	v4l2_std_id norm;	/* selected tv norm */
 
 	/* Frame properties */
@@ -606,8 +607,6 @@ struct em28xx {
 	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
 	atomic_t       stream_started;	/* stream should be running if true */
 
-	struct em28xx_fmt *format;
-
 	/* Some older em28xx chips needs a waiting time after writing */
 	unsigned int wait_after_write;
 
-- 
1.8.4.5

[PATCH 12/19] em28xx: move progressive/interlaced fields from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |  2 --
 drivers/media/usb/em28xx/em28xx-video.c | 27 +++++++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h       | 10 +++++-----
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index a21cce1..64ea25a 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2682,8 +2682,6 @@ static void em28xx_card_setup(struct em28xx *dev)
 	if (dev->board.is_webcam) {
 		if (em28xx_detect_sensor(dev) < 0)
 			dev->board.is_webcam = 0;
-		else
-			dev->progressive = 1;
 	}
 
 	switch (dev->model) {
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index c316147..abb4e8e 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -447,9 +447,10 @@ static void em28xx_copy_video(struct em28xx *dev,
 			      unsigned char *usb_buf,
 			      unsigned long len)
 {
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	void *fieldstart, *startwrite, *startread;
 	int  linesdone, currlinedone, offset, lencopy, remain;
-	int bytesperline = dev->v4l2->width << 1;
+	int bytesperline = v4l2->width << 1;
 
 	if (buf->pos + len > buf->length)
 		len = buf->length - buf->pos;
@@ -457,7 +458,7 @@ static void em28xx_copy_video(struct em28xx *dev,
 	startread = usb_buf;
 	remain = len;
 
-	if (dev->progressive || buf->top_field)
+	if (v4l2->progressive || buf->top_field)
 		fieldstart = buf->vb_buf;
 	else /* interlaced mode, even nr. of lines */
 		fieldstart = buf->vb_buf + bytesperline;
@@ -465,7 +466,7 @@ static void em28xx_copy_video(struct em28xx *dev,
 	linesdone = buf->pos / bytesperline;
 	currlinedone = buf->pos % bytesperline;
 
-	if (dev->progressive)
+	if (v4l2->progressive)
 		offset = linesdone * bytesperline + currlinedone;
 	else
 		offset = linesdone * bytesperline * 2 + currlinedone;
@@ -489,7 +490,7 @@ static void em28xx_copy_video(struct em28xx *dev,
 	remain -= lencopy;
 
 	while (remain > 0) {
-		if (dev->progressive)
+		if (v4l2->progressive)
 			startwrite += lencopy;
 		else
 			startwrite += lencopy + bytesperline;
@@ -611,7 +612,9 @@ finish_field_prepare_next(struct em28xx *dev,
 			  struct em28xx_buffer *buf,
 			  struct em28xx_dmaqueue *dma_q)
 {
-	if (dev->progressive || dev->top_field) { /* Brand new frame */
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
+
+	if (v4l2->progressive || dev->top_field) { /* Brand new frame */
 		if (buf != NULL)
 			finish_buffer(dev, buf);
 		buf = get_next_buf(dev, dma_q);
@@ -1234,10 +1237,10 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
 
 	/* FIXME: TOP? NONE? BOTTOM? ALTENATE? */
-	if (dev->progressive)
+	if (v4l2->progressive)
 		f->fmt.pix.field = V4L2_FIELD_NONE;
 	else
-		f->fmt.pix.field = dev->interlaced ?
+		f->fmt.pix.field = v4l2->interlaced_fieldmode ?
 			   V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
 	return 0;
 }
@@ -1258,6 +1261,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh    = priv;
 	struct em28xx         *dev   = fh->dev;
+	struct em28xx_v4l2    *v4l2  = dev->v4l2;
 	unsigned int          width  = f->fmt.pix.width;
 	unsigned int          height = f->fmt.pix.height;
 	unsigned int          maxw   = norm_maxw(dev);
@@ -1299,10 +1303,10 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.bytesperline = (width * fmt->depth + 7) >> 3;
 	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * height;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
-	if (dev->progressive)
+	if (v4l2->progressive)
 		f->fmt.pix.field = V4L2_FIELD_NONE;
 	else
-		f->fmt.pix.field = dev->interlaced ?
+		f->fmt.pix.field = v4l2->interlaced_fieldmode ?
 			   V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
 	f->fmt.pix.priv = 0;
 
@@ -2316,6 +2320,9 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	v4l2_ctrl_handler_init(hdl, 8);
 	v4l2->v4l2_dev.ctrl_handler = hdl;
 
+	if (dev->board.is_webcam)
+		v4l2->progressive = 1;
+
 	/*
 	 * Default format, used for tvp5150 or saa711x output formats
 	 */
@@ -2430,7 +2437,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* set default norm */
 	v4l2->norm = V4L2_STD_PAL;
 	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_std, v4l2->norm);
-	dev->interlaced = EM28XX_INTERLACED_DEFAULT;
+	v4l2->interlaced_fieldmode = EM28XX_INTERLACED_DEFAULT;
 
 	/* Analog specific initialization */
 	v4l2->format = &format[0];
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index dd93a37..1491879 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -521,6 +521,11 @@ struct em28xx_v4l2 {
 	struct em28xx_fmt *format;
 	v4l2_std_id norm;	/* selected tv norm */
 
+	/* Progressive/interlaced mode */
+	bool progressive;
+	int interlaced_fieldmode; /* 1=interlaced fields, 0=just top fields */
+	/* FIXME: everything else than interlaced_fieldmode=1 doesn't work */
+
 	/* Frame properties */
 	int width;		/* current frame width */
 	int height;		/* current frame height */
@@ -600,9 +605,6 @@ struct em28xx {
 	int sensor_xres, sensor_yres;
 	int sensor_xtal;
 
-	/* Progressive (non-interlaced) mode */
-	int progressive;
-
 	/* Controls audio streaming */
 	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
 	atomic_t       stream_started;	/* stream should be running if true */
@@ -640,8 +642,6 @@ struct em28xx {
 	int mute;
 	int volume;
 
-	int interlaced;		/* 1=interlace fileds, 0=just top fileds */
-
 	unsigned long hash;	/* eeprom hash - for boards with generic ID */
 	unsigned long i2c_hash;	/* i2c devicelist hash -
 				   for boards with generic ID */
-- 
1.8.4.5

[PATCH 13/19] em28xx: move sensor parameter fields from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 20 ++++++++++----------
 drivers/media/usb/em28xx/em28xx-video.c  |  6 ++++--
 drivers/media/usb/em28xx/em28xx.h        | 10 ++++++----
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index 3a88867..12d4c03 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -349,8 +349,8 @@ int em28xx_init_camera(struct em28xx *dev)
 			.platform_data = &pdata,
 		};
 
-		dev->sensor_xres = 640;
-		dev->sensor_yres = 480;
+		v4l2->sensor_xres = 640;
+		v4l2->sensor_yres = 480;
 
 		/*
 		 * FIXME: mt9v011 uses I2S speed as xtal clk - at least with
@@ -363,8 +363,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		 */
 		dev->board.xclk = EM28XX_XCLK_FREQUENCY_4_3MHZ;
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
-		dev->sensor_xtal = 4300000;
-		pdata.xtal = dev->sensor_xtal;
+		v4l2->sensor_xtal = 4300000;
+		pdata.xtal = v4l2->sensor_xtal;
 		if (NULL ==
 		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
 					      &mt9v011_info, NULL)) {
@@ -378,8 +378,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		break;
 	}
 	case EM28XX_MT9M001:
-		dev->sensor_xres = 1280;
-		dev->sensor_yres = 1024;
+		v4l2->sensor_xres = 1280;
+		v4l2->sensor_yres = 1024;
 
 		em28xx_initialize_mt9m001(dev);
 
@@ -389,8 +389,8 @@ int em28xx_init_camera(struct em28xx *dev)
 
 		break;
 	case EM28XX_MT9M111:
-		dev->sensor_xres = 640;
-		dev->sensor_yres = 512;
+		v4l2->sensor_xres = 640;
+		v4l2->sensor_yres = 512;
 
 		dev->board.xclk = EM28XX_XCLK_FREQUENCY_48MHZ;
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
@@ -419,8 +419,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		 * - adjust bridge xclk
 		 * - disable 16 bit (12 bit) output formats on high resolutions
 		 */
-		dev->sensor_xres = 640;
-		dev->sensor_yres = 480;
+		v4l2->sensor_xres = 640;
+		v4l2->sensor_yres = 480;
 
 		subdev =
 		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index abb4e8e..640c0b0 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -144,8 +144,10 @@ static struct em28xx_fmt format[] = {
 /*FIXME: maxw should be dependent of alt mode */
 static inline unsigned int norm_maxw(struct em28xx *dev)
 {
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
+
 	if (dev->board.is_webcam)
-		return dev->sensor_xres;
+		return v4l2->sensor_xres;
 
 	if (dev->board.max_range_640_480)
 		return 640;
@@ -158,7 +160,7 @@ static inline unsigned int norm_maxh(struct em28xx *dev)
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
 	if (dev->board.is_webcam)
-		return dev->sensor_yres;
+		return v4l2->sensor_yres;
 
 	if (dev->board.max_range_640_480)
 		return 480;
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 1491879..f447108 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -518,6 +518,11 @@ struct em28xx_v4l2 {
 	u8 vinmode;
 	u8 vinctl;
 
+	/* Camera specific fields */
+	int sensor_xres;
+	int sensor_yres;
+	int sensor_xtal;
+
 	struct em28xx_fmt *format;
 	v4l2_std_id norm;	/* selected tv norm */
 
@@ -600,10 +605,7 @@ struct em28xx {
 
 	struct em28xx_board board;
 
-	/* Webcam specific fields */
-	enum em28xx_sensor em28xx_sensor;
-	int sensor_xres, sensor_yres;
-	int sensor_xtal;
+	enum em28xx_sensor em28xx_sensor;	/* camera specific */
 
 	/* Controls audio streaming */
 	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
-- 
1.8.4.5

[PATCH 14/19] em28xx: move capture state tracking fields from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 44 +++++++++++++++++----------------
 drivers/media/usb/em28xx/em28xx.h       | 13 +++++-----
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 640c0b0..496dcef 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -434,7 +434,7 @@ static inline void finish_buffer(struct em28xx *dev,
 {
 	em28xx_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
 
-	buf->vb.v4l2_buf.sequence = dev->field_count++;
+	buf->vb.v4l2_buf.sequence = dev->v4l2->field_count++;
 	buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
 	v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
 
@@ -616,13 +616,13 @@ finish_field_prepare_next(struct em28xx *dev,
 {
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
-	if (v4l2->progressive || dev->top_field) { /* Brand new frame */
+	if (v4l2->progressive || v4l2->top_field) { /* Brand new frame */
 		if (buf != NULL)
 			finish_buffer(dev, buf);
 		buf = get_next_buf(dev, dma_q);
 	}
 	if (buf != NULL) {
-		buf->top_field = dev->top_field;
+		buf->top_field = v4l2->top_field;
 		buf->pos = 0;
 	}
 
@@ -656,17 +656,17 @@ static inline void process_frame_data_em28xx(struct em28xx *dev,
 			data_len -= 4;
 		} else if (data_pkt[0] == 0x33 && data_pkt[1] == 0x95) {
 			/* Field start (VBI mode) */
-			dev->capture_type = 0;
-			dev->vbi_read = 0;
+			v4l2->capture_type = 0;
+			v4l2->vbi_read = 0;
 			em28xx_isocdbg("VBI START HEADER !!!\n");
-			dev->top_field = !(data_pkt[2] & 1);
+			v4l2->top_field = !(data_pkt[2] & 1);
 			data_pkt += 4;
 			data_len -= 4;
 		} else if (data_pkt[0] == 0x22 && data_pkt[1] == 0x5a) {
 			/* Field start (VBI disabled) */
-			dev->capture_type = 2;
+			v4l2->capture_type = 2;
 			em28xx_isocdbg("VIDEO START HEADER !!!\n");
-			dev->top_field = !(data_pkt[2] & 1);
+			v4l2->top_field = !(data_pkt[2] & 1);
 			data_pkt += 4;
 			data_len -= 4;
 		}
@@ -674,37 +674,37 @@ static inline void process_frame_data_em28xx(struct em28xx *dev,
 	/* NOTE: With bulk transfers, intermediate data packets
 	 * have no continuation header */
 
-	if (dev->capture_type == 0) {
+	if (v4l2->capture_type == 0) {
 		vbi_buf = finish_field_prepare_next(dev, vbi_buf, vbi_dma_q);
 		dev->usb_ctl.vbi_buf = vbi_buf;
-		dev->capture_type = 1;
+		v4l2->capture_type = 1;
 	}
 
-	if (dev->capture_type == 1) {
+	if (v4l2->capture_type == 1) {
 		int vbi_size = v4l2->vbi_width * v4l2->vbi_height;
-		int vbi_data_len = ((dev->vbi_read + data_len) > vbi_size) ?
-				   (vbi_size - dev->vbi_read) : data_len;
+		int vbi_data_len = ((v4l2->vbi_read + data_len) > vbi_size) ?
+				   (vbi_size - v4l2->vbi_read) : data_len;
 
 		/* Copy VBI data */
 		if (vbi_buf != NULL)
 			em28xx_copy_vbi(dev, vbi_buf, data_pkt, vbi_data_len);
-		dev->vbi_read += vbi_data_len;
+		v4l2->vbi_read += vbi_data_len;
 
 		if (vbi_data_len < data_len) {
 			/* Continue with copying video data */
-			dev->capture_type = 2;
+			v4l2->capture_type = 2;
 			data_pkt += vbi_data_len;
 			data_len -= vbi_data_len;
 		}
 	}
 
-	if (dev->capture_type == 2) {
+	if (v4l2->capture_type == 2) {
 		buf = finish_field_prepare_next(dev, buf, dma_q);
 		dev->usb_ctl.vid_buf = buf;
-		dev->capture_type = 3;
+		v4l2->capture_type = 3;
 	}
 
-	if (dev->capture_type == 3 && buf != NULL && data_len > 0)
+	if (v4l2->capture_type == 3 && buf != NULL && data_len > 0)
 		em28xx_copy_video(dev, buf, data_pkt, data_len);
 }
 
@@ -717,6 +717,7 @@ static inline void process_frame_data_em25xx(struct em28xx *dev,
 {
 	struct em28xx_buffer    *buf = dev->usb_ctl.vid_buf;
 	struct em28xx_dmaqueue  *dmaq = &dev->vidq;
+	struct em28xx_v4l2      *v4l2 = dev->v4l2;
 	bool frame_end = 0;
 
 	/* Check for header */
@@ -725,7 +726,7 @@ static inline void process_frame_data_em25xx(struct em28xx *dev,
 	if (data_len >= 2) {	/* em25xx header is only 2 bytes long */
 		if ((data_pkt[0] == EM25XX_FRMDATAHDR_BYTE1) &&
 		    ((data_pkt[1] & ~EM25XX_FRMDATAHDR_BYTE2_MASK) == 0x00)) {
-			dev->top_field = !(data_pkt[1] &
+			v4l2->top_field = !(data_pkt[1] &
 					   EM25XX_FRMDATAHDR_BYTE2_FRAME_ID);
 			frame_end = data_pkt[1] &
 				    EM25XX_FRMDATAHDR_BYTE2_FRAME_END;
@@ -921,6 +922,7 @@ buffer_prepare(struct vb2_buffer *vb)
 int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct v4l2_frequency f;
 	int rc = 0;
 
@@ -943,7 +945,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 		*/
 		em28xx_wake_i2c(dev);
 
-		dev->capture_type = -1;
+		v4l2->capture_type = -1;
 		rc = em28xx_init_usb_xfer(dev, EM28XX_ANALOG_MODE,
 					  dev->analog_xfer_bulk,
 					  EM28XX_NUM_BUFS,
@@ -966,7 +968,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 			f.type = V4L2_TUNER_RADIO;
 		else
 			f.type = V4L2_TUNER_ANALOG_TV;
-		v4l2_device_call_all(&dev->v4l2->v4l2_dev,
+		v4l2_device_call_all(&v4l2->v4l2_dev,
 				     0, tuner, s_frequency, &f);
 	}
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index f447108..91bb624 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -538,6 +538,12 @@ struct em28xx_v4l2 {
 	unsigned vscale;	/* vertical scale factor (see datasheet) */
 	unsigned int vbi_width;
 	unsigned int vbi_height; /* lines per field */
+
+	/* Capture state tracking */
+	int capture_type;
+	bool top_field;
+	int vbi_read;
+	unsigned int field_count;
 };
 
 struct em28xx_audio {
@@ -648,11 +654,6 @@ struct em28xx {
 	unsigned long i2c_hash;	/* i2c devicelist hash -
 				   for boards with generic ID */
 
-	/* capture state tracking */
-	int capture_type;
-	unsigned char top_field:1;
-	int vbi_read;
-
 	struct work_struct         request_module_wk;
 
 	/* locks */
@@ -672,8 +673,6 @@ struct em28xx {
 	struct em28xx_usb_ctl usb_ctl;
 	spinlock_t slock;
 
-	unsigned int field_count;
-
 	/* usb transfer */
 	struct usb_device *udev;	/* the usb device */
 	u8 ifnum;		/* number of the assigned usb interface */
-- 
1.8.4.5

[PATCH 15/19] em28xx: move v4l2 user counting fields from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 27 +++++++++++++++------------
 drivers/media/usb/em28xx/em28xx.h       |  5 +++--
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 496dcef..aaab111 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -934,7 +934,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 	if (rc)
 		return rc;
 
-	if (dev->streaming_users == 0) {
+	if (v4l2->streaming_users == 0) {
 		/* First active streaming user, so allocate all the URBs */
 
 		/* Allocate the USB bandwidth */
@@ -972,7 +972,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 				     0, tuner, s_frequency, &f);
 	}
 
-	dev->streaming_users++;
+	v4l2->streaming_users++;
 
 	return rc;
 }
@@ -980,6 +980,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 static int em28xx_stop_streaming(struct vb2_queue *vq)
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct em28xx_dmaqueue *vidq = &dev->vidq;
 	unsigned long flags = 0;
 
@@ -987,7 +988,7 @@ static int em28xx_stop_streaming(struct vb2_queue *vq)
 
 	res_free(dev, vq->type);
 
-	if (dev->streaming_users-- == 1) {
+	if (v4l2->streaming_users-- == 1) {
 		/* Last active user, so shutdown all the URBS */
 		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 	}
@@ -1008,6 +1009,7 @@ static int em28xx_stop_streaming(struct vb2_queue *vq)
 int em28xx_stop_vbi_streaming(struct vb2_queue *vq)
 {
 	struct em28xx *dev = vb2_get_drv_priv(vq);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 	struct em28xx_dmaqueue *vbiq = &dev->vbiq;
 	unsigned long flags = 0;
 
@@ -1015,7 +1017,7 @@ int em28xx_stop_vbi_streaming(struct vb2_queue *vq)
 
 	res_free(dev, vq->type);
 
-	if (dev->streaming_users-- == 1) {
+	if (v4l2->streaming_users-- == 1) {
 		/* Last active user, so shutdown all the URBS */
 		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 	}
@@ -1344,8 +1346,9 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 			struct v4l2_format *f)
 {
 	struct em28xx *dev = video_drvdata(file);
+	struct em28xx_v4l2 *v4l2 = dev->v4l2;
 
-	if (dev->streaming_users > 0)
+	if (v4l2->streaming_users > 0)
 		return -EBUSY;
 
 	vidioc_try_fmt_vid_cap(file, priv, f);
@@ -1384,7 +1387,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 	if (norm == v4l2->norm)
 		return 0;
 
-	if (dev->streaming_users > 0)
+	if (v4l2->streaming_users > 0)
 		return -EBUSY;
 
 	v4l2->norm = norm;
@@ -1907,7 +1910,7 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	em28xx_videodbg("open dev=%s type=%s users=%d\n",
 			video_device_node_name(vdev), v4l2_type_names[fh_type],
-			dev->users);
+			v4l2->users);
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;
@@ -1922,7 +1925,7 @@ static int em28xx_v4l2_open(struct file *filp)
 	fh->type = fh_type;
 	filp->private_data = fh;
 
-	if (dev->users == 0) {
+	if (v4l2->users == 0) {
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
 
 		if (vdev->vfl_type != VFL_TYPE_RADIO)
@@ -1942,7 +1945,7 @@ static int em28xx_v4l2_open(struct file *filp)
 
 	kref_get(&dev->ref);
 	kref_get(&v4l2->ref);
-	dev->users++;
+	v4l2->users++;
 
 	mutex_unlock(&dev->lock);
 	v4l2_fh_add(&fh->fh);
@@ -2051,12 +2054,12 @@ static int em28xx_v4l2_close(struct file *filp)
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	int              errCode;
 
-	em28xx_videodbg("users=%d\n", dev->users);
+	em28xx_videodbg("users=%d\n", v4l2->users);
 
 	vb2_fop_release(filp);
 	mutex_lock(&dev->lock);
 
-	if (dev->users == 1) {
+	if (v4l2->users == 1) {
 		/* No sense to try to write to the device */
 		if (dev->disconnected)
 			goto exit;
@@ -2078,8 +2081,8 @@ static int em28xx_v4l2_close(struct file *filp)
 	}
 
 exit:
+	v4l2->users--;
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
-	dev->users--;
 	mutex_unlock(&dev->lock);
 	kref_put(&dev->ref, em28xx_free_device);
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 91bb624..0585217 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -523,6 +523,9 @@ struct em28xx_v4l2 {
 	int sensor_yres;
 	int sensor_xtal;
 
+	int users;		/* user count for exclusive use */
+	int streaming_users;    /* number of actively streaming users */
+
 	struct em28xx_fmt *format;
 	v4l2_std_id norm;	/* selected tv norm */
 
@@ -641,8 +644,6 @@ struct em28xx {
 	struct rt_mutex i2c_bus_lock;
 
 	/* video for linux */
-	int users;		/* user count for exclusive use */
-	int streaming_users;    /* Number of actively streaming users */
 	int ctl_freq;		/* selected frequency */
 	unsigned int ctl_input;	/* selected input */
 	unsigned int ctl_ainput;/* selected audio input */
-- 
1.8.4.5

[PATCH 16/19] em28xx: move tuner frequency field from struct em28xx to struct v4l2

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 16 +++++++++-------
 drivers/media/usb/em28xx/em28xx.h       |  3 ++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index aaab111..35bf2b9 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -963,7 +963,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 
 		/* Ask tuner to go to analog or radio mode */
 		memset(&f, 0, sizeof(f));
-		f.frequency = dev->ctl_freq;
+		f.frequency = v4l2->frequency;
 		if (vq->owner && vq->owner->vdev->vfl_type == VFL_TYPE_RADIO)
 			f.type = V4L2_TUNER_RADIO;
 		else
@@ -1597,11 +1597,12 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 {
 	struct em28xx_fh      *fh  = priv;
 	struct em28xx         *dev = fh->dev;
+	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 
 	if (0 != f->tuner)
 		return -EINVAL;
 
-	f->frequency = dev->ctl_freq;
+	f->frequency = v4l2->frequency;
 	return 0;
 }
 
@@ -1618,7 +1619,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f);
 	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq);
-	dev->ctl_freq = new_freq.frequency;
+	v4l2->frequency = new_freq.frequency;
 
 	return 0;
 }
@@ -2224,9 +2225,10 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 
 static void em28xx_tuner_setup(struct em28xx *dev)
 {
-	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
-	struct tuner_setup           tun_setup;
-	struct v4l2_frequency        f;
+	struct em28xx_v4l2      *v4l2 = dev->v4l2;
+	struct v4l2_device      *v4l2_dev = &v4l2->v4l2_dev;
+	struct tuner_setup      tun_setup;
+	struct v4l2_frequency   f;
 
 	if (dev->tuner_type == TUNER_ABSENT)
 		return;
@@ -2281,7 +2283,7 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 	f.tuner = 0;
 	f.type = V4L2_TUNER_ANALOG_TV;
 	f.frequency = 9076;     /* just a magic number */
-	dev->ctl_freq = f.frequency;
+	v4l2->frequency = f.frequency;
 	v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f);
 }
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 0585217..8a0ed93 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -526,6 +526,8 @@ struct em28xx_v4l2 {
 	int users;		/* user count for exclusive use */
 	int streaming_users;    /* number of actively streaming users */
 
+	u32 frequency;		/* selected tuner frequency */
+
 	struct em28xx_fmt *format;
 	v4l2_std_id norm;	/* selected tv norm */
 
@@ -644,7 +646,6 @@ struct em28xx {
 	struct rt_mutex i2c_bus_lock;
 
 	/* video for linux */
-	int ctl_freq;		/* selected frequency */
 	unsigned int ctl_input;	/* selected input */
 	unsigned int ctl_ainput;/* selected audio input */
 	unsigned int ctl_aoutput;/* selected audio output */
-- 
1.8.4.5

[PATCH 17/19] em28xx: remove field tda9887_conf from struct em28xx

[Frank Schäfer]

The tda9887 configuration is already stored in the board data, and it is used
only one time by the v4l2 sub-module at tuner setup.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 3 ---
 drivers/media/usb/em28xx/em28xx-video.c | 6 +++---
 drivers/media/usb/em28xx/em28xx.h       | 1 -
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 64ea25a..b81946f 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2719,9 +2719,6 @@ static void em28xx_card_setup(struct em28xx *dev)
 	if (em28xx_boards[dev->model].tuner_addr)
 		dev->tuner_addr = em28xx_boards[dev->model].tuner_addr;
 
-	if (em28xx_boards[dev->model].tda9887_conf)
-		dev->tda9887_conf = em28xx_boards[dev->model].tda9887_conf;
-
 	/* request some modules */
 	switch (dev->model) {
 	case EM2820_BOARD_HAUPPAUGE_WINTV_USB_2:
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 35bf2b9..8c0082c 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2254,11 +2254,11 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 				     0, tuner, s_type_addr, &tun_setup);
 	}
 
-	if (dev->tda9887_conf) {
+	if (dev->board.tda9887_conf) {
 		struct v4l2_priv_tun_config tda9887_cfg;
 
 		tda9887_cfg.tuner = TUNER_TDA9887;
-		tda9887_cfg.priv = &dev->tda9887_conf;
+		tda9887_cfg.priv = &dev->board.tda9887_conf;
 
 		v4l2_device_call_all(v4l2_dev,
 				     0, tuner, s_config, &tda9887_cfg);
@@ -2364,7 +2364,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* Initialize tuner and camera */
 
 	if (dev->board.tuner_type != TUNER_ABSENT) {
-		int has_demod = (dev->tda9887_conf & TDA9887_PRESENT);
+		int has_demod = (dev->board.tda9887_conf & TDA9887_PRESENT);
 
 		if (dev->board.radio.type)
 			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 8a0ed93..917cb25 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -633,7 +633,6 @@ struct em28xx {
 
 	int tuner_type;		/* type of the tuner */
 	int tuner_addr;		/* tuner address */
-	int tda9887_conf;
 
 	/* i2c i/o */
 	struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
-- 
1.8.4.5

[PATCH 18/19] em28xx: remove field tuner_addr from struct em28xx

[Frank Schäfer]

The tuner address is only used by the v4l submodule and at tuner setup and
can be obtained from the board data directly (if specified).

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |  2 --
 drivers/media/usb/em28xx/em28xx-video.c | 17 ++++++++---------
 drivers/media/usb/em28xx/em28xx.h       |  1 -
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index b81946f..e552375 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2716,8 +2716,6 @@ static void em28xx_card_setup(struct em28xx *dev)
 		    dev->board.name, dev->model);
 
 	dev->tuner_type = em28xx_boards[dev->model].tuner_type;
-	if (em28xx_boards[dev->model].tuner_addr)
-		dev->tuner_addr = em28xx_boards[dev->model].tuner_addr;
 
 	/* request some modules */
 	switch (dev->model) {
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 8c0082c..254a7ff 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2223,16 +2223,13 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 	return vfd;
 }
 
-static void em28xx_tuner_setup(struct em28xx *dev)
+static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr)
 {
 	struct em28xx_v4l2      *v4l2 = dev->v4l2;
 	struct v4l2_device      *v4l2_dev = &v4l2->v4l2_dev;
 	struct tuner_setup      tun_setup;
 	struct v4l2_frequency   f;
 
-	if (dev->tuner_type == TUNER_ABSENT)
-		return;
-
 	memset(&tun_setup, 0, sizeof(tun_setup));
 
 	tun_setup.mode_mask = T_ANALOG_TV | T_RADIO;
@@ -2248,7 +2245,7 @@ static void em28xx_tuner_setup(struct em28xx *dev)
 
 	if ((dev->tuner_type != TUNER_ABSENT) && (dev->tuner_type)) {
 		tun_setup.type   = dev->tuner_type;
-		tun_setup.addr   = dev->tuner_addr;
+		tun_setup.addr   = tuner_addr;
 
 		v4l2_device_call_all(v4l2_dev,
 				     0, tuner, s_type_addr, &tun_setup);
@@ -2364,6 +2361,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* Initialize tuner and camera */
 
 	if (dev->board.tuner_type != TUNER_ABSENT) {
+		unsigned short tuner_addr = dev->board.tuner_addr;
 		int has_demod = (dev->board.tda9887_conf & TDA9887_PRESENT);
 
 		if (dev->board.radio.type)
@@ -2375,7 +2373,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
 				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
 				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
-		if (dev->tuner_addr == 0) {
+		if (tuner_addr == 0) {
 			enum v4l2_i2c_tuner_type type =
 				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
 			struct v4l2_subdev *sd;
@@ -2385,15 +2383,16 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 				0, v4l2_i2c_tuner_addrs(type));
 
 			if (sd)
-				dev->tuner_addr = v4l2_i2c_subdev_addr(sd);
+				tuner_addr = v4l2_i2c_subdev_addr(sd);
 		} else {
 			v4l2_i2c_new_subdev(&v4l2->v4l2_dev,
 					    &dev->i2c_adap[dev->def_i2c_bus],
-					    "tuner", dev->tuner_addr, NULL);
+					    "tuner", tuner_addr, NULL);
 		}
+
+		em28xx_tuner_setup(dev, tuner_addr);
 	}
 
-	em28xx_tuner_setup(dev);
 	if (dev->em28xx_sensor != EM28XX_NOSENSOR)
 		em28xx_init_camera(dev);
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 917cb25..3a3fe16 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -632,7 +632,6 @@ struct em28xx {
 	struct em28xx_audio_mode audio_mode;
 
 	int tuner_type;		/* type of the tuner */
-	int tuner_addr;		/* tuner address */
 
 	/* i2c i/o */
 	struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
-- 
1.8.4.5

[PATCH 19/19] em28xx: move fields wq_trigger and streaming_started from struct em28xx to struct em28xx_audio

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-audio.c | 39 ++++++++++++++++++---------------
 drivers/media/usb/em28xx/em28xx.h       |  8 +++----
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
index c1937ea..e5ed5b9 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -92,7 +92,7 @@ static void em28xx_audio_isocirq(struct urb *urb)
 
 	if (dev->disconnected) {
 		dprintk("device disconnected while streaming. URB status=%d.\n", urb->status);
-		atomic_set(&dev->stream_started, 0);
+		atomic_set(&dev->adev.stream_started, 0);
 		return;
 	}
 
@@ -109,7 +109,7 @@ static void em28xx_audio_isocirq(struct urb *urb)
 		break;
 	}
 
-	if (atomic_read(&dev->stream_started) == 0)
+	if (atomic_read(&dev->adev.stream_started) == 0)
 		return;
 
 	if (dev->adev.capture_pcm_substream) {
@@ -185,7 +185,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev)
 			em28xx_errdev("submit of audio urb failed (error=%i)\n",
 				      errCode);
 			em28xx_deinit_isoc_audio(dev);
-			atomic_set(&dev->stream_started, 0);
+			atomic_set(&dev->adev.stream_started, 0);
 			return errCode;
 		}
 
@@ -332,9 +332,9 @@ static int snd_em28xx_pcm_close(struct snd_pcm_substream *substream)
 	dev->mute = 1;
 	mutex_lock(&dev->lock);
 	dev->adev.users--;
-	if (atomic_read(&dev->stream_started) > 0) {
-		atomic_set(&dev->stream_started, 0);
-		schedule_work(&dev->wq_trigger);
+	if (atomic_read(&dev->adev.stream_started) > 0) {
+		atomic_set(&dev->adev.stream_started, 0);
+		schedule_work(&dev->adev.wq_trigger);
 	}
 
 	em28xx_audio_analog_set(dev);
@@ -381,12 +381,13 @@ static int snd_em28xx_hw_capture_params(struct snd_pcm_substream *substream,
 static int snd_em28xx_hw_capture_free(struct snd_pcm_substream *substream)
 {
 	struct em28xx *dev = snd_pcm_substream_chip(substream);
+	struct em28xx_audio *adev = &dev->adev;
 
 	dprintk("Stop capture, if needed\n");
 
-	if (atomic_read(&dev->stream_started) > 0) {
-		atomic_set(&dev->stream_started, 0);
-		schedule_work(&dev->wq_trigger);
+	if (atomic_read(&adev->stream_started) > 0) {
+		atomic_set(&adev->stream_started, 0);
+		schedule_work(&adev->wq_trigger);
 	}
 
 	return 0;
@@ -407,9 +408,11 @@ static int snd_em28xx_prepare(struct snd_pcm_substream *substream)
 
 static void audio_trigger(struct work_struct *work)
 {
-	struct em28xx *dev = container_of(work, struct em28xx, wq_trigger);
+	struct em28xx_audio *adev =
+			    container_of(work, struct em28xx_audio, wq_trigger);
+	struct em28xx *dev = container_of(adev, struct em28xx, adev);
 
-	if (atomic_read(&dev->stream_started)) {
+	if (atomic_read(&adev->stream_started)) {
 		dprintk("starting capture");
 		em28xx_init_audio_isoc(dev);
 	} else {
@@ -431,17 +434,17 @@ static int snd_em28xx_capture_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: /* fall through */
 	case SNDRV_PCM_TRIGGER_RESUME: /* fall through */
 	case SNDRV_PCM_TRIGGER_START:
-		atomic_set(&dev->stream_started, 1);
+		atomic_set(&dev->adev.stream_started, 1);
 		break;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* fall through */
 	case SNDRV_PCM_TRIGGER_SUSPEND: /* fall through */
 	case SNDRV_PCM_TRIGGER_STOP:
-		atomic_set(&dev->stream_started, 0);
+		atomic_set(&dev->adev.stream_started, 0);
 		break;
 	default:
 		retval = -EINVAL;
 	}
-	schedule_work(&dev->wq_trigger);
+	schedule_work(&dev->adev.wq_trigger);
 	return retval;
 }
 
@@ -929,7 +932,7 @@ static int em28xx_audio_init(struct em28xx *dev)
 	strcpy(card->shortname, "Em28xx Audio");
 	strcpy(card->longname, "Empia Em28xx Audio");
 
-	INIT_WORK(&dev->wq_trigger, audio_trigger);
+	INIT_WORK(&adev->wq_trigger, audio_trigger);
 
 	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
 		em28xx_cvol_new(card, dev, "Video", AC97_VIDEO);
@@ -984,7 +987,7 @@ static int em28xx_audio_fini(struct em28xx *dev)
 
 	if (dev->adev.sndcard) {
 		snd_card_disconnect(dev->adev.sndcard);
-		flush_work(&dev->wq_trigger);
+		flush_work(&dev->adev.wq_trigger);
 
 		em28xx_audio_free_urb(dev);
 
@@ -1006,7 +1009,7 @@ static int em28xx_audio_suspend(struct em28xx *dev)
 
 	em28xx_info("Suspending audio extension");
 	em28xx_deinit_isoc_audio(dev);
-	atomic_set(&dev->stream_started, 0);
+	atomic_set(&dev->adev.stream_started, 0);
 	return 0;
 }
 
@@ -1020,7 +1023,7 @@ static int em28xx_audio_resume(struct em28xx *dev)
 
 	em28xx_info("Resuming audio extension");
 	/* Nothing to do other than schedule_work() ?? */
-	schedule_work(&dev->wq_trigger);
+	schedule_work(&dev->adev.wq_trigger);
 	return 0;
 }
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 3a3fe16..c1102ba 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -567,6 +567,10 @@ struct em28xx_audio {
 
 	int users;
 	spinlock_t slock;
+
+	/* Controls streaming */
+	struct work_struct wq_trigger;	/* trigger to start/stop audio */
+	atomic_t       stream_started;	/* stream should be running if true */
 };
 
 struct em28xx;
@@ -618,10 +622,6 @@ struct em28xx {
 
 	enum em28xx_sensor em28xx_sensor;	/* camera specific */
 
-	/* Controls audio streaming */
-	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
-	atomic_t       stream_started;	/* stream should be running if true */
-
 	/* Some older em28xx chips needs a waiting time after writing */
 	unsigned int wait_after_write;
 
-- 
1.8.4.5

Re: [PATCH 00/19] em28xx: clean up the main device struct and move sub-module specific data to its own data structs

[Hans Verkuil]

Hi Frank,

This looks good to me. I do have some comments for future cleanups and I'll
reply to the relevant patches for that.

However, before I can apply this patch series you need to take a look at my comments
for this pre-requisite patch:

https://patchwork.linuxtv.org/patch/23179/

That needs to be sorted before I can apply this series.

Regards,

	Hans

On 03/24/2014 08:33 PM, Frank Schäfer wrote:
> This patch series cleans up the main device struct of the em28xx driver.
> 
> Most of the patches (patches 3-16) are about moving the em28xx-v4l specific data
> to it's own dynamically allocated data structure.
> Patch 19 moves two em28xx-alsa specific fields to the em28xx_audio struct.
> Patches 17 and 18 remove two fields which aren't needed.
> 
> 
> Frank Schäfer (19):
>   em28xx: move sub-module data structs to a common place in the main
>     struct
>   em28xx-video: simplify usage of the pointer to struct
>     v4l2_ctrl_handler in em28xx_v4l2_init()
>   em28xx: start moving em28xx-v4l specific data to its own struct
>   em28xx: move struct v4l2_ctrl_handler ctrl_handler from struct em28xx
>     to struct v4l2
>   em28xx: move struct v4l2_clk *clk from struct em28xx to struct v4l2
>   em28xx: move video_device structs from struct em28xx to struct v4l2
>   em28xx: move videobuf2 related data from struct em28xx to struct v4l2
>   em28xx: move v4l2 frame resolutions and scale data from struct em28xx
>     to struct v4l2
>   em28xx: move vinmode and vinctrl data from struct em28xx to struct
>     v4l2
>   em28xx: move TV norm from struct em28xx to struct v4l2
>   em28xx: move struct em28xx_fmt *format from struct em28xx to struct
>     v4l2
>   em28xx: move progressive/interlaced fields from struct em28xx to
>     struct v4l2
>   em28xx: move sensor parameter fields from struct em28xx to struct v4l2
>   em28xx: move capture state tracking fields from struct em28xx to
>     struct v4l2
>   em28xx: move v4l2 user counting fields from struct em28xx to struct
>     v4l2
>   em28xx: move tuner frequency field from struct em28xx to struct v4l2
>   em28xx: remove field tda9887_conf from struct em28xx
>   em28xx: remove field tuner_addr from struct em28xx
>   em28xx: move fields wq_trigger and streaming_started from struct
>     em28xx to struct em28xx_audio
> 
>  drivers/media/usb/em28xx/em28xx-audio.c  |  39 +-
>  drivers/media/usb/em28xx/em28xx-camera.c |  51 +--
>  drivers/media/usb/em28xx/em28xx-cards.c  |   9 -
>  drivers/media/usb/em28xx/em28xx-vbi.c    |  10 +-
>  drivers/media/usb/em28xx/em28xx-video.c  | 592 +++++++++++++++++--------------
>  drivers/media/usb/em28xx/em28xx.h        | 120 ++++---
>  6 files changed, 452 insertions(+), 369 deletions(-)
> 

Re: [PATCH 03/19] em28xx: start moving em28xx-v4l specific data to its own struct

[Hans Verkuil]

Some comments for future improvements:

On 03/24/2014 08:33 PM, Frank Schäfer wrote:
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-camera.c |   4 +-
>  drivers/media/usb/em28xx/em28xx-video.c  | 160 +++++++++++++++++++++----------
>  drivers/media/usb/em28xx/em28xx.h        |   8 +-
>  3 files changed, 116 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
> index 505e050..daebef3 100644
> --- a/drivers/media/usb/em28xx/em28xx-camera.c
> +++ b/drivers/media/usb/em28xx/em28xx-camera.c
> @@ -365,7 +365,7 @@ int em28xx_init_camera(struct em28xx *dev)
>  		dev->sensor_xtal = 4300000;
>  		pdata.xtal = dev->sensor_xtal;
>  		if (NULL ==
> -		    v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
> +		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>  					      &mt9v011_info, NULL)) {
>  			ret = -ENODEV;
>  			break;
> @@ -422,7 +422,7 @@ int em28xx_init_camera(struct em28xx *dev)
>  		dev->sensor_yres = 480;
>  
>  		subdev =
> -		     v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
> +		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>  					       &ov2640_info, NULL);
>  		if (NULL == subdev) {
>  			ret = -ENODEV;
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 45ad471..89947db 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -189,10 +189,11 @@ static int em28xx_vbi_supported(struct em28xx *dev)
>   */
>  static void em28xx_wake_i2c(struct em28xx *dev)
>  {
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
> +	v4l2_device_call_all(v4l2_dev, 0, core,  reset, 0);
> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>  			INPUT(dev->ctl_input)->vmux, 0, 0);
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> +	v4l2_device_call_all(v4l2_dev, 0, video, s_stream, 0);
>  }
>  
>  static int em28xx_colorlevels_set_default(struct em28xx *dev)
> @@ -952,7 +953,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>  			f.type = V4L2_TUNER_RADIO;
>  		else
>  			f.type = V4L2_TUNER_ANALOG_TV;
> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
> +		v4l2_device_call_all(&dev->v4l2->v4l2_dev,
> +				     0, tuner, s_frequency, &f);
>  	}
>  
>  	dev->streaming_users++;
> @@ -1083,6 +1085,7 @@ static int em28xx_vb2_setup(struct em28xx *dev)
>  
>  static void video_mux(struct em28xx *dev, int index)
>  {
> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
>  	dev->ctl_input = index;
>  	dev->ctl_ainput = INPUT(index)->amux;
>  	dev->ctl_aoutput = INPUT(index)->aout;
> @@ -1090,21 +1093,21 @@ static void video_mux(struct em28xx *dev, int index)
>  	if (!dev->ctl_aoutput)
>  		dev->ctl_aoutput = EM28XX_AOUT_MASTER;
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>  			INPUT(index)->vmux, 0, 0);
>  
>  	if (dev->board.has_msp34xx) {
>  		if (dev->i2s_speed) {
> -			v4l2_device_call_all(&dev->v4l2_dev, 0, audio,
> +			v4l2_device_call_all(v4l2_dev, 0, audio,
>  				s_i2s_clock_freq, dev->i2s_speed);
>  		}
>  		/* Note: this is msp3400 specific */
> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>  			 dev->ctl_ainput, MSP_OUTPUT(MSP_SC_IN_DSP_SCART1), 0);
>  	}
>  
>  	if (dev->board.adecoder != EM28XX_NOADECODER) {
> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>  			dev->ctl_ainput, dev->ctl_aoutput, 0);
>  	}
>  
> @@ -1344,7 +1347,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
>  	struct em28xx_fh   *fh  = priv;
>  	struct em28xx      *dev = fh->dev;
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm);
>  
>  	return 0;
>  }
> @@ -1374,7 +1377,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
>  	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
>  
>  	em28xx_resolution_set(dev);
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, core, s_std, dev->norm);
>  
>  	return 0;
>  }
> @@ -1388,7 +1391,7 @@ static int vidioc_g_parm(struct file *file, void *priv,
>  
>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
>  	if (dev->board.is_webcam)
> -		rc = v4l2_device_call_until_err(&dev->v4l2_dev, 0,
> +		rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
>  						video, g_parm, p);
>  	else
>  		v4l2_video_std_frame_period(dev->norm,
> @@ -1404,7 +1407,8 @@ static int vidioc_s_parm(struct file *file, void *priv,
>  	struct em28xx      *dev = fh->dev;
>  
>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
> -	return v4l2_device_call_until_err(&dev->v4l2_dev, 0, video, s_parm, p);
> +	return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,
> +					  0, video, s_parm, p);
>  }
>  
>  static const char *iname[] = {
> @@ -1543,7 +1547,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
>  
>  	strcpy(t->name, "Tuner");
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>  	return 0;
>  }
>  
> @@ -1556,7 +1560,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>  	if (0 != t->index)
>  		return -EINVAL;
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>  	return 0;
>  }
>  
> @@ -1576,15 +1580,16 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>  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 v4l2_frequency  new_freq = *f;
> +	struct em28xx_fh          *fh   = priv;
> +	struct em28xx             *dev  = fh->dev;
> +	struct em28xx_v4l2        *v4l2 = dev->v4l2;
>  
>  	if (0 != f->tuner)
>  		return -EINVAL;
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, f);
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, &new_freq);
> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f);
> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq);
>  	dev->ctl_freq = new_freq.frequency;
>  
>  	return 0;
> @@ -1602,7 +1607,8 @@ static int vidioc_g_chip_info(struct file *file, void *priv,
>  	if (chip->match.addr == 1)
>  		strlcpy(chip->name, "ac97", sizeof(chip->name));
>  	else
> -		strlcpy(chip->name, dev->v4l2_dev.name, sizeof(chip->name));
> +		strlcpy(chip->name,
> +			dev->v4l2->v4l2_dev.name, sizeof(chip->name));
>  	return 0;
>  }
>  
> @@ -1814,7 +1820,7 @@ static int radio_g_tuner(struct file *file, void *priv,
>  
>  	strcpy(t->name, "Radio");
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>  
>  	return 0;
>  }
> @@ -1827,12 +1833,26 @@ static int radio_s_tuner(struct file *file, void *priv,
>  	if (0 != t->index)
>  		return -EINVAL;
>  
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>  
>  	return 0;
>  }
>  
>  /*
> + * em28xx_free_v4l2() - Free struct em28xx_v4l2
> + *
> + * @ref: struct kref for struct em28xx_v4l2
> + *
> + * Called when all users of struct em28xx_v4l2 are gone
> + */
> +void em28xx_free_v4l2(struct kref *ref)
> +{
> +	struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref);
> +
> +	kfree(v4l2);
> +}
> +
> +/*
>   * em28xx_v4l2_open()
>   * inits the device and starts isoc transfer
>   */
> @@ -1840,6 +1860,7 @@ static int em28xx_v4l2_open(struct file *filp)
>  {
>  	struct video_device *vdev = video_devdata(filp);
>  	struct em28xx *dev = video_drvdata(filp);
> +	struct em28xx_v4l2 *v4l2 = dev->v4l2;
>  	enum v4l2_buf_type fh_type = 0;
>  	struct em28xx_fh *fh;
>  
> @@ -1888,10 +1909,11 @@ static int em28xx_v4l2_open(struct file *filp)
>  
>  	if (vdev->vfl_type == VFL_TYPE_RADIO) {
>  		em28xx_videodbg("video_open: setting radio device\n");
> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_radio);
> +		v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio);
>  	}
>  
>  	kref_get(&dev->ref);
> +	kref_get(&v4l2->ref);

I never like these kref things. Especially for usb devices I strongly recommend
using the release() callback from v4l2_device instead: this callback will only
be called once all references to video_device nodes have been closed. In other
words, only once all filehandles to /dev/videoX (and radio, vbi etc) are closed
will the release callback be called.

As such it is a perfect place to put the final cleanup, and there is no more need
to mess around with krefs.


>  	dev->users++;

The same for these user counters. You can use v4l2_fh_is_singular_file() to check
if the file open is the first file. However, this function assumes that v4l2_fh_add
has been called first.

So for this driver it might be easier if we add a v4l2_fh_is_empty() to v4l2-fh.c
so we can call this before v4l2_fh_add.

For that matter, you can almost certainly remove struct em28xx_fh altogether.
The type field of that struct can be determined by vdev->vfl_type and 'dev' can be
obtained via video_get_drvdata().

Regards,

	Hans

>  
>  	mutex_unlock(&dev->lock);

Re: [PATCH 06/19] em28xx: move video_device structs from struct em28xx to struct v4l2

[Hans Verkuil]

On 03/24/2014 08:33 PM, Frank Schäfer wrote:
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-video.c | 120 ++++++++++++++------------------
>  drivers/media/usb/em28xx/em28xx.h       |   7 +-
>  2 files changed, 56 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 4fb0053..7252eef 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -1447,7 +1447,7 @@ static int vidioc_enum_input(struct file *file, void *priv,
>  		(EM28XX_VMUX_CABLE == INPUT(n)->type))
>  		i->type = V4L2_INPUT_TYPE_TUNER;
>  
> -	i->std = dev->vdev->tvnorms;
> +	i->std = dev->v4l2->vdev->tvnorms;
>  	/* webcams do not have the STD API */
>  	if (dev->board.is_webcam)
>  		i->capabilities = 0;
> @@ -1691,9 +1691,10 @@ static int vidioc_s_register(struct file *file, void *priv,
>  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 video_device   *vdev = video_devdata(file);
> +	struct em28xx_fh      *fh   = priv;
> +	struct em28xx         *dev  = fh->dev;
> +	struct em28xx_v4l2    *v4l2 = dev->v4l2;
>  
>  	strlcpy(cap->driver, "em28xx", sizeof(cap->driver));
>  	strlcpy(cap->card, em28xx_boards[dev->model].name, sizeof(cap->card));
> @@ -1715,9 +1716,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>  	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
>  		V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> -	if (dev->vbi_dev)
> +	if (v4l2->vbi_dev)
>  		cap->capabilities |= V4L2_CAP_VBI_CAPTURE;
> -	if (dev->radio_dev)
> +	if (v4l2->radio_dev)
>  		cap->capabilities |= V4L2_CAP_RADIO;
>  	return 0;
>  }
> @@ -1955,20 +1956,20 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
>  
>  	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
>  
> -	if (dev->radio_dev) {
> +	if (v4l2->radio_dev) {
>  		em28xx_info("V4L2 device %s deregistered\n",
> -			    video_device_node_name(dev->radio_dev));
> -		video_unregister_device(dev->radio_dev);
> +			    video_device_node_name(v4l2->radio_dev));
> +		video_unregister_device(v4l2->radio_dev);
>  	}
> -	if (dev->vbi_dev) {
> +	if (v4l2->vbi_dev) {
>  		em28xx_info("V4L2 device %s deregistered\n",
> -			    video_device_node_name(dev->vbi_dev));
> -		video_unregister_device(dev->vbi_dev);
> +			    video_device_node_name(v4l2->vbi_dev));
> +		video_unregister_device(v4l2->vbi_dev);
>  	}
> -	if (dev->vdev) {
> +	if (v4l2->vdev) {
>  		em28xx_info("V4L2 device %s deregistered\n",
> -			    video_device_node_name(dev->vdev));
> -		video_unregister_device(dev->vdev);
> +			    video_device_node_name(v4l2->vdev));
> +		video_unregister_device(v4l2->vdev);
>  	}
>  
>  	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
> @@ -2061,23 +2062,6 @@ exit:
>  	return 0;
>  }
>  
> -/*
> - * em28xx_videodevice_release()
> - * called when the last user of the video device exits and frees the memeory
> - */
> -static void em28xx_videodevice_release(struct video_device *vdev)
> -{
> -	struct em28xx *dev = video_get_drvdata(vdev);
> -
> -	video_device_release(vdev);
> -	if (vdev == dev->vdev)
> -		dev->vdev = NULL;
> -	else if (vdev == dev->vbi_dev)
> -		dev->vbi_dev = NULL;
> -	else if (vdev == dev->radio_dev)
> -		dev->radio_dev = NULL;
> -}
> -
>  static const struct v4l2_file_operations em28xx_v4l_fops = {
>  	.owner         = THIS_MODULE,
>  	.open          = em28xx_v4l2_open,
> @@ -2134,7 +2118,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>  static const struct video_device em28xx_video_template = {
>  	.fops		= &em28xx_v4l_fops,
>  	.ioctl_ops	= &video_ioctl_ops,
> -	.release	= em28xx_videodevice_release,
> +	.release	= video_device_release,
>  	.tvnorms	= V4L2_STD_ALL,
>  };
>  
> @@ -2163,7 +2147,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
>  static struct video_device em28xx_radio_template = {
>  	.fops		= &radio_fops,
>  	.ioctl_ops	= &radio_ioctl_ops,
> -	.release	= em28xx_videodevice_release,
> +	.release	= video_device_release,
>  };
>  
>  /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
> @@ -2493,36 +2477,36 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>  		goto unregister_dev;
>  
>  	/* allocate and fill video video_device struct */
> -	dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
> -	if (!dev->vdev) {
> +	v4l2->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
> +	if (!v4l2->vdev) {
>  		em28xx_errdev("cannot allocate video_device.\n");
>  		ret = -ENODEV;
>  		goto unregister_dev;
>  	}
> -	dev->vdev->queue = &dev->vb_vidq;
> -	dev->vdev->queue->lock = &dev->vb_queue_lock;
> +	v4l2->vdev->queue = &dev->vb_vidq;
> +	v4l2->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);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD);
>  	} else {
> -		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM);
>  	}
>  	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);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY);
> +		v4l2_disable_ioctl(v4l2->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);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO);
> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO);
>  	}
>  
>  	/* register v4l2 video video_device */
> -	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
> +	ret = video_register_device(v4l2->vdev, VFL_TYPE_GRABBER,
>  				       video_nr[dev->devno]);
>  	if (ret) {
>  		em28xx_errdev("unable to register video device (error=%i).\n",
> @@ -2532,27 +2516,27 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>  
>  	/* Allocate and fill vbi video_device struct */
>  	if (em28xx_vbi_supported(dev) == 1) {
> -		dev->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
> +		v4l2->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
>  						"vbi");
>  
> -		dev->vbi_dev->queue = &dev->vb_vbiq;
> -		dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
> +		v4l2->vbi_dev->queue = &dev->vb_vbiq;
> +		v4l2->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
>  
>  		/* disable inapplicable ioctls */
> -		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
> +		v4l2_disable_ioctl(v4l2->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);
> -			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_FREQUENCY);
> -			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_FREQUENCY);
> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_TUNER);
> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_TUNER);
> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_FREQUENCY);
> +			v4l2_disable_ioctl(v4l2->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);
> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_AUDIO);
> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_AUDIO);
>  		}
>  
>  		/* register v4l2 vbi video_device */
> -		ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
> +		ret = video_register_device(v4l2->vbi_dev, VFL_TYPE_VBI,
>  					    vbi_nr[dev->devno]);
>  		if (ret < 0) {
>  			em28xx_errdev("unable to register vbi device\n");
> @@ -2561,29 +2545,29 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>  	}
>  
>  	if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) {
> -		dev->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
> -						  "radio");
> -		if (!dev->radio_dev) {
> +		v4l2->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
> +						   "radio");
> +		if (!v4l2->radio_dev) {
>  			em28xx_errdev("cannot allocate video_device.\n");
>  			ret = -ENODEV;
>  			goto unregister_dev;
>  		}
> -		ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO,
> +		ret = video_register_device(v4l2->radio_dev, VFL_TYPE_RADIO,
>  					    radio_nr[dev->devno]);
>  		if (ret < 0) {
>  			em28xx_errdev("can't register radio device\n");
>  			goto unregister_dev;
>  		}
>  		em28xx_info("Registered radio device as %s\n",
> -			    video_device_node_name(dev->radio_dev));
> +			    video_device_node_name(v4l2->radio_dev));
>  	}
>  
>  	em28xx_info("V4L2 video device registered as %s\n",
> -		    video_device_node_name(dev->vdev));
> +		    video_device_node_name(v4l2->vdev));
>  
> -	if (dev->vbi_dev)
> +	if (v4l2->vbi_dev)
>  		em28xx_info("V4L2 VBI device registered as %s\n",
> -			    video_device_node_name(dev->vbi_dev));
> +			    video_device_node_name(v4l2->vbi_dev));
>  
>  	/* Save some power by putting tuner to sleep */
>  	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_power, 0);
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index a4d26bf..88d0589 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -504,6 +504,10 @@ struct em28xx_v4l2 {
>  	struct v4l2_device v4l2_dev;
>  	struct v4l2_ctrl_handler ctrl_handler;
>  	struct v4l2_clk *clk;
> +
> +	struct video_device *vdev;
> +	struct video_device *vbi_dev;
> +	struct video_device *radio_dev;

Think about embedding these structs. That way you don't have to allocate them which
removes the complexity of checking for ENOMEM errors.

Regards,

	Hans

>  };
>  
>  struct em28xx_audio {
> @@ -614,7 +618,6 @@ struct em28xx {
>  	/* video for linux */
>  	int users;		/* user count for exclusive use */
>  	int streaming_users;    /* Number of actively streaming users */
> -	struct video_device *vdev;	/* video for linux device struct */
>  	v4l2_std_id norm;	/* selected tv norm */
>  	int ctl_freq;		/* selected frequency */
>  	unsigned int ctl_input;	/* selected input */
> @@ -645,8 +648,6 @@ struct em28xx {
>  	/* locks */
>  	struct mutex lock;
>  	struct mutex ctrl_urb_lock;	/* protects urb_buf */
> -	struct video_device *vbi_dev;
> -	struct video_device *radio_dev;
>  
>  	/* Videobuf2 */
>  	struct vb2_queue vb_vidq;
> 

Re: [PATCH 03/19] em28xx: start moving em28xx-v4l specific data to its own struct

[Frank Schäfer]

Am 09.05.2014 11:17, schrieb Hans Verkuil:
> Some comments for future improvements:
>
> On 03/24/2014 08:33 PM, Frank Schäfer wrote:
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-camera.c |   4 +-
>>  drivers/media/usb/em28xx/em28xx-video.c  | 160 +++++++++++++++++++++----------
>>  drivers/media/usb/em28xx/em28xx.h        |   8 +-
>>  3 files changed, 116 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
>> index 505e050..daebef3 100644
>> --- a/drivers/media/usb/em28xx/em28xx-camera.c
>> +++ b/drivers/media/usb/em28xx/em28xx-camera.c
>> @@ -365,7 +365,7 @@ int em28xx_init_camera(struct em28xx *dev)
>>  		dev->sensor_xtal = 4300000;
>>  		pdata.xtal = dev->sensor_xtal;
>>  		if (NULL ==
>> -		    v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
>> +		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>>  					      &mt9v011_info, NULL)) {
>>  			ret = -ENODEV;
>>  			break;
>> @@ -422,7 +422,7 @@ int em28xx_init_camera(struct em28xx *dev)
>>  		dev->sensor_yres = 480;
>>  
>>  		subdev =
>> -		     v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
>> +		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>>  					       &ov2640_info, NULL);
>>  		if (NULL == subdev) {
>>  			ret = -ENODEV;
>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
>> index 45ad471..89947db 100644
>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>> @@ -189,10 +189,11 @@ static int em28xx_vbi_supported(struct em28xx *dev)
>>   */
>>  static void em28xx_wake_i2c(struct em28xx *dev)
>>  {
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
>> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
>> +	v4l2_device_call_all(v4l2_dev, 0, core,  reset, 0);
>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>>  			INPUT(dev->ctl_input)->vmux, 0, 0);
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_stream, 0);
>>  }
>>  
>>  static int em28xx_colorlevels_set_default(struct em28xx *dev)
>> @@ -952,7 +953,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>>  			f.type = V4L2_TUNER_RADIO;
>>  		else
>>  			f.type = V4L2_TUNER_ANALOG_TV;
>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
>> +		v4l2_device_call_all(&dev->v4l2->v4l2_dev,
>> +				     0, tuner, s_frequency, &f);
>>  	}
>>  
>>  	dev->streaming_users++;
>> @@ -1083,6 +1085,7 @@ static int em28xx_vb2_setup(struct em28xx *dev)
>>  
>>  static void video_mux(struct em28xx *dev, int index)
>>  {
>> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
>>  	dev->ctl_input = index;
>>  	dev->ctl_ainput = INPUT(index)->amux;
>>  	dev->ctl_aoutput = INPUT(index)->aout;
>> @@ -1090,21 +1093,21 @@ static void video_mux(struct em28xx *dev, int index)
>>  	if (!dev->ctl_aoutput)
>>  		dev->ctl_aoutput = EM28XX_AOUT_MASTER;
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>>  			INPUT(index)->vmux, 0, 0);
>>  
>>  	if (dev->board.has_msp34xx) {
>>  		if (dev->i2s_speed) {
>> -			v4l2_device_call_all(&dev->v4l2_dev, 0, audio,
>> +			v4l2_device_call_all(v4l2_dev, 0, audio,
>>  				s_i2s_clock_freq, dev->i2s_speed);
>>  		}
>>  		/* Note: this is msp3400 specific */
>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
>> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>>  			 dev->ctl_ainput, MSP_OUTPUT(MSP_SC_IN_DSP_SCART1), 0);
>>  	}
>>  
>>  	if (dev->board.adecoder != EM28XX_NOADECODER) {
>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
>> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>>  			dev->ctl_ainput, dev->ctl_aoutput, 0);
>>  	}
>>  
>> @@ -1344,7 +1347,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
>>  	struct em28xx_fh   *fh  = priv;
>>  	struct em28xx      *dev = fh->dev;
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm);
>>  
>>  	return 0;
>>  }
>> @@ -1374,7 +1377,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
>>  	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
>>  
>>  	em28xx_resolution_set(dev);
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, core, s_std, dev->norm);
>>  
>>  	return 0;
>>  }
>> @@ -1388,7 +1391,7 @@ static int vidioc_g_parm(struct file *file, void *priv,
>>  
>>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
>>  	if (dev->board.is_webcam)
>> -		rc = v4l2_device_call_until_err(&dev->v4l2_dev, 0,
>> +		rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
>>  						video, g_parm, p);
>>  	else
>>  		v4l2_video_std_frame_period(dev->norm,
>> @@ -1404,7 +1407,8 @@ static int vidioc_s_parm(struct file *file, void *priv,
>>  	struct em28xx      *dev = fh->dev;
>>  
>>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
>> -	return v4l2_device_call_until_err(&dev->v4l2_dev, 0, video, s_parm, p);
>> +	return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,
>> +					  0, video, s_parm, p);
>>  }
>>  
>>  static const char *iname[] = {
>> @@ -1543,7 +1547,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
>>  
>>  	strcpy(t->name, "Tuner");
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>>  	return 0;
>>  }
>>  
>> @@ -1556,7 +1560,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>>  	if (0 != t->index)
>>  		return -EINVAL;
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>>  	return 0;
>>  }
>>  
>> @@ -1576,15 +1580,16 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>>  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 v4l2_frequency  new_freq = *f;
>> +	struct em28xx_fh          *fh   = priv;
>> +	struct em28xx             *dev  = fh->dev;
>> +	struct em28xx_v4l2        *v4l2 = dev->v4l2;
>>  
>>  	if (0 != f->tuner)
>>  		return -EINVAL;
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, f);
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, &new_freq);
>> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f);
>> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq);
>>  	dev->ctl_freq = new_freq.frequency;
>>  
>>  	return 0;
>> @@ -1602,7 +1607,8 @@ static int vidioc_g_chip_info(struct file *file, void *priv,
>>  	if (chip->match.addr == 1)
>>  		strlcpy(chip->name, "ac97", sizeof(chip->name));
>>  	else
>> -		strlcpy(chip->name, dev->v4l2_dev.name, sizeof(chip->name));
>> +		strlcpy(chip->name,
>> +			dev->v4l2->v4l2_dev.name, sizeof(chip->name));
>>  	return 0;
>>  }
>>  
>> @@ -1814,7 +1820,7 @@ static int radio_g_tuner(struct file *file, void *priv,
>>  
>>  	strcpy(t->name, "Radio");
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>>  
>>  	return 0;
>>  }
>> @@ -1827,12 +1833,26 @@ static int radio_s_tuner(struct file *file, void *priv,
>>  	if (0 != t->index)
>>  		return -EINVAL;
>>  
>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>>  
>>  	return 0;
>>  }
>>  
>>  /*
>> + * em28xx_free_v4l2() - Free struct em28xx_v4l2
>> + *
>> + * @ref: struct kref for struct em28xx_v4l2
>> + *
>> + * Called when all users of struct em28xx_v4l2 are gone
>> + */
>> +void em28xx_free_v4l2(struct kref *ref)
>> +{
>> +	struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref);
>> +
>> +	kfree(v4l2);
>> +}
>> +
>> +/*
>>   * em28xx_v4l2_open()
>>   * inits the device and starts isoc transfer
>>   */
>> @@ -1840,6 +1860,7 @@ static int em28xx_v4l2_open(struct file *filp)
>>  {
>>  	struct video_device *vdev = video_devdata(filp);
>>  	struct em28xx *dev = video_drvdata(filp);
>> +	struct em28xx_v4l2 *v4l2 = dev->v4l2;
>>  	enum v4l2_buf_type fh_type = 0;
>>  	struct em28xx_fh *fh;
>>  
>> @@ -1888,10 +1909,11 @@ static int em28xx_v4l2_open(struct file *filp)
>>  
>>  	if (vdev->vfl_type == VFL_TYPE_RADIO) {
>>  		em28xx_videodbg("video_open: setting radio device\n");
>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_radio);
>> +		v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio);
>>  	}
>>  
>>  	kref_get(&dev->ref);
>> +	kref_get(&v4l2->ref);
> I never like these kref things. Especially for usb devices I strongly recommend
> using the release() callback from v4l2_device instead: this callback will only
> be called once all references to video_device nodes have been closed. In other
> words, only once all filehandles to /dev/videoX (and radio, vbi etc) are closed
> will the release callback be called.
>
> As such it is a perfect place to put the final cleanup, and there is no more need
> to mess around with krefs.

The v4l2 submodule data struct can not be cleared before 1) the
submodule is unloaded/unregistered AND 2) all users of all device nodes
are gone.
Using a kref is much easier (and also safer) than dealing with
non-trivial case checks in em28xx_v4l2_fini() and the v4l2_device
release() callbacks.

What we could do is to call kref_get() only one time at the first open()
of a device node and kref_put() only at the last close().
But it seems that this would just complicate the code without any real
benefit.


>>  	dev->users++;
> The same for these user counters. You can use v4l2_fh_is_singular_file() to check
> if the file open is the first file. However, this function assumes that v4l2_fh_add
> has been called first.
>
> So for this driver it might be easier if we add a v4l2_fh_is_empty() to v4l2-fh.c
> so we can call this before v4l2_fh_add.
>
> For that matter, you can almost certainly remove struct em28xx_fh altogether.
> The type field of that struct can be determined by vdev->vfl_type and 'dev' can be
> obtained via video_get_drvdata().

Yes, fields "dev" and "type" can definitly be removed from struct em28xx_fh.
Then struct v4l2_fh fh is the last member, but I didn't have the time
yet to take a deeper look at it.
At a first glance its usage seems to be incomplete/broken.
There are no v4l2_fh_del() and v4l2_fh_exit() calls and I wonder who
deallocates the structs memory !?

Regards,
Frank

>
> Regards,
>
> 	Hans
>
>>  
>>  	mutex_unlock(&dev->lock);

Re: [PATCH 06/19] em28xx: move video_device structs from struct em28xx to struct v4l2

[Frank Schäfer]

Am 09.05.2014 11:19, schrieb Hans Verkuil:
> On 03/24/2014 08:33 PM, Frank Schäfer wrote:
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-video.c | 120 ++++++++++++++------------------
>>  drivers/media/usb/em28xx/em28xx.h       |   7 +-
>>  2 files changed, 56 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
>> index 4fb0053..7252eef 100644
>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>> @@ -1447,7 +1447,7 @@ static int vidioc_enum_input(struct file *file, void *priv,
>>  		(EM28XX_VMUX_CABLE == INPUT(n)->type))
>>  		i->type = V4L2_INPUT_TYPE_TUNER;
>>  
>> -	i->std = dev->vdev->tvnorms;
>> +	i->std = dev->v4l2->vdev->tvnorms;
>>  	/* webcams do not have the STD API */
>>  	if (dev->board.is_webcam)
>>  		i->capabilities = 0;
>> @@ -1691,9 +1691,10 @@ static int vidioc_s_register(struct file *file, void *priv,
>>  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 video_device   *vdev = video_devdata(file);
>> +	struct em28xx_fh      *fh   = priv;
>> +	struct em28xx         *dev  = fh->dev;
>> +	struct em28xx_v4l2    *v4l2 = dev->v4l2;
>>  
>>  	strlcpy(cap->driver, "em28xx", sizeof(cap->driver));
>>  	strlcpy(cap->card, em28xx_boards[dev->model].name, sizeof(cap->card));
>> @@ -1715,9 +1716,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
>>  
>>  	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
>>  		V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
>> -	if (dev->vbi_dev)
>> +	if (v4l2->vbi_dev)
>>  		cap->capabilities |= V4L2_CAP_VBI_CAPTURE;
>> -	if (dev->radio_dev)
>> +	if (v4l2->radio_dev)
>>  		cap->capabilities |= V4L2_CAP_RADIO;
>>  	return 0;
>>  }
>> @@ -1955,20 +1956,20 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
>>  
>>  	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
>>  
>> -	if (dev->radio_dev) {
>> +	if (v4l2->radio_dev) {
>>  		em28xx_info("V4L2 device %s deregistered\n",
>> -			    video_device_node_name(dev->radio_dev));
>> -		video_unregister_device(dev->radio_dev);
>> +			    video_device_node_name(v4l2->radio_dev));
>> +		video_unregister_device(v4l2->radio_dev);
>>  	}
>> -	if (dev->vbi_dev) {
>> +	if (v4l2->vbi_dev) {
>>  		em28xx_info("V4L2 device %s deregistered\n",
>> -			    video_device_node_name(dev->vbi_dev));
>> -		video_unregister_device(dev->vbi_dev);
>> +			    video_device_node_name(v4l2->vbi_dev));
>> +		video_unregister_device(v4l2->vbi_dev);
>>  	}
>> -	if (dev->vdev) {
>> +	if (v4l2->vdev) {
>>  		em28xx_info("V4L2 device %s deregistered\n",
>> -			    video_device_node_name(dev->vdev));
>> -		video_unregister_device(dev->vdev);
>> +			    video_device_node_name(v4l2->vdev));
>> +		video_unregister_device(v4l2->vdev);
>>  	}
>>  
>>  	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
>> @@ -2061,23 +2062,6 @@ exit:
>>  	return 0;
>>  }
>>  
>> -/*
>> - * em28xx_videodevice_release()
>> - * called when the last user of the video device exits and frees the memeory
>> - */
>> -static void em28xx_videodevice_release(struct video_device *vdev)
>> -{
>> -	struct em28xx *dev = video_get_drvdata(vdev);
>> -
>> -	video_device_release(vdev);
>> -	if (vdev == dev->vdev)
>> -		dev->vdev = NULL;
>> -	else if (vdev == dev->vbi_dev)
>> -		dev->vbi_dev = NULL;
>> -	else if (vdev == dev->radio_dev)
>> -		dev->radio_dev = NULL;
>> -}
>> -
>>  static const struct v4l2_file_operations em28xx_v4l_fops = {
>>  	.owner         = THIS_MODULE,
>>  	.open          = em28xx_v4l2_open,
>> @@ -2134,7 +2118,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>>  static const struct video_device em28xx_video_template = {
>>  	.fops		= &em28xx_v4l_fops,
>>  	.ioctl_ops	= &video_ioctl_ops,
>> -	.release	= em28xx_videodevice_release,
>> +	.release	= video_device_release,
>>  	.tvnorms	= V4L2_STD_ALL,
>>  };
>>  
>> @@ -2163,7 +2147,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
>>  static struct video_device em28xx_radio_template = {
>>  	.fops		= &radio_fops,
>>  	.ioctl_ops	= &radio_ioctl_ops,
>> -	.release	= em28xx_videodevice_release,
>> +	.release	= video_device_release,
>>  };
>>  
>>  /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
>> @@ -2493,36 +2477,36 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>>  		goto unregister_dev;
>>  
>>  	/* allocate and fill video video_device struct */
>> -	dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
>> -	if (!dev->vdev) {
>> +	v4l2->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
>> +	if (!v4l2->vdev) {
>>  		em28xx_errdev("cannot allocate video_device.\n");
>>  		ret = -ENODEV;
>>  		goto unregister_dev;
>>  	}
>> -	dev->vdev->queue = &dev->vb_vidq;
>> -	dev->vdev->queue->lock = &dev->vb_queue_lock;
>> +	v4l2->vdev->queue = &dev->vb_vidq;
>> +	v4l2->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);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD);
>>  	} else {
>> -		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM);
>>  	}
>>  	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);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY);
>> +		v4l2_disable_ioctl(v4l2->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);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO);
>> +		v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO);
>>  	}
>>  
>>  	/* register v4l2 video video_device */
>> -	ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
>> +	ret = video_register_device(v4l2->vdev, VFL_TYPE_GRABBER,
>>  				       video_nr[dev->devno]);
>>  	if (ret) {
>>  		em28xx_errdev("unable to register video device (error=%i).\n",
>> @@ -2532,27 +2516,27 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>>  
>>  	/* Allocate and fill vbi video_device struct */
>>  	if (em28xx_vbi_supported(dev) == 1) {
>> -		dev->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
>> +		v4l2->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template,
>>  						"vbi");
>>  
>> -		dev->vbi_dev->queue = &dev->vb_vbiq;
>> -		dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
>> +		v4l2->vbi_dev->queue = &dev->vb_vbiq;
>> +		v4l2->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
>>  
>>  		/* disable inapplicable ioctls */
>> -		v4l2_disable_ioctl(dev->vdev, VIDIOC_S_PARM);
>> +		v4l2_disable_ioctl(v4l2->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);
>> -			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_G_FREQUENCY);
>> -			v4l2_disable_ioctl(dev->vbi_dev, VIDIOC_S_FREQUENCY);
>> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_TUNER);
>> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_TUNER);
>> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_FREQUENCY);
>> +			v4l2_disable_ioctl(v4l2->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);
>> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_G_AUDIO);
>> +			v4l2_disable_ioctl(v4l2->vbi_dev, VIDIOC_S_AUDIO);
>>  		}
>>  
>>  		/* register v4l2 vbi video_device */
>> -		ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
>> +		ret = video_register_device(v4l2->vbi_dev, VFL_TYPE_VBI,
>>  					    vbi_nr[dev->devno]);
>>  		if (ret < 0) {
>>  			em28xx_errdev("unable to register vbi device\n");
>> @@ -2561,29 +2545,29 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>>  	}
>>  
>>  	if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) {
>> -		dev->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
>> -						  "radio");
>> -		if (!dev->radio_dev) {
>> +		v4l2->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template,
>> +						   "radio");
>> +		if (!v4l2->radio_dev) {
>>  			em28xx_errdev("cannot allocate video_device.\n");
>>  			ret = -ENODEV;
>>  			goto unregister_dev;
>>  		}
>> -		ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO,
>> +		ret = video_register_device(v4l2->radio_dev, VFL_TYPE_RADIO,
>>  					    radio_nr[dev->devno]);
>>  		if (ret < 0) {
>>  			em28xx_errdev("can't register radio device\n");
>>  			goto unregister_dev;
>>  		}
>>  		em28xx_info("Registered radio device as %s\n",
>> -			    video_device_node_name(dev->radio_dev));
>> +			    video_device_node_name(v4l2->radio_dev));
>>  	}
>>  
>>  	em28xx_info("V4L2 video device registered as %s\n",
>> -		    video_device_node_name(dev->vdev));
>> +		    video_device_node_name(v4l2->vdev));
>>  
>> -	if (dev->vbi_dev)
>> +	if (v4l2->vbi_dev)
>>  		em28xx_info("V4L2 VBI device registered as %s\n",
>> -			    video_device_node_name(dev->vbi_dev));
>> +			    video_device_node_name(v4l2->vbi_dev));
>>  
>>  	/* Save some power by putting tuner to sleep */
>>  	v4l2_device_call_all(&v4l2->v4l2_dev, 0, core, s_power, 0);
>> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
>> index a4d26bf..88d0589 100644
>> --- a/drivers/media/usb/em28xx/em28xx.h
>> +++ b/drivers/media/usb/em28xx/em28xx.h
>> @@ -504,6 +504,10 @@ struct em28xx_v4l2 {
>>  	struct v4l2_device v4l2_dev;
>>  	struct v4l2_ctrl_handler ctrl_handler;
>>  	struct v4l2_clk *clk;
>> +
>> +	struct video_device *vdev;
>> +	struct video_device *vbi_dev;
>> +	struct video_device *radio_dev;
> Think about embedding these structs. That way you don't have to allocate them which
> removes the complexity of checking for ENOMEM errors.

Yes, but consider that only em286x and em288x devices provide VBI
support and we have even less devices with radio support (~ 3 of 100).
So with most devices, we would waste memory.

Regards,
Frank


>
> Regards,
>
> 	Hans
>
>>  };
>>  
>>  struct em28xx_audio {
>> @@ -614,7 +618,6 @@ struct em28xx {
>>  	/* video for linux */
>>  	int users;		/* user count for exclusive use */
>>  	int streaming_users;    /* Number of actively streaming users */
>> -	struct video_device *vdev;	/* video for linux device struct */
>>  	v4l2_std_id norm;	/* selected tv norm */
>>  	int ctl_freq;		/* selected frequency */
>>  	unsigned int ctl_input;	/* selected input */
>> @@ -645,8 +648,6 @@ struct em28xx {
>>  	/* locks */
>>  	struct mutex lock;
>>  	struct mutex ctrl_urb_lock;	/* protects urb_buf */
>> -	struct video_device *vbi_dev;
>> -	struct video_device *radio_dev;
>>  
>>  	/* Videobuf2 */
>>  	struct vb2_queue vb_vidq;
>>

Re: [PATCH 00/19] em28xx: clean up the main device struct and move sub-module specific data to its own data structs

[Frank Schäfer]

Hi Hans,

Am 09.05.2014 11:04, schrieb Hans Verkuil:
> Hi Frank,
>
> This looks good to me. I do have some comments for future cleanups and I'll
> reply to the relevant patches for that.
>
> However, before I can apply this patch series you need to take a look at my comments
> for this pre-requisite patch:
>
> https://patchwork.linuxtv.org/patch/23179/
>
> That needs to be sorted before I can apply this series.

Done:
https://patchwork.linuxtv.org/patch/23882/

I've also sent a rebased version of patch 15/19:
https://patchwork.linuxtv.org/patch/23883/

All other patches still apply.

Many thanks for reviewing the patches and your comments !

Regards,
Frank


>
> Regards,
>
> 	Hans
>
> On 03/24/2014 08:33 PM, Frank Schäfer wrote:
>> This patch series cleans up the main device struct of the em28xx driver.
>>
>> Most of the patches (patches 3-16) are about moving the em28xx-v4l specific data
>> to it's own dynamically allocated data structure.
>> Patch 19 moves two em28xx-alsa specific fields to the em28xx_audio struct.
>> Patches 17 and 18 remove two fields which aren't needed.
>>
>>
>> Frank Schäfer (19):
>>   em28xx: move sub-module data structs to a common place in the main
>>     struct
>>   em28xx-video: simplify usage of the pointer to struct
>>     v4l2_ctrl_handler in em28xx_v4l2_init()
>>   em28xx: start moving em28xx-v4l specific data to its own struct
>>   em28xx: move struct v4l2_ctrl_handler ctrl_handler from struct em28xx
>>     to struct v4l2
>>   em28xx: move struct v4l2_clk *clk from struct em28xx to struct v4l2
>>   em28xx: move video_device structs from struct em28xx to struct v4l2
>>   em28xx: move videobuf2 related data from struct em28xx to struct v4l2
>>   em28xx: move v4l2 frame resolutions and scale data from struct em28xx
>>     to struct v4l2
>>   em28xx: move vinmode and vinctrl data from struct em28xx to struct
>>     v4l2
>>   em28xx: move TV norm from struct em28xx to struct v4l2
>>   em28xx: move struct em28xx_fmt *format from struct em28xx to struct
>>     v4l2
>>   em28xx: move progressive/interlaced fields from struct em28xx to
>>     struct v4l2
>>   em28xx: move sensor parameter fields from struct em28xx to struct v4l2
>>   em28xx: move capture state tracking fields from struct em28xx to
>>     struct v4l2
>>   em28xx: move v4l2 user counting fields from struct em28xx to struct
>>     v4l2
>>   em28xx: move tuner frequency field from struct em28xx to struct v4l2
>>   em28xx: remove field tda9887_conf from struct em28xx
>>   em28xx: remove field tuner_addr from struct em28xx
>>   em28xx: move fields wq_trigger and streaming_started from struct
>>     em28xx to struct em28xx_audio
>>
>>  drivers/media/usb/em28xx/em28xx-audio.c  |  39 +-
>>  drivers/media/usb/em28xx/em28xx-camera.c |  51 +--
>>  drivers/media/usb/em28xx/em28xx-cards.c  |   9 -
>>  drivers/media/usb/em28xx/em28xx-vbi.c    |  10 +-
>>  drivers/media/usb/em28xx/em28xx-video.c  | 592 +++++++++++++++++--------------
>>  drivers/media/usb/em28xx/em28xx.h        | 120 ++++---
>>  6 files changed, 452 insertions(+), 369 deletions(-)
>>

Re: [PATCH 06/19] em28xx: move video_device structs from struct em28xx to struct v4l2

[Hans Verkuil]

On 05/11/2014 10:50 PM, Frank Schäfer wrote:
> 
> Am 09.05.2014 11:19, schrieb Hans Verkuil:
>> On 03/24/2014 08:33 PM, Frank Schäfer wrote:
>>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-video.c | 120 ++++++++++++++------------------
>>>  drivers/media/usb/em28xx/em28xx.h       |   7 +-
>>>  2 files changed, 56 insertions(+), 71 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
>>> index a4d26bf..88d0589 100644
>>> --- a/drivers/media/usb/em28xx/em28xx.h
>>> +++ b/drivers/media/usb/em28xx/em28xx.h
>>> @@ -504,6 +504,10 @@ struct em28xx_v4l2 {
>>>  	struct v4l2_device v4l2_dev;
>>>  	struct v4l2_ctrl_handler ctrl_handler;
>>>  	struct v4l2_clk *clk;
>>> +
>>> +	struct video_device *vdev;
>>> +	struct video_device *vbi_dev;
>>> +	struct video_device *radio_dev;
>> Think about embedding these structs. That way you don't have to allocate them which
>> removes the complexity of checking for ENOMEM errors.
> 
> Yes, but consider that only em286x and em288x devices provide VBI
> support and we have even less devices with radio support (~ 3 of 100).
> So with most devices, we would waste memory.

The problem with v4l drivers is always complexity, never performance or memory.
Anything that reduces complexity is always a good thing. The extra memory used
is negligible. Since kmalloc rounds up the requested memory to the next power
of two you might even end up with allocating more memory instead of less, but
you'd have to calculate that to see if it is true.

Simplification is always key to media drivers.

Regards,

	Hans

Re: [PATCH 03/19] em28xx: start moving em28xx-v4l specific data to its own struct

[Hans Verkuil]

On 05/11/2014 10:46 PM, Frank Schäfer wrote:
> 
> Am 09.05.2014 11:17, schrieb Hans Verkuil:
>> Some comments for future improvements:
>>
>> On 03/24/2014 08:33 PM, Frank Schäfer wrote:
>>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-camera.c |   4 +-
>>>  drivers/media/usb/em28xx/em28xx-video.c  | 160 +++++++++++++++++++++----------
>>>  drivers/media/usb/em28xx/em28xx.h        |   8 +-
>>>  3 files changed, 116 insertions(+), 56 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
>>> index 505e050..daebef3 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-camera.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-camera.c
>>> @@ -365,7 +365,7 @@ int em28xx_init_camera(struct em28xx *dev)
>>>  		dev->sensor_xtal = 4300000;
>>>  		pdata.xtal = dev->sensor_xtal;
>>>  		if (NULL ==
>>> -		    v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
>>> +		    v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>>>  					      &mt9v011_info, NULL)) {
>>>  			ret = -ENODEV;
>>>  			break;
>>> @@ -422,7 +422,7 @@ int em28xx_init_camera(struct em28xx *dev)
>>>  		dev->sensor_yres = 480;
>>>  
>>>  		subdev =
>>> -		     v4l2_i2c_new_subdev_board(&dev->v4l2_dev, adap,
>>> +		     v4l2_i2c_new_subdev_board(&dev->v4l2->v4l2_dev, adap,
>>>  					       &ov2640_info, NULL);
>>>  		if (NULL == subdev) {
>>>  			ret = -ENODEV;
>>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
>>> index 45ad471..89947db 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>>> @@ -189,10 +189,11 @@ static int em28xx_vbi_supported(struct em28xx *dev)
>>>   */
>>>  static void em28xx_wake_i2c(struct em28xx *dev)
>>>  {
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
>>> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
>>> +	v4l2_device_call_all(v4l2_dev, 0, core,  reset, 0);
>>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>>>  			INPUT(dev->ctl_input)->vmux, 0, 0);
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_stream, 0);
>>>  }
>>>  
>>>  static int em28xx_colorlevels_set_default(struct em28xx *dev)
>>> @@ -952,7 +953,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>>>  			f.type = V4L2_TUNER_RADIO;
>>>  		else
>>>  			f.type = V4L2_TUNER_ANALOG_TV;
>>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
>>> +		v4l2_device_call_all(&dev->v4l2->v4l2_dev,
>>> +				     0, tuner, s_frequency, &f);
>>>  	}
>>>  
>>>  	dev->streaming_users++;
>>> @@ -1083,6 +1085,7 @@ static int em28xx_vb2_setup(struct em28xx *dev)
>>>  
>>>  static void video_mux(struct em28xx *dev, int index)
>>>  {
>>> +	struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev;
>>>  	dev->ctl_input = index;
>>>  	dev->ctl_ainput = INPUT(index)->amux;
>>>  	dev->ctl_aoutput = INPUT(index)->aout;
>>> @@ -1090,21 +1093,21 @@ static void video_mux(struct em28xx *dev, int index)
>>>  	if (!dev->ctl_aoutput)
>>>  		dev->ctl_aoutput = EM28XX_AOUT_MASTER;
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
>>> +	v4l2_device_call_all(v4l2_dev, 0, video, s_routing,
>>>  			INPUT(index)->vmux, 0, 0);
>>>  
>>>  	if (dev->board.has_msp34xx) {
>>>  		if (dev->i2s_speed) {
>>> -			v4l2_device_call_all(&dev->v4l2_dev, 0, audio,
>>> +			v4l2_device_call_all(v4l2_dev, 0, audio,
>>>  				s_i2s_clock_freq, dev->i2s_speed);
>>>  		}
>>>  		/* Note: this is msp3400 specific */
>>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
>>> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>>>  			 dev->ctl_ainput, MSP_OUTPUT(MSP_SC_IN_DSP_SCART1), 0);
>>>  	}
>>>  
>>>  	if (dev->board.adecoder != EM28XX_NOADECODER) {
>>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
>>> +		v4l2_device_call_all(v4l2_dev, 0, audio, s_routing,
>>>  			dev->ctl_ainput, dev->ctl_aoutput, 0);
>>>  	}
>>>  
>>> @@ -1344,7 +1347,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm)
>>>  	struct em28xx_fh   *fh  = priv;
>>>  	struct em28xx      *dev = fh->dev;
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, video, querystd, norm);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -1374,7 +1377,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
>>>  	size_to_scale(dev, dev->width, dev->height, &dev->hscale, &dev->vscale);
>>>  
>>>  	em28xx_resolution_set(dev);
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, core, s_std, dev->norm);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -1388,7 +1391,7 @@ static int vidioc_g_parm(struct file *file, void *priv,
>>>  
>>>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
>>>  	if (dev->board.is_webcam)
>>> -		rc = v4l2_device_call_until_err(&dev->v4l2_dev, 0,
>>> +		rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0,
>>>  						video, g_parm, p);
>>>  	else
>>>  		v4l2_video_std_frame_period(dev->norm,
>>> @@ -1404,7 +1407,8 @@ static int vidioc_s_parm(struct file *file, void *priv,
>>>  	struct em28xx      *dev = fh->dev;
>>>  
>>>  	p->parm.capture.readbuffers = EM28XX_MIN_BUF;
>>> -	return v4l2_device_call_until_err(&dev->v4l2_dev, 0, video, s_parm, p);
>>> +	return v4l2_device_call_until_err(&dev->v4l2->v4l2_dev,
>>> +					  0, video, s_parm, p);
>>>  }
>>>  
>>>  static const char *iname[] = {
>>> @@ -1543,7 +1547,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
>>>  
>>>  	strcpy(t->name, "Tuner");
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1556,7 +1560,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>>>  	if (0 != t->index)
>>>  		return -EINVAL;
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1576,15 +1580,16 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>>>  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 v4l2_frequency  new_freq = *f;
>>> +	struct em28xx_fh          *fh   = priv;
>>> +	struct em28xx             *dev  = fh->dev;
>>> +	struct em28xx_v4l2        *v4l2 = dev->v4l2;
>>>  
>>>  	if (0 != f->tuner)
>>>  		return -EINVAL;
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, f);
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, &new_freq);
>>> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f);
>>> +	v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq);
>>>  	dev->ctl_freq = new_freq.frequency;
>>>  
>>>  	return 0;
>>> @@ -1602,7 +1607,8 @@ static int vidioc_g_chip_info(struct file *file, void *priv,
>>>  	if (chip->match.addr == 1)
>>>  		strlcpy(chip->name, "ac97", sizeof(chip->name));
>>>  	else
>>> -		strlcpy(chip->name, dev->v4l2_dev.name, sizeof(chip->name));
>>> +		strlcpy(chip->name,
>>> +			dev->v4l2->v4l2_dev.name, sizeof(chip->name));
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1814,7 +1820,7 @@ static int radio_g_tuner(struct file *file, void *priv,
>>>  
>>>  	strcpy(t->name, "Radio");
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -1827,12 +1833,26 @@ static int radio_s_tuner(struct file *file, void *priv,
>>>  	if (0 != t->index)
>>>  		return -EINVAL;
>>>  
>>> -	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
>>> +	v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t);
>>>  
>>>  	return 0;
>>>  }
>>>  
>>>  /*
>>> + * em28xx_free_v4l2() - Free struct em28xx_v4l2
>>> + *
>>> + * @ref: struct kref for struct em28xx_v4l2
>>> + *
>>> + * Called when all users of struct em28xx_v4l2 are gone
>>> + */
>>> +void em28xx_free_v4l2(struct kref *ref)
>>> +{
>>> +	struct em28xx_v4l2 *v4l2 = container_of(ref, struct em28xx_v4l2, ref);
>>> +
>>> +	kfree(v4l2);
>>> +}
>>> +
>>> +/*
>>>   * em28xx_v4l2_open()
>>>   * inits the device and starts isoc transfer
>>>   */
>>> @@ -1840,6 +1860,7 @@ static int em28xx_v4l2_open(struct file *filp)
>>>  {
>>>  	struct video_device *vdev = video_devdata(filp);
>>>  	struct em28xx *dev = video_drvdata(filp);
>>> +	struct em28xx_v4l2 *v4l2 = dev->v4l2;
>>>  	enum v4l2_buf_type fh_type = 0;
>>>  	struct em28xx_fh *fh;
>>>  
>>> @@ -1888,10 +1909,11 @@ static int em28xx_v4l2_open(struct file *filp)
>>>  
>>>  	if (vdev->vfl_type == VFL_TYPE_RADIO) {
>>>  		em28xx_videodbg("video_open: setting radio device\n");
>>> -		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_radio);
>>> +		v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio);
>>>  	}
>>>  
>>>  	kref_get(&dev->ref);
>>> +	kref_get(&v4l2->ref);
>> I never like these kref things. Especially for usb devices I strongly recommend
>> using the release() callback from v4l2_device instead: this callback will only
>> be called once all references to video_device nodes have been closed. In other
>> words, only once all filehandles to /dev/videoX (and radio, vbi etc) are closed
>> will the release callback be called.
>>
>> As such it is a perfect place to put the final cleanup, and there is no more need
>> to mess around with krefs.
> 
> The v4l2 submodule data struct can not be cleared before 1) the
> submodule is unloaded/unregistered AND 2) all users of all device nodes
> are gone.

Indeed. That's the whole point of the v4l2_device release callback. It's only
called after all devices are unregistered AND the last user of those device nodes
close has gone.

Basically you are duplicating the v4l2_device functionality here since v4l2_device
uses its own kref.

> Using a kref is much easier (and also safer) than dealing with
> non-trivial case checks in em28xx_v4l2_fini() and the v4l2_device
> release() callbacks.

You don't need any case checks in the release callback. All those checks are
already done for you.

> What we could do is to call kref_get() only one time at the first open()
> of a device node and kref_put() only at the last close().
> But it seems that this would just complicate the code without any real
> benefit.
> 
> 
>>>  	dev->users++;
>> The same for these user counters. You can use v4l2_fh_is_singular_file() to check
>> if the file open is the first file. However, this function assumes that v4l2_fh_add
>> has been called first.
>>
>> So for this driver it might be easier if we add a v4l2_fh_is_empty() to v4l2-fh.c
>> so we can call this before v4l2_fh_add.
>>
>> For that matter, you can almost certainly remove struct em28xx_fh altogether.
>> The type field of that struct can be determined by vdev->vfl_type and 'dev' can be
>> obtained via video_get_drvdata().
> 
> Yes, fields "dev" and "type" can definitly be removed from struct em28xx_fh.
> Then struct v4l2_fh fh is the last member, but I didn't have the time
> yet to take a deeper look at it.
> At a first glance its usage seems to be incomplete/broken.
> There are no v4l2_fh_del() and v4l2_fh_exit() calls and I wonder who
> deallocates the structs memory !?

vb2_fop_release() calls v4l2_fh_release() which calls del/exit. I admit, it's
not obvious.

Regards,

	Hans