[PATCH/RFC 00/13 v3] Converting soc_camera to the control framework

[PATCH/RFC 00/13 v3] Converting soc_camera to the control framework

[Guennadi Liakhovetski]

This is a re-send of the patch-series by Hans Verkuil, partially modified 
and tested by me. The original patch-series can be found at

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

Significant changes has been applied to patches 1 and 2. Unfortunately, 
Hans didn't address my comments to his original submission, so, those 
issues had to be fixed here too. This (almost) applies to the same base, 
as the above tree. I pushed all my for-3.2 and Hans' relevant patches to

git://linuxtv.org/gliakhovetski/v4l-dvb.git rc1-for-3.2

History:
v3: my changes
v2: above git-tree by Hans
v1: Hans' original posts of 12.01.2011

Hans Verkuil (13):
  soc_camera: add control handler support
  sh_mobile_ceu_camera: implement the control handler.
  ov9640: convert to the control framework.
  ov772x: convert to the control framework.
  rj54n1cb0c: convert to the control framework.
  mt9v022: convert to the control framework.
  ov2640: convert to the control framework.
  ov6650: convert to the control framework.
  ov9740: convert to the control framework.
  mt9m001: convert to the control framework.
  mt9m111: convert to the control framework.
  mt9t031: convert to the control framework.
  soc_camera: remove the now obsolete struct soc_camera_ops

 drivers/media/video/imx074.c               |    1 -
 drivers/media/video/mt9m001.c              |  218 ++++++----------
 drivers/media/video/mt9m111.c              |  203 ++++-----------
 drivers/media/video/mt9t031.c              |  252 +++++++------------
 drivers/media/video/mt9t112.c              |    2 -
 drivers/media/video/mt9v022.c              |  265 ++++++++------------
 drivers/media/video/ov2640.c               |   90 ++-----
 drivers/media/video/ov6650.c               |  381 +++++++++-------------------
 drivers/media/video/ov772x.c               |  114 +++------
 drivers/media/video/ov9640.c               |  119 +++------
 drivers/media/video/ov9640.h               |    4 +-
 drivers/media/video/ov9740.c               |   83 ++----
 drivers/media/video/rj54n1cb0c.c           |  141 +++--------
 drivers/media/video/sh_mobile_ceu_camera.c |   91 +++----
 drivers/media/video/soc_camera.c           |   94 ++------
 drivers/media/video/soc_camera_platform.c  |    2 -
 drivers/media/video/tw9910.c               |    1 -
 include/media/soc_camera.h                 |   24 +--
 18 files changed, 659 insertions(+), 1426 deletions(-)

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 01/13 v3] soc_camera: add control handler support

[Guennadi Liakhovetski]

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

The soc_camera framework is switched over to use the control framework.
After this patch none of the controls in subdevs or host drivers are available,
until those drivers are also converted to the control framework.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: moved code around, fixed problems]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/soc_camera.c |   94 ++++++++------------------------------
 include/media/soc_camera.h       |    2 +
 2 files changed, 21 insertions(+), 75 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index f0996de..6bd6fd9 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -554,6 +554,7 @@ static int soc_camera_open(struct file *file)
 			if (ret < 0)
 				goto einitvb;
 		}
+		v4l2_ctrl_handler_setup(&icd->ctrl_handler);
 	}
 
 	file->private_data = icd;
@@ -823,78 +824,6 @@ static int soc_camera_streamoff(struct file *file, void *priv,
 	return 0;
 }
 
