[REVIEW PATCH 00/15] au0828: v4l2-compliance cleanups

[REVIEW PATCH 00/15] au0828: v4l2-compliance cleanups

[Hans Verkuil]

Hi all,

This patch series converts the au0828/au8522 drivers to the latest frameworks,
except for vb2 as usual.

Tested with a WinTV aero generously donated by Hauppauge some time ago.

I also did a lot of fixes in the disconnect handling and setting up the
right routing/std information at the right time.

It is now working correctly as far as I can tell: if I stick it in my PC
and run qv4l2 it actually picks up the tuner signal right away (if I set
it to the correct frequency of course).

If someone has additional hardware, then that would be nice if that can be
tested as well.

My git branch is here:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/au0828

Regards,

	Hans

[REVIEW PATCH 01/15] au8522_decoder: convert to the control framework.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/dvb-frontends/au8522_decoder.c |  130 +++++++++-----------------
 drivers/media/dvb-frontends/au8522_priv.h    |    6 +-
 2 files changed, 46 insertions(+), 90 deletions(-)

diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c
index 5243ba6..be2c802 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -229,15 +229,11 @@ static void setup_decoder_defaults(struct au8522_state *state, u8 input_mode)
 	/* Provide reasonable defaults for picture tuning values */
 	au8522_writereg(state, AU8522_TVDEC_SHARPNESSREG009H, 0x07);
 	au8522_writereg(state, AU8522_TVDEC_BRIGHTNESS_REG00AH, 0xed);
-	state->brightness = 0xed - 128;
 	au8522_writereg(state, AU8522_TVDEC_CONTRAST_REG00BH, 0x79);
-	state->contrast = 0x79;
 	au8522_writereg(state, AU8522_TVDEC_SATURATION_CB_REG00CH, 0x80);
 	au8522_writereg(state, AU8522_TVDEC_SATURATION_CR_REG00DH, 0x80);
-	state->saturation = 0x80;
 	au8522_writereg(state, AU8522_TVDEC_HUE_H_REG00EH, 0x00);
 	au8522_writereg(state, AU8522_TVDEC_HUE_L_REG00FH, 0x00);
-	state->hue = 0x00;
 
 	/* Other decoder registers */
 	au8522_writereg(state, AU8522_TVDEC_INT_MASK_REG010H, 0x00);
@@ -489,75 +485,32 @@ static void set_audio_input(struct au8522_state *state, int aud_input)
 
 /* ----------------------------------------------------------------------- */
 
-static int au8522_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int au8522_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct au8522_state *state = to_state(sd);
+	struct au8522_state *state =
+		container_of(ctrl->handler, struct au8522_state, hdl);
 
 	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
-		state->brightness = ctrl->value;
 		au8522_writereg(state, AU8522_TVDEC_BRIGHTNESS_REG00AH,
-				ctrl->value - 128);
+				ctrl->val - 128);
 		break;
 	case V4L2_CID_CONTRAST:
-		state->contrast = ctrl->value;
 		au8522_writereg(state, AU8522_TVDEC_CONTRAST_REG00BH,
-				ctrl->value);
+				ctrl->val);
 		break;
 	case V4L2_CID_SATURATION:
-		state->saturation = ctrl->value;
 		au8522_writereg(state, AU8522_TVDEC_SATURATION_CB_REG00CH,
-				ctrl->value);
+				ctrl->val);
 		au8522_writereg(state, AU8522_TVDEC_SATURATION_CR_REG00DH,
-				ctrl->value);
+				ctrl->val);
 		break;
 	case V4L2_CID_HUE:
-		state->hue = ctrl->value;
 		au8522_writereg(state, AU8522_TVDEC_HUE_H_REG00EH,
-				ctrl->value >> 8);
+				ctrl->val >> 8);
 		au8522_writereg(state, AU8522_TVDEC_HUE_L_REG00FH,
-				ctrl->value & 0xFF);
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-	case V4L2_CID_AUDIO_BASS:
-	case V4L2_CID_AUDIO_TREBLE:
-	case V4L2_CID_AUDIO_BALANCE:
-	case V4L2_CID_AUDIO_MUTE:
-		/* Not yet implemented */
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int au8522_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct au8522_state *state = to_state(sd);
-
-	/* Note that we are using values cached in the state structure instead
-	   of reading the registers due to issues with i2c reads not working
-	   properly/consistently yet on the HVR-950q */
-
-	switch (ctrl->id) {
-	case V4L2_CID_BRIGHTNESS:
-		ctrl->value = state->brightness;
-		break;
-	case V4L2_CID_CONTRAST:
-		ctrl->value = state->contrast;
-		break;
-	case V4L2_CID_SATURATION:
-		ctrl->value = state->saturation;
-		break;
-	case V4L2_CID_HUE:
-		ctrl->value = state->hue;
+				ctrl->val & 0xFF);
 		break;
-	case V4L2_CID_AUDIO_VOLUME:
-	case V4L2_CID_AUDIO_BASS:
-	case V4L2_CID_AUDIO_TREBLE:
-	case V4L2_CID_AUDIO_BALANCE:
-	case V4L2_CID_AUDIO_MUTE:
-		/* Not yet supported */
 	default:
 		return -EINVAL;
 	}