-static int soc_camera_queryctrl(struct file *file, void *priv,
-				struct v4l2_queryctrl *qc)
-{
-	struct soc_camera_device *icd = file->private_data;
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	int i;
-
-	WARN_ON(priv != file->private_data);
-
-	if (!qc->id)
-		return -EINVAL;
-
-	/* First check host controls */
-	for (i = 0; i < ici->ops->num_controls; i++)
-		if (qc->id == ici->ops->controls[i].id) {
-			memcpy(qc, &(ici->ops->controls[i]),
-				sizeof(*qc));
-			return 0;
-		}
-
-	if (!icd->ops)
-		return -EINVAL;
-
-	/* Then device controls */
-	for (i = 0; i < icd->ops->num_controls; i++)
-		if (qc->id == icd->ops->controls[i].id) {
-			memcpy(qc, &(icd->ops->controls[i]),
-				sizeof(*qc));
-			return 0;
-		}
-
-	return -EINVAL;
-}
-
-static int soc_camera_g_ctrl(struct file *file, void *priv,
-			     struct v4l2_control *ctrl)
-{
-	struct soc_camera_device *icd = file->private_data;
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	int ret;
-
-	WARN_ON(priv != file->private_data);
-
-	if (ici->ops->get_ctrl) {
-		ret = ici->ops->get_ctrl(icd, ctrl);
-		if (ret != -ENOIOCTLCMD)
-			return ret;
-	}
-
-	return v4l2_subdev_call(sd, core, g_ctrl, ctrl);
-}
-
-static int soc_camera_s_ctrl(struct file *file, void *priv,
-			     struct v4l2_control *ctrl)
-{
-	struct soc_camera_device *icd = file->private_data;
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	int ret;
-
-	WARN_ON(priv != file->private_data);
-
-	if (ici->ops->set_ctrl) {
-		ret = ici->ops->set_ctrl(icd, ctrl);
-		if (ret != -ENOIOCTLCMD)
-			return ret;
-	}
-
-	return v4l2_subdev_call(sd, core, s_ctrl, ctrl);
-}
-
 static int soc_camera_cropcap(struct file *file, void *fh,
 			      struct v4l2_cropcap *a)
 {
@@ -1097,6 +1026,17 @@ static int soc_camera_probe(struct soc_camera_device *icd)
 
 	dev_info(icd->pdev, "Probing %s\n", dev_name(icd->pdev));
 
+	/*
+	 * Currently the subdev with the largest number of controls (13) is
+	 * ov6550. So let's pick 16 as a hint for the control handler. Note
+	 * that this is a hint only: too large and you waste some memory, too
+	 * small and there is a (very) small performance hit when looking up
+	 * controls in the internal hash.
+	 */
+	ret = v4l2_ctrl_handler_init(&icd->ctrl_handler, 16);
+	if (ret < 0)
+		return ret;
+
 	ret = regulator_bulk_get(icd->pdev, icl->num_regulators,
 				 icl->regulators);
 	if (ret < 0)
@@ -1157,6 +1097,9 @@ static int soc_camera_probe(struct soc_camera_device *icd)
 	sd = soc_camera_to_subdev(icd);
 	sd->grp_id = (long)icd;
 
+	if (v4l2_ctrl_add_handler(&icd->ctrl_handler, sd->ctrl_handler))
+		goto ectrl;
+
 	/* At this point client .probe() should have run already */
 	ret = soc_camera_init_user_formats(icd);
 	if (ret < 0)
@@ -1201,6 +1144,7 @@ evidstart:
 	mutex_unlock(&icd->video_lock);
 	soc_camera_free_user_formats(icd);
 eiufmt:
+ectrl:
 	if (icl->board_info) {
 		soc_camera_free_i2c(icd);
 	} else {
@@ -1218,6 +1162,7 @@ eadd:
 epower:
 	regulator_bulk_free(icl->num_regulators, icl->regulators);
 ereg:
+	v4l2_ctrl_handler_free(&icd->ctrl_handler);
 	return ret;
 }
 
@@ -1232,6 +1177,7 @@ static int soc_camera_remove(struct soc_camera_device *icd)
 
 	BUG_ON(!icd->parent);
 
+	v4l2_ctrl_handler_free(&icd->ctrl_handler);
 	if (vdev) {
 		video_unregister_device(vdev);
 		icd->vdev = NULL;
@@ -1439,9 +1385,6 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
 	.vidioc_prepare_buf	 = soc_camera_prepare_buf,
 	.vidioc_streamon	 = soc_camera_streamon,
 	.vidioc_streamoff	 = soc_camera_streamoff,
-	.vidioc_queryctrl	 = soc_camera_queryctrl,
-	.vidioc_g_ctrl		 = soc_camera_g_ctrl,
-	.vidioc_s_ctrl		 = soc_camera_s_ctrl,
 	.vidioc_cropcap		 = soc_camera_cropcap,
 	.vidioc_g_crop		 = soc_camera_g_crop,
 	.vidioc_s_crop		 = soc_camera_s_crop,
@@ -1470,6 +1413,7 @@ static int video_dev_create(struct soc_camera_device *icd)
 	vdev->ioctl_ops		= &soc_camera_ioctl_ops;
 	vdev->release		= video_device_release;
 	vdev->tvnorms		= V4L2_STD_UNKNOWN;
+	vdev->ctrl_handler	= &icd->ctrl_handler;
 	vdev->lock		= &icd->video_lock;
 
 	icd->vdev = vdev;
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 1864e22..2e15e17 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -19,6 +19,7 @@
 #include <linux/videodev2.h>
 #include <media/videobuf-core.h>
 #include <media/videobuf2-core.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 
 struct file;
@@ -40,6 +41,7 @@ struct soc_camera_device {
 	struct soc_camera_sense *sense;	/* See comment in struct definition */
 	struct soc_camera_ops *ops;
 	struct video_device *vdev;
+	struct v4l2_ctrl_handler ctrl_handler;
 	const struct soc_camera_format_xlate *current_fmt;
 	struct soc_camera_format_xlate *user_formats;
 	int num_user_formats;
-- 
1.7.2.5

[PATCH 02/13 v3] sh_mobile_ceu_camera: implement the control handler.

[Guennadi Liakhovetski]

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

And since this is the last and only host driver that uses controls, also
remove the now obsolete control fields from soc_camera.h.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: moved code around, fixed problems]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/sh_mobile_ceu_camera.c |   91 ++++++++++++----------------
 include/media/soc_camera.h                 |    4 -
 2 files changed, 38 insertions(+), 57 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 8711aa8..955947a 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -997,6 +997,38 @@ static bool sh_mobile_ceu_packing_supported(const struct soc_mbus_pixelfmt *fmt)
 
 static int client_g_rect(struct v4l2_subdev *sd, struct v4l2_rect *rect);
 
+static struct soc_camera_device *ctrl_to_icd(struct v4l2_ctrl *ctrl)
+{
+	return container_of(ctrl->handler, struct soc_camera_device,
+							ctrl_handler);
+}
+
+static int sh_mobile_ceu_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct soc_camera_device *icd = ctrl_to_icd(ctrl);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+
+	switch (ctrl->id) {
+	case V4L2_CID_SHARPNESS:
+		switch (icd->current_fmt->host_fmt->fourcc) {
+		case V4L2_PIX_FMT_NV12:
+		case V4L2_PIX_FMT_NV21:
+		case V4L2_PIX_FMT_NV16:
+		case V4L2_PIX_FMT_NV61:
+			ceu_write(pcdev, CLFCR, !ctrl->val);
+			return 0;
+		}
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops sh_mobile_ceu_ctrl_ops = {
+	.s_ctrl = sh_mobile_ceu_s_ctrl,
+};
+
 static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, unsigned int idx,
 				     struct soc_camera_format_xlate *xlate)
 {
@@ -1033,6 +1065,12 @@ static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, unsigned int
 		struct v4l2_rect rect;
 		int shift = 0;
 
+		/* Add our control */
+		v4l2_ctrl_new_std(&icd->ctrl_handler, &sh_mobile_ceu_ctrl_ops,
+				  V4L2_CID_SHARPNESS, 0, 1, 1, 0);
+		if (icd->ctrl_handler.error)
+			return icd->ctrl_handler.error;
+
 		/* FIXME: subwindow is lost between close / open */
 
 		/* Cache current client geometry */
@@ -1961,55 +1999,6 @@ static int sh_mobile_ceu_init_videobuf(struct vb2_queue *q,
 	return vb2_queue_init(q);
 }
 
-static int sh_mobile_ceu_get_ctrl(struct soc_camera_device *icd,
-				  struct v4l2_control *ctrl)
-{
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-	u32 val;
-
-	switch (ctrl->id) {
-	case V4L2_CID_SHARPNESS:
-		val = ceu_read(pcdev, CLFCR);
-		ctrl->value = val ^ 1;
-		return 0;
-	}
-	return -ENOIOCTLCMD;
-}
-
-static int sh_mobile_ceu_set_ctrl(struct soc_camera_device *icd,
-				  struct v4l2_control *ctrl)
-{
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-
-	switch (ctrl->id) {
-	case V4L2_CID_SHARPNESS:
-		switch (icd->current_fmt->host_fmt->fourcc) {
-		case V4L2_PIX_FMT_NV12:
-		case V4L2_PIX_FMT_NV21:
-		case V4L2_PIX_FMT_NV16:
-		case V4L2_PIX_FMT_NV61:
-			ceu_write(pcdev, CLFCR, !ctrl->value);
-			return 0;
-		}
-		return -EINVAL;
-	}
-	return -ENOIOCTLCMD;
-}
-
-static const struct v4l2_queryctrl sh_mobile_ceu_controls[] = {
-	{
-		.id		= V4L2_CID_SHARPNESS,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Low-pass filter",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
 static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.owner		= THIS_MODULE,
 	.add		= sh_mobile_ceu_add_device,
@@ -2021,14 +2010,10 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.set_livecrop	= sh_mobile_ceu_set_livecrop,
 	.set_fmt	= sh_mobile_ceu_set_fmt,
 	.try_fmt	= sh_mobile_ceu_try_fmt,
-	.set_ctrl	= sh_mobile_ceu_set_ctrl,
-	.get_ctrl	= sh_mobile_ceu_get_ctrl,
 	.poll		= sh_mobile_ceu_poll,
 	.querycap	= sh_mobile_ceu_querycap,
 	.set_bus_param	= sh_mobile_ceu_set_bus_param,
 	.init_videobuf2	= sh_mobile_ceu_init_videobuf,
-	.controls	= sh_mobile_ceu_controls,
-	.num_controls	= ARRAY_SIZE(sh_mobile_ceu_controls),
 };
 
 struct bus_wait {
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 2e15e17..d41b8bd 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -96,14 +96,10 @@ struct soc_camera_host_ops {
 	int (*reqbufs)(struct soc_camera_device *, struct v4l2_requestbuffers *);
 	int (*querycap)(struct soc_camera_host *, struct v4l2_capability *);
 	int (*set_bus_param)(struct soc_camera_device *, __u32);
-	int (*get_ctrl)(struct soc_camera_device *, struct v4l2_control *);
-	int (*set_ctrl)(struct soc_camera_device *, struct v4l2_control *);
 	int (*get_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
 	int (*set_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
 	int (*enum_fsizes)(struct soc_camera_device *, struct v4l2_frmsizeenum *);
 	unsigned int (*poll)(struct file *, poll_table *);
-	const struct v4l2_queryctrl *controls;
-	int num_controls;
 };
 
 #define SOCAM_SENSOR_INVERT_PCLK	(1 << 0)
-- 
1.7.2.5

[PATCH 03/13 v3] ov9640: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/ov9640.c |  119 +++++++++++++-----------------------------
 drivers/media/video/ov9640.h |    4 +-
 2 files changed, 38 insertions(+), 85 deletions(-)

diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
index 352a8fb..12d33a9 100644
--- a/drivers/media/video/ov9640.c
+++ b/drivers/media/video/ov9640.c
@@ -30,6 +30,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-common.h>
+#include <media/v4l2-ctrls.h>
 
 #include "ov9640.h"
 
@@ -164,27 +165,6 @@ static enum v4l2_mbus_pixelcode ov9640_codes[] = {
 	V4L2_MBUS_FMT_RGB565_2X8_LE,
 };
 
-static const struct v4l2_queryctrl ov9640_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
 /* read a register */
 static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
 {
@@ -286,52 +266,25 @@ static int ov9640_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-/* Get status of additional camera capabilities */
-static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct ov9640_priv *priv = to_ov9640_sensor(sd);
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		ctrl->value = priv->flag_vflip;
-		break;
-	case V4L2_CID_HFLIP:
-		ctrl->value = priv->flag_hflip;
-		break;
-	}
-	return 0;
-}
-
 /* Set status of additional camera capabilities */
-static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov9640_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov9640_priv *priv = to_ov9640_sensor(sd);
-
-	int ret = 0;
+	struct ov9640_priv *priv = container_of(ctrl->handler, struct ov9640_priv, hdl);
+	struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		priv->flag_vflip = ctrl->value;
-		if (ctrl->value)
-			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+		if (ctrl->val)
+			return ov9640_reg_rmw(client, OV9640_MVFP,
 							OV9640_MVFP_V, 0);
-		else
-			ret = ov9640_reg_rmw(client, OV9640_MVFP,
-							0, OV9640_MVFP_V);
-		break;
+		return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_V);
 	case V4L2_CID_HFLIP:
-		priv->flag_hflip = ctrl->value;
-		if (ctrl->value)
-			ret = ov9640_reg_rmw(client, OV9640_MVFP,
+		if (ctrl->val)
+			return ov9640_reg_rmw(client, OV9640_MVFP,
 							OV9640_MVFP_H, 0);
-		else
-			ret = ov9640_reg_rmw(client, OV9640_MVFP,
-							0, OV9640_MVFP_H);
-		break;
+		return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_H);
 	}
-
-	return ret;
+	return -EINVAL;
 }
 
 /* Get chip identification */
@@ -643,20 +596,14 @@ static int ov9640_video_probe(struct soc_camera_device *icd,
 	 */
 
 	ret = ov9640_reg_read(client, OV9640_PID, &pid);
+	if (!ret)
+		ret = ov9640_reg_read(client, OV9640_VER, &ver);
+	if (!ret)
+		ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
+	if (!ret)
+		ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
 	if (ret)
-		goto err;
-
-	ret = ov9640_reg_read(client, OV9640_VER, &ver);
-	if (ret)
-		goto err;
-
-	ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
-	if (ret)
-		goto err;
-
-	ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
-	if (ret)
-		goto err;
+		return ret;
 
 	switch (VERSION(pid, ver)) {
 	case OV9640_V2:
@@ -670,25 +617,20 @@ static int ov9640_video_probe(struct soc_camera_device *icd,
 		break;
 	default:
 		dev_err(&client->dev, "Product ID error %x:%x\n", pid, ver);
-		ret = -ENODEV;
-		goto err;
+		return -ENODEV;
 	}
 
 	dev_info(&client->dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
 		 devname, pid, ver, midh, midl);
 
-err:
-	return ret;
+	return v4l2_ctrl_handler_setup(&priv->hdl);
 }
 
-static struct soc_camera_ops ov9640_ops = {
-	.controls		= ov9640_controls,
-	.num_controls		= ARRAY_SIZE(ov9640_controls),
+static const struct v4l2_ctrl_ops ov9640_ctrl_ops = {
+	.s_ctrl = ov9640_s_ctrl,
 };
 
 static struct v4l2_subdev_core_ops ov9640_core_ops = {
-	.g_ctrl			= ov9640_g_ctrl,
-	.s_ctrl			= ov9640_s_ctrl,
 	.g_chip_ident		= ov9640_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register		= ov9640_get_register,
@@ -760,12 +702,23 @@ static int ov9640_probe(struct i2c_client *client,
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov9640_subdev_ops);
 
-	icd->ops	= &ov9640_ops;
+	v4l2_ctrl_handler_init(&priv->hdl, 2);
+	v4l2_ctrl_new_std(&priv->hdl, &ov9640_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov9640_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	priv->subdev.ctrl_handler = &priv->hdl;
+	if (priv->hdl.error) {
+		int err = priv->hdl.error;
+
+		kfree(priv);
+		return err;
+	}
 
 	ret = ov9640_video_probe(icd, client);
 
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	}
 
@@ -777,6 +730,8 @@ static int ov9640_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct ov9640_priv *priv = to_ov9640_sensor(sd);
 
+	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_ctrl_handler_free(&priv->hdl);
 	kfree(priv);
 	return 0;
 }
diff --git a/drivers/media/video/ov9640.h b/drivers/media/video/ov9640.h
index f8a51b7..6b33a97 100644
--- a/drivers/media/video/ov9640.h
+++ b/drivers/media/video/ov9640.h
@@ -198,12 +198,10 @@ struct ov9640_reg {
 
 struct ov9640_priv {
 	struct v4l2_subdev		subdev;
+	struct v4l2_ctrl_handler	hdl;
 
 	int				model;
 	int				revision;
-
-	bool				flag_vflip;
-	bool				flag_hflip;
 };
 
 #endif	/* __DRIVERS_MEDIA_VIDEO_OV9640_H__ */
-- 
1.7.2.5

[PATCH 04/13 v3] ov772x: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/ov772x.c |  114 +++++++++++++----------------------------
 1 files changed, 36 insertions(+), 78 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index f77e899..9b54042 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -25,6 +25,7 @@
 #include <media/ov772x.h>
 #include <media/soc_camera.h>
 #include <media/soc_mediabus.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-subdev.h>
 
@@ -401,6 +402,7 @@ struct ov772x_win_size {
 
 struct ov772x_priv {
 	struct v4l2_subdev                subdev;
+	struct v4l2_ctrl_handler	  hdl;
 	struct ov772x_camera_info        *info;
 	const struct ov772x_color_format *cfmt;
 	const struct ov772x_win_size     *win;
@@ -518,36 +520,6 @@ static const struct ov772x_win_size ov772x_win_qvga = {
 	.regs     = ov772x_qvga_regs,
 };
 
-static const struct v4l2_queryctrl ov772x_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_BAND_STOP_FILTER,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Band-stop filter",
-		.minimum	= 0,
-		.maximum	= 256,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
 /*
  * general function
  */
@@ -621,52 +593,30 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-static int ov772x_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		ctrl->value = priv->flag_vflip;
-		break;
-	case V4L2_CID_HFLIP:
-		ctrl->value = priv->flag_hflip;
-		break;
-	case V4L2_CID_BAND_STOP_FILTER:
-		ctrl->value = priv->band_filter;
-		break;
-	}
-	return 0;
-}
-
-static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct ov772x_priv *priv = container_of(ctrl->handler,
+						struct ov772x_priv, hdl);
+	struct v4l2_subdev *sd = &priv->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
 	int ret = 0;
 	u8 val;
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		val = ctrl->value ? VFLIP_IMG : 0x00;
-		priv->flag_vflip = ctrl->value;
+		val = ctrl->val ? VFLIP_IMG : 0x00;
+		priv->flag_vflip = ctrl->val;
 		if (priv->info->flags & OV772X_FLAG_VFLIP)
 			val ^= VFLIP_IMG;
-		ret = ov772x_mask_set(client, COM3, VFLIP_IMG, val);
-		break;
+		return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
 	case V4L2_CID_HFLIP:
-		val = ctrl->value ? HFLIP_IMG : 0x00;
-		priv->flag_hflip = ctrl->value;
+		val = ctrl->val ? HFLIP_IMG : 0x00;
+		priv->flag_hflip = ctrl->val;
 		if (priv->info->flags & OV772X_FLAG_HFLIP)
 			val ^= HFLIP_IMG;
-		ret = ov772x_mask_set(client, COM3, HFLIP_IMG, val);
-		break;
+		return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
 	case V4L2_CID_BAND_STOP_FILTER:
-		if ((unsigned)ctrl->value > 256)
-			ctrl->value = 256;
-		if (ctrl->value == priv->band_filter)
-			break;
-		if (!ctrl->value) {
+		if (!ctrl->val) {
 			/* Switch the filter off, it is on now */
 			ret = ov772x_mask_set(client, BDBASE, 0xff, 0xff);
 			if (!ret)
@@ -674,7 +624,7 @@ static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 						      BNDF_ON_OFF, 0);
 		} else {
 			/* Switch the filter on, set AEC low limit */
-			val = 256 - ctrl->value;
+			val = 256 - ctrl->val;
 			ret = ov772x_mask_set(client, COM8,
 					      BNDF_ON_OFF, BNDF_ON_OFF);
 			if (!ret)
@@ -682,11 +632,11 @@ static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 						      0xff, val);
 		}
 		if (!ret)
-			priv->band_filter = ctrl->value;
-		break;
+			priv->band_filter = ctrl->val;
+		return ret;
 	}
 
-	return ret;
+	return -EINVAL;
 }
 
 static int ov772x_g_chip_ident(struct v4l2_subdev *sd,
@@ -1042,18 +992,14 @@ static int ov772x_video_probe(struct soc_camera_device *icd,
 		 ver,
 		 i2c_smbus_read_byte_data(client, MIDH),
 		 i2c_smbus_read_byte_data(client, MIDL));
-
-	return 0;
+	return v4l2_ctrl_handler_setup(&priv->hdl);
 }
 
-static struct soc_camera_ops ov772x_ops = {
-	.controls		= ov772x_controls,
-	.num_controls		= ARRAY_SIZE(ov772x_controls),
+static const struct v4l2_ctrl_ops ov772x_ctrl_ops = {
+	.s_ctrl = ov772x_s_ctrl,
 };
 
 static struct v4l2_subdev_core_ops ov772x_subdev_core_ops = {
-	.g_ctrl		= ov772x_g_ctrl,
-	.s_ctrl		= ov772x_s_ctrl,
 	.g_chip_ident	= ov772x_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= ov772x_g_register,
@@ -1139,12 +1085,24 @@ static int ov772x_probe(struct i2c_client *client,
 	priv->info = icl->priv;
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops);
+	v4l2_ctrl_handler_init(&priv->hdl, 3);
+	v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+			V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
+	priv->subdev.ctrl_handler = &priv->hdl;
+	if (priv->hdl.error) {
+		int err = priv->hdl.error;
 
-	icd->ops		= &ov772x_ops;
+		kfree(priv);
+		return err;
+	}
 
 	ret = ov772x_video_probe(icd, client);
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	}
 
@@ -1154,9 +1112,9 @@ static int ov772x_probe(struct i2c_client *client,
 static int ov772x_remove(struct i2c_client *client)
 {
 	struct ov772x_priv *priv = to_ov772x(client);
-	struct soc_camera_device *icd = client->dev.platform_data;
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_ctrl_handler_free(&priv->hdl);
 	kfree(priv);
 	return 0;
 }
-- 
1.7.2.5

[PATCH 05/13 v3] rj54n1cb0c: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/rj54n1cb0c.c |  141 ++++++++++---------------------------
 1 files changed, 38 insertions(+), 103 deletions(-)

diff --git a/drivers/media/video/rj54n1cb0c.c b/drivers/media/video/rj54n1cb0c.c
index c302211..9a87153 100644
--- a/drivers/media/video/rj54n1cb0c.c
+++ b/drivers/media/video/rj54n1cb0c.c
@@ -18,6 +18,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-subdev.h>
 #include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
 
 #define RJ54N1_DEV_CODE			0x0400
 #define RJ54N1_DEV_CODE2		0x0401
@@ -148,6 +149,7 @@ struct rj54n1_clock_div {
 
 struct rj54n1 {
 	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
 	struct rj54n1_clock_div clk_div;
 	const struct rj54n1_datafmt *fmt;
 	struct v4l2_rect rect;	/* Sensor window */
@@ -1177,132 +1179,51 @@ static int rj54n1_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
-static const struct v4l2_queryctrl rj54n1_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gain",
-		.minimum	= 0,
-		.maximum	= 127,
-		.step		= 1,
-		.default_value	= 66,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_AUTO_WHITE_BALANCE,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Auto white balance",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	},
-};
-
-static struct soc_camera_ops rj54n1_ops = {
-	.controls		= rj54n1_controls,
-	.num_controls		= ARRAY_SIZE(rj54n1_controls),
-};
-
-static int rj54n1_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int rj54n1_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct rj54n1 *rj54n1 = container_of(ctrl->handler, struct rj54n1, hdl);
+	struct v4l2_subdev *sd = &rj54n1->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct rj54n1 *rj54n1 = to_rj54n1(client);
 	int data;
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		data = reg_read(client, RJ54N1_MIRROR_STILL_MODE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !(data & 1);
-		break;
-	case V4L2_CID_HFLIP:
-		data = reg_read(client, RJ54N1_MIRROR_STILL_MODE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !(data & 2);
-		break;
-	case V4L2_CID_GAIN:
-		data = reg_read(client, RJ54N1_Y_GAIN);
-		if (data < 0)
-			return -EIO;
-
-		ctrl->value = data / 2;
-		break;
-	case V4L2_CID_AUTO_WHITE_BALANCE:
-		ctrl->value = rj54n1->auto_wb;
-		break;
-	}
-
-	return 0;
-}
-
-static int rj54n1_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	int data;
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct rj54n1 *rj54n1 = to_rj54n1(client);
-	const struct v4l2_queryctrl *qctrl;
-
-	qctrl = soc_camera_find_qctrl(&rj54n1_ops, ctrl->id);
-	if (!qctrl)
-		return -EINVAL;
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 0, 1);
 		else
 			data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 1, 1);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_HFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 0, 2);
 		else
 			data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 2, 2);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_GAIN:
-		if (ctrl->value > qctrl->maximum ||
-		    ctrl->value < qctrl->minimum)
-			return -EINVAL;
-		else if (reg_write(client, RJ54N1_Y_GAIN, ctrl->value * 2) < 0)
+		if (reg_write(client, RJ54N1_Y_GAIN, ctrl->val * 2) < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_AUTO_WHITE_BALANCE:
 		/* Auto WB area - whole image */
-		if (reg_set(client, RJ54N1_WB_SEL_WEIGHT_I, ctrl->value << 7,
+		if (reg_set(client, RJ54N1_WB_SEL_WEIGHT_I, ctrl->val << 7,
 			    0x80) < 0)
 			return -EIO;
-		rj54n1->auto_wb = ctrl->value;
-		break;
+		rj54n1->auto_wb = ctrl->val;
+		return 0;
 	}
 
-	return 0;
+	return -EINVAL;
 }
 
+static const struct v4l2_ctrl_ops rj54n1_ctrl_ops = {
+	.s_ctrl = rj54n1_s_ctrl,
+};
+
 static struct v4l2_subdev_core_ops rj54n1_subdev_core_ops = {
-	.g_ctrl		= rj54n1_g_ctrl,
-	.s_ctrl		= rj54n1_s_ctrl,
 	.g_chip_ident	= rj54n1_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= rj54n1_g_register,
@@ -1432,8 +1353,22 @@ static int rj54n1_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&rj54n1->subdev, client, &rj54n1_subdev_ops);
+	v4l2_ctrl_handler_init(&rj54n1->hdl, 4);
+	v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+			V4L2_CID_GAIN, 0, 127, 1, 66);
+	v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+			V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+	rj54n1->subdev.ctrl_handler = &rj54n1->hdl;
+	if (rj54n1->hdl.error) {
+		int err = rj54n1->hdl.error;
 
-	icd->ops		= &rj54n1_ops;
+		kfree(rj54n1);
+		return err;
+	}
 
 	rj54n1->clk_div		= clk_div;
 	rj54n1->rect.left	= RJ54N1_COLUMN_SKIP;
@@ -1449,12 +1384,11 @@ static int rj54n1_probe(struct i2c_client *client,
 
 	ret = rj54n1_video_probe(icd, client, rj54n1_priv);
 	if (ret < 0) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&rj54n1->hdl);
 		kfree(rj54n1);
 		return ret;
 	}
-
-	return ret;
+	return v4l2_ctrl_handler_setup(&rj54n1->hdl);
 }
 
 static int rj54n1_remove(struct i2c_client *client)
@@ -1463,9 +1397,10 @@ static int rj54n1_remove(struct i2c_client *client)
 	struct soc_camera_device *icd = client->dev.platform_data;
 	struct soc_camera_link *icl = to_soc_camera_link(icd);
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&rj54n1->subdev);
 	if (icl->free_bus)
 		icl->free_bus(icl);
+	v4l2_ctrl_handler_free(&rj54n1->hdl);
 	kfree(rj54n1);
 
 	return 0;
-- 
1.7.2.5

[PATCH 06/13 v3] mt9v022: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9v022.c |  265 +++++++++++++++++------------------------
 1 files changed, 107 insertions(+), 158 deletions(-)

diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 53149a7..7e2aeda 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -18,6 +18,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-subdev.h>
 #include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
 
 /*
  * mt9v022 i2c address 0x48, 0x4c, 0x58, 0x5c
@@ -101,6 +102,17 @@ static const struct mt9v022_datafmt mt9v022_monochrome_fmts[] = {
 
 struct mt9v022 {
 	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
+	struct {
+		/* exposure/auto-exposure cluster */
+		struct v4l2_ctrl *autoexposure;
+		struct v4l2_ctrl *exposure;
+	};
+	struct {
+		/* gain/auto-gain cluster */
+		struct v4l2_ctrl *autogain;
+		struct v4l2_ctrl *gain;
+	};
 	struct v4l2_rect rect;	/* Sensor window */
 	const struct mt9v022_datafmt *fmt;
 	const struct mt9v022_datafmt *fmts;
@@ -179,6 +191,8 @@ static int mt9v022_init(struct i2c_client *client)
 		ret = reg_clear(client, MT9V022_BLACK_LEVEL_CALIB_CTRL, 1);
 	if (!ret)
 		ret = reg_write(client, MT9V022_DIGITAL_TEST_PATTERN, 0);
+	if (!ret)
+		return v4l2_ctrl_handler_setup(&mt9v022->hdl);
 
 	return ret;
 }
@@ -431,215 +445,117 @@ static int mt9v022_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
-static const struct v4l2_queryctrl mt9v022_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Analog Gain",
-		.minimum	= 64,
-		.maximum	= 127,
-		.step		= 1,
-		.default_value	= 64,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_EXPOSURE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Exposure",
-		.minimum	= 1,
-		.maximum	= 255,
-		.step		= 1,
-		.default_value	= 255,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_AUTOGAIN,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Automatic Gain",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	}, {
-		.id		= V4L2_CID_EXPOSURE_AUTO,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Automatic Exposure",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	}
-};
-
-static struct soc_camera_ops mt9v022_ops = {
-	.controls		= mt9v022_controls,
-	.num_controls		= ARRAY_SIZE(mt9v022_controls),
-};
-
-static int mt9v022_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9v022_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct mt9v022 *mt9v022 = container_of(ctrl->handler,
+					       struct mt9v022, hdl);
+	struct v4l2_subdev *sd = &mt9v022->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	const struct v4l2_queryctrl *qctrl;
+	struct v4l2_ctrl *gain = mt9v022->gain;
+	struct v4l2_ctrl *exp = mt9v022->exposure;
 	unsigned long range;
 	int data;
 
-	qctrl = soc_camera_find_qctrl(&mt9v022_ops, ctrl->id);
-
 	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		data = reg_read(client, MT9V022_READ_MODE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x10);
-		break;
-	case V4L2_CID_HFLIP:
-		data = reg_read(client, MT9V022_READ_MODE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x20);
-		break;
-	case V4L2_CID_EXPOSURE_AUTO:
-		data = reg_read(client, MT9V022_AEC_AGC_ENABLE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x1);
-		break;
 	case V4L2_CID_AUTOGAIN:
-		data = reg_read(client, MT9V022_AEC_AGC_ENABLE);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x2);
-		break;
-	case V4L2_CID_GAIN:
 		data = reg_read(client, MT9V022_ANALOG_GAIN);
 		if (data < 0)
 			return -EIO;
 
-		range = qctrl->maximum - qctrl->minimum;
-		ctrl->value = ((data - 16) * range + 24) / 48 + qctrl->minimum;
-
-		break;
-	case V4L2_CID_EXPOSURE:
+		range = gain->maximum - gain->minimum;
+		gain->val = ((data - 16) * range + 24) / 48 + gain->minimum;
+		return 0;
+	case V4L2_CID_EXPOSURE_AUTO:
 		data = reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH);
 		if (data < 0)
 			return -EIO;
 
-		range = qctrl->maximum - qctrl->minimum;
-		ctrl->value = ((data - 1) * range + 239) / 479 + qctrl->minimum;
-
-		break;
+		range = exp->maximum - exp->minimum;
+		exp->val = ((data - 1) * range + 239) / 479 + exp->minimum;
+		return 0;
 	}
-	return 0;
+	return -EINVAL;
 }
 
-static int mt9v022_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9v022_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	int data;
+	struct mt9v022 *mt9v022 = container_of(ctrl->handler,
+					       struct mt9v022, hdl);
+	struct v4l2_subdev *sd = &mt9v022->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	const struct v4l2_queryctrl *qctrl;
-
-	qctrl = soc_camera_find_qctrl(&mt9v022_ops, ctrl->id);
-	if (!qctrl)
-		return -EINVAL;
+	int data;
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, MT9V022_READ_MODE, 0x10);
 		else
 			data = reg_clear(client, MT9V022_READ_MODE, 0x10);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_HFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, MT9V022_READ_MODE, 0x20);
 		else
 			data = reg_clear(client, MT9V022_READ_MODE, 0x20);
 		if (data < 0)
 			return -EIO;
-		break;
-	case V4L2_CID_GAIN:
-		/* mt9v022 has minimum == default */
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
-		else {
-			unsigned long range = qctrl->maximum - qctrl->minimum;
+		return 0;
+	case V4L2_CID_AUTOGAIN:
+		if (ctrl->val) {
+			if (reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x2) < 0)
+				return -EIO;
+		} else {
+			struct v4l2_ctrl *gain = mt9v022->gain;
+			/* mt9v022 has minimum == default */
+			unsigned long range = gain->maximum - gain->minimum;
 			/* Valid values 16 to 64, 32 to 64 must be even. */
-			unsigned long gain = ((ctrl->value - qctrl->minimum) *
+			unsigned long gain_val = ((gain->val - gain->minimum) *
 					      48 + range / 2) / range + 16;
-			if (gain >= 32)
-				gain &= ~1;
+
+			if (gain_val >= 32)
+				gain_val &= ~1;
+
 			/*
 			 * The user wants to set gain manually, hope, she
 			 * knows, what she's doing... Switch AGC off.
 			 */
-
 			if (reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x2) < 0)
 				return -EIO;
 
 			dev_dbg(&client->dev, "Setting gain from %d to %lu\n",
-				reg_read(client, MT9V022_ANALOG_GAIN), gain);
-			if (reg_write(client, MT9V022_ANALOG_GAIN, gain) < 0)
+				reg_read(client, MT9V022_ANALOG_GAIN), gain_val);
+			if (reg_write(client, MT9V022_ANALOG_GAIN, gain_val) < 0)
 				return -EIO;
 		}
-		break;
-	case V4L2_CID_EXPOSURE:
-		/* mt9v022 has maximum == default */
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
-		else {
-			unsigned long range = qctrl->maximum - qctrl->minimum;
-			unsigned long shutter = ((ctrl->value - qctrl->minimum) *
-						 479 + range / 2) / range + 1;
+		return 0;
+	case V4L2_CID_EXPOSURE_AUTO:
+		if (ctrl->val == V4L2_EXPOSURE_AUTO) {
+			data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x1);
+		} else {
+			struct v4l2_ctrl *exp = mt9v022->exposure;
+			unsigned long range = exp->maximum - exp->minimum;
+			unsigned long shutter = ((exp->val - exp->minimum) *
+					479 + range / 2) / range + 1;
+
 			/*
 			 * The user wants to set shutter width manually, hope,
 			 * she knows, what she's doing... Switch AEC off.
 			 */
-
-			if (reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1) < 0)
+			data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1);
+			if (data < 0)
 				return -EIO;
-
 			dev_dbg(&client->dev, "Shutter width from %d to %lu\n",
-				reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH),
-				shutter);
+					reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH),
+					shutter);
 			if (reg_write(client, MT9V022_TOTAL_SHUTTER_WIDTH,
-				      shutter) < 0)
+						shutter) < 0)
 				return -EIO;
 		}
-		break;
-	case V4L2_CID_AUTOGAIN:
-		if (ctrl->value)
-			data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x2);
-		else
-			data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x2);
-		if (data < 0)
-			return -EIO;
-		break;
-	case V4L2_CID_EXPOSURE_AUTO:
-		if (ctrl->value)
-			data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x1);
-		else
-			data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1);
-		if (data < 0)
-			return -EIO;
-		break;
+		return 0;
 	}
-	return 0;
+	return -EINVAL;
 }
 
 /*
@@ -752,9 +668,12 @@ static int mt9v022_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
 	return 0;
 }
 
+static const struct v4l2_ctrl_ops mt9v022_ctrl_ops = {
+	.g_volatile_ctrl = mt9v022_g_volatile_ctrl,
+	.s_ctrl = mt9v022_s_ctrl,
+};
+
 static struct v4l2_subdev_core_ops mt9v022_subdev_core_ops = {
-	.g_ctrl		= mt9v022_g_ctrl,
-	.s_ctrl		= mt9v022_s_ctrl,
 	.g_chip_ident	= mt9v022_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= mt9v022_g_register,
@@ -906,10 +825,39 @@ static int mt9v022_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&mt9v022->subdev, client, &mt9v022_subdev_ops);
+	v4l2_ctrl_handler_init(&mt9v022->hdl, 6);
+	v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	mt9v022->autogain = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+			V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
+	mt9v022->gain = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+			V4L2_CID_GAIN, 0, 127, 1, 64);
+
+	/*
+	 * Simulated autoexposure. If enabled, we calculate shutter width
+	 * ourselves in the driver based on vertical blanking and frame width
+	 */
+	mt9v022->autoexposure = v4l2_ctrl_new_std_menu(&mt9v022->hdl,
+			&mt9v022_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+			V4L2_EXPOSURE_AUTO);
+	mt9v022->exposure = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+			V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+
+	mt9v022->subdev.ctrl_handler = &mt9v022->hdl;
+	if (mt9v022->hdl.error) {
+		int err = mt9v022->hdl.error;
+
+		kfree(mt9v022);
+		return err;
+	}
+	v4l2_ctrl_auto_cluster(2, &mt9v022->autoexposure,
+				V4L2_EXPOSURE_MANUAL, true);
+	v4l2_ctrl_auto_cluster(2, &mt9v022->autogain, 0, true);
 
 	mt9v022->chip_control = MT9V022_CHIP_CONTROL_DEFAULT;
 
-	icd->ops		= &mt9v022_ops;
 	/*
 	 * MT9V022 _really_ corrupts the first read out line.
 	 * TODO: verify on i.MX31
@@ -922,7 +870,7 @@ static int mt9v022_probe(struct i2c_client *client,
 
 	ret = mt9v022_video_probe(icd, client);
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&mt9v022->hdl);
 		kfree(mt9v022);
 	}
 
@@ -934,8 +882,9 @@ static int mt9v022_remove(struct i2c_client *client)
 	struct mt9v022 *mt9v022 = to_mt9v022(client);
 	struct soc_camera_device *icd = client->dev.platform_data;
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&mt9v022->subdev);
 	mt9v022_video_remove(icd);
+	v4l2_ctrl_handler_free(&mt9v022->hdl);
 	kfree(mt9v022);
 
 	return 0;
-- 
1.7.2.5

[PATCH 07/13 v3] ov2640: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/ov2640.c |   90 ++++++++++++-----------------------------
 1 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/drivers/media/video/ov2640.c b/drivers/media/video/ov2640.c
index 2826aff..981767f 100644
--- a/drivers/media/video/ov2640.c
+++ b/drivers/media/video/ov2640.c
@@ -24,6 +24,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
 
 #define VAL_SET(x, mask, rshift, lshift)  \
 		((((x) >> rshift) & mask) << lshift)
@@ -300,11 +301,10 @@ struct ov2640_win_size {
 
 struct ov2640_priv {
 	struct v4l2_subdev		subdev;
+	struct v4l2_ctrl_handler	hdl;
 	enum v4l2_mbus_pixelcode	cfmt_code;
 	const struct ov2640_win_size	*win;
 	int				model;
-	u16				flag_vflip:1;
-	u16				flag_hflip:1;
 };
 
 /*
@@ -610,29 +610,6 @@ static enum v4l2_mbus_pixelcode ov2640_codes[] = {
 };
 
 /*
- * Supported controls
- */
-static const struct v4l2_queryctrl ov2640_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
-/*
  * General functions
  */
 static struct ov2640_priv *to_ov2640(const struct i2c_client *client)
@@ -701,43 +678,23 @@ static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-static int ov2640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct i2c_client  *client = v4l2_get_subdevdata(sd);
-	struct ov2640_priv *priv = to_ov2640(client);
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		ctrl->value = priv->flag_vflip;
-		break;
-	case V4L2_CID_HFLIP:
-		ctrl->value = priv->flag_hflip;
-		break;
-	}
-	return 0;
-}
-
-static int ov2640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct v4l2_subdev *sd =
+		&container_of(ctrl->handler, struct ov2640_priv, hdl)->subdev;
 	struct i2c_client  *client = v4l2_get_subdevdata(sd);
-	struct ov2640_priv *priv = to_ov2640(client);
-	int ret = 0;
 	u8 val;
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		val = ctrl->value ? REG04_VFLIP_IMG : 0x00;
-		priv->flag_vflip = ctrl->value ? 1 : 0;
-		ret = ov2640_mask_set(client, REG04, REG04_VFLIP_IMG, val);
-		break;
+		val = ctrl->val ? REG04_VFLIP_IMG : 0x00;
+		return ov2640_mask_set(client, REG04, REG04_VFLIP_IMG, val);
 	case V4L2_CID_HFLIP:
-		val = ctrl->value ? REG04_HFLIP_IMG : 0x00;
-		priv->flag_hflip = ctrl->value ? 1 : 0;
-		ret = ov2640_mask_set(client, REG04, REG04_HFLIP_IMG, val);
-		break;
+		val = ctrl->val ? REG04_HFLIP_IMG : 0x00;
+		return ov2640_mask_set(client, REG04, REG04_HFLIP_IMG, val);
 	}
 
-	return ret;
+	return -EINVAL;
 }
 
 static int ov2640_g_chip_ident(struct v4l2_subdev *sd,
@@ -1022,20 +979,17 @@ static int ov2640_video_probe(struct soc_camera_device *icd,
 		 "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
 		 devname, pid, ver, midh, midl);
 
-	return 0;
+	return v4l2_ctrl_handler_setup(&priv->hdl);
 
 err:
 	return ret;
 }
 