@@ -616,26 +569,6 @@ static int au8522_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-static int au8522_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
-{
-	switch (qc->id) {
-	case V4L2_CID_CONTRAST:
-		return v4l2_ctrl_query_fill(qc, 0, 255, 1,
-					    AU8522_TVDEC_CONTRAST_REG00BH_CVBS);
-	case V4L2_CID_BRIGHTNESS:
-		return v4l2_ctrl_query_fill(qc, 0, 255, 1, 109);
-	case V4L2_CID_SATURATION:
-		return v4l2_ctrl_query_fill(qc, 0, 255, 1, 128);
-	case V4L2_CID_HUE:
-		return v4l2_ctrl_query_fill(qc, -32768, 32768, 1, 0);
-	default:
-		break;
-	}
-
-	qc->type = 0;
-	return -EINVAL;
-}
-
 static int au8522_reset(struct v4l2_subdev *sd, u32 val)
 {
 	struct au8522_state *state = to_state(sd);
@@ -712,20 +645,18 @@ static int au8522_g_chip_ident(struct v4l2_subdev *sd,
 	return v4l2_chip_ident_i2c_client(client, chip, state->id, state->rev);
 }
 
-static int au8522_log_status(struct v4l2_subdev *sd)
-{
-	/* FIXME: Add some status info here */
-	return 0;
-}
-
 /* ----------------------------------------------------------------------- */
 
 static const struct v4l2_subdev_core_ops au8522_core_ops = {
-	.log_status = au8522_log_status,
+	.log_status = v4l2_ctrl_subdev_log_status,
+	.g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
+	.try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
+	.s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
+	.g_ctrl = v4l2_subdev_g_ctrl,
+	.s_ctrl = v4l2_subdev_s_ctrl,
+	.queryctrl = v4l2_subdev_queryctrl,
+	.querymenu = v4l2_subdev_querymenu,
 	.g_chip_ident = au8522_g_chip_ident,
-	.g_ctrl = au8522_g_ctrl,
-	.s_ctrl = au8522_s_ctrl,
-	.queryctrl = au8522_queryctrl,
 	.reset = au8522_reset,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register = au8522_g_register,
@@ -753,12 +684,17 @@ static const struct v4l2_subdev_ops au8522_ops = {
 	.video = &au8522_video_ops,
 };
 
+static const struct v4l2_ctrl_ops au8522_ctrl_ops = {
+	.s_ctrl = au8522_s_ctrl,
+};
+
 /* ----------------------------------------------------------------------- */
 
 static int au8522_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 {
 	struct au8522_state *state;
+	struct v4l2_ctrl_handler *hdl;
 	struct v4l2_subdev *sd;
 	int instance;
 	struct au8522_config *demod_config;
@@ -799,6 +735,27 @@ static int au8522_probe(struct i2c_client *client,
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &au8522_ops);
 
+	hdl = &state->hdl;
+	v4l2_ctrl_handler_init(hdl, 4);
+	v4l2_ctrl_new_std(hdl, &au8522_ctrl_ops,
+			V4L2_CID_BRIGHTNESS, 0, 255, 1, 109);
+	v4l2_ctrl_new_std(hdl, &au8522_ctrl_ops,
+			V4L2_CID_CONTRAST, 0, 255, 1,
+			AU8522_TVDEC_CONTRAST_REG00BH_CVBS);
+	v4l2_ctrl_new_std(hdl, &au8522_ctrl_ops,
+			V4L2_CID_SATURATION, 0, 255, 1, 128);
+	v4l2_ctrl_new_std(hdl, &au8522_ctrl_ops,
+			V4L2_CID_HUE, -32768, 32767, 1, 0);
+	sd->ctrl_handler = hdl;
+	if (hdl->error) {
+		int err = hdl->error;
+
+		v4l2_ctrl_handler_free(hdl);
+		kfree(demod_config);
+		kfree(state);
+		return err;
+	}
+
 	state->c = client;
 	state->vid_input = AU8522_COMPOSITE_CH1;
 	state->aud_input = AU8522_AUDIO_NONE;
@@ -815,6 +772,7 @@ static int au8522_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	v4l2_device_unregister_subdev(sd);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	au8522_release_state(to_state(sd));
 	return 0;
 }
diff --git a/drivers/media/dvb-frontends/au8522_priv.h b/drivers/media/dvb-frontends/au8522_priv.h
index 0529699..aa0f16d 100644
--- a/drivers/media/dvb-frontends/au8522_priv.h
+++ b/drivers/media/dvb-frontends/au8522_priv.h
@@ -29,6 +29,7 @@
 #include <linux/delay.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
 #include <linux/i2c.h>
 #include "dvb_frontend.h"
 #include "au8522.h"
@@ -65,10 +66,7 @@ struct au8522_state {
 	int aud_input;
 	u32 id;
 	u32 rev;
-	u8 brightness;
-	u8 contrast;
-	u8 saturation;
-	s16 hue;
+	struct v4l2_ctrl_handler hdl;
 };
 
 /* These are routines shared by both the VSB/QAM demodulator and the analog
-- 
1.7.10.4

[REVIEW PATCH 02/15] au0828: fix querycap.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 8b9e826..4254b2c 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1241,20 +1241,25 @@ static int vidioc_queryctrl(struct file *file, void *priv,
 static int vidioc_querycap(struct file *file, void  *priv,
 			   struct v4l2_capability *cap)
 {
-	struct au0828_fh *fh  = priv;
+	struct video_device *vdev = video_devdata(file);
+	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
 
 	strlcpy(cap->driver, "au0828", sizeof(cap->driver));
 	strlcpy(cap->card, dev->board.name, sizeof(cap->card));
-	strlcpy(cap->bus_info, dev->v4l2_dev.name, sizeof(cap->bus_info));
+	usb_make_path(dev->usbdev, cap->bus_info, sizeof(cap->bus_info));
 
-	/*set the device capabilities */
-	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE |
-		V4L2_CAP_VBI_CAPTURE |
-		V4L2_CAP_AUDIO |
+	/* set the device capabilities */
+	cap->device_caps = V4L2_CAP_AUDIO |
 		V4L2_CAP_READWRITE |
 		V4L2_CAP_STREAMING |
 		V4L2_CAP_TUNER;
+	if (vdev->vfl_type == VFL_TYPE_GRABBER)
+		cap->device_caps |= V4L2_CAP_VIDEO_CAPTURE;
+	else
+		cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS |
+		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_VIDEO_CAPTURE;
 	return 0;
 }
 
-- 
1.7.10.4

[REVIEW PATCH 03/15] au0828: frequency handling fixes.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

- define an initial frequency
- return an error if g_frequency is called for an invalid tuner index
- get the clamped frequency value after setting it: i.e. the tuner driver
  may clamp the given frequency to a valid frequency range and ctrl_freq
  should get that actual clamped frequency.
- remove obsolete tuner type checks (done by the core).

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 4254b2c..25b18d397 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1521,8 +1521,6 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 	if (t->index != 0)
 		return -EINVAL;
 
-	t->type = V4L2_TUNER_ANALOG_TV;
-
 	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
 		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
 
@@ -1544,7 +1542,8 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
 
-	freq->type = V4L2_TUNER_ANALOG_TV;
+	if (freq->tuner != 0)
+		return -EINVAL;
 	freq->frequency = dev->ctrl_freq;
 	return 0;
 }