-static struct soc_camera_ops ov2640_ops = {
-	.controls		= ov2640_controls,
-	.num_controls		= ARRAY_SIZE(ov2640_controls),
+static const struct v4l2_ctrl_ops ov2640_ctrl_ops = {
+	.s_ctrl = ov2640_s_ctrl,
 };
 
 static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
-	.g_ctrl		= ov2640_g_ctrl,
-	.s_ctrl		= ov2640_s_ctrl,
 	.g_chip_ident	= ov2640_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= ov2640_g_register,
@@ -1113,12 +1067,22 @@ static int ov2640_probe(struct i2c_client *client,
 	}
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
+	v4l2_ctrl_handler_init(&priv->hdl, 2);
+	v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	priv->subdev.ctrl_handler = &priv->hdl;
+	if (priv->hdl.error) {
+		int err = priv->hdl.error;
 
-	icd->ops = &ov2640_ops;
+		kfree(priv);
+		return err;
+	}
 
 	ret = ov2640_video_probe(icd, client);
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	} else {
 		dev_info(&adapter->dev, "OV2640 Probed\n");
@@ -1130,9 +1094,9 @@ static int ov2640_probe(struct i2c_client *client,
 static int ov2640_remove(struct i2c_client *client)
 {
 	struct ov2640_priv       *priv = to_ov2640(client);
-	struct soc_camera_device *icd = client->dev.platform_data;
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_ctrl_handler_free(&priv->hdl);
 	kfree(priv);
 	return 0;
 }
-- 
1.7.2.5

[PATCH 08/13 v3] ov6650: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/ov6650.c |  381 +++++++++++++-----------------------------
 1 files changed, 115 insertions(+), 266 deletions(-)

diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
index 654b2f5..089a4aa 100644
--- a/drivers/media/video/ov6650.c
+++ b/drivers/media/video/ov6650.c
@@ -32,6 +32,7 @@
 #include <media/soc_camera.h>
 #include <media/soc_mediabus.h>
 #include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
 
 /* Register definitions */
 #define REG_GAIN		0x00	/* range 00 - 3F */
@@ -177,20 +178,23 @@ struct ov6650_reg {
 
 struct ov6650 {
 	struct v4l2_subdev	subdev;
-
-	int			gain;
-	int			blue;
-	int			red;
-	int			saturation;
-	int			hue;
-	int			brightness;
-	int			exposure;
-	int			gamma;
-	int			aec;
-	bool			vflip;
-	bool			hflip;
-	bool			awb;
-	bool			agc;
+	struct v4l2_ctrl_handler hdl;
+	struct {
+		/* exposure/autoexposure cluster */
+		struct v4l2_ctrl *autoexposure;
+		struct v4l2_ctrl *exposure;
+	};
+	struct {
+		/* gain/autogain cluster */
+		struct v4l2_ctrl *autogain;
+		struct v4l2_ctrl *gain;
+	};
+	struct {
+		/* blue/red/autowhitebalance cluster */
+		struct v4l2_ctrl *autowb;
+		struct v4l2_ctrl *blue;
+		struct v4l2_ctrl *red;
+	};
 	bool			half_scale;	/* scale down output by 2 */
 	struct v4l2_rect	rect;		/* sensor cropping window */
 	unsigned long		pclk_limit;	/* from host */
@@ -210,126 +214,6 @@ static enum v4l2_mbus_pixelcode ov6650_codes[] = {
 	V4L2_MBUS_FMT_Y8_1X8,
 };
 
-static const struct v4l2_queryctrl ov6650_controls[] = {
-	{
-		.id		= V4L2_CID_AUTOGAIN,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "AGC",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	},
-	{
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gain",
-		.minimum	= 0,
-		.maximum	= 0x3f,
-		.step		= 1,
-		.default_value	= DEF_GAIN,
-	},
-	{
-		.id		= V4L2_CID_AUTO_WHITE_BALANCE,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "AWB",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	},
-	{
-		.id		= V4L2_CID_BLUE_BALANCE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Blue",
-		.minimum	= 0,
-		.maximum	= 0xff,
-		.step		= 1,
-		.default_value	= DEF_BLUE,
-	},
-	{
-		.id		= V4L2_CID_RED_BALANCE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Red",
-		.minimum	= 0,
-		.maximum	= 0xff,
-		.step		= 1,
-		.default_value	= DEF_RED,
-	},
-	{
-		.id		= V4L2_CID_SATURATION,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Saturation",
-		.minimum	= 0,
-		.maximum	= 0xf,
-		.step		= 1,
-		.default_value	= 0x8,
-	},
-	{
-		.id		= V4L2_CID_HUE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Hue",
-		.minimum	= 0,
-		.maximum	= HUE_MASK,
-		.step		= 1,
-		.default_value	= DEF_HUE,
-	},
-	{
-		.id		= V4L2_CID_BRIGHTNESS,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Brightness",
-		.minimum	= 0,
-		.maximum	= 0xff,
-		.step		= 1,
-		.default_value	= 0x80,
-	},
-	{
-		.id		= V4L2_CID_EXPOSURE_AUTO,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "AEC",
-		.minimum	= 0,
-		.maximum	= 3,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_EXPOSURE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Exposure",
-		.minimum	= 0,
-		.maximum	= 0xff,
-		.step		= 1,
-		.default_value	= DEF_AECH,
-	},
-	{
-		.id		= V4L2_CID_GAMMA,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gamma",
-		.minimum	= 0,
-		.maximum	= 0xff,
-		.step		= 1,
-		.default_value	= 0x12,
-	},
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
 /* read a register */
 static int ov6650_reg_read(struct i2c_client *client, u8 reg, u8 *val)
 {
@@ -420,166 +304,91 @@ static int ov6650_s_stream(struct v4l2_subdev *sd, int enable)
 }
 
 /* Get status of additional camera capabilities */
-static int ov6650_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
+	struct v4l2_subdev *sd = &priv->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov6650 *priv = to_ov6650(client);
-	uint8_t reg;
+	uint8_t reg, reg2;
 	int ret = 0;
 
 	switch (ctrl->id) {
 	case V4L2_CID_AUTOGAIN:
-		ctrl->value = priv->agc;
-		break;
-	case V4L2_CID_GAIN:
-		if (priv->agc) {
-			ret = ov6650_reg_read(client, REG_GAIN, &reg);
-			ctrl->value = reg;
-		} else {
-			ctrl->value = priv->gain;
-		}
-		break;
+		ret = ov6650_reg_read(client, REG_GAIN, &reg);
+		if (!ret)
+			priv->gain->val = reg;
+		return ret;
 	case V4L2_CID_AUTO_WHITE_BALANCE:
-		ctrl->value = priv->awb;
-		break;
-	case V4L2_CID_BLUE_BALANCE:
-		if (priv->awb) {
-			ret = ov6650_reg_read(client, REG_BLUE, &reg);
-			ctrl->value = reg;
-		} else {
-			ctrl->value = priv->blue;
-		}
-		break;
-	case V4L2_CID_RED_BALANCE:
-		if (priv->awb) {
-			ret = ov6650_reg_read(client, REG_RED, &reg);
-			ctrl->value = reg;
-		} else {
-			ctrl->value = priv->red;
+		ret = ov6650_reg_read(client, REG_BLUE, &reg);
+		if (!ret)
+			ret = ov6650_reg_read(client, REG_RED, &reg2);
+		if (!ret) {
+			priv->blue->val = reg;
+			priv->red->val = reg2;
 		}
-		break;
-	case V4L2_CID_SATURATION:
-		ctrl->value = priv->saturation;
-		break;
-	case V4L2_CID_HUE:
-		ctrl->value = priv->hue;
-		break;
-	case V4L2_CID_BRIGHTNESS:
-		ctrl->value = priv->brightness;
-		break;
+		return ret;
 	case V4L2_CID_EXPOSURE_AUTO:
-		ctrl->value = priv->aec;
-		break;
-	case V4L2_CID_EXPOSURE:
-		if (priv->aec) {
-			ret = ov6650_reg_read(client, REG_AECH, &reg);
-			ctrl->value = reg;
-		} else {
-			ctrl->value = priv->exposure;
-		}
-		break;
-	case V4L2_CID_GAMMA:
-		ctrl->value = priv->gamma;
-		break;
-	case V4L2_CID_VFLIP:
-		ctrl->value = priv->vflip;
-		break;
-	case V4L2_CID_HFLIP:
-		ctrl->value = priv->hflip;
-		break;
+		ret = ov6650_reg_read(client, REG_AECH, &reg);
+		if (!ret)
+			priv->exposure->val = reg;
+		return ret;
 	}
-	return ret;
+	return -EINVAL;
 }
 
 /* Set status of additional camera capabilities */
-static int ov6650_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
+	struct v4l2_subdev *sd = &priv->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov6650 *priv = to_ov6650(client);
 	int ret = 0;
 
 	switch (ctrl->id) {
 	case V4L2_CID_AUTOGAIN:
 		ret = ov6650_reg_rmw(client, REG_COMB,
-				ctrl->value ? COMB_AGC : 0, COMB_AGC);
-		if (!ret)
-			priv->agc = ctrl->value;
-		break;
-	case V4L2_CID_GAIN:
-		ret = ov6650_reg_write(client, REG_GAIN, ctrl->value);
-		if (!ret)
-			priv->gain = ctrl->value;
-		break;
+				ctrl->val ? COMB_AGC : 0, COMB_AGC);
+		if (!ret && !ctrl->val)
+			ret = ov6650_reg_write(client, REG_GAIN, priv->gain->val);
+		return ret;
 	case V4L2_CID_AUTO_WHITE_BALANCE:
 		ret = ov6650_reg_rmw(client, REG_COMB,
-				ctrl->value ? COMB_AWB : 0, COMB_AWB);
-		if (!ret)
-			priv->awb = ctrl->value;
-		break;
-	case V4L2_CID_BLUE_BALANCE:
-		ret = ov6650_reg_write(client, REG_BLUE, ctrl->value);
-		if (!ret)
-			priv->blue = ctrl->value;
-		break;
-	case V4L2_CID_RED_BALANCE:
-		ret = ov6650_reg_write(client, REG_RED, ctrl->value);
-		if (!ret)
-			priv->red = ctrl->value;
-		break;
+				ctrl->val ? COMB_AWB : 0, COMB_AWB);
+		if (!ret && !ctrl->val) {
+			ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
+			if (!ret)
+				ret = ov6650_reg_write(client, REG_BLUE,
+							priv->red->val);
+		}
+		return ret;
 	case V4L2_CID_SATURATION:
-		ret = ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->value),
+		return ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->val),
 				SAT_MASK);
-		if (!ret)
-			priv->saturation = ctrl->value;
-		break;
 	case V4L2_CID_HUE:
-		ret = ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->value),
+		return ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->val),
 				HUE_MASK);
-		if (!ret)
-			priv->hue = ctrl->value;
-		break;
 	case V4L2_CID_BRIGHTNESS:
-		ret = ov6650_reg_write(client, REG_BRT, ctrl->value);
-		if (!ret)
-			priv->brightness = ctrl->value;
-		break;
+		return ov6650_reg_write(client, REG_BRT, ctrl->val);
 	case V4L2_CID_EXPOSURE_AUTO:
-		switch (ctrl->value) {
-		case V4L2_EXPOSURE_AUTO:
+		if (ctrl->val == V4L2_EXPOSURE_AUTO)
 			ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
-			break;
-		default:
+		else
 			ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
-			break;
-		}
-		if (!ret)
-			priv->aec = ctrl->value;
-		break;
-	case V4L2_CID_EXPOSURE:
-		ret = ov6650_reg_write(client, REG_AECH, ctrl->value);
-		if (!ret)
-			priv->exposure = ctrl->value;
-		break;
+		if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
+			ret = ov6650_reg_write(client, REG_AECH,
+						priv->exposure->val);
+		return ret;
 	case V4L2_CID_GAMMA:
-		ret = ov6650_reg_write(client, REG_GAM1, ctrl->value);
-		if (!ret)
-			priv->gamma = ctrl->value;
-		break;
+		return ov6650_reg_write(client, REG_GAM1, ctrl->val);
 	case V4L2_CID_VFLIP:
-		ret = ov6650_reg_rmw(client, REG_COMB,
-				ctrl->value ? COMB_FLIP_V : 0, COMB_FLIP_V);
-		if (!ret)
-			priv->vflip = ctrl->value;
-		break;
+		return ov6650_reg_rmw(client, REG_COMB,
+				ctrl->val ? COMB_FLIP_V : 0, COMB_FLIP_V);
 	case V4L2_CID_HFLIP:
-		ret = ov6650_reg_rmw(client, REG_COMB,
-				ctrl->value ? COMB_FLIP_H : 0, COMB_FLIP_H);
-		if (!ret)
-			priv->hflip = ctrl->value;
-		break;
+		return ov6650_reg_rmw(client, REG_COMB,
+				ctrl->val ? COMB_FLIP_H : 0, COMB_FLIP_H);
 	}
 
-	return ret;
+	return -EINVAL;
 }
 
 /* Get chip identification */
@@ -1048,14 +857,12 @@ static int ov6650_video_probe(struct soc_camera_device *icd,
 	return ret;
 }
 
-static struct soc_camera_ops ov6650_ops = {
-	.controls		= ov6650_controls,
-	.num_controls		= ARRAY_SIZE(ov6650_controls),
+static const struct v4l2_ctrl_ops ov6550_ctrl_ops = {
+	.g_volatile_ctrl = ov6550_g_volatile_ctrl,
+	.s_ctrl = ov6550_s_ctrl,
 };
 
 static struct v4l2_subdev_core_ops ov6650_core_ops = {
-	.g_ctrl			= ov6650_g_ctrl,
-	.s_ctrl			= ov6650_s_ctrl,
 	.g_chip_ident		= ov6650_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register		= ov6650_get_register,
@@ -1164,8 +971,46 @@ static int ov6650_probe(struct i2c_client *client,
 	}
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov6650_subdev_ops);
+	v4l2_ctrl_handler_init(&priv->hdl, 13);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	priv->autogain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
+	priv->gain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_GAIN, 0, 0x3f, 1, DEF_GAIN);
+	priv->autowb = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+	priv->blue = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_BLUE_BALANCE, 0, 0xff, 1, DEF_BLUE);
+	priv->red = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_RED_BALANCE, 0, 0xff, 1, DEF_RED);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_SATURATION, 0, 0xf, 1, 0x8);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_HUE, 0, HUE_MASK, 1, DEF_HUE);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
+	priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
+			&ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+			V4L2_EXPOSURE_AUTO);
+	priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
+	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+			V4L2_CID_GAMMA, 0, 0xff, 1, 0x12);
+
+	priv->subdev.ctrl_handler = &priv->hdl;
+	if (priv->hdl.error) {
+		int err = priv->hdl.error;
 
-	icd->ops = &ov6650_ops;
+		kfree(priv);
+		return err;
+	}
+	v4l2_ctrl_auto_cluster(2, &priv->autogain, 0, true);
+	v4l2_ctrl_auto_cluster(3, &priv->autowb, 0, true);
+	v4l2_ctrl_auto_cluster(2, &priv->autoexposure,
+				V4L2_EXPOSURE_MANUAL, true);
 
 	priv->rect.left	  = DEF_HSTRT << 1;
 	priv->rect.top	  = DEF_VSTRT << 1;
@@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
 	priv->colorspace  = V4L2_COLORSPACE_JPEG;
 
 	ret = ov6650_video_probe(icd, client);
+	if (!ret)
+		ret = v4l2_ctrl_handler_setup(&priv->hdl);
 
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	}
 
@@ -1189,6 +1036,8 @@ static int ov6650_remove(struct i2c_client *client)
 {
 	struct ov6650 *priv = to_ov6650(client);
 
+	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_ctrl_handler_free(&priv->hdl);
 	kfree(priv);
 	return 0;
 }
-- 
1.7.2.5

[PATCH 09/13 v3] ov9740: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/ov9740.c |   83 ++++++++++++++----------------------------
 1 files changed, 28 insertions(+), 55 deletions(-)

diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c
index 5920f60..3dd910d 100644
--- a/drivers/media/video/ov9740.c
+++ b/drivers/media/video/ov9740.c
@@ -18,6 +18,7 @@
 #include <media/soc_camera.h>
 #include <media/soc_mediabus.h>
 #include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
 
 #define to_ov9740(sd)		container_of(sd, struct ov9740_priv, subdev)
 
@@ -194,6 +195,7 @@ struct ov9740_reg {
 
 struct ov9740_priv {
 	struct v4l2_subdev		subdev;
+	struct v4l2_ctrl_handler	hdl;
 
 	int				ident;
 	u16				model;
@@ -394,27 +396,6 @@ static enum v4l2_mbus_pixelcode ov9740_codes[] = {
 	V4L2_MBUS_FMT_YUYV8_2X8,
 };
 
-static const struct v4l2_queryctrl ov9740_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	{
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-};
-
 /* read a register */
 static int ov9740_reg_read(struct i2c_client *client, u16 reg, u8 *val)
 {
@@ -771,36 +752,18 @@ static int ov9740_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	return 0;
 }
 
-/* Get status of additional camera capabilities */
-static int ov9740_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct ov9740_priv *priv = to_ov9740(sd);
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		ctrl->value = priv->flag_vflip;
-		break;
-	case V4L2_CID_HFLIP:
-		ctrl->value = priv->flag_hflip;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 /* Set status of additional camera capabilities */
-static int ov9740_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov9740_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct ov9740_priv *priv = to_ov9740(sd);
+	struct ov9740_priv *priv =
+		container_of(ctrl->handler, struct ov9740_priv, hdl);
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		priv->flag_vflip = ctrl->value;
+		priv->flag_vflip = ctrl->val;
 		break;
 	case V4L2_CID_HFLIP:
-		priv->flag_hflip = ctrl->value;
+		priv->flag_hflip = ctrl->val;
 		break;
 	default:
 		return -EINVAL;
@@ -925,11 +888,6 @@ err:
 	return ret;
 }
 
-static struct soc_camera_ops ov9740_ops = {
-	.controls		= ov9740_controls,
-	.num_controls		= ARRAY_SIZE(ov9740_controls),
-};
-
 /* Request bus settings on camera side */
 static int ov9740_g_mbus_config(struct v4l2_subdev *sd,
 				struct v4l2_mbus_config *cfg)
@@ -958,8 +916,6 @@ static struct v4l2_subdev_video_ops ov9740_video_ops = {
 };
 
 static struct v4l2_subdev_core_ops ov9740_core_ops = {
-	.g_ctrl			= ov9740_g_ctrl,
-	.s_ctrl			= ov9740_s_ctrl,
 	.g_chip_ident		= ov9740_g_chip_ident,
 	.s_power		= ov9740_s_power,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -973,6 +929,10 @@ static struct v4l2_subdev_ops ov9740_subdev_ops = {
 	.video			= &ov9740_video_ops,
 };
 
+static const struct v4l2_ctrl_ops ov9740_ctrl_ops = {
+	.s_ctrl = ov9740_s_ctrl,
+};
+
 /*
  * i2c_driver function
  */
@@ -980,7 +940,7 @@ static int ov9740_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 {
 	struct ov9740_priv *priv;
-	struct soc_camera_device *icd	= client->dev.platform_data;
+	struct soc_camera_device *icd = client->dev.platform_data;
 	struct soc_camera_link *icl;
 	int ret;
 
@@ -1002,12 +962,24 @@ static int ov9740_probe(struct i2c_client *client,
 	}
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov9740_subdev_ops);
+	v4l2_ctrl_handler_init(&priv->hdl, 13);
+	v4l2_ctrl_new_std(&priv->hdl, &ov9740_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&priv->hdl, &ov9740_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	priv->subdev.ctrl_handler = &priv->hdl;
+	if (priv->hdl.error) {
+		int err = priv->hdl.error;
 
-	icd->ops = &ov9740_ops;
+		kfree(priv);
+		return err;
+	}
 
 	ret = ov9740_video_probe(icd, client);
+	if (!ret)
+		ret = v4l2_ctrl_handler_setup(&priv->hdl);
 	if (ret < 0) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	}
 
@@ -1018,8 +990,9 @@ static int ov9740_remove(struct i2c_client *client)
 {
 	struct ov9740_priv *priv = i2c_get_clientdata(client);
 
+	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_ctrl_handler_free(&priv->hdl);
 	kfree(priv);
-
 	return 0;
 }
 
-- 
1.7.2.5

[PATCH 10/13 v3] mt9m001: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9m001.c |  218 +++++++++++++++--------------------------
 1 files changed, 77 insertions(+), 141 deletions(-)

diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index 3555e85..42bb3c8 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -17,6 +17,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-subdev.h>
 #include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
 
 /*
  * mt9m001 i2c address 0x5d
@@ -85,15 +86,19 @@ static const struct mt9m001_datafmt mt9m001_monochrome_fmts[] = {
 
 struct mt9m001 {
 	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
+	struct {
+		/* exposure/auto-exposure cluster */
+		struct v4l2_ctrl *autoexposure;
+		struct v4l2_ctrl *exposure;
+	};
 	struct v4l2_rect rect;	/* Sensor window */
 	const struct mt9m001_datafmt *fmt;
 	const struct mt9m001_datafmt *fmts;
 	int num_fmts;
 	int model;	/* V4L2_IDENT_MT9M001* codes from v4l2-chip-ident.h */
-	unsigned int gain;
-	unsigned int exposure;
+	unsigned int total_h;
 	unsigned short y_skip_top;	/* Lines to skip at the top */
-	unsigned char autoexposure;
 };
 
 static struct mt9m001 *to_mt9m001(const struct i2c_client *client)
@@ -171,10 +176,8 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct mt9m001 *mt9m001 = to_mt9m001(client);
 	struct v4l2_rect rect = a->c;
-	struct soc_camera_device *icd = client->dev.platform_data;
 	int ret;
 	const u16 hblank = 9, vblank = 25;
-	unsigned int total_h;
 
 	if (mt9m001->fmts == mt9m001_colour_fmts)
 		/*
@@ -193,7 +196,7 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	soc_camera_limit_side(&rect.top, &rect.height,
 		     MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
 
-	total_h = rect.height + mt9m001->y_skip_top + vblank;
+	mt9m001->total_h = rect.height + mt9m001->y_skip_top + vblank;
 
 	/* Blanking and start values - default... */
 	ret = reg_write(client, MT9M001_HORIZONTAL_BLANKING, hblank);
@@ -213,17 +216,8 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	if (!ret)
 		ret = reg_write(client, MT9M001_WINDOW_HEIGHT,
 				rect.height + mt9m001->y_skip_top - 1);
-	if (!ret && mt9m001->autoexposure) {
-		ret = reg_write(client, MT9M001_SHUTTER_WIDTH, total_h);
-		if (!ret) {
-			const struct v4l2_queryctrl *qctrl =
-				soc_camera_find_qctrl(icd->ops,
-						      V4L2_CID_EXPOSURE);
-			mt9m001->exposure = (524 + (total_h - 1) *
-				 (qctrl->maximum - qctrl->minimum)) /
-				1048 + qctrl->minimum;
-		}
-	}
+	if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
+		ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
 
 	if (!ret)
 		mt9m001->rect = rect;
@@ -383,105 +377,48 @@ static int mt9m001_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
-static const struct v4l2_queryctrl mt9m001_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gain",
-		.minimum	= 0,
-		.maximum	= 127,
-		.step		= 1,
-		.default_value	= 64,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_EXPOSURE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Exposure",
-		.minimum	= 1,
-		.maximum	= 255,
-		.step		= 1,
-		.default_value	= 255,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_EXPOSURE_AUTO,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Automatic Exposure",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	}
-};
-
-static struct soc_camera_ops mt9m001_ops = {
-	.controls		= mt9m001_controls,
-	.num_controls		= ARRAY_SIZE(mt9m001_controls),
-};
-
-static int mt9m001_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct mt9m001 *mt9m001 = to_mt9m001(client);
-	int data;
+	struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+					       struct mt9m001, hdl);
+	s32 min, max;
 
 	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		data = reg_read(client, MT9M001_READ_OPTIONS2);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x8000);
-		break;
 	case V4L2_CID_EXPOSURE_AUTO:
-		ctrl->value = mt9m001->autoexposure;
-		break;
-	case V4L2_CID_GAIN:
-		ctrl->value = mt9m001->gain;
-		break;
-	case V4L2_CID_EXPOSURE:
-		ctrl->value = mt9m001->exposure;
+		min = mt9m001->exposure->minimum;
+		max = mt9m001->exposure->maximum;
+		mt9m001->exposure->val =
+			(524 + (mt9m001->total_h - 1) * (max - min)) / 1048 + min;
 		break;
 	}
 	return 0;
 }
 
-static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+					       struct mt9m001, hdl);
+	struct v4l2_subdev *sd = &mt9m001->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct mt9m001 *mt9m001 = to_mt9m001(client);
-	struct soc_camera_device *icd = client->dev.platform_data;
-	const struct v4l2_queryctrl *qctrl;
+	struct v4l2_ctrl *exp = mt9m001->exposure;
 	int data;
 
-	qctrl = soc_camera_find_qctrl(&mt9m001_ops, ctrl->id);
-
-	if (!qctrl)
-		return -EINVAL;
-
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
 		else
 			data = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
+
 	case V4L2_CID_GAIN:
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
 		/* See Datasheet Table 7, Gain settings. */
-		if (ctrl->value <= qctrl->default_value) {
+		if (ctrl->val <= ctrl->default_value) {
 			/* Pack it into 0..1 step 0.125, register values 0..8 */
-			unsigned long range = qctrl->default_value - qctrl->minimum;
-			data = ((ctrl->value - qctrl->minimum) * 8 + range / 2) / range;
+			unsigned long range = ctrl->default_value - ctrl->minimum;
+			data = ((ctrl->val - ctrl->minimum) * 8 + range / 2) / range;
 
 			dev_dbg(&client->dev, "Setting gain %d\n", data);
 			data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
@@ -490,8 +427,8 @@ static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 		} else {
 			/* Pack it into 1.125..15 variable step, register values 9..67 */
 			/* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
-			unsigned long range = qctrl->maximum - qctrl->default_value - 1;
-			unsigned long gain = ((ctrl->value - qctrl->default_value - 1) *
+			unsigned long range = ctrl->maximum - ctrl->default_value - 1;
+			unsigned long gain = ((ctrl->val - ctrl->default_value - 1) *
 					       111 + range / 2) / range + 9;
 
 			if (gain <= 32)
@@ -507,47 +444,30 @@ static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 			if (data < 0)
 				return -EIO;
 		}
+		return 0;
 
-		/* Success */
-		mt9m001->gain = ctrl->value;
-		break;
-	case V4L2_CID_EXPOSURE:
-		/* mt9m001 has maximum == default */
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
-		else {
-			unsigned long range = qctrl->maximum - qctrl->minimum;
-			unsigned long shutter = ((ctrl->value - qctrl->minimum) * 1048 +
+	case V4L2_CID_EXPOSURE_AUTO:
+		if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
+			unsigned long range = exp->maximum - exp->minimum;
+			unsigned long shutter = ((exp->val - exp->minimum) * 1048 +
 						 range / 2) / range + 1;
 
 			dev_dbg(&client->dev,
 				"Setting shutter width from %d to %lu\n",
-				reg_read(client, MT9M001_SHUTTER_WIDTH),
-				shutter);
+				reg_read(client, MT9M001_SHUTTER_WIDTH), shutter);
 			if (reg_write(client, MT9M001_SHUTTER_WIDTH, shutter) < 0)
 				return -EIO;
-			mt9m001->exposure = ctrl->value;
-			mt9m001->autoexposure = 0;
-		}
-		break;
-	case V4L2_CID_EXPOSURE_AUTO:
-		if (ctrl->value) {
+		} else {
 			const u16 vblank = 25;
-			unsigned int total_h = mt9m001->rect.height +
+
+			mt9m001->total_h = mt9m001->rect.height +
 				mt9m001->y_skip_top + vblank;
-			if (reg_write(client, MT9M001_SHUTTER_WIDTH,
-				      total_h) < 0)
+			if (reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h) < 0)
 				return -EIO;
-			qctrl = soc_camera_find_qctrl(icd->ops, V4L2_CID_EXPOSURE);
-			mt9m001->exposure = (524 + (total_h - 1) *
-				 (qctrl->maximum - qctrl->minimum)) /
-				1048 + qctrl->minimum;
-			mt9m001->autoexposure = 1;
-		} else
-			mt9m001->autoexposure = 0;
-		break;
+		}
+		return 0;
 	}
-	return 0;
+	return -EINVAL;
 }
 
 /*
@@ -621,10 +541,7 @@ static int mt9m001_video_probe(struct soc_camera_device *icd,
 		dev_err(&client->dev, "Failed to initialise the camera\n");
 
 	/* mt9m001_init() has reset the chip, returning registers to defaults */
-	mt9m001->gain = 64;
-	mt9m001->exposure = 255;
-
-	return ret;
+	return v4l2_ctrl_handler_setup(&mt9m001->hdl);
 }
 
 static void mt9m001_video_remove(struct soc_camera_device *icd)
@@ -647,9 +564,12 @@ static int mt9m001_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
 	return 0;
 }
 
+static const struct v4l2_ctrl_ops mt9m001_ctrl_ops = {
+	.g_volatile_ctrl = mt9m001_g_volatile_ctrl,
+	.s_ctrl = mt9m001_s_ctrl,
+};
+
 static struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
-	.g_ctrl		= mt9m001_g_ctrl,
-	.s_ctrl		= mt9m001_s_ctrl,
 	.g_chip_ident	= mt9m001_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= mt9m001_g_register,
@@ -765,25 +685,40 @@ static int mt9m001_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
+	v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
+	v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+			V4L2_CID_GAIN, 0, 127, 1, 64);
+	mt9m001->exposure = v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+			V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+	/*
+	 * Simulated autoexposure. If enabled, we calculate shutter width
+	 * ourselves in the driver based on vertical blanking and frame width
+	 */
+	mt9m001->autoexposure = v4l2_ctrl_new_std_menu(&mt9m001->hdl,
+			&mt9m001_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+			V4L2_EXPOSURE_AUTO);
+	mt9m001->subdev.ctrl_handler = &mt9m001->hdl;
+	if (mt9m001->hdl.error) {
+		int err = mt9m001->hdl.error;
 
-	/* Second stage probe - when a capture adapter is there */
-	icd->ops		= &mt9m001_ops;
+		kfree(mt9m001);
+		return err;
+	}
+	v4l2_ctrl_auto_cluster(2, &mt9m001->autoexposure,
+					V4L2_EXPOSURE_MANUAL, true);
 
+	/* Second stage probe - when a capture adapter is there */
 	mt9m001->y_skip_top	= 0;
 	mt9m001->rect.left	= MT9M001_COLUMN_SKIP;
 	mt9m001->rect.top	= MT9M001_ROW_SKIP;
 	mt9m001->rect.width	= MT9M001_MAX_WIDTH;
 	mt9m001->rect.height	= MT9M001_MAX_HEIGHT;
 
-	/*
-	 * Simulated autoexposure. If enabled, we calculate shutter width
-	 * ourselves in the driver based on vertical blanking and frame width
-	 */
-	mt9m001->autoexposure = 1;
-
 	ret = mt9m001_video_probe(icd, client);
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&mt9m001->hdl);
 		kfree(mt9m001);
 	}
 
@@ -795,7 +730,8 @@ static int mt9m001_remove(struct i2c_client *client)
 	struct mt9m001 *mt9m001 = to_mt9m001(client);
 	struct soc_camera_device *icd = client->dev.platform_data;
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&mt9m001->subdev);
+	v4l2_ctrl_handler_free(&mt9m001->hdl);
 	mt9m001_video_remove(icd);
 	kfree(mt9m001);
 
-- 
1.7.2.5

[PATCH 11/13 v3] mt9m111: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9m111.c |  203 ++++++++++-------------------------------
 1 files changed, 48 insertions(+), 155 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index d7a0773..41df6c3 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -17,6 +17,7 @@
 #include <media/soc_camera.h>
 #include <media/soc_mediabus.h>
 #include <media/v4l2-common.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-chip-ident.h>
 
 /*
@@ -178,6 +179,8 @@ enum mt9m111_context {
 
 struct mt9m111 {
 	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
+	struct v4l2_ctrl *gain;
 	int model;	/* V4L2_IDENT_MT9M111 or V4L2_IDENT_MT9M112 code
 			 * from v4l2-chip-ident.h */
 	enum mt9m111_context context;
@@ -185,13 +188,8 @@ struct mt9m111 {
 	struct mutex power_lock; /* lock to protect power_count */
 	int power_count;
 	const struct mt9m111_datafmt *fmt;
-	unsigned int gain;
-	unsigned char autoexposure;
 	unsigned char datawidth;
 	unsigned int powered:1;
-	unsigned int hflip:1;
-	unsigned int vflip:1;
-	unsigned int autowhitebalance:1;
 };
 
 static struct mt9m111 *to_mt9m111(const struct i2c_client *client)
@@ -645,48 +643,6 @@ static int mt9m111_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
-static const struct v4l2_queryctrl mt9m111_controls[] = {
-	{
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Verticaly",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontaly",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	}, {	/* gain = 1/32*val (=>gain=1 if val==32) */
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gain",
-		.minimum	= 0,
-		.maximum	= 63 * 2 * 2,
-		.step		= 1,
-		.default_value	= 32,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	}, {
-		.id		= V4L2_CID_EXPOSURE_AUTO,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Auto Exposure",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	}
-};
-
-static struct soc_camera_ops mt9m111_ops = {
-	.controls		= mt9m111_controls,
-	.num_controls		= ARRAY_SIZE(mt9m111_controls),
-};
-
 static int mt9m111_set_flip(struct mt9m111 *mt9m111, int flip, int mask)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
@@ -727,7 +683,6 @@ static int mt9m111_set_global_gain(struct mt9m111 *mt9m111, int gain)
 	if (gain > 63 * 2 * 2)
 		return -EINVAL;
 
-	mt9m111->gain = gain;
 	if ((gain >= 64 * 2) && (gain < 63 * 2 * 2))
 		val = (1 << 10) | (1 << 9) | (gain / 4);
 	else if ((gain >= 64) && (gain < 64 * 2))
@@ -741,118 +696,47 @@ static int mt9m111_set_global_gain(struct mt9m111 *mt9m111, int gain)
 static int mt9m111_set_autoexposure(struct mt9m111 *mt9m111, int on)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
-	int ret;
 
 	if (on)
-		ret = reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
-	else
-		ret = reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
-
-	if (!ret)
-		mt9m111->autoexposure = on;
-
-	return ret;
+		return reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
+	return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
 }
 
 static int mt9m111_set_autowhitebalance(struct mt9m111 *mt9m111, int on)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
-	int ret;
 
 	if (on)
-		ret = reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
-	else
-		ret = reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
-
-	if (!ret)
-		mt9m111->autowhitebalance = on;
-
-	return ret;
+		return reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
+	return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
 }
 
-static int mt9m111_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
-	int data;
+	struct mt9m111 *mt9m111 = container_of(ctrl->handler,
+					       struct mt9m111, hdl);
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		if (mt9m111->context == HIGHPOWER)
-			data = reg_read(READ_MODE_B);
-		else
-			data = reg_read(READ_MODE_A);
-
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & MT9M111_RMB_MIRROR_ROWS);
-		break;
-	case V4L2_CID_HFLIP:
-		if (mt9m111->context == HIGHPOWER)
-			data = reg_read(READ_MODE_B);
-		else
-			data = reg_read(READ_MODE_A);
-
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & MT9M111_RMB_MIRROR_COLS);
-		break;
-	case V4L2_CID_GAIN:
-		data = mt9m111_get_global_gain(mt9m111);
-		if (data < 0)
-			return data;
-		ctrl->value = data;
-		break;
-	case V4L2_CID_EXPOSURE_AUTO:
-		ctrl->value = mt9m111->autoexposure;
-		break;
-	case V4L2_CID_AUTO_WHITE_BALANCE:
-		ctrl->value = mt9m111->autowhitebalance;
-		break;
-	}
-	return 0;
-}
-
-static int mt9m111_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
-	const struct v4l2_queryctrl *qctrl;
-	int ret;
-
-	qctrl = soc_camera_find_qctrl(&mt9m111_ops, ctrl->id);
-	if (!qctrl)
-		return -EINVAL;
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		mt9m111->vflip = ctrl->value;
-		ret = mt9m111_set_flip(mt9m111, ctrl->value,
+		return mt9m111_set_flip(mt9m111, ctrl->val,
 					MT9M111_RMB_MIRROR_ROWS);
-		break;
 	case V4L2_CID_HFLIP:
-		mt9m111->hflip = ctrl->value;
-		ret = mt9m111_set_flip(mt9m111, ctrl->value,
+		return mt9m111_set_flip(mt9m111, ctrl->val,
 					MT9M111_RMB_MIRROR_COLS);
-		break;
 	case V4L2_CID_GAIN:
-		ret = mt9m111_set_global_gain(mt9m111, ctrl->value);
-		break;
+		return mt9m111_set_global_gain(mt9m111, ctrl->val);
 	case V4L2_CID_EXPOSURE_AUTO:
-		ret =  mt9m111_set_autoexposure(mt9m111, ctrl->value);
-		break;
+		return mt9m111_set_autoexposure(mt9m111, ctrl->val);
 	case V4L2_CID_AUTO_WHITE_BALANCE:
-		ret =  mt9m111_set_autowhitebalance(mt9m111, ctrl->value);
-		break;
-	default:
-		ret = -EINVAL;
+		return mt9m111_set_autowhitebalance(mt9m111, ctrl->val);
 	}
 
-	return ret;
+	return -EINVAL;
 }
 
 static int mt9m111_suspend(struct mt9m111 *mt9m111)
 {
-	mt9m111->gain = mt9m111_get_global_gain(mt9m111);
+	v4l2_ctrl_s_ctrl(mt9m111->gain, mt9m111_get_global_gain(mt9m111));
 
 	return 0;
 }
@@ -862,11 +746,7 @@ static void mt9m111_restore_state(struct mt9m111 *mt9m111)
 	mt9m111_set_context(mt9m111, mt9m111->context);
 	mt9m111_set_pixfmt(mt9m111, mt9m111->fmt->code);
 	mt9m111_setup_rect(mt9m111, &mt9m111->rect);
-	mt9m111_set_flip(mt9m111, mt9m111->hflip, MT9M111_RMB_MIRROR_COLS);
-	mt9m111_set_flip(mt9m111, mt9m111->vflip, MT9M111_RMB_MIRROR_ROWS);
-	mt9m111_set_global_gain(mt9m111, mt9m111->gain);
-	mt9m111_set_autoexposure(mt9m111, mt9m111->autoexposure);
-	mt9m111_set_autowhitebalance(mt9m111, mt9m111->autowhitebalance);
+	v4l2_ctrl_handler_setup(&mt9m111->hdl);
 }
 
 static int mt9m111_resume(struct mt9m111 *mt9m111)
@@ -894,8 +774,6 @@ static int mt9m111_init(struct mt9m111 *mt9m111)
 		ret = mt9m111_reset(mt9m111);
 	if (!ret)
 		ret = mt9m111_set_context(mt9m111, mt9m111->context);