@@ -1557,10 +1556,6 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	if (freq->tuner != 0)
 		return -EINVAL;
-	if (freq->type != V4L2_TUNER_ANALOG_TV)
-		return -EINVAL;
-
-	dev->ctrl_freq = freq->frequency;
 
 	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
 		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
@@ -1575,6 +1570,9 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	}
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, freq);
+	/* Get the actual set (and possibly clamped) frequency */
+	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, freq);
+	dev->ctrl_freq = freq->frequency;
 
 	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
 		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
@@ -1978,6 +1976,7 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->frame_size = dev->field_size << 1;
 	dev->bytesperline = dev->width << 1;
 	dev->ctrl_ainput = 0;
+	dev->ctrl_freq = 960;
 
 	/* allocate and fill v4l2 video struct */
 	dev->vdev = video_device_alloc();
-- 
1.7.10.4

[REVIEW PATCH 04/15] au0828: fix intendation coding style issue.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

No code change, just fixing a wrong indentation.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 25b18d397..e3d8a3c 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1561,12 +1561,12 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
 
 	if (dev->std_set_in_tuner_core == 0) {
-	  /* If we've never sent the standard in tuner core, do so now.  We
-	     don't do this at device probe because we don't want to incur
-	     the cost of a firmware load */
-	  v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std,
-			       dev->vdev->tvnorms);
-	  dev->std_set_in_tuner_core = 1;
+		/* If we've never sent the standard in tuner core, do so now.
+		   We don't do this at device probe because we don't want to
+		   incur the cost of a firmware load */
+		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std,
+				     dev->vdev->tvnorms);
+		dev->std_set_in_tuner_core = 1;
 	}
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, freq);
-- 
1.7.10.4

[REVIEW PATCH 05/15] au0828: fix audio input handling.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

- V4L2_CAP_AUDIO was set, but enumaudio was not implemented.
- audioset was never filled by enum_input
- ctrl_ainput was never updated when switching the video input
- g_audio was broken due to faulty logic: g_audio should set the
  index, it doesn't receive it from the user.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   35 +++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index e3d8a3c..e4a24fa 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1373,10 +1373,13 @@ static int vidioc_enum_input(struct file *file, void *priv,
 	input->index = tmp;
 	strcpy(input->name, inames[AUVI_INPUT(tmp).type]);
 	if ((AUVI_INPUT(tmp).type == AU0828_VMUX_TELEVISION) ||
-	    (AUVI_INPUT(tmp).type == AU0828_VMUX_CABLE))
+	    (AUVI_INPUT(tmp).type == AU0828_VMUX_CABLE)) {
 		input->type |= V4L2_INPUT_TYPE_TUNER;
-	else
+		input->audioset = 1;
+	} else {
 		input->type |= V4L2_INPUT_TYPE_CAMERA;
+		input->audioset = 2;
+	}
 
 	input->std = dev->vdev->tvnorms;
 
@@ -1408,12 +1411,15 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 	switch (AUVI_INPUT(index).type) {
 	case AU0828_VMUX_SVIDEO:
 		dev->input_type = AU0828_VMUX_SVIDEO;
+		dev->ctrl_ainput = 1;
 		break;
 	case AU0828_VMUX_COMPOSITE:
 		dev->input_type = AU0828_VMUX_COMPOSITE;
+		dev->ctrl_ainput = 1;
 		break;
 	case AU0828_VMUX_TELEVISION:
 		dev->input_type = AU0828_VMUX_TELEVISION;
+		dev->ctrl_ainput = 0;
 		break;
 	default:
 		dprintk(1, "VIDIOC_S_INPUT unknown input type set [%d]\n",
@@ -1450,23 +1456,32 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 	return 0;
 }
 
+static int vidioc_enumaudio(struct file *file, void *priv, struct v4l2_audio *a)
+{
+	if (a->index > 1)
+		return -EINVAL;
+
+	if (a->index == 0)
+		strcpy(a->name, "Television");
+	else
+		strcpy(a->name, "Line in");
+
+	a->capability = V4L2_AUDCAP_STEREO;
+	return 0;
+}
+
 static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 {
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
-	unsigned int index = a->index;
 
-	if (a->index > 1)
-		return -EINVAL;
-
-	index = dev->ctrl_ainput;
-	if (index == 0)
+	a->index = dev->ctrl_ainput;
+	if (a->index == 0)
 		strcpy(a->name, "Television");
 	else
 		strcpy(a->name, "Line in");
 
 	a->capability = V4L2_AUDCAP_STEREO;
-	a->index = index;
 	return 0;
 }
 
@@ -1474,6 +1489,7 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio
 {
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
+
 	if (a->index != dev->ctrl_ainput)
 		return -EINVAL;
 	return 0;
@@ -1876,6 +1892,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap       = vidioc_s_fmt_vid_cap,
 	.vidioc_g_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
+	.vidioc_enumaudio           = vidioc_enumaudio,
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
 	.vidioc_cropcap             = vidioc_cropcap,
-- 
1.7.10.4

[REVIEW PATCH 06/15] au0828: convert to the control framework.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-core.c  |   15 ++++++++++--
 drivers/media/usb/au0828/au0828-video.c |   39 ++-----------------------------
 drivers/media/usb/au0828/au0828.h       |    2 ++
 3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index 1e6f40e..ffd3bcb 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -143,6 +143,7 @@ static void au0828_usb_disconnect(struct usb_interface *interface)
 	au0828_i2c_unregister(dev);
 
 #ifdef CONFIG_VIDEO_AU0828_V4L2
+	v4l2_ctrl_handler_free(&dev->v4l2_ctrl_hdl);
 	v4l2_device_unregister(&dev->v4l2_dev);
 #endif
 
@@ -205,12 +206,22 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	/* Create the v4l2_device */
 	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
 	if (retval) {
-		printk(KERN_ERR "%s() v4l2_device_register failed\n",
+		pr_err("%s() v4l2_device_register failed\n",
 		       __func__);
 		mutex_unlock(&dev->lock);
 		kfree(dev);
-		return -EIO;
+		return retval;
 	}
+	/* This control handler will inherit the controls from au8522 */
+	retval = v4l2_ctrl_handler_init(&dev->v4l2_ctrl_hdl, 4);
+	if (retval) {
+		pr_err("%s() v4l2_ctrl_handler_init failed\n",
+		       __func__);
+		mutex_unlock(&dev->lock);
+		kfree(dev);
+		return retval;
+	}
+	dev->v4l2_dev.ctrl_handler = &dev->v4l2_ctrl_hdl;
 #endif
 
 	/* Power Up the bridge */
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index e4a24fa..7d762c0 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1226,18 +1226,6 @@ static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
 }
 
 
-static int vidioc_queryctrl(struct file *file, void *priv,
-			    struct v4l2_queryctrl *qc)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, queryctrl, qc);
-	if (qc->type)
-		return 0;
-	else
-		return -EINVAL;
-}
-
 static int vidioc_querycap(struct file *file, void  *priv,
 			   struct v4l2_capability *cap)
 {
@@ -1495,26 +1483,6 @@ static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio
 	return 0;
 }
 