-	if (!ret)
-		ret = mt9m111_set_autoexposure(mt9m111, mt9m111->autoexposure);
 	if (ret)
 		dev_err(&client->dev, "mt9m111 init failed: %d\n", ret);
 	return ret;
@@ -916,9 +794,6 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 	BUG_ON(!icd->parent ||
 	       to_soc_camera_host(icd->parent)->nr != icd->iface);
 
-	mt9m111->autoexposure = 1;
-	mt9m111->autowhitebalance = 1;
-
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
@@ -932,17 +807,16 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 		dev_info(&client->dev, "Detected a MT9M112 chip ID %x\n", data);
 		break;
 	default:
-		ret = -ENODEV;
 		dev_err(&client->dev,
 			"No MT9M111/MT9M112/MT9M131 chip detected register read %x\n",
 			data);
-		goto ei2c;
+		return -ENODEV;
 	}
 
 	ret = mt9m111_init(mt9m111);
-
-ei2c:
-	return ret;
+	if (ret)
+		return ret;
+	return v4l2_ctrl_handler_setup(&mt9m111->hdl);
 }
 
 static int mt9m111_s_power(struct v4l2_subdev *sd, int on)
@@ -979,9 +853,11 @@ out:
 	return ret;
 }
 
+static const struct v4l2_ctrl_ops mt9m111_ctrl_ops = {
+	.s_ctrl = mt9m111_s_ctrl,
+};
+
 static struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = {
-	.g_ctrl		= mt9m111_g_ctrl,
-	.s_ctrl		= mt9m111_s_ctrl,
 	.g_chip_ident	= mt9m111_g_chip_ident,
 	.s_power	= mt9m111_s_power,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -1063,10 +939,27 @@ static int mt9m111_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&mt9m111->subdev, client, &mt9m111_subdev_ops);
+	v4l2_ctrl_handler_init(&mt9m111->hdl, 5);
+	v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+			V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+	mt9m111->gain = v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+			V4L2_CID_GAIN, 0, 63 * 2 * 2, 1, 32);
+	v4l2_ctrl_new_std_menu(&mt9m111->hdl,
+			&mt9m111_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+			V4L2_EXPOSURE_AUTO);
+	mt9m111->subdev.ctrl_handler = &mt9m111->hdl;
+	if (mt9m111->hdl.error) {
+		int err = mt9m111->hdl.error;
 
-	/* Second stage probe - when a capture adapter is there */
-	icd->ops		= &mt9m111_ops;
+		kfree(mt9m111);
+		return err;
+	}
 
+	/* Second stage probe - when a capture adapter is there */
 	mt9m111->rect.left	= MT9M111_MIN_DARK_COLS;
 	mt9m111->rect.top	= MT9M111_MIN_DARK_ROWS;
 	mt9m111->rect.width	= MT9M111_MAX_WIDTH;
@@ -1075,7 +968,7 @@ static int mt9m111_probe(struct i2c_client *client,
 
 	ret = mt9m111_video_probe(icd, client);
 	if (ret) {
-		icd->ops = NULL;
+		v4l2_ctrl_handler_free(&mt9m111->hdl);
 		kfree(mt9m111);
 	}
 
@@ -1085,9 +978,9 @@ static int mt9m111_probe(struct i2c_client *client,
 static int mt9m111_remove(struct i2c_client *client)
 {
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	struct soc_camera_device *icd = client->dev.platform_data;
 
-	icd->ops = NULL;
+	v4l2_device_unregister_subdev(&mt9m111->subdev);
+	v4l2_ctrl_handler_free(&mt9m111->hdl);
 	kfree(mt9m111);
 
 	return 0;
-- 
1.7.2.5

[PATCH 12/13 v3] mt9t031: convert to the control framework.

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9t031.c |  252 ++++++++++++++---------------------------
 1 files changed, 86 insertions(+), 166 deletions(-)

diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c
index 25fb833..7ce3799 100644
--- a/drivers/media/video/mt9t031.c
+++ b/drivers/media/video/mt9t031.c
@@ -19,6 +19,7 @@
 #include <media/soc_mediabus.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
 
 /*
  * mt9t031 i2c address 0x5d
@@ -60,14 +61,18 @@
 
 struct mt9t031 {
 	struct v4l2_subdev subdev;
+	struct v4l2_ctrl_handler hdl;
+	struct {
+		/* exposure/auto-exposure cluster */
+		struct v4l2_ctrl *autoexposure;
+		struct v4l2_ctrl *exposure;
+	};
 	struct v4l2_rect rect;	/* Sensor window */
 	int model;	/* V4L2_IDENT_MT9T031* codes from v4l2-chip-ident.h */
 	u16 xskip;
 	u16 yskip;
-	unsigned int gain;
+	unsigned int total_h;
 	unsigned short y_skip_top;	/* Lines to skip at the top */
-	unsigned int exposure;
-	unsigned char autoexposure;
 };
 
 static struct mt9t031 *to_mt9t031(const struct i2c_client *client)
@@ -175,69 +180,6 @@ static int mt9t031_s_stream(struct v4l2_subdev *sd, int enable)
 	return 0;
 }
 
-enum {
-	MT9T031_CTRL_VFLIP,
-	MT9T031_CTRL_HFLIP,
-	MT9T031_CTRL_GAIN,
-	MT9T031_CTRL_EXPOSURE,
-	MT9T031_CTRL_EXPOSURE_AUTO,
-};
-
-static const struct v4l2_queryctrl mt9t031_controls[] = {
-	[MT9T031_CTRL_VFLIP] = {
-		.id		= V4L2_CID_VFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Vertically",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	[MT9T031_CTRL_HFLIP] = {
-		.id		= V4L2_CID_HFLIP,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Flip Horizontally",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 0,
-	},
-	[MT9T031_CTRL_GAIN] = {
-		.id		= V4L2_CID_GAIN,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Gain",
-		.minimum	= 0,
-		.maximum	= 127,
-		.step		= 1,
-		.default_value	= 64,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	},
-	[MT9T031_CTRL_EXPOSURE] = {
-		.id		= V4L2_CID_EXPOSURE,
-		.type		= V4L2_CTRL_TYPE_INTEGER,
-		.name		= "Exposure",
-		.minimum	= 1,
-		.maximum	= 255,
-		.step		= 1,
-		.default_value	= 255,
-		.flags		= V4L2_CTRL_FLAG_SLIDER,
-	},
-	[MT9T031_CTRL_EXPOSURE_AUTO] = {
-		.id		= V4L2_CID_EXPOSURE_AUTO,
-		.type		= V4L2_CTRL_TYPE_BOOLEAN,
-		.name		= "Automatic Exposure",
-		.minimum	= 0,
-		.maximum	= 1,
-		.step		= 1,
-		.default_value	= 1,
-	}
-};
-
-static struct soc_camera_ops mt9t031_ops = {
-	.controls		= mt9t031_controls,
-	.num_controls		= ARRAY_SIZE(mt9t031_controls),
-};
-
 /* target must be _even_ */
 static u16 mt9t031_skip(s32 *source, s32 target, s32 max)
 {
@@ -334,17 +276,10 @@ static int mt9t031_set_params(struct i2c_client *client,
 	if (ret >= 0)
 		ret = reg_write(client, MT9T031_WINDOW_HEIGHT,
 				rect->height + mt9t031->y_skip_top - 1);
-	if (ret >= 0 && mt9t031->autoexposure) {
-		unsigned int total_h = rect->height + mt9t031->y_skip_top + vblank;
-		ret = set_shutter(client, total_h);
-		if (ret >= 0) {
-			const u32 shutter_max = MT9T031_MAX_HEIGHT + vblank;
-			const struct v4l2_queryctrl *qctrl =
-				&mt9t031_controls[MT9T031_CTRL_EXPOSURE];
-			mt9t031->exposure = (shutter_max / 2 + (total_h - 1) *
-				 (qctrl->maximum - qctrl->minimum)) /
-				shutter_max + qctrl->minimum;
-		}
+	if (ret >= 0 && v4l2_ctrl_g_ctrl(mt9t031->autoexposure) == V4L2_EXPOSURE_AUTO) {
+		mt9t031->total_h = rect->height + mt9t031->y_skip_top + vblank;
+
+		ret = set_shutter(client, mt9t031->total_h);
 	}
 
 	/* Re-enable register update, commit all changes */
@@ -513,71 +448,57 @@ static int mt9t031_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
-static int mt9t031_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9t031_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct mt9t031 *mt9t031 = to_mt9t031(client);
-	int data;
+	struct mt9t031 *mt9t031 = container_of(ctrl->handler,
+					       struct mt9t031, hdl);
+	const u32 shutter_max = MT9T031_MAX_HEIGHT + MT9T031_VERTICAL_BLANK;
+	s32 min, max;
 
 	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		data = reg_read(client, MT9T031_READ_MODE_2);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x8000);
-		break;
-	case V4L2_CID_HFLIP:
-		data = reg_read(client, MT9T031_READ_MODE_2);
-		if (data < 0)
-			return -EIO;
-		ctrl->value = !!(data & 0x4000);
-		break;
 	case V4L2_CID_EXPOSURE_AUTO:
-		ctrl->value = mt9t031->autoexposure;
-		break;
-	case V4L2_CID_GAIN:
-		ctrl->value = mt9t031->gain;
-		break;
-	case V4L2_CID_EXPOSURE:
-		ctrl->value = mt9t031->exposure;
+		min = mt9t031->exposure->minimum;
+		max = mt9t031->exposure->maximum;
+		mt9t031->exposure->val =
+			(shutter_max / 2 + (mt9t031->total_h - 1) * (max - min))
+				/ shutter_max + min;
 		break;
 	}
 	return 0;
 }
 
-static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9t031_s_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct mt9t031 *mt9t031 = container_of(ctrl->handler,
+					       struct mt9t031, hdl);
+	struct v4l2_subdev *sd = &mt9t031->subdev;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct mt9t031 *mt9t031 = to_mt9t031(client);
-	const struct v4l2_queryctrl *qctrl;
+	struct v4l2_ctrl *exp = mt9t031->exposure;
 	int data;
 
 	switch (ctrl->id) {
 	case V4L2_CID_VFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, MT9T031_READ_MODE_2, 0x8000);
 		else
 			data = reg_clear(client, MT9T031_READ_MODE_2, 0x8000);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_HFLIP:
-		if (ctrl->value)
+		if (ctrl->val)
 			data = reg_set(client, MT9T031_READ_MODE_2, 0x4000);
 		else
 			data = reg_clear(client, MT9T031_READ_MODE_2, 0x4000);
 		if (data < 0)
 			return -EIO;
-		break;
+		return 0;
 	case V4L2_CID_GAIN:
-		qctrl = &mt9t031_controls[MT9T031_CTRL_GAIN];
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
 		/* See Datasheet Table 7, Gain settings. */
-		if (ctrl->value <= qctrl->default_value) {
+		if (ctrl->val <= ctrl->default_value) {
 			/* Pack it into 0..1 step 0.125, register values 0..8 */
-			unsigned long range = qctrl->default_value - qctrl->minimum;
-			data = ((ctrl->value - qctrl->minimum) * 8 + range / 2) / range;
+			unsigned long range = ctrl->default_value - ctrl->minimum;
+			data = ((ctrl->val - ctrl->minimum) * 8 + range / 2) / range;
 
 			dev_dbg(&client->dev, "Setting gain %d\n", data);
 			data = reg_write(client, MT9T031_GLOBAL_GAIN, data);
@@ -586,9 +507,9 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 		} else {
 			/* Pack it into 1.125..128 variable step, register values 9..0x7860 */
 			/* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
-			unsigned long range = qctrl->maximum - qctrl->default_value - 1;
+			unsigned long range = ctrl->maximum - ctrl->default_value - 1;
 			/* calculated gain: map 65..127 to 9..1024 step 0.125 */
-			unsigned long gain = ((ctrl->value - qctrl->default_value - 1) *
+			unsigned long gain = ((ctrl->val - ctrl->default_value - 1) *
 					       1015 + range / 2) / range + 9;
 
 			if (gain <= 32)		/* calculated gain 9..32 -> 9..32 */
@@ -605,19 +526,13 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 			if (data < 0)
 				return -EIO;
 		}
+		return 0;
 
-		/* Success */
-		mt9t031->gain = ctrl->value;
-		break;
-	case V4L2_CID_EXPOSURE:
-		qctrl = &mt9t031_controls[MT9T031_CTRL_EXPOSURE];
-		/* mt9t031 has maximum == default */
-		if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
-			return -EINVAL;
-		else {
-			const unsigned long range = qctrl->maximum - qctrl->minimum;
-			const u32 shutter = ((ctrl->value - qctrl->minimum) * 1048 +
-					     range / 2) / range + 1;
+	case V4L2_CID_EXPOSURE_AUTO:
+		if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
+			unsigned int range = exp->maximum - exp->minimum;
+			unsigned int shutter = ((exp->val - exp->minimum) * 1048 +
+						 range / 2) / range + 1;
 			u32 old;
 
 			get_shutter(client, &old);
@@ -625,27 +540,15 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
 				old, shutter);
 			if (set_shutter(client, shutter) < 0)
 				return -EIO;
-			mt9t031->exposure = ctrl->value;
-			mt9t031->autoexposure = 0;
-		}
-		break;
-	case V4L2_CID_EXPOSURE_AUTO:
-		if (ctrl->value) {
+		} else {
 			const u16 vblank = MT9T031_VERTICAL_BLANK;
-			const u32 shutter_max = MT9T031_MAX_HEIGHT + vblank;
-			unsigned int total_h = mt9t031->rect.height +
+			mt9t031->total_h = mt9t031->rect.height +
 				mt9t031->y_skip_top + vblank;
 
-			if (set_shutter(client, total_h) < 0)
+			if (set_shutter(client, mt9t031->total_h) < 0)
 				return -EIO;
-			qctrl = &mt9t031_controls[MT9T031_CTRL_EXPOSURE];
-			mt9t031->exposure = (shutter_max / 2 + (total_h - 1) *
-				 (qctrl->maximum - qctrl->minimum)) /
-				shutter_max + qctrl->minimum;
-			mt9t031->autoexposure = 1;
-		} else
-			mt9t031->autoexposure = 0;
-		break;
+		}
+		return 0;
 	default:
 		return -EINVAL;
 	}
@@ -735,15 +638,12 @@ static int mt9t031_video_probe(struct i2c_client *client)
 	dev_info(&client->dev, "Detected a MT9T031 chip ID %x\n", data);
 
 	ret = mt9t031_idle(client);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(&client->dev, "Failed to initialise the camera\n");
-	else
+	} else {
 		vdev->dev.type = &mt9t031_dev_type;
-
-	/* mt9t031_idle() has reset the chip to default. */
-	mt9t031->exposure = 255;
-	mt9t031->gain = 64;
-
+		v4l2_ctrl_handler_setup(&mt9t031->hdl);
+	}
 	return ret;
 }
 
@@ -757,9 +657,12 @@ static int mt9t031_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
 	return 0;
 }
 
+static const struct v4l2_ctrl_ops mt9t031_ctrl_ops = {
+	.g_volatile_ctrl = mt9t031_g_volatile_ctrl,
+	.s_ctrl = mt9t031_s_ctrl,
+};
+
 static struct v4l2_subdev_core_ops mt9t031_subdev_core_ops = {
-	.g_ctrl		= mt9t031_g_ctrl,
-	.s_ctrl		= mt9t031_s_ctrl,
 	.g_chip_ident	= mt9t031_g_chip_ident,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	.g_register	= mt9t031_g_register,
@@ -844,8 +747,6 @@ static int mt9t031_probe(struct i2c_client *client,
 			dev_err(&client->dev, "MT9T031 driver needs platform data\n");
 			return -EINVAL;
 		}
-
-		icd->ops = &mt9t031_ops;
 	}
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
@@ -859,6 +760,33 @@ static int mt9t031_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&mt9t031->subdev, client, &mt9t031_subdev_ops);
+	v4l2_ctrl_handler_init(&mt9t031->hdl, 5);
+	v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+			V4L2_CID_VFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+			V4L2_CID_HFLIP, 0, 1, 1, 0);
+	v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+			V4L2_CID_GAIN, 0, 127, 1, 64);
+
+	/*
+	 * Simulated autoexposure. If enabled, we calculate shutter width
+	 * ourselves in the driver based on vertical blanking and frame width
+	 */
+	mt9t031->autoexposure = v4l2_ctrl_new_std_menu(&mt9t031->hdl,
+			&mt9t031_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+			V4L2_EXPOSURE_AUTO);
+	mt9t031->exposure = v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+			V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+
+	mt9t031->subdev.ctrl_handler = &mt9t031->hdl;
+	if (mt9t031->hdl.error) {
+		int err = mt9t031->hdl.error;
+
+		kfree(mt9t031);
+		return err;
+	}
+	v4l2_ctrl_auto_cluster(2, &mt9t031->autoexposure,
+				V4L2_EXPOSURE_MANUAL, true);
 
 	mt9t031->y_skip_top	= 0;
 	mt9t031->rect.left	= MT9T031_COLUMN_SKIP;