-static int vidioc_g_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, g_ctrl, ctrl);
-	return 0;
-
-}
-
-static int vidioc_s_ctrl(struct file *file, void *priv,
-				struct v4l2_control *ctrl)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_ctrl, ctrl);
-	return 0;
-}
-
 static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 {
 	struct au0828_fh *fh = priv;
@@ -1904,9 +1872,6 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_enum_input          = vidioc_enum_input,
 	.vidioc_g_input             = vidioc_g_input,
 	.vidioc_s_input             = vidioc_s_input,
-	.vidioc_queryctrl           = vidioc_queryctrl,
-	.vidioc_g_ctrl              = vidioc_g_ctrl,
-	.vidioc_s_ctrl              = vidioc_s_ctrl,
 	.vidioc_streamon            = vidioc_streamon,
 	.vidioc_streamoff           = vidioc_streamoff,
 	.vidioc_g_tuner             = vidioc_g_tuner,
@@ -2012,13 +1977,13 @@ int au0828_analog_register(struct au0828_dev *dev,
 
 	/* Fill the video capture device struct */
 	*dev->vdev = au0828_video_template;
-	dev->vdev->parent = &dev->usbdev->dev;
+	dev->vdev->v4l2_dev = &dev->v4l2_dev;
 	dev->vdev->lock = &dev->lock;
 	strcpy(dev->vdev->name, "au0828a video");
 
 	/* Setup the VBI device */
 	*dev->vbi_dev = au0828_video_template;
-	dev->vbi_dev->parent = &dev->usbdev->dev;
+	dev->vbi_dev->v4l2_dev = &dev->v4l2_dev;
 	dev->vbi_dev->lock = &dev->lock;
 	strcpy(dev->vbi_dev->name, "au0828a vbi");
 
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index e579ff6..803af10 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -28,6 +28,7 @@
 #include <linux/videodev2.h>
 #include <media/videobuf-vmalloc.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
 
 /* DVB */
 #include "demux.h"
@@ -202,6 +203,7 @@ struct au0828_dev {
 #ifdef CONFIG_VIDEO_AU0828_V4L2
 	/* Analog */
 	struct v4l2_device v4l2_dev;
+	struct v4l2_ctrl_handler v4l2_ctrl_hdl;
 #endif
 	int users;
 	unsigned int resources;	/* resources in use */
-- 
1.7.10.4

[REVIEW PATCH 07/15] au0828: add prio, control event and log_status support

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   42 ++++++++++++++++++++++++-------
 drivers/media/usb/au0828/au0828.h       |    4 +++
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 7d762c0..07287ef 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -35,6 +35,7 @@
 #include <linux/suspend.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/tuner.h>
 #include "au0828.h"
@@ -988,6 +989,7 @@ static int au0828_v4l2_open(struct file *filp)
 
 	fh->type = type;
 	fh->dev = dev;
+	v4l2_fh_init(&fh->fh, vdev);
 	filp->private_data = fh;
 
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && dev->users == 0) {
@@ -1031,6 +1033,7 @@ static int au0828_v4l2_open(struct file *filp)
 				    V4L2_FIELD_SEQ_TB,
 				    sizeof(struct au0828_buffer), fh,
 				    &dev->lock);
+	v4l2_fh_add(&fh->fh);
 	return ret;
 }
 
@@ -1040,6 +1043,8 @@ static int au0828_v4l2_close(struct file *filp)
 	struct au0828_fh *fh = filp->private_data;
 	struct au0828_dev *dev = fh->dev;
 
+	v4l2_fh_del(&fh->fh);
+	v4l2_fh_exit(&fh->fh);
 	if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
 		/* Cancel timeout thread in case they didn't call streamoff */
 		dev->vid_timeout_running = 0;
@@ -1061,6 +1066,7 @@ static int au0828_v4l2_close(struct file *filp)
 	if (dev->users == 1) {
 		if (dev->dev_state & DEV_DISCONNECTED) {
 			au0828_analog_unregister(dev);
+			kfree(fh);
 			kfree(dev);
 			return 0;
 		}
@@ -1128,23 +1134,27 @@ static unsigned int au0828_v4l2_poll(struct file *filp, poll_table *wait)
 {
 	struct au0828_fh *fh = filp->private_data;
 	struct au0828_dev *dev = fh->dev;
-	int rc;
+	unsigned long req_events = poll_requested_events(wait);
+	unsigned int res;
 
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
+	if (check_dev(dev) < 0)
+		return POLLERR;
+
+	res = v4l2_ctrl_poll(filp, wait);
+	if (!(req_events & (POLLIN | POLLRDNORM)))
+		return res;
 
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		if (!res_get(fh, AU0828_RESOURCE_VIDEO))
 			return POLLERR;
-		return videobuf_poll_stream(filp, &fh->vb_vidq, wait);
-	} else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
+		return res | videobuf_poll_stream(filp, &fh->vb_vidq, wait);
+	}
+	if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
 		if (!res_get(fh, AU0828_RESOURCE_VBI))
 			return POLLERR;
-		return videobuf_poll_stream(filp, &fh->vb_vbiq, wait);
-	} else {
-		return POLLERR;
+		return res | videobuf_poll_stream(filp, &fh->vb_vbiq, wait);
 	}
+	return POLLERR;
 }
 
 static int au0828_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
@@ -1760,6 +1770,15 @@ static int vidioc_s_register(struct file *file, void *priv,
 }
 #endif
 
+static int vidioc_log_status(struct file *file, void *fh)
+{
+	struct video_device *vdev = video_devdata(file);
+
+	v4l2_ctrl_log_status(file, fh);
+	v4l2_device_call_all(vdev->v4l2_dev, 0, core, log_status);
+	return 0;
+}
+
 static int vidioc_reqbufs(struct file *file, void *priv,
 			  struct v4l2_requestbuffers *rb)
 {
@@ -1883,6 +1902,9 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_s_register          = vidioc_s_register,
 #endif
 	.vidioc_g_chip_ident        = vidioc_g_chip_ident,
+	.vidioc_log_status	    = vidioc_log_status,
+	.vidioc_subscribe_event     = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
 };
 
 static const struct video_device au0828_video_template = {
@@ -1979,12 +2001,14 @@ int au0828_analog_register(struct au0828_dev *dev,
 	*dev->vdev = au0828_video_template;
 	dev->vdev->v4l2_dev = &dev->v4l2_dev;
 	dev->vdev->lock = &dev->lock;
+	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vdev->flags);
 	strcpy(dev->vdev->name, "au0828a video");
 
 	/* Setup the VBI device */
 	*dev->vbi_dev = au0828_video_template;
 	dev->vbi_dev->v4l2_dev = &dev->v4l2_dev;
 	dev->vbi_dev->lock = &dev->lock;
+	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vbi_dev->flags);
 	strcpy(dev->vbi_dev->name, "au0828a vbi");
 
 	/* Register the v4l2 device */
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index 803af10..ad40048 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -29,6 +29,7 @@
 #include <media/videobuf-vmalloc.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-fh.h>
 
 /* DVB */
 #include "demux.h"
@@ -119,6 +120,9 @@ enum au0828_dev_state {
 };
 
 struct au0828_fh {
+	/* must be the first field of this struct! */
+	struct v4l2_fh fh;
+
 	struct au0828_dev *dev;
 	unsigned int  resources;
 
-- 
1.7.10.4

[REVIEW PATCH 08/15] au0828: add try_fmt_vbi support, zero vbi.reserved, pix.priv.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Also get rid of unnecessary format type check.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 07287ef..3c3e4d6 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1182,9 +1182,6 @@ static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
 	int width = format->fmt.pix.width;
 	int height = format->fmt.pix.height;
 
-	if (format->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		return -EINVAL;
-
 	/* If they are demanding a format other than the one we support,
 	   bail out (tvtime asks for UYVY and then retries with YUYV) */
 	if (format->fmt.pix.pixelformat != V4L2_PIX_FMT_UYVY)
@@ -1203,6 +1200,7 @@ static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
 	format->fmt.pix.sizeimage = width * height * 2;
 	format->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
 	format->fmt.pix.field = V4L2_FIELD_INTERLACED;
+	format->fmt.pix.priv = 0;
 
 	if (cmd == VIDIOC_TRY_FMT)
 		return 0;
@@ -1289,6 +1287,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.sizeimage = dev->frame_size;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M; /* NTSC/PAL */
 	f->fmt.pix.field = V4L2_FIELD_INTERLACED;
+	f->fmt.pix.priv = 0;
 	return 0;
 }
 
@@ -1595,6 +1594,7 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 	format->fmt.vbi.count[1] = dev->vbi_height;
 	format->fmt.vbi.start[0] = 21;
 	format->fmt.vbi.start[1] = 284;
+	memset(format->fmt.vbi.reserved, 0, sizeof(format->fmt.vbi.reserved));
 
 	return 0;
 }
@@ -1878,6 +1878,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_try_fmt_vid_cap     = vidioc_try_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap       = vidioc_s_fmt_vid_cap,
 	.vidioc_g_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
+	.vidioc_try_fmt_vbi_cap     = vidioc_g_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap       = vidioc_g_fmt_vbi_cap,
 	.vidioc_enumaudio           = vidioc_enumaudio,
 	.vidioc_g_audio             = vidioc_g_audio,
-- 
1.7.10.4

[REVIEW PATCH 09/15] au0828: replace deprecated current_norm by g_std.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   15 +++++++++++++--
 drivers/media/usb/au0828/au0828.h       |    1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 3c3e4d6..62308fe 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1322,7 +1322,7 @@ out:
 	return rc;
 }
 
-static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id * norm)
+static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 {
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
@@ -1339,10 +1339,20 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id * norm)
 
 	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
 		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+	dev->std = *norm;
 
 	return 0;
 }
 