@@ -866,12 +794,6 @@ static int mt9t031_probe(struct i2c_client *client,
 	mt9t031->rect.width	= MT9T031_MAX_WIDTH;
 	mt9t031->rect.height	= MT9T031_MAX_HEIGHT;
 
-	/*
-	 * Simulated autoexposure. If enabled, we calculate shutter width
-	 * ourselves in the driver based on vertical blanking and frame width
-	 */
-	mt9t031->autoexposure = 1;
-
 	mt9t031->xskip = 1;
 	mt9t031->yskip = 1;
 
@@ -882,8 +804,7 @@ static int mt9t031_probe(struct i2c_client *client,
 	mt9t031_disable(client);
 
 	if (ret) {
-		if (icd)
-			icd->ops = NULL;
+		v4l2_ctrl_handler_free(&mt9t031->hdl);
 		kfree(mt9t031);
 	}
 
@@ -893,10 +814,9 @@ static int mt9t031_probe(struct i2c_client *client,
 static int mt9t031_remove(struct i2c_client *client)
 {
 	struct mt9t031 *mt9t031 = to_mt9t031(client);
-	struct soc_camera_device *icd = client->dev.platform_data;
 
-	if (icd)
-		icd->ops = NULL;
+	v4l2_device_unregister_subdev(&mt9t031->subdev);
+	v4l2_ctrl_handler_free(&mt9t031->hdl);
 	kfree(mt9t031);
 
 	return 0;
-- 
1.7.2.5

[PATCH 13/13 v3] soc_camera: remove the now obsolete struct soc_camera_ops

[Guennadi Liakhovetski]

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: mt9m001 hunk moved to an earlier patch]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/imx074.c              |    1 -
 drivers/media/video/mt9t112.c             |    2 --
 drivers/media/video/soc_camera_platform.c |    2 --
 drivers/media/video/tw9910.c              |    1 -
 include/media/soc_camera.h                |   18 ------------------
 5 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/drivers/media/video/imx074.c b/drivers/media/video/imx074.c
index 20756e0..3f5d4de 100644
--- a/drivers/media/video/imx074.c
+++ b/drivers/media/video/imx074.c
@@ -437,7 +437,6 @@ static int imx074_probe(struct i2c_client *client,
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &imx074_subdev_ops);
 
-	icd->ops	= NULL;
 	priv->fmt	= &imx074_colour_fmts[0];
 
 	ret = imx074_video_probe(icd, client);
diff --git a/drivers/media/video/mt9t112.c b/drivers/media/video/mt9t112.c
index 25cdcb9..b8da7fe 100644
--- a/drivers/media/video/mt9t112.c
+++ b/drivers/media/video/mt9t112.c
@@ -1095,8 +1095,6 @@ static int mt9t112_probe(struct i2c_client *client,
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
 
-	icd->ops = NULL;
-
 	ret = mt9t112_camera_probe(icd, client);
 	if (ret)
 		kfree(priv);
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index c8f6b18..4402a8a 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -150,8 +150,6 @@ static int soc_camera_platform_probe(struct platform_device *pdev)
 	/* Set the control device reference */
 	icd->control = &pdev->dev;
 
-	icd->ops = NULL;
-
 	ici = to_soc_camera_host(icd->parent);
 
 	v4l2_subdev_init(&priv->subdev, &platform_subdev_ops);
diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
index 40cc149..2fddd1f 100644
--- a/drivers/media/video/tw9910.c
+++ b/drivers/media/video/tw9910.c
@@ -921,7 +921,6 @@ static int tw9910_probe(struct i2c_client *client,
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops);
 
-	icd->ops     = NULL;
 	icd->iface   = icl->bus_id;
 
 	ret = tw9910_video_probe(icd, client);
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index d41b8bd..6398ff0 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -39,7 +39,6 @@ struct soc_camera_device {
 	unsigned char iface;		/* Host number */
 	unsigned char devnum;		/* Device number per host */
 	struct soc_camera_sense *sense;	/* See comment in struct definition */
-	struct soc_camera_ops *ops;
 	struct video_device *vdev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	const struct soc_camera_format_xlate *current_fmt;
@@ -192,11 +191,6 @@ struct soc_camera_format_xlate {
 	const struct soc_mbus_pixelfmt *host_fmt;
 };
 
-struct soc_camera_ops {
-	const struct v4l2_queryctrl *controls;
-	int num_controls;
-};
-
 #define SOCAM_SENSE_PCLK_CHANGED	(1 << 0)
 
 /**
@@ -223,18 +217,6 @@ struct soc_camera_sense {
 	unsigned long pixel_clock;
 };
 
-static inline struct v4l2_queryctrl const *soc_camera_find_qctrl(
-	struct soc_camera_ops *ops, int id)
-{
-	int i;
-
-	for (i = 0; i < ops->num_controls; i++)
-		if (ops->controls[i].id == id)
-			return &ops->controls[i];
-
-	return NULL;
-}
-
 #define SOCAM_DATAWIDTH(x)	BIT((x) - 1)
 #define SOCAM_DATAWIDTH_4	SOCAM_DATAWIDTH(4)
 #define SOCAM_DATAWIDTH_8	SOCAM_DATAWIDTH(8)
-- 
1.7.2.5

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Janusz Krzysztofik]

On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Hi,
I've successfully tested this one for you, to the extent possible using 
mplayer, i.e., only saturation, hue and brightness controls, and an 
overall functionality.

Modifications to other (not runtime tested) controls look good to me, 
except for one copy-paste mistake, see below. With that erratic REG_BLUE 
corrected:

Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

There are also a few minor comments for you to consider.

Thanks,
Janusz

> ---
>  drivers/media/video/ov6650.c |  381 +++++++++++++-----------------------------
>  1 files changed, 115 insertions(+), 266 deletions(-)
> 
> diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
> index 654b2f5..089a4aa 100644
> --- a/drivers/media/video/ov6650.c
> +++ b/drivers/media/video/ov6650.c
> @@ -32,6 +32,7 @@
>  #include <media/soc_camera.h>
>  #include <media/soc_mediabus.h>
>  #include <media/v4l2-chip-ident.h>
> +#include <media/v4l2-ctrls.h>
>  
>  /* Register definitions */
>  #define REG_GAIN		0x00	/* range 00 - 3F */
> @@ -177,20 +178,23 @@ struct ov6650_reg {
>  
>  struct ov6650 {
>  	struct v4l2_subdev	subdev;
> -
> -	int			gain;
> -	int			blue;
> -	int			red;
> -	int			saturation;
> -	int			hue;
> -	int			brightness;
> -	int			exposure;
> -	int			gamma;
> -	int			aec;
> -	bool			vflip;
> -	bool			hflip;
> -	bool			awb;
> -	bool			agc;
> +	struct v4l2_ctrl_handler hdl;
> +	struct {
> +		/* exposure/autoexposure cluster */
> +		struct v4l2_ctrl *autoexposure;
> +		struct v4l2_ctrl *exposure;
> +	};
> +	struct {
> +		/* gain/autogain cluster */
> +		struct v4l2_ctrl *autogain;
> +		struct v4l2_ctrl *gain;
> +	};
> +	struct {
> +		/* blue/red/autowhitebalance cluster */
> +		struct v4l2_ctrl *autowb;
> +		struct v4l2_ctrl *blue;
> +		struct v4l2_ctrl *red;
> +	};
>  	bool			half_scale;	/* scale down output by 2 */
>  	struct v4l2_rect	rect;		/* sensor cropping window */
>  	unsigned long		pclk_limit;	/* from host */
> @@ -210,126 +214,6 @@ static enum v4l2_mbus_pixelcode ov6650_codes[] = {
>  	V4L2_MBUS_FMT_Y8_1X8,
>  };
>  
> -static const struct v4l2_queryctrl ov6650_controls[] = {
> -	{
> -		.id		= V4L2_CID_AUTOGAIN,
> -		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> -		.name		= "AGC",
> -		.minimum	= 0,
> -		.maximum	= 1,
> -		.step		= 1,
> -		.default_value	= 1,
> -	},
> -	{
> -		.id		= V4L2_CID_GAIN,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Gain",
> -		.minimum	= 0,
> -		.maximum	= 0x3f,
> -		.step		= 1,
> -		.default_value	= DEF_GAIN,
> -	},
> -	{
> -		.id		= V4L2_CID_AUTO_WHITE_BALANCE,
> -		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> -		.name		= "AWB",
> -		.minimum	= 0,
> -		.maximum	= 1,
> -		.step		= 1,
> -		.default_value	= 1,
> -	},
> -	{
> -		.id		= V4L2_CID_BLUE_BALANCE,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Blue",
> -		.minimum	= 0,
> -		.maximum	= 0xff,
> -		.step		= 1,
> -		.default_value	= DEF_BLUE,
> -	},
> -	{
> -		.id		= V4L2_CID_RED_BALANCE,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Red",
> -		.minimum	= 0,
> -		.maximum	= 0xff,
> -		.step		= 1,
> -		.default_value	= DEF_RED,
> -	},
> -	{
> -		.id		= V4L2_CID_SATURATION,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Saturation",
> -		.minimum	= 0,
> -		.maximum	= 0xf,
> -		.step		= 1,
> -		.default_value	= 0x8,
> -	},
> -	{
> -		.id		= V4L2_CID_HUE,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Hue",
> -		.minimum	= 0,
> -		.maximum	= HUE_MASK,
> -		.step		= 1,
> -		.default_value	= DEF_HUE,
> -	},
> -	{
> -		.id		= V4L2_CID_BRIGHTNESS,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Brightness",
> -		.minimum	= 0,
> -		.maximum	= 0xff,
> -		.step		= 1,
> -		.default_value	= 0x80,
> -	},
> -	{
> -		.id		= V4L2_CID_EXPOSURE_AUTO,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "AEC",
> -		.minimum	= 0,
> -		.maximum	= 3,
> -		.step		= 1,
> -		.default_value	= 0,
> -	},
> -	{
> -		.id		= V4L2_CID_EXPOSURE,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Exposure",
> -		.minimum	= 0,
> -		.maximum	= 0xff,
> -		.step		= 1,
> -		.default_value	= DEF_AECH,
> -	},
> -	{
> -		.id		= V4L2_CID_GAMMA,
> -		.type		= V4L2_CTRL_TYPE_INTEGER,
> -		.name		= "Gamma",
> -		.minimum	= 0,
> -		.maximum	= 0xff,
> -		.step		= 1,
> -		.default_value	= 0x12,
> -	},
> -	{
> -		.id		= V4L2_CID_VFLIP,
> -		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> -		.name		= "Flip Vertically",
> -		.minimum	= 0,
> -		.maximum	= 1,
> -		.step		= 1,
> -		.default_value	= 0,
> -	},
> -	{
> -		.id		= V4L2_CID_HFLIP,
> -		.type		= V4L2_CTRL_TYPE_BOOLEAN,
> -		.name		= "Flip Horizontally",
> -		.minimum	= 0,
> -		.maximum	= 1,
> -		.step		= 1,
> -		.default_value	= 0,
> -	},
> -};
> -
>  /* read a register */
>  static int ov6650_reg_read(struct i2c_client *client, u8 reg, u8 *val)
>  {
> @@ -420,166 +304,91 @@ static int ov6650_s_stream(struct v4l2_subdev *sd, int enable)
>  }
>  
>  /* Get status of additional camera capabilities */
> -static int ov6650_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>  {
> +	struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
> +	struct v4l2_subdev *sd = &priv->subdev;
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct ov6650 *priv = to_ov6650(client);
> -	uint8_t reg;
> +	uint8_t reg, reg2;
>  	int ret = 0;

With no "retrun ret;" at the end, there is no need to initialize ret any 
longer.

>  	switch (ctrl->id) {
>  	case V4L2_CID_AUTOGAIN:
> -		ctrl->value = priv->agc;
> -		break;
> -	case V4L2_CID_GAIN:
> -		if (priv->agc) {
> -			ret = ov6650_reg_read(client, REG_GAIN, &reg);
> -			ctrl->value = reg;
> -		} else {
> -			ctrl->value = priv->gain;
> -		}
> -		break;
> +		ret = ov6650_reg_read(client, REG_GAIN, &reg);
> +		if (!ret)
> +			priv->gain->val = reg;
> +		return ret;
>  	case V4L2_CID_AUTO_WHITE_BALANCE:
> -		ctrl->value = priv->awb;
> -		break;
> -	case V4L2_CID_BLUE_BALANCE:
> -		if (priv->awb) {
> -			ret = ov6650_reg_read(client, REG_BLUE, &reg);
> -			ctrl->value = reg;
> -		} else {
> -			ctrl->value = priv->blue;
> -		}
> -		break;
> -	case V4L2_CID_RED_BALANCE:
> -		if (priv->awb) {
> -			ret = ov6650_reg_read(client, REG_RED, &reg);
> -			ctrl->value = reg;
> -		} else {
> -			ctrl->value = priv->red;
> +		ret = ov6650_reg_read(client, REG_BLUE, &reg);
> +		if (!ret)
> +			ret = ov6650_reg_read(client, REG_RED, &reg2);
> +		if (!ret) {
> +			priv->blue->val = reg;
> +			priv->red->val = reg2;
>  		}
> -		break;
> -	case V4L2_CID_SATURATION:
> -		ctrl->value = priv->saturation;
> -		break;
> -	case V4L2_CID_HUE:
> -		ctrl->value = priv->hue;
> -		break;
> -	case V4L2_CID_BRIGHTNESS:
> -		ctrl->value = priv->brightness;
> -		break;
> +		return ret;
>  	case V4L2_CID_EXPOSURE_AUTO:
> -		ctrl->value = priv->aec;
> -		break;
> -	case V4L2_CID_EXPOSURE:
> -		if (priv->aec) {
> -			ret = ov6650_reg_read(client, REG_AECH, &reg);
> -			ctrl->value = reg;
> -		} else {
> -			ctrl->value = priv->exposure;
> -		}
> -		break;
> -	case V4L2_CID_GAMMA:
> -		ctrl->value = priv->gamma;
> -		break;
> -	case V4L2_CID_VFLIP:
> -		ctrl->value = priv->vflip;
> -		break;
> -	case V4L2_CID_HFLIP:
> -		ctrl->value = priv->hflip;
> -		break;
> +		ret = ov6650_reg_read(client, REG_AECH, &reg);
> +		if (!ret)
> +			priv->exposure->val = reg;
> +		return ret;
>  	}
> -	return ret;
> +	return -EINVAL;
>  }
>  
>  /* Set status of additional camera capabilities */
> -static int ov6650_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
> +	struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
> +	struct v4l2_subdev *sd = &priv->subdev;
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct ov6650 *priv = to_ov6650(client);
>  	int ret = 0;

ditto

>  	switch (ctrl->id) {
>  	case V4L2_CID_AUTOGAIN:
>  		ret = ov6650_reg_rmw(client, REG_COMB,
> -				ctrl->value ? COMB_AGC : 0, COMB_AGC);
> -		if (!ret)
> -			priv->agc = ctrl->value;
> -		break;
> -	case V4L2_CID_GAIN:
> -		ret = ov6650_reg_write(client, REG_GAIN, ctrl->value);
> -		if (!ret)
> -			priv->gain = ctrl->value;
> -		break;
> +				ctrl->val ? COMB_AGC : 0, COMB_AGC);
> +		if (!ret && !ctrl->val)
> +			ret = ov6650_reg_write(client, REG_GAIN, priv->gain->val);
> +		return ret;
>  	case V4L2_CID_AUTO_WHITE_BALANCE:
>  		ret = ov6650_reg_rmw(client, REG_COMB,
> -				ctrl->value ? COMB_AWB : 0, COMB_AWB);
> -		if (!ret)
> -			priv->awb = ctrl->value;
> -		break;
> -	case V4L2_CID_BLUE_BALANCE:
> -		ret = ov6650_reg_write(client, REG_BLUE, ctrl->value);
> -		if (!ret)
> -			priv->blue = ctrl->value;
> -		break;
> -	case V4L2_CID_RED_BALANCE:
> -		ret = ov6650_reg_write(client, REG_RED, ctrl->value);
> -		if (!ret)
> -			priv->red = ctrl->value;
> -		break;
> +				ctrl->val ? COMB_AWB : 0, COMB_AWB);
> +		if (!ret && !ctrl->val) {
> +			ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
> +			if (!ret)
> +				ret = ov6650_reg_write(client, REG_BLUE,

REG_RED

> +							priv->red->val);
> +		}
> +		return ret;
>  	case V4L2_CID_SATURATION:
> -		ret = ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->value),
> +		return ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->val),
>  				SAT_MASK);
> -		if (!ret)
> -			priv->saturation = ctrl->value;
> -		break;
>  	case V4L2_CID_HUE:
> -		ret = ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->value),
> +		return ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->val),
>  				HUE_MASK);
> -		if (!ret)
> -			priv->hue = ctrl->value;
> -		break;
>  	case V4L2_CID_BRIGHTNESS:
> -		ret = ov6650_reg_write(client, REG_BRT, ctrl->value);
> -		if (!ret)
> -			priv->brightness = ctrl->value;
> -		break;
> +		return ov6650_reg_write(client, REG_BRT, ctrl->val);
>  	case V4L2_CID_EXPOSURE_AUTO:
> -		switch (ctrl->value) {
> -		case V4L2_EXPOSURE_AUTO:

For consitency with other cases (V4L2_CID_AUTOGAIN, 
V4L2_CID_AUTO_WHITE_BALANCE, V4L2_CID_VFLIP, V4L2_CID_HFLIP), the 
following snippet:

> +		if (ctrl->val == V4L2_EXPOSURE_AUTO)
>  			ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
> -			break;
> -		default:
> +		else
>  			ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);

could perhaps be replaced with something like:

-		if (ctrl->val == V4L2_EXPOSURE_AUTO)
-			ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
-			break;
-		default:
-			ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
+		ret = ov6650_reg_rmw(client, REG_COMB, ctrl->val ==
+				V4L2_EXPOSURE_AUTO ? COMB_AEC : 0, COMB_AEC);

or, the other way around, conditional expressions could be replaced with 
if...else constructs in those other cases consequently.

> -			break;
> -		}
> -		if (!ret)
> -			priv->aec = ctrl->value;
> -		break;
> -	case V4L2_CID_EXPOSURE:
> -		ret = ov6650_reg_write(client, REG_AECH, ctrl->value);
> -		if (!ret)
> -			priv->exposure = ctrl->value;
> -		break;
> +		if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
> +			ret = ov6650_reg_write(client, REG_AECH,
> +						priv->exposure->val);
> +		return ret;
>  	case V4L2_CID_GAMMA:
> -		ret = ov6650_reg_write(client, REG_GAM1, ctrl->value);
> -		if (!ret)
> -			priv->gamma = ctrl->value;
> -		break;
> +		return ov6650_reg_write(client, REG_GAM1, ctrl->val);
>  	case V4L2_CID_VFLIP:
> -		ret = ov6650_reg_rmw(client, REG_COMB,
> -				ctrl->value ? COMB_FLIP_V : 0, COMB_FLIP_V);
> -		if (!ret)
> -			priv->vflip = ctrl->value;
> -		break;
> +		return ov6650_reg_rmw(client, REG_COMB,
> +				ctrl->val ? COMB_FLIP_V : 0, COMB_FLIP_V);
>  	case V4L2_CID_HFLIP:
> -		ret = ov6650_reg_rmw(client, REG_COMB,
> -				ctrl->value ? COMB_FLIP_H : 0, COMB_FLIP_H);
> -		if (!ret)
> -			priv->hflip = ctrl->value;
> -		break;
> +		return ov6650_reg_rmw(client, REG_COMB,
> +				ctrl->val ? COMB_FLIP_H : 0, COMB_FLIP_H);
>  	}
>  
> -	return ret;
> +	return -EINVAL;
>  }
>  
>  /* Get chip identification */
> @@ -1048,14 +857,12 @@ static int ov6650_video_probe(struct soc_camera_device *icd,
>  	return ret;
>  }
>  
> -static struct soc_camera_ops ov6650_ops = {
> -	.controls		= ov6650_controls,
> -	.num_controls		= ARRAY_SIZE(ov6650_controls),
> +static const struct v4l2_ctrl_ops ov6550_ctrl_ops = {
> +	.g_volatile_ctrl = ov6550_g_volatile_ctrl,
> +	.s_ctrl = ov6550_s_ctrl,
>  };
>  
>  static struct v4l2_subdev_core_ops ov6650_core_ops = {
> -	.g_ctrl			= ov6650_g_ctrl,
> -	.s_ctrl			= ov6650_s_ctrl,
>  	.g_chip_ident		= ov6650_g_chip_ident,
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>  	.g_register		= ov6650_get_register,
> @@ -1164,8 +971,46 @@ static int ov6650_probe(struct i2c_client *client,
>  	}
>  
>  	v4l2_i2c_subdev_init(&priv->subdev, client, &ov6650_subdev_ops);
> +	v4l2_ctrl_handler_init(&priv->hdl, 13);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_VFLIP, 0, 1, 1, 0);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_HFLIP, 0, 1, 1, 0);
> +	priv->autogain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
> +	priv->gain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_GAIN, 0, 0x3f, 1, DEF_GAIN);
> +	priv->autowb = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
> +	priv->blue = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_BLUE_BALANCE, 0, 0xff, 1, DEF_BLUE);
> +	priv->red = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_RED_BALANCE, 0, 0xff, 1, DEF_RED);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_SATURATION, 0, 0xf, 1, 0x8);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_HUE, 0, HUE_MASK, 1, DEF_HUE);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
> +	priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
> +			&ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,

max value of V4L2_EXPOSURE_MANUAL instead of equivalent 1 could be more 
clear.

> +			V4L2_EXPOSURE_AUTO);
> +	priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
> +	v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> +			V4L2_CID_GAMMA, 0, 0xff, 1, 0x12);
> +
> +	priv->subdev.ctrl_handler = &priv->hdl;
> +	if (priv->hdl.error) {
> +		int err = priv->hdl.error;
>  
> -	icd->ops = &ov6650_ops;
> +		kfree(priv);
> +		return err;
> +	}
> +	v4l2_ctrl_auto_cluster(2, &priv->autogain, 0, true);
> +	v4l2_ctrl_auto_cluster(3, &priv->autowb, 0, true);
> +	v4l2_ctrl_auto_cluster(2, &priv->autoexposure,
> +				V4L2_EXPOSURE_MANUAL, true);
>  
>  	priv->rect.left	  = DEF_HSTRT << 1;
>  	priv->rect.top	  = DEF_VSTRT << 1;
> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
>  	priv->colorspace  = V4L2_COLORSPACE_JPEG;
>  
>  	ret = ov6650_video_probe(icd, client);
> +	if (!ret)
> +		ret = v4l2_ctrl_handler_setup(&priv->hdl);

Are you sure the probe function should fail if v4l2_ctrl_handler_setup() 
fails? Its usage is documented as optional.

>  
>  	if (ret) {
> -		icd->ops = NULL;
> +		v4l2_ctrl_handler_free(&priv->hdl);
>  		kfree(priv);
>  	}
>  
> @@ -1189,6 +1036,8 @@ static int ov6650_remove(struct i2c_client *client)
>  {
>  	struct ov6650 *priv = to_ov6650(client);
>  
> +	v4l2_device_unregister_subdev(&priv->subdev);
> +	v4l2_ctrl_handler_free(&priv->hdl);
>  	kfree(priv);
>  	return 0;
>  }
> 

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Guennadi Liakhovetski]

Hi Janusz

On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:

> On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> Hi,
> I've successfully tested this one for you, to the extent possible using 
> mplayer, i.e., only saturation, hue and brightness controls, and an 
> overall functionality.

Thanks for testing and reviewing!

> Modifications to other (not runtime tested) controls look good to me, 
> except for one copy-paste mistake, see below. With that erratic REG_BLUE 
> corrected:
> 
> Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
> There are also a few minor comments for you to consider.

Well, some of them are not so minor, I would say;-) But I personally would 
be happy to have this just as an incremental patch. Would you like to 
prepare one or should I do it?

I basically agree with all your comments apart from maybe

[snip]

> > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> >  	priv->colorspace  = V4L2_COLORSPACE_JPEG;
> >  
> >  	ret = ov6650_video_probe(icd, client);
> > +	if (!ret)
> > +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
> 
> Are you sure the probe function should fail if v4l2_ctrl_handler_setup() 
> fails? Its usage is documented as optional.

Not sure what the standard really meant, but it looks like this is done in 
all patches in this series. So, we'd have to change this everywhere. Most 
other drivers indeed do not care.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Sylwester Nawrocki]

Hi,

On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
> Hi Janusz
> 
> On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> 
>> On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
>>> From: Hans Verkuil<hans.verkuil@cisco.com>
>>>
>>> Signed-off-by: Hans Verkuil<hans.verkuil@cisco.com>
>>> [g.liakhovetski@gmx.de: simplified pointer arithmetic]
>>> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
>>
>> Hi,
>> I've successfully tested this one for you, to the extent possible using
>> mplayer, i.e., only saturation, hue and brightness controls, and an
>> overall functionality.
> 
> Thanks for testing and reviewing!
> 
>> Modifications to other (not runtime tested) controls look good to me,
>> except for one copy-paste mistake, see below. With that erratic REG_BLUE
>> corrected:
>>
>> Acked-by: Janusz Krzysztofik<jkrzyszt@tis.icnet.pl>
>>
>> There are also a few minor comments for you to consider.
> 
> Well, some of them are not so minor, I would say;-) But I personally would
> be happy to have this just as an incremental patch. Would you like to
> prepare one or should I do it?
> 
> I basically agree with all your comments apart from maybe
> 
> [snip]
> 
>>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
>>>   	priv->colorspace  = V4L2_COLORSPACE_JPEG;
>>>
>>>   	ret = ov6650_video_probe(icd, client);
>>> +	if (!ret)
>>> +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
>>
>> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
>> fails? Its usage is documented as optional.
> 
> Not sure what the standard really meant, but it looks like this is done in
> all patches in this series. So, we'd have to change this everywhere. Most
> other drivers indeed do not care.

The usage of v4l2_ctrl_handler_setup() is optional, but if this function
is not used, then AFAIU the driver writer needs to ensure the control's 
values after the device is initialized are exactly as those specified during
the control creation. Of course v4l2_ctrl_handler_setup() failure might
mean s_ctrl op failed, which might be caused by some H/W access errors.
So IMHO it is always a good idea to check the return value if we know
the batch controls setup shouldn't fail.

--
Regards,
Sylwester

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Janusz Krzysztofik]

On Fri, 9 Sep 2011 at 20:01:14 Guennadi Liakhovetski wrote:
> Hi Janusz
> 
> On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> 
> > On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > 
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > 
> > Hi,
> > I've successfully tested this one for you, to the extent possible using 
> > mplayer, i.e., only saturation, hue and brightness controls, and an 
> > overall functionality.
> 
> Thanks for testing and reviewing!
> 
> > Modifications to other (not runtime tested) controls look good to me, 
> > except for one copy-paste mistake, see below. With that erratic REG_BLUE 
> > corrected:
> > 
> > Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > 
> > There are also a few minor comments for you to consider.
> 
> Well, some of them are not so minor, I would say;-) But I personally would 
> be happy to have this just as an incremental patch. Would you like to 
> prepare one or should I do it?

OK, I can do it.

Do you want them all (except the one below) go into the incremental 
patch, or would you rather fix that REG_RED bug still before applying?

Thanks,
Janusz

> 
> I basically agree with all your comments apart from maybe
> 
> [snip]
> 
> > > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > >  	priv->colorspace  = V4L2_COLORSPACE_JPEG;
> > >  
> > >  	ret = ov6650_video_probe(icd, client);
> > > +	if (!ret)
> > > +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
> > 
> > Are you sure the probe function should fail if v4l2_ctrl_handler_setup() 
> > fails? Its usage is documented as optional.
> 
> Not sure what the standard really meant, but it looks like this is done in 
> all patches in this series. So, we'd have to change this everywhere. Most 
> other drivers indeed do not care.
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Janusz Krzysztofik]

On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
> Hi,
> 
> On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:

[snip]

> > I basically agree with all your comments apart from maybe
> > 
> > [snip]
> > 
> >>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> >>>   	priv->colorspace  = V4L2_COLORSPACE_JPEG;
> >>>
> >>>   	ret = ov6650_video_probe(icd, client);
> >>> +	if (!ret)
> >>> +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
> >>
> >> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> >> fails? Its usage is documented as optional.
> > 
> > Not sure what the standard really meant, but it looks like this is done in
> > all patches in this series. So, we'd have to change this everywhere. Most
> > other drivers indeed do not care.
> 
> The usage of v4l2_ctrl_handler_setup() is optional, but if this function
> is not used, then AFAIU the driver writer needs to ensure the control's 
> values after the device is initialized are exactly as those specified during
> the control creation. Of course v4l2_ctrl_handler_setup() failure might
> mean s_ctrl op failed, which might be caused by some H/W access errors.
> So IMHO it is always a good idea to check the return value if we know
> the batch controls setup shouldn't fail.

I'm not for ignoring that return value, only wondering if the i2c_driver 
.probe handler should really fail in such cases, effectivelly preventing 
the device from being accessible at all.

Perhaps a warning message would be sufficient?

Thanks,
Janusz

> 
> --
> Regards,
> Sylwester
> 

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Guennadi Liakhovetski]

On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:

> On Fri, 9 Sep 2011 at 20:01:14 Guennadi Liakhovetski wrote:
> > Hi Janusz
> > 
> > On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> > 
> > > On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > > 
> > > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > 
> > > Hi,
> > > I've successfully tested this one for you, to the extent possible using 
> > > mplayer, i.e., only saturation, hue and brightness controls, and an 
> > > overall functionality.
> > 
> > Thanks for testing and reviewing!
> > 
> > > Modifications to other (not runtime tested) controls look good to me, 
> > > except for one copy-paste mistake, see below. With that erratic REG_BLUE 
> > > corrected:
> > > 
> > > Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > > 
> > > There are also a few minor comments for you to consider.
> > 
> > Well, some of them are not so minor, I would say;-) But I personally would 
> > be happy to have this just as an incremental patch. Would you like to 
> > prepare one or should I do it?
> 
> OK, I can do it.
> 
> Do you want them all (except the one below) go into the incremental 
> patch, or would you rather fix that REG_RED bug still before applying?

Just put them all in one.

Thanks
Guennadi

> 
> Thanks,
> Janusz
> 
> > 
> > I basically agree with all your comments apart from maybe
> > 
> > [snip]
> > 
> > > > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > > >  	priv->colorspace  = V4L2_COLORSPACE_JPEG;
> > > >  
> > > >  	ret = ov6650_video_probe(icd, client);
> > > > +	if (!ret)
> > > > +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
> > > 
> > > Are you sure the probe function should fail if v4l2_ctrl_handler_setup() 
> > > fails? Its usage is documented as optional.
> > 
> > Not sure what the standard really meant, but it looks like this is done in 
> > all patches in this series. So, we'd have to change this everywhere. Most 
> > other drivers indeed do not care.
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Guennadi Liakhovetski]

On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:

> On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
> > Hi,
> > 
> > On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > > I basically agree with all your comments apart from maybe
> > > 
> > > [snip]
> > > 
> > >>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > >>>   	priv->colorspace  = V4L2_COLORSPACE_JPEG;
> > >>>
> > >>>   	ret = ov6650_video_probe(icd, client);
> > >>> +	if (!ret)
> > >>> +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
> > >>
> > >> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> > >> fails? Its usage is documented as optional.
> > > 
> > > Not sure what the standard really meant, but it looks like this is done in
> > > all patches in this series. So, we'd have to change this everywhere. Most
> > > other drivers indeed do not care.
> > 
> > The usage of v4l2_ctrl_handler_setup() is optional, but if this function
> > is not used, then AFAIU the driver writer needs to ensure the control's 
> > values after the device is initialized are exactly as those specified during
> > the control creation. Of course v4l2_ctrl_handler_setup() failure might
> > mean s_ctrl op failed, which might be caused by some H/W access errors.
> > So IMHO it is always a good idea to check the return value if we know
> > the batch controls setup shouldn't fail.
> 
> I'm not for ignoring that return value, only wondering if the i2c_driver 
> .probe handler should really fail in such cases, effectivelly preventing 
> the device from being accessible at all.
> 
> Perhaps a warning message would be sufficient?

Well, restoring the state is normally something pretty basic like writing 
a couple of registers via i2c. If that didn't work out, there's little 
chance you'll be able to use the device anyway. So, I'd feel pretty safe 
with erroring out. If we ever encounter a case, where this can fail in a 
non-fatal way, we can always relax it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 08/13 v3] ov6650: convert to the control framework.

[Sylwester Nawrocki]

On 09/09/2011 10:58 PM, Janusz Krzysztofik wrote:
> On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
>> On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
> 
> [snip]
> 
>>> I basically agree with all your comments apart from maybe
>>>
>>> [snip]
>>>
>>>>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
>>>>>    	priv->colorspace  = V4L2_COLORSPACE_JPEG;
>>>>>
>>>>>    	ret = ov6650_video_probe(icd, client);
>>>>> +	if (!ret)
>>>>> +		ret = v4l2_ctrl_handler_setup(&priv->hdl);
>>>>
>>>> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
>>>> fails? Its usage is documented as optional.
>>>
>>> Not sure what the standard really meant, but it looks like this is done in
>>> all patches in this series. So, we'd have to change this everywhere. Most
>>> other drivers indeed do not care.
>>
>> The usage of v4l2_ctrl_handler_setup() is optional, but if this function
>> is not used, then AFAIU the driver writer needs to ensure the control's
>> values after the device is initialized are exactly as those specified during
>> the control creation. Of course v4l2_ctrl_handler_setup() failure might
>> mean s_ctrl op failed, which might be caused by some H/W access errors.
>> So IMHO it is always a good idea to check the return value if we know
>> the batch controls setup shouldn't fail.
> 
> I'm not for ignoring that return value, only wondering if the i2c_driver
> .probe handler should really fail in such cases, effectivelly preventing
> the device from being accessible at all.
> 
> Perhaps a warning message would be sufficient?

I guess, on individual cases - yes, but I wouldn't take it as a general rule.
If the device fails from the beginning it will probably not be really usable
thereafter.

--
Regards,
Sylwester