+static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
+{
+	struct au0828_fh *fh = priv;
+	struct au0828_dev *dev = fh->dev;
+
+	*norm = dev->std;
+	return 0;
+}
+
 static int vidioc_enum_input(struct file *file, void *priv,
 				struct v4l2_input *input)
 {
@@ -1889,6 +1899,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_qbuf                = vidioc_qbuf,
 	.vidioc_dqbuf               = vidioc_dqbuf,
 	.vidioc_s_std               = vidioc_s_std,
+	.vidioc_g_std               = vidioc_g_std,
 	.vidioc_enum_input          = vidioc_enum_input,
 	.vidioc_g_input             = vidioc_g_input,
 	.vidioc_s_input             = vidioc_s_input,
@@ -1913,7 +1924,6 @@ static const struct video_device au0828_video_template = {
 	.release                    = video_device_release,
 	.ioctl_ops 		    = &video_ioctl_ops,
 	.tvnorms                    = V4L2_STD_NTSC_M,
-	.current_norm               = V4L2_STD_NTSC_M,
 };
 
 /**************************************************************************/
@@ -1982,6 +1992,7 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->bytesperline = dev->width << 1;
 	dev->ctrl_ainput = 0;
 	dev->ctrl_freq = 960;
+	dev->std = V4L2_STD_NTSC_M;
 
 	/* allocate and fill v4l2 video struct */
 	dev->vdev = video_device_alloc();
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index ad40048..ef1f57f 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -222,6 +222,7 @@ struct au0828_dev {
 	int vbi_width;
 	int vbi_height;
 	u32 vbi_read;
+	v4l2_std_id std;
 	u32 field_size;
 	u32 frame_size;
 	u32 bytesperline;
-- 
1.7.10.4

[REVIEW PATCH 10/15] au8522_decoder: remove obsolete control ops.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Now that au0828 has been converted to the control framework these
compatilibity ops are no longer needed.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/dvb-frontends/au8522_decoder.c |    7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c
index be2c802..aa7be74 100644
--- a/drivers/media/dvb-frontends/au8522_decoder.c
+++ b/drivers/media/dvb-frontends/au8522_decoder.c
@@ -649,13 +649,6 @@ static int au8522_g_chip_ident(struct v4l2_subdev *sd,
 
 static const struct v4l2_subdev_core_ops au8522_core_ops = {
 	.log_status = v4l2_ctrl_subdev_log_status,
-	.g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
-	.try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
-	.s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
-	.g_ctrl = v4l2_subdev_g_ctrl,
-	.s_ctrl = v4l2_subdev_s_ctrl,
-	.queryctrl = v4l2_subdev_queryctrl,
-	.querymenu = v4l2_subdev_querymenu,
 	.g_chip_ident = au8522_g_chip_ident,
 	.reset = au8522_reset,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
-- 
1.7.10.4

[REVIEW PATCH 11/15] au0828: fix disconnect sequence.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The driver crashed when the device was disconnected while an application
still had a device node open. Fixed by using the release() callback of struct
v4l2_device.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-core.c  |   48 ++++++++++++++++++++-----------
 drivers/media/usb/au0828/au0828-video.c |    9 +-----
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index ffd3bcb..bd9d19a 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -125,36 +125,48 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	return status;
 }
 
-static void au0828_usb_disconnect(struct usb_interface *interface)
+static void au0828_usb_release(struct au0828_dev *dev)
 {
-	struct au0828_dev *dev = usb_get_intfdata(interface);
-
-	dprintk(1, "%s()\n", __func__);
-
-	/* Digital TV */
-	au0828_dvb_unregister(dev);
-
-#ifdef CONFIG_VIDEO_AU0828_V4L2
-	if (AUVI_INPUT(0).type != AU0828_VMUX_UNDEFINED)
-		au0828_analog_unregister(dev);
-#endif
-
 	/* I2C */
 	au0828_i2c_unregister(dev);
 
+	kfree(dev);
+}
+
 #ifdef CONFIG_VIDEO_AU0828_V4L2
+static void au0828_usb_v4l2_release(struct v4l2_device *v4l2_dev)
+{
+	struct au0828_dev *dev =
+		container_of(v4l2_dev, struct au0828_dev, v4l2_dev);
+
 	v4l2_ctrl_handler_free(&dev->v4l2_ctrl_hdl);
 	v4l2_device_unregister(&dev->v4l2_dev);
+	au0828_usb_release(dev);
+}
 #endif
 
-	usb_set_intfdata(interface, NULL);
+static void au0828_usb_disconnect(struct usb_interface *interface)
+{
+	struct au0828_dev *dev = usb_get_intfdata(interface);
+
+	dprintk(1, "%s()\n", __func__);
+
+	/* Digital TV */
+	au0828_dvb_unregister(dev);
 
+	usb_set_intfdata(interface, NULL);
 	mutex_lock(&dev->mutex);
 	dev->usbdev = NULL;
 	mutex_unlock(&dev->mutex);
-
-	kfree(dev);
-
+#ifdef CONFIG_VIDEO_AU0828_V4L2
+	if (AUVI_INPUT(0).type != AU0828_VMUX_UNDEFINED) {
+		au0828_analog_unregister(dev);
+		v4l2_device_disconnect(&dev->v4l2_dev);
+		v4l2_device_put(&dev->v4l2_dev);
+		return;
+	}
+#endif
+	au0828_usb_release(dev);
 }
 
 static int au0828_usb_probe(struct usb_interface *interface,
@@ -203,6 +215,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	dev->boardnr = id->driver_info;
 
 #ifdef CONFIG_VIDEO_AU0828_V4L2
+	dev->v4l2_dev.release = au0828_usb_v4l2_release;
+
 	/* Create the v4l2_device */
 	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
 	if (retval) {
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 62308fe..a41e5ae 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1063,14 +1063,7 @@ static int au0828_v4l2_close(struct file *filp)
 		res_free(fh, AU0828_RESOURCE_VBI);
 	}
 
-	if (dev->users == 1) {
-		if (dev->dev_state & DEV_DISCONNECTED) {
-			au0828_analog_unregister(dev);
-			kfree(fh);
-			kfree(dev);
-			return 0;
-		}
-
+	if (dev->users == 1 && video_is_registered(video_devdata(filp))) {
 		au0828_analog_stream_disable(dev);
 
 		au0828_uninit_isoc(dev);
-- 
1.7.10.4

[REVIEW PATCH 12/15] au0828: simplify i2c_gate_ctrl.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Turn it into a simple function.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index a41e5ae..ac89b2c5 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -59,6 +59,12 @@ do {\
 	} \
   } while (0)
 
+static inline void i2c_gate_ctrl(struct au0828_dev *dev, int val)
+{
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, val);
+}
+
 static inline void print_err_status(struct au0828_dev *dev,
 				    int packet, int status)
 {
@@ -1320,8 +1326,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+	i2c_gate_ctrl(dev, 1);
 
 	/* FIXME: when we support something other than NTSC, we are going to
 	   have to make the au0828 bridge adjust the size of its capture
@@ -1330,8 +1335,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, *norm);
 	dev->std_set_in_tuner_core = 1;
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+	i2c_gate_ctrl(dev, 0);
 	dev->std = *norm;
 
 	return 0;
@@ -1517,13 +1521,11 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 	if (t->index != 0)
 		return -EINVAL;
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+	i2c_gate_ctrl(dev, 1);
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+	i2c_gate_ctrl(dev, 0);
 
 	dprintk(1, "VIDIOC_S_TUNER: signal = %x, afc = %x\n", t->signal,
 		t->afc);
@@ -1553,8 +1555,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	if (freq->tuner != 0)
 		return -EINVAL;
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+	i2c_gate_ctrl(dev, 1);
 
 	if (dev->std_set_in_tuner_core == 0) {
 		/* If we've never sent the standard in tuner core, do so now.
@@ -1570,8 +1571,7 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, freq);
 	dev->ctrl_freq = freq->frequency;
 
-	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
-		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+	i2c_gate_ctrl(dev, 0);
 
 	au0828_analog_stream_reset(dev);
 
-- 
1.7.10.4

[REVIEW PATCH 13/15] au0828: don't change global state information on open().

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Just opening a device shouldn't have any side-effects.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index ac89b2c5..1f06d97 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1005,11 +1005,6 @@ static int au0828_v4l2_open(struct file *filp)
 			printk(KERN_INFO "Au0828 can't set alternate to 5!\n");
 			return -EBUSY;
 		}
-		dev->width = NTSC_STD_W;
-		dev->height = NTSC_STD_H;
-		dev->frame_size = dev->width * dev->height * 2;
-		dev->field_size = dev->width * dev->height;
-		dev->bytesperline = dev->width * 2;
 
 		au0828_analog_stream_enable(dev);
 		au0828_analog_stream_reset(dev);
@@ -1031,8 +1026,6 @@ static int au0828_v4l2_open(struct file *filp)
 				    &dev->lock);
 
 	/* VBI Setup */
-	dev->vbi_width = 720;
-	dev->vbi_height = 1;
 	videobuf_queue_vmalloc_init(&fh->vb_vbiq, &au0828_vbi_qops,
 				    NULL, &dev->slock,
 				    V4L2_BUF_TYPE_VBI_CAPTURE,
@@ -1983,6 +1976,8 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->field_size = dev->width * dev->height;
 	dev->frame_size = dev->field_size << 1;
 	dev->bytesperline = dev->width << 1;
+	dev->vbi_width = 720;
+	dev->vbi_height = 1;
 	dev->ctrl_ainput = 0;
 	dev->ctrl_freq = 960;
 	dev->std = V4L2_STD_NTSC_M;
-- 
1.7.10.4

[REVIEW PATCH 14/15] au0828: fix initial video routing.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

After loading the module the initial video routing is not setup.
Explicitly call s_input to get this right.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 1f06d97..cc1e861 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1391,20 +1391,10 @@ static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 	return 0;
 }
 
-static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
+static void au0828_s_input(struct au0828_dev *dev, int index)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
 	int i;
 
-	dprintk(1, "VIDIOC_S_INPUT in function %s, input=%d\n", __func__,
-		index);
-	if (index >= AU0828_MAX_INPUT)
-		return -EINVAL;
-	if (AUVI_INPUT(index).type == 0)
-		return -EINVAL;
-	dev->ctrl_input = index;
-
 	switch (AUVI_INPUT(index).type) {
 	case AU0828_VMUX_SVIDEO:
 		dev->input_type = AU0828_VMUX_SVIDEO;
@@ -1419,7 +1409,7 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 		dev->ctrl_ainput = 0;
 		break;
 	default:
-		dprintk(1, "VIDIOC_S_INPUT unknown input type set [%d]\n",
+		dprintk(1, "unknown input type set [%d]\n",
 			AUVI_INPUT(index).type);
 		break;
 	}
@@ -1450,6 +1440,21 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, audio, s_routing,
 			AUVI_INPUT(index).amux, 0, 0);
+}
+
+static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
+{
+	struct au0828_fh *fh = priv;
+	struct au0828_dev *dev = fh->dev;
+
+	dprintk(1, "VIDIOC_S_INPUT in function %s, input=%d\n", __func__,
+		index);
+	if (index >= AU0828_MAX_INPUT)
+		return -EINVAL;
+	if (AUVI_INPUT(index).type == 0)
+		return -EINVAL;
+	dev->ctrl_input = index;
+	au0828_s_input(dev, index);
 	return 0;
 }
 
@@ -1981,6 +1986,7 @@ int au0828_analog_register(struct au0828_dev *dev,
 	dev->ctrl_ainput = 0;
 	dev->ctrl_freq = 960;
 	dev->std = V4L2_STD_NTSC_M;
+	au0828_s_input(dev, 0);
 
 	/* allocate and fill v4l2 video struct */
 	dev->vdev = video_device_alloc();
-- 
1.7.10.4

[REVIEW PATCH 15/15] au0828: improve firmware loading & locking.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

- open/close/read and poll need to take the core lock as well.
- when the tuner goes to sleep we should set std_set_in_tuner_core
  to 0 since the tuner loses the firmware at that time.
- initialize the tuner if std_set_in_tuner_core == 0 whenever:
  1) g/s_tuner, s_std or s_frequency is called
  2) read or poll is called
  3) streamon is called

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/usb/au0828/au0828-video.c |   66 ++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index cc1e861..1aee330 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -998,11 +998,17 @@ static int au0828_v4l2_open(struct file *filp)
 	v4l2_fh_init(&fh->fh, vdev);
 	filp->private_data = fh;
 
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && dev->users == 0) {
+	if (mutex_lock_interruptible(&dev->lock)) {
+		kfree(fh);
+		return -ERESTARTSYS;
+	}
+	if (dev->users == 0) {
 		/* set au0828 interface0 to AS5 here again */
 		ret = usb_set_interface(dev->usbdev, 0, 5);
 		if (ret < 0) {
+			mutex_unlock(&dev->lock);
 			printk(KERN_INFO "Au0828 can't set alternate to 5!\n");
+			kfree(fh);
 			return -EBUSY;
 		}
 
@@ -1017,6 +1023,7 @@ static int au0828_v4l2_open(struct file *filp)
 	}
 
 	dev->users++;
+	mutex_unlock(&dev->lock);
 
 	videobuf_queue_vmalloc_init(&fh->vb_vidq, &au0828_video_qops,
 				    NULL, &dev->slock,
@@ -1044,6 +1051,7 @@ static int au0828_v4l2_close(struct file *filp)
 
 	v4l2_fh_del(&fh->fh);
 	v4l2_fh_exit(&fh->fh);
+	mutex_lock(&dev->lock);
 	if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
 		/* Cancel timeout thread in case they didn't call streamoff */
 		dev->vid_timeout_running = 0;
@@ -1069,6 +1077,7 @@ static int au0828_v4l2_close(struct file *filp)
 
 		/* Save some power by putting tuner to sleep */
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
+		dev->std_set_in_tuner_core = 0;
 
 		/* When close the device, set the usb intf0 into alt0 to free
 		   USB bandwidth */
@@ -1076,6 +1085,7 @@ static int au0828_v4l2_close(struct file *filp)
 		if (ret < 0)
 			printk(KERN_INFO "Au0828 can't set alternate to 0!\n");
 	}
+	mutex_unlock(&dev->lock);
 
 	videobuf_mmap_free(&fh->vb_vidq);
 	videobuf_mmap_free(&fh->vb_vbiq);
@@ -1085,6 +1095,26 @@ static int au0828_v4l2_close(struct file *filp)
 	return 0;
 }
 
+/* Must be called with dev->lock held */
+static void au0828_init_tuner(struct au0828_dev *dev)
+{
+	struct v4l2_frequency f = {
+		.frequency = dev->ctrl_freq,
+		.type = V4L2_TUNER_ANALOG_TV,
+	};
+
+	if (dev->std_set_in_tuner_core)
+		return;
+	dev->std_set_in_tuner_core = 1;
+	i2c_gate_ctrl(dev, 1);
+	/* If we've never sent the standard in tuner core, do so now.
+	   We don't do this at device probe because we don't want to
+	   incur the cost of a firmware load */
+	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->std);
+	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
+	i2c_gate_ctrl(dev, 0);
+}
+
 static ssize_t au0828_v4l2_read(struct file *filp, char __user *buf,
 				size_t count, loff_t *pos)
 {
@@ -1096,6 +1126,11 @@ static ssize_t au0828_v4l2_read(struct file *filp, char __user *buf,
 	if (rc < 0)
 		return rc;
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
+	au0828_init_tuner(dev);
+	mutex_unlock(&dev->lock);
+
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		if (res_locked(dev, AU0828_RESOURCE_VIDEO))
 			return -EBUSY;
@@ -1136,6 +1171,11 @@ static unsigned int au0828_v4l2_poll(struct file *filp, poll_table *wait)
 	if (!(req_events & (POLLIN | POLLRDNORM)))
 		return res;
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
+	au0828_init_tuner(dev);
+	mutex_unlock(&dev->lock);
+
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		if (!res_get(fh, AU0828_RESOURCE_VIDEO))
 			return POLLERR;
@@ -1319,6 +1359,10 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
 
+	dev->std = *norm;
+
+	au0828_init_tuner(dev);
+
 	i2c_gate_ctrl(dev, 1);
 
 	/* FIXME: when we support something other than NTSC, we are going to
@@ -1326,10 +1370,8 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *norm)
 	   buffer, which is currently hardcoded at 720x480 */
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, *norm);
-	dev->std_set_in_tuner_core = 1;
 
 	i2c_gate_ctrl(dev, 0);
-	dev->std = *norm;
 
 	return 0;
 }
@@ -1506,7 +1548,11 @@ static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 		return -EINVAL;
 
 	strcpy(t->name, "Auvitek tuner");
+
+	au0828_init_tuner(dev);
+	i2c_gate_ctrl(dev, 1);
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_tuner, t);
+	i2c_gate_ctrl(dev, 0);
 	return 0;
 }
 
@@ -1519,10 +1565,9 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 	if (t->index != 0)
 		return -EINVAL;
 
+	au0828_init_tuner(dev);
 	i2c_gate_ctrl(dev, 1);
-
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
-
 	i2c_gate_ctrl(dev, 0);
 
 	dprintk(1, "VIDIOC_S_TUNER: signal = %x, afc = %x\n", t->signal,
@@ -1553,17 +1598,9 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	if (freq->tuner != 0)
 		return -EINVAL;
 
+	au0828_init_tuner(dev);
 	i2c_gate_ctrl(dev, 1);
 
-	if (dev->std_set_in_tuner_core == 0) {
-		/* If we've never sent the standard in tuner core, do so now.
-		   We don't do this at device probe because we don't want to
-		   incur the cost of a firmware load */
-		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std,
-				     dev->vdev->tvnorms);
-		dev->std_set_in_tuner_core = 1;
-	}
-
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, freq);
 	/* Get the actual set (and possibly clamped) frequency */
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, g_frequency, freq);
@@ -1662,6 +1699,7 @@ static int vidioc_streamon(struct file *file, void *priv,
 	if (unlikely(!res_get(fh, get_ressource(fh))))
 		return -EBUSY;
 
+	au0828_init_tuner(dev);
 	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		au0828_analog_stream_enable(dev);
 		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1);
-- 
1.7.10.4

Re: [REVIEW PATCH 11/15] au0828: fix disconnect sequence.

[Devin Heitmueller]

On Mon, Mar 11, 2013 at 5:00 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The driver crashed when the device was disconnected while an application
> still had a device node open. Fixed by using the release() callback of struct
> v4l2_device.

This is all obviously good stuff.  I actually spent a couple of days
working through various disconnect scenarios, but hadn't had a chance
to do a PULL request.  I will review my tree and see if you missed any
other cases that I took care of.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [REVIEW PATCH 11/15] au0828: fix disconnect sequence.

[Hans Verkuil]

On Tue March 12 2013 03:05:50 Devin Heitmueller wrote:
> On Mon, Mar 11, 2013 at 5:00 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > The driver crashed when the device was disconnected while an application
> > still had a device node open. Fixed by using the release() callback of struct
> > v4l2_device.
> 
> This is all obviously good stuff.  I actually spent a couple of days
> working through various disconnect scenarios, but hadn't had a chance
> to do a PULL request.  I will review my tree and see if you missed any
> other cases that I took care of.

I want to make a pull request for this. Can I have your Acked-by or do you
want to look at this some more?

Regards,

	Hans

Re: [REVIEW PATCH 11/15] au0828: fix disconnect sequence.

[Devin Heitmueller]

On Wed, Mar 20, 2013 at 3:20 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> I want to make a pull request for this. Can I have your Acked-by or do you
> want to look at this some more?

I *looked* at all the patches, and they all look fine.  That said, I
haven't actually installed them at all and seen if anything got
broken.  The logic is so convoluted that it's entirely possible there
is breakage that wouldn't be obvious simply by reviewing the patches
without actual testing with real application (and no, v4l-2ctl and
v4l2-compliance do *not* count as real applications).

Did you try the resulting patches with anything other than
v4l2-compliance/v4l2-ctl?  tvtime?  xawtv?  mythtv?

Hence, for what it's worth:

Reviewed-by: Devin Heitmueller <dheitmueller@kernellabs.com>

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com