[PATCH 00/10] Miscellaneous ov772x cleanups and fixes

[PATCH 00/10] Miscellaneous ov772x cleanups and fixes

[Laurent Pinchart]

Hi Guennadi,

The subject line says it all :-) These patches have been written while
preparing the ov772x driver for being used with a non soc-camera host.
They apply on top of my previous soc-camera patch series.

Laurent Pinchart (10):
  ov772x: Reorganize the code in sections
  ov772x: Fix memory leak in probe error path
  ov772x: Select the default format at probe time
  ov772x: Don't fail in s_fmt if the requested format isn't supported
  ov772x: try_fmt must not default to the current format
  ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv
  ov772x: Add ov772x_read() and ov772x_write() functions
  ov772x: Add support for SBGGR10 format
  ov772x: Compute window size registers at runtime
  ov772x: Stop sensor readout right after reset

 drivers/media/video/ov772x.c |  801 +++++++++++++++++++++---------------------
 1 files changed, 398 insertions(+), 403 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 01/10] ov772x: Reorganize the code in sections

[Laurent Pinchart]

Functions are only moved around without any other code change.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |  503 +++++++++++++++++++++---------------------
 1 files changed, 256 insertions(+), 247 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 641f6f4..7c645dd 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -439,8 +439,55 @@ static const struct regval_list ov772x_vga_regs[] = {
 };
 
 /*
- * supported color format list
+ * general function
  */
+
+static struct ov772x_priv *to_ov772x(const struct i2c_client *client)
+{
+	return container_of(i2c_get_clientdata(client), struct ov772x_priv,
+			    subdev);
+}
+
+static int ov772x_write_array(struct i2c_client        *client,
+			      const struct regval_list *vals)
+{
+	while (vals->reg_num != 0xff) {
+		int ret = i2c_smbus_write_byte_data(client,
+						    vals->reg_num,
+						    vals->value);
+		if (ret < 0)
+			return ret;
+		vals++;
+	}
+	return 0;
+}
+
+static int ov772x_mask_set(struct i2c_client *client,
+					  u8  command,
+					  u8  mask,
+					  u8  set)
+{
+	s32 val = i2c_smbus_read_byte_data(client, command);
+	if (val < 0)
+		return val;
+
+	val &= ~mask;
+	val |= set & mask;
+
+	return i2c_smbus_write_byte_data(client, command, val);
+}
+
+static int ov772x_reset(struct i2c_client *client)
+{
+	int ret = i2c_smbus_write_byte_data(client, COM7, SCCB_RESET);
+	msleep(1);
+	return ret;
+}
+
+/* -----------------------------------------------------------------------------
+ * Formats and hardware configuration
+ */
+
 static const struct ov772x_color_format ov772x_cfmts[] = {
 	{
 		.code		= V4L2_MBUS_FMT_YUYV8_2X8,
@@ -493,10 +540,6 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 	},
 };
 
-
-/*
- * window size list
- */
 #define VGA_WIDTH   640
 #define VGA_HEIGHT  480
 #define QVGA_WIDTH  320
@@ -520,177 +563,6 @@ static const struct ov772x_win_size ov772x_win_qvga = {
 	.regs     = ov772x_qvga_regs,
 };
 
-/*
- * general function
- */
-
-static struct ov772x_priv *to_ov772x(const struct i2c_client *client)
-{
-	return container_of(i2c_get_clientdata(client), struct ov772x_priv,
-			    subdev);
-}
-
-static int ov772x_write_array(struct i2c_client        *client,
-			      const struct regval_list *vals)
-{
-	while (vals->reg_num != 0xff) {
-		int ret = i2c_smbus_write_byte_data(client,
-						    vals->reg_num,
-						    vals->value);
-		if (ret < 0)
-			return ret;
-		vals++;
-	}
-	return 0;
-}
-
-static int ov772x_mask_set(struct i2c_client *client,
-					  u8  command,
-					  u8  mask,
-					  u8  set)
-{
-	s32 val = i2c_smbus_read_byte_data(client, command);
-	if (val < 0)
-		return val;
-
-	val &= ~mask;
-	val |= set & mask;
-
-	return i2c_smbus_write_byte_data(client, command, val);
-}
-
-static int ov772x_reset(struct i2c_client *client)
-{
-	int ret = i2c_smbus_write_byte_data(client, COM7, SCCB_RESET);
-	msleep(1);
-	return ret;
-}
-
-/*
- * soc_camera_ops function
- */
-
-static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
-
-	if (!enable) {
-		ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
-		return 0;
-	}
-
-	if (!priv->win || !priv->cfmt) {
-		dev_err(&client->dev, "norm or win select error\n");
-		return -EPERM;
-	}
-
-	ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0);
-
-	dev_dbg(&client->dev, "format %d, win %s\n",
-		priv->cfmt->code, priv->win->name);
-
-	return 0;
-}
-
-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);
-	int ret = 0;
-	u8 val;
-
-	switch (ctrl->id) {
-	case V4L2_CID_VFLIP:
-		val = ctrl->val ? VFLIP_IMG : 0x00;
-		priv->flag_vflip = ctrl->val;
-		if (priv->info->flags & OV772X_FLAG_VFLIP)
-			val ^= VFLIP_IMG;
-		return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
-	case V4L2_CID_HFLIP:
-		val = ctrl->val ? HFLIP_IMG : 0x00;
-		priv->flag_hflip = ctrl->val;
-		if (priv->info->flags & OV772X_FLAG_HFLIP)
-			val ^= HFLIP_IMG;
-		return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
-	case V4L2_CID_BAND_STOP_FILTER:
-		if (!ctrl->val) {
-			/* Switch the filter off, it is on now */
-			ret = ov772x_mask_set(client, BDBASE, 0xff, 0xff);
-			if (!ret)
-				ret = ov772x_mask_set(client, COM8,
-						      BNDF_ON_OFF, 0);
-		} else {
-			/* Switch the filter on, set AEC low limit */
-			val = 256 - ctrl->val;
-			ret = ov772x_mask_set(client, COM8,
-					      BNDF_ON_OFF, BNDF_ON_OFF);
-			if (!ret)
-				ret = ov772x_mask_set(client, BDBASE,
-						      0xff, val);
-		}
-		if (!ret)
-			priv->band_filter = ctrl->val;
-		return ret;
-	}
-
-	return -EINVAL;
-}
-
-static int ov772x_g_chip_ident(struct v4l2_subdev *sd,
-			       struct v4l2_dbg_chip_ident *id)
-{
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
-
-	id->ident    = priv->model;
-	id->revision = 0;
-
-	return 0;
-}
-
-#ifdef CONFIG_VIDEO_ADV_DEBUG
-static int ov772x_g_register(struct v4l2_subdev *sd,
-			     struct v4l2_dbg_register *reg)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	int ret;
-
-	reg->size = 1;
-	if (reg->reg > 0xff)
-		return -EINVAL;
-
-	ret = i2c_smbus_read_byte_data(client, reg->reg);
-	if (ret < 0)
-		return ret;
-
-	reg->val = (__u64)ret;
-
-	return 0;
-}
-
-static int ov772x_s_register(struct v4l2_subdev *sd,
-			     struct v4l2_dbg_register *reg)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-
-	if (reg->reg > 0xff ||
-	    reg->val > 0xff)
-		return -EINVAL;
-
-	return i2c_smbus_write_byte_data(client, reg->reg, reg->val);
-}
-#endif
-
-static int ov772x_s_power(struct v4l2_subdev *sd, int on)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct soc_camera_link *icl = soc_camera_i2c_to_link(client);
-
-	return soc_camera_set_power(&client->dev, icl, on);
-}
-
 static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
 {
 	__u32 diff;
@@ -860,27 +732,139 @@ ov772x_set_fmt_error:
 	return ret;
 }
 
-static int ov772x_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+/* -----------------------------------------------------------------------------
+ * Controls
+ */
+
+static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	a->c.left	= 0;
-	a->c.top	= 0;
-	a->c.width	= VGA_WIDTH;
-	a->c.height	= VGA_HEIGHT;
-	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	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);
+	int ret = 0;
+	u8 val;
+
+	switch (ctrl->id) {
+	case V4L2_CID_VFLIP:
+		val = ctrl->val ? VFLIP_IMG : 0x00;
+		priv->flag_vflip = ctrl->val;
+		if (priv->info->flags & OV772X_FLAG_VFLIP)
+			val ^= VFLIP_IMG;
+		return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
+	case V4L2_CID_HFLIP:
+		val = ctrl->val ? HFLIP_IMG : 0x00;
+		priv->flag_hflip = ctrl->val;
+		if (priv->info->flags & OV772X_FLAG_HFLIP)
+			val ^= HFLIP_IMG;
+		return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
+	case V4L2_CID_BAND_STOP_FILTER:
+		if (!ctrl->val) {
+			/* Switch the filter off, it is on now */
+			ret = ov772x_mask_set(client, BDBASE, 0xff, 0xff);
+			if (!ret)
+				ret = ov772x_mask_set(client, COM8,
+						      BNDF_ON_OFF, 0);
+		} else {
+			/* Switch the filter on, set AEC low limit */
+			val = 256 - ctrl->val;
+			ret = ov772x_mask_set(client, COM8,
+					      BNDF_ON_OFF, BNDF_ON_OFF);
+			if (!ret)
+				ret = ov772x_mask_set(client, BDBASE,
+						      0xff, val);
+		}
+		if (!ret)
+			priv->band_filter = ctrl->val;
+		return ret;
+	}
+
+	return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops ov772x_ctrl_ops = {
+	.s_ctrl = ov772x_s_ctrl,
+};
+
+/* -----------------------------------------------------------------------------
+ * Core operations
+ */
+
+static int ov772x_g_chip_ident(struct v4l2_subdev *sd,
+			       struct v4l2_dbg_chip_ident *id)
+{
+	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+
+	id->ident    = priv->model;
+	id->revision = 0;
 
 	return 0;
 }
 
-static int ov772x_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int ov772x_g_register(struct v4l2_subdev *sd,
+			     struct v4l2_dbg_register *reg)
 {
-	a->bounds.left			= 0;
-	a->bounds.top			= 0;
-	a->bounds.width			= VGA_WIDTH;
-	a->bounds.height		= VGA_HEIGHT;
-	a->defrect			= a->bounds;
-	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	a->pixelaspect.numerator	= 1;
-	a->pixelaspect.denominator	= 1;
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	int ret;
+
+	reg->size = 1;
+	if (reg->reg > 0xff)
+		return -EINVAL;
+
+	ret = i2c_smbus_read_byte_data(client, reg->reg);
+	if (ret < 0)
+		return ret;
+
+	reg->val = (__u64)ret;
+
+	return 0;
+}
+
+static int ov772x_s_register(struct v4l2_subdev *sd,
+			     struct v4l2_dbg_register *reg)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+	if (reg->reg > 0xff ||
+	    reg->val > 0xff)
+		return -EINVAL;
+
+	return i2c_smbus_write_byte_data(client, reg->reg, reg->val);
+}
+#endif
+
+static int ov772x_s_power(struct v4l2_subdev *sd, int on)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct soc_camera_link *icl = soc_camera_i2c_to_link(client);
+
+	return soc_camera_set_power(&client->dev, icl, on);
+}
+
+/* -----------------------------------------------------------------------------
+ * Video operations
+ */
+
+static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+
+	if (!enable) {
+		ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
+		return 0;
+	}
+
+	if (!priv->win || !priv->cfmt) {
+		dev_err(&client->dev, "norm or win select error\n");
+		return -EPERM;
+	}
+
+	ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0);
+
+	dev_dbg(&client->dev, "format %d, win %s\n",
+		priv->cfmt->code, priv->win->name);
 
 	return 0;
 }
@@ -957,65 +941,30 @@ static int ov772x_try_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static int ov772x_video_probe(struct i2c_client *client)
+static int ov772x_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 {
-	struct ov772x_priv *priv = to_ov772x(client);
-	u8                  pid, ver;
-	const char         *devname;
-	int		    ret;
-
-	ret = ov772x_s_power(&priv->subdev, 1);
-	if (ret < 0)
-		return ret;
-
-	/*
-	 * check and show product ID and manufacturer ID
-	 */
-	pid = i2c_smbus_read_byte_data(client, PID);
-	ver = i2c_smbus_read_byte_data(client, VER);
-
-	switch (VERSION(pid, ver)) {
-	case OV7720:
-		devname     = "ov7720";
-		priv->model = V4L2_IDENT_OV7720;
-		break;
-	case OV7725:
-		devname     = "ov7725";
-		priv->model = V4L2_IDENT_OV7725;
-		break;
-	default:
-		dev_err(&client->dev,
-			"Product ID error %x:%x\n", pid, ver);
-		ret = -ENODEV;
-		goto done;
-	}
-
-	dev_info(&client->dev,
-		 "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
-		 devname,
-		 pid,
-		 ver,
-		 i2c_smbus_read_byte_data(client, MIDH),
-		 i2c_smbus_read_byte_data(client, MIDL));
-	ret = v4l2_ctrl_handler_setup(&priv->hdl);
+	a->c.left	= 0;
+	a->c.top	= 0;
+	a->c.width	= VGA_WIDTH;
+	a->c.height	= VGA_HEIGHT;
+	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
-done:
-	ov772x_s_power(&priv->subdev, 0);
-	return ret;
+	return 0;
 }
 
-static const struct v4l2_ctrl_ops ov772x_ctrl_ops = {
-	.s_ctrl = ov772x_s_ctrl,
-};
+static int ov772x_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+	a->bounds.left			= 0;
+	a->bounds.top			= 0;
+	a->bounds.width			= VGA_WIDTH;
+	a->bounds.height		= VGA_HEIGHT;
+	a->defrect			= a->bounds;
+	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	a->pixelaspect.numerator	= 1;
+	a->pixelaspect.denominator	= 1;
 
-static struct v4l2_subdev_core_ops ov772x_subdev_core_ops = {
-	.g_chip_ident	= ov772x_g_chip_ident,
-#ifdef CONFIG_VIDEO_ADV_DEBUG
-	.g_register	= ov772x_g_register,
-	.s_register	= ov772x_s_register,
-#endif
-	.s_power	= ov772x_s_power,
-};
+	return 0;
+}
 
 static int ov772x_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
 			   enum v4l2_mbus_pixelcode *code)
@@ -1042,6 +991,19 @@ static int ov772x_g_mbus_config(struct v4l2_subdev *sd,
 	return 0;
 }
 
+/* -----------------------------------------------------------------------------
+ * Subdev operations
+ */
+
+static struct v4l2_subdev_core_ops ov772x_subdev_core_ops = {
+	.g_chip_ident	= ov772x_g_chip_ident,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+	.g_register	= ov772x_g_register,
+	.s_register	= ov772x_s_register,
+#endif
+	.s_power	= ov772x_s_power,
+};
+
 static struct v4l2_subdev_video_ops ov772x_subdev_video_ops = {
 	.s_stream	= ov772x_s_stream,
 	.g_mbus_fmt	= ov772x_g_fmt,
@@ -1058,10 +1020,57 @@ static struct v4l2_subdev_ops ov772x_subdev_ops = {
 	.video	= &ov772x_subdev_video_ops,
 };
 
-/*
- * i2c_driver function
+/* -----------------------------------------------------------------------------
+ * Initialization and cleanup
  */
 
+static int ov772x_video_probe(struct i2c_client *client)
+{
+	struct ov772x_priv *priv = to_ov772x(client);
+	u8                  pid, ver;
+	const char         *devname;
+	int		    ret;
+
+	ret = ov772x_s_power(&priv->subdev, 1);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * check and show product ID and manufacturer ID
+	 */
+	pid = i2c_smbus_read_byte_data(client, PID);
+	ver = i2c_smbus_read_byte_data(client, VER);
+
+	switch (VERSION(pid, ver)) {
+	case OV7720:
+		devname     = "ov7720";
+		priv->model = V4L2_IDENT_OV7720;
+		break;
+	case OV7725:
+		devname     = "ov7725";
+		priv->model = V4L2_IDENT_OV7725;
+		break;
+	default:
+		dev_err(&client->dev,
+			"Product ID error %x:%x\n", pid, ver);
+		ret = -ENODEV;
+		goto done;
+	}
+
+	dev_info(&client->dev,
+		 "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
+		 devname,
+		 pid,
+		 ver,
+		 i2c_smbus_read_byte_data(client, MIDH),
+		 i2c_smbus_read_byte_data(client, MIDL));
+	ret = v4l2_ctrl_handler_setup(&priv->hdl);
+
+done:
+	ov772x_s_power(&priv->subdev, 0);
+	return ret;
+}
+
 static int ov772x_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 {
-- 
1.7.8.6

[PATCH 02/10] ov772x: Fix memory leak in probe error path

[Laurent Pinchart]

The control handler isn't freed if its initialization fails. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 7c645dd..066bac6 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -1107,18 +1107,17 @@ static int ov772x_probe(struct i2c_client *client,
 			V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
 	priv->subdev.ctrl_handler = &priv->hdl;
 	if (priv->hdl.error) {
-		int err = priv->hdl.error;
-
-		kfree(priv);
-		return err;
+		ret = priv->hdl.error;
+		goto done;
 	}
 
 	ret = ov772x_video_probe(client);
+
+done:
 	if (ret) {
 		v4l2_ctrl_handler_free(&priv->hdl);
 		kfree(priv);
 	}
-
 	return ret;
 }
 
-- 
1.7.8.6

[PATCH 03/10] ov772x: Select the default format at probe time

[Laurent Pinchart]

The format and window size are only initialized during the first g_fmt
call. This leaves the device in an inconsistent state after
initialization, which will cause problems when implementing pad
operations. Move the format and window size initialization to probe
time.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   63 ++++++++++++++++++++---------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 066bac6..576780a 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -547,37 +547,36 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 #define MAX_WIDTH   VGA_WIDTH
 #define MAX_HEIGHT  VGA_HEIGHT
 
-static const struct ov772x_win_size ov772x_win_vga = {
-	.name     = "VGA",
-	.width    = VGA_WIDTH,
-	.height   = VGA_HEIGHT,
-	.com7_bit = SLCT_VGA,
-	.regs     = ov772x_vga_regs,
-};
-
-static const struct ov772x_win_size ov772x_win_qvga = {
-	.name     = "QVGA",
-	.width    = QVGA_WIDTH,
-	.height   = QVGA_HEIGHT,
-	.com7_bit = SLCT_QVGA,
-	.regs     = ov772x_qvga_regs,
+static const struct ov772x_win_size ov772x_win_sizes[] = {
+	{
+		.name     = "VGA",
+		.width    = VGA_WIDTH,
+		.height   = VGA_HEIGHT,
+		.com7_bit = SLCT_VGA,
+		.regs     = ov772x_vga_regs,
+	}, {
+		.name     = "QVGA",
+		.width    = QVGA_WIDTH,
+		.height   = QVGA_HEIGHT,
+		.com7_bit = SLCT_QVGA,
+		.regs     = ov772x_qvga_regs,
+	},
 };
 
 static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
 {
-	__u32 diff;
-	const struct ov772x_win_size *win;
-
-	/* default is QVGA */
-	diff = abs(width - ov772x_win_qvga.width) +
-		abs(height - ov772x_win_qvga.height);
-	win = &ov772x_win_qvga;
-
-	/* VGA */
-	if (diff >
-	    abs(width  - ov772x_win_vga.width) +
-	    abs(height - ov772x_win_vga.height))
-		win = &ov772x_win_vga;
+	const struct ov772x_win_size *win = &ov772x_win_sizes[0];
+	unsigned int i;
+	u32 best_diff = (u32)-1;
+
+	for (i = 0; i < ARRAY_SIZE(ov772x_win_sizes); ++i) {
+		u32 diff = abs(width - ov772x_win_sizes[i].width)
+			 + abs(height - ov772x_win_sizes[i].height);
+		if (diff < best_diff) {
+			best_diff = diff;
+			win = &ov772x_win_sizes[i];
+		}
+	}
 
 	return win;
 }
@@ -874,11 +873,6 @@ static int ov772x_g_fmt(struct v4l2_subdev *sd,
 {
 	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
 
-	if (!priv->win || !priv->cfmt) {
-		priv->cfmt = &ov772x_cfmts[0];
-		priv->win = ov772x_select_win(VGA_WIDTH, VGA_HEIGHT);
-	}
-
 	mf->width	= priv->win->width;
 	mf->height	= priv->win->height;
 	mf->code	= priv->cfmt->code;
@@ -1112,6 +1106,11 @@ static int ov772x_probe(struct i2c_client *client,
 	}
 
 	ret = ov772x_video_probe(client);
+	if (ret < 0)
+		goto done;
+
+	priv->cfmt = &ov772x_cfmts[0];
+	priv->win = &ov772x_win_sizes[0];
 
 done:
 	if (ret) {
-- 
1.7.8.6

[PATCH 04/10] ov772x: Don't fail in s_fmt if the requested format isn't supported

[Laurent Pinchart]

Select a default format instead.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   83 ++++++++++++++++++++++--------------------
 1 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 576780a..fcb338a 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -581,31 +581,33 @@ static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
 	return win;
 }
 
-static int ov772x_set_params(struct i2c_client *client, u32 *width, u32 *height,
-			     enum v4l2_mbus_pixelcode code)
+static void ov772x_select_params(const struct v4l2_mbus_framefmt *mf,
+				 const struct ov772x_color_format **cfmt,
+				 const struct ov772x_win_size **win)
 {
-	struct ov772x_priv *priv = to_ov772x(client);
-	int ret = -EINVAL;
-	u8  val;
-	int i;
+	unsigned int i;
+
+	/* Select the format format. */
+	*cfmt = &ov772x_cfmts[0];
 
-	/*
-	 * select format
-	 */
-	priv->cfmt = NULL;
 	for (i = 0; i < ARRAY_SIZE(ov772x_cfmts); i++) {
-		if (code == ov772x_cfmts[i].code) {
-			priv->cfmt = ov772x_cfmts + i;
+		if (mf->code == ov772x_cfmts[i].code) {
+			*cfmt = &ov772x_cfmts[i];
 			break;
 		}
 	}
-	if (!priv->cfmt)
-		goto ov772x_set_fmt_error;
 
-	/*
-	 * select win
-	 */
-	priv->win = ov772x_select_win(*width, *height);
+	/* Select the window size. */
+	*win = ov772x_select_win(mf->width, mf->height);
+}
+
+static int ov772x_set_params(struct ov772x_priv *priv,
+			     const struct ov772x_color_format *cfmt,
+			     const struct ov772x_win_size *win)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
+	int ret;
+	u8  val;
 
 	/*
 	 * reset hardware
@@ -662,14 +664,14 @@ static int ov772x_set_params(struct i2c_client *client, u32 *width, u32 *height,
 	/*
 	 * set size format
 	 */
-	ret = ov772x_write_array(client, priv->win->regs);
+	ret = ov772x_write_array(client, win->regs);
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
 	/*
 	 * set DSP_CTRL3
 	 */
-	val = priv->cfmt->dsp3;
+	val = cfmt->dsp3;
 	if (val) {
 		ret = ov772x_mask_set(client,
 				      DSP_CTRL3, UV_MASK, val);
@@ -680,7 +682,7 @@ static int ov772x_set_params(struct i2c_client *client, u32 *width, u32 *height,
 	/*
 	 * set COM3
 	 */
-	val = priv->cfmt->com3;
+	val = cfmt->com3;
 	if (priv->info->flags & OV772X_FLAG_VFLIP)
 		val |= VFLIP_IMG;
 	if (priv->info->flags & OV772X_FLAG_HFLIP)
@@ -698,7 +700,7 @@ static int ov772x_set_params(struct i2c_client *client, u32 *width, u32 *height,
 	/*
 	 * set COM7
 	 */
-	val = priv->win->com7_bit | priv->cfmt->com7;
+	val = win->com7_bit | cfmt->com7;
 	ret = ov772x_mask_set(client,
 			      COM7, SLCT_MASK | FMT_MASK | OFMT_MASK,
 			      val);
@@ -717,16 +719,11 @@ static int ov772x_set_params(struct i2c_client *client, u32 *width, u32 *height,
 			goto ov772x_set_fmt_error;
 	}
 
-	*width = priv->win->width;
-	*height = priv->win->height;
-
 	return ret;
 
 ov772x_set_fmt_error:
 
 	ov772x_reset(client);
-	priv->win = NULL;
-	priv->cfmt = NULL;
 
 	return ret;
 }
@@ -855,11 +852,6 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
 		return 0;
 	}
 
-	if (!priv->win || !priv->cfmt) {
-		dev_err(&client->dev, "norm or win select error\n");
-		return -EPERM;
-	}
-
 	ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0);
 
 	dev_dbg(&client->dev, "format %d, win %s\n",
@@ -882,18 +874,29 @@ static int ov772x_g_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static int ov772x_s_fmt(struct v4l2_subdev *sd,
-			struct v4l2_mbus_framefmt *mf)
+static int ov772x_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
-	int ret = ov772x_set_params(client, &mf->width, &mf->height,
-				    mf->code);
+	const struct ov772x_color_format *cfmt;
+	const struct ov772x_win_size *win;
+	int ret;
 
-	if (!ret)
-		mf->colorspace = priv->cfmt->colorspace;
+	ov772x_select_params(mf, &cfmt, &win);
 
-	return ret;
+	ret = ov772x_set_params(priv, cfmt, win);
+	if (ret < 0)
+		return ret;
+
+	priv->win = win;
+	priv->cfmt = cfmt;
+
+	mf->code = cfmt->code;
+	mf->width = win->width;
+	mf->height = win->height;
+	mf->field = V4L2_FIELD_NONE;
+	mf->colorspace = cfmt->colorspace;
+
+	return 0;
 }
 
 static int ov772x_try_fmt(struct v4l2_subdev *sd,
-- 
1.7.8.6

[PATCH 05/10] ov772x: try_fmt must not default to the current format

[Laurent Pinchart]

If the requested format isn't supported, return a fixed default format
instead of the current format.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   36 +++++++-----------------------------
 1 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index fcb338a..e3de4de 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -902,38 +902,16 @@ static int ov772x_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf)
 static int ov772x_try_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_mbus_framefmt *mf)
 {
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+	const struct ov772x_color_format *cfmt;
 	const struct ov772x_win_size *win;
-	int i;
-
-	/*
-	 * select suitable win
-	 */
-	win = ov772x_select_win(mf->width, mf->height);
 
-	mf->width	= win->width;
-	mf->height	= win->height;
-	mf->field	= V4L2_FIELD_NONE;
-
-	for (i = 0; i < ARRAY_SIZE(ov772x_cfmts); i++)
-		if (mf->code == ov772x_cfmts[i].code)
-			break;
+	ov772x_select_params(mf, &cfmt, &win);
 
-	if (i == ARRAY_SIZE(ov772x_cfmts)) {
-		/* Unsupported format requested. Propose either */
-		if (priv->cfmt) {
-			/* the current one or */
-			mf->colorspace = priv->cfmt->colorspace;
-			mf->code = priv->cfmt->code;
-		} else {
-			/* the default one */
-			mf->colorspace = ov772x_cfmts[0].colorspace;
-			mf->code = ov772x_cfmts[0].code;
-		}
-	} else {
-		/* Also return the colorspace */
-		mf->colorspace	= ov772x_cfmts[i].colorspace;
-	}
+	mf->code = cfmt->code;
+	mf->width = win->width;
+	mf->height = win->height;
+	mf->field = V4L2_FIELD_NONE;
+	mf->colorspace = cfmt->colorspace;
 
 	return 0;
 }
-- 
1.7.8.6

[PATCH 06/10] ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv

[Laurent Pinchart]

Conversion from i2c_client to ov772x_priv is only needed in a single
location, while conversion from v4l2_subdev to ov772x_priv is needed in
several locations. Perform the former manually, and use to_ov772x for
the later.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index e3de4de..9055ba4 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -442,10 +442,9 @@ static const struct regval_list ov772x_vga_regs[] = {
  * general function
  */
 
-static struct ov772x_priv *to_ov772x(const struct i2c_client *client)
+static struct ov772x_priv *to_ov772x(struct v4l2_subdev *sd)
 {
-	return container_of(i2c_get_clientdata(client), struct ov772x_priv,
-			    subdev);
+	return container_of(sd, struct ov772x_priv, subdev);
 }
 
 static int ov772x_write_array(struct i2c_client        *client,
@@ -789,7 +788,7 @@ static const struct v4l2_ctrl_ops ov772x_ctrl_ops = {
 static int ov772x_g_chip_ident(struct v4l2_subdev *sd,
 			       struct v4l2_dbg_chip_ident *id)
 {
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+	struct ov772x_priv *priv = to_ov772x(sd);
 
 	id->ident    = priv->model;
 	id->revision = 0;
@@ -845,7 +844,7 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
 static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+	struct ov772x_priv *priv = to_ov772x(sd);
 
 	if (!enable) {
 		ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
@@ -863,7 +862,7 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
 static int ov772x_g_fmt(struct v4l2_subdev *sd,
 			struct v4l2_mbus_framefmt *mf)
 {
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+	struct ov772x_priv *priv = to_ov772x(sd);
 
 	mf->width	= priv->win->width;
 	mf->height	= priv->win->height;
@@ -876,7 +875,7 @@ static int ov772x_g_fmt(struct v4l2_subdev *sd,
 
 static int ov772x_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf)
 {
-	struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
+	struct ov772x_priv *priv = to_ov772x(sd);
 	const struct ov772x_color_format *cfmt;
 	const struct ov772x_win_size *win;
 	int ret;
@@ -999,9 +998,9 @@ static struct v4l2_subdev_ops ov772x_subdev_ops = {
  * Initialization and cleanup
  */
 
-static int ov772x_video_probe(struct i2c_client *client)
+static int ov772x_video_probe(struct ov772x_priv *priv)
 {
-	struct ov772x_priv *priv = to_ov772x(client);
+	struct i2c_client  *client = v4l2_get_subdevdata(&priv->subdev);
 	u8                  pid, ver;
 	const char         *devname;
 	int		    ret;
@@ -1086,7 +1085,7 @@ static int ov772x_probe(struct i2c_client *client,
 		goto done;
 	}
 
-	ret = ov772x_video_probe(client);
+	ret = ov772x_video_probe(priv);
 	if (ret < 0)
 		goto done;
 
@@ -1103,7 +1102,7 @@ done:
 
 static int ov772x_remove(struct i2c_client *client)
 {
-	struct ov772x_priv *priv = to_ov772x(client);
+	struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
 
 	v4l2_device_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
-- 
1.7.8.6

[PATCH 07/10] ov772x: Add ov772x_read() and ov772x_write() functions

[Laurent Pinchart]

And use them instead of calling SMBus access functions directly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   38 ++++++++++++++++++++++----------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 9055ba4..67c385b 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -447,13 +447,21 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev *sd)
 	return container_of(sd, struct ov772x_priv, subdev);
 }
 
+static inline int ov772x_read(struct i2c_client *client, u8 addr)
+{
+	return i2c_smbus_read_byte_data(client, addr);
+}
+
+static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
+{
+	return i2c_smbus_write_byte_data(client, addr, value);
+}
+
 static int ov772x_write_array(struct i2c_client        *client,
 			      const struct regval_list *vals)
 {
 	while (vals->reg_num != 0xff) {
-		int ret = i2c_smbus_write_byte_data(client,
-						    vals->reg_num,
-						    vals->value);
+		int ret = ov772x_write(client, vals->reg_num, vals->value);
 		if (ret < 0)
 			return ret;
 		vals++;
@@ -461,24 +469,22 @@ static int ov772x_write_array(struct i2c_client        *client,
 	return 0;
 }
 
-static int ov772x_mask_set(struct i2c_client *client,
-					  u8  command,
-					  u8  mask,
-					  u8  set)
+static int ov772x_mask_set(struct i2c_client *client, u8  command, u8  mask,
+			   u8  set)
 {
-	s32 val = i2c_smbus_read_byte_data(client, command);
+	s32 val = ov772x_read(client, command);
 	if (val < 0)
 		return val;
 
 	val &= ~mask;
 	val |= set & mask;
 
-	return i2c_smbus_write_byte_data(client, command, val);
+	return ov772x_write(client, command, val);
 }
 
 static int ov772x_reset(struct i2c_client *client)
 {
-	int ret = i2c_smbus_write_byte_data(client, COM7, SCCB_RESET);
+	int ret = ov772x_write(client, COM7, SCCB_RESET);
 	msleep(1);
 	return ret;
 }
@@ -807,7 +813,7 @@ static int ov772x_g_register(struct v4l2_subdev *sd,
 	if (reg->reg > 0xff)
 		return -EINVAL;
 
-	ret = i2c_smbus_read_byte_data(client, reg->reg);
+	ret = ov772x_write(client, reg->reg);
 	if (ret < 0)
 		return ret;
 
@@ -825,7 +831,7 @@ static int ov772x_s_register(struct v4l2_subdev *sd,
 	    reg->val > 0xff)
 		return -EINVAL;
 
-	return i2c_smbus_write_byte_data(client, reg->reg, reg->val);
+	return ov772x_write(client, reg->reg, reg->val);
 }
 #endif
 
@@ -1012,8 +1018,8 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
 	/*
 	 * check and show product ID and manufacturer ID
 	 */
-	pid = i2c_smbus_read_byte_data(client, PID);
-	ver = i2c_smbus_read_byte_data(client, VER);
+	pid = ov772x_read(client, PID);
+	ver = ov772x_read(client, VER);
 
 	switch (VERSION(pid, ver)) {
 	case OV7720:
@@ -1036,8 +1042,8 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
 		 devname,
 		 pid,
 		 ver,
-		 i2c_smbus_read_byte_data(client, MIDH),
-		 i2c_smbus_read_byte_data(client, MIDL));
+		 ov772x_read(client, MIDH),
+		 ov772x_read(client, MIDL));
 	ret = v4l2_ctrl_handler_setup(&priv->hdl);
 
 done:
-- 
1.7.8.6

[PATCH 08/10] ov772x: Add support for SBGGR10 format

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   43 +++++++++++++++++++++++++++++++++++------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 67c385b..07ff709 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -274,6 +274,7 @@
 #define SLCT_VGA        0x00	/*   0 : VGA */
 #define SLCT_QVGA       0x40	/*   1 : QVGA */
 #define ITU656_ON_OFF   0x20	/* ITU656 protocol ON/OFF selection */
+#define SENSOR_RAW	0x10	/* Sensor RAW */
 				/* RGB output format control */
 #define FMT_MASK        0x0c	/*      Mask of color format */
 #define FMT_GBR422      0x00	/*      00 : GBR 4:2:2 */
@@ -337,6 +338,12 @@
 #define CBAR_ON         0x20	/*   ON */
 #define CBAR_OFF        0x00	/*   OFF */
 
+/* DSP_CTRL4 */
+#define DSP_OFMT_YUV	0x00
+#define DSP_OFMT_RGB	0x00
+#define DSP_OFMT_RAW8	0x02
+#define DSP_OFMT_RAW10	0x03
+
 /* HSTART */
 #define HST_VGA         0x23
 #define HST_QVGA        0x3F
@@ -388,6 +395,7 @@ struct ov772x_color_format {
 	enum v4l2_mbus_pixelcode code;
 	enum v4l2_colorspace colorspace;
 	u8 dsp3;
+	u8 dsp4;
 	u8 com3;
 	u8 com7;
 };
@@ -498,6 +506,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_YUYV8_2X8,
 		.colorspace	= V4L2_COLORSPACE_JPEG,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= SWAP_YUV,
 		.com7		= OFMT_YUV,
 	},
@@ -505,6 +514,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_YVYU8_2X8,
 		.colorspace	= V4L2_COLORSPACE_JPEG,
 		.dsp3		= UV_ON,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= SWAP_YUV,
 		.com7		= OFMT_YUV,
 	},
@@ -512,6 +522,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_UYVY8_2X8,
 		.colorspace	= V4L2_COLORSPACE_JPEG,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= 0x0,
 		.com7		= OFMT_YUV,
 	},
@@ -519,6 +530,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= SWAP_RGB,
 		.com7		= FMT_RGB555 | OFMT_RGB,
 	},
@@ -526,6 +538,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= 0x0,
 		.com7		= FMT_RGB555 | OFMT_RGB,
 	},
@@ -533,6 +546,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_RGB565_2X8_LE,
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= SWAP_RGB,
 		.com7		= FMT_RGB565 | OFMT_RGB,
 	},
@@ -540,9 +554,22 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 		.code		= V4L2_MBUS_FMT_RGB565_2X8_BE,
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_YUV,
 		.com3		= 0x0,
 		.com7		= FMT_RGB565 | OFMT_RGB,
 	},
+	{
+		/* Setting DSP4 to DSP_OFMT_RAW8 still gives 10-bit output,
+		 * regardless of the COM7 value. We can thus only support 10-bit
+		 * Bayer until someone figures it out.
+		 */
+		.code		= V4L2_MBUS_FMT_SBGGR10_1X10,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.dsp3		= 0x0,
+		.dsp4		= DSP_OFMT_RAW10,
+		.com3		= 0x0,
+		.com7		= SENSOR_RAW | OFMT_BRAW,
+	},
 };
 
 #define VGA_WIDTH   640
@@ -684,6 +711,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 			goto ov772x_set_fmt_error;
 	}
 
+	/* DSP_CTRL4: AEC reference point and DSP output format. */
+	if (cfmt->dsp4) {
+		ret = ov772x_write(client, DSP_CTRL4, cfmt->dsp4);
+		if (ret < 0)
+			goto ov772x_set_fmt_error;
+	}
+
 	/*
 	 * set COM3
 	 */
@@ -702,13 +736,8 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
-	/*
-	 * set COM7
-	 */
-	val = win->com7_bit | cfmt->com7;
-	ret = ov772x_mask_set(client,
-			      COM7, SLCT_MASK | FMT_MASK | OFMT_MASK,
-			      val);
+	/* COM7: Sensor resolution and output format control. */
+	ret = ov772x_write(client, COM7, win->com7_bit | cfmt->com7);
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
-- 
1.7.8.6

[PATCH 09/10] ov772x: Compute window size registers at runtime

[Laurent Pinchart]

Instead of hardcoding register arrays, compute the values at runtime.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |  149 ++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 91 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 07ff709..98b1bdf 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -317,8 +317,15 @@
 #define SGLF_ON_OFF     0x02	/* Single frame ON/OFF selection */
 #define SGLF_TRIG       0x01	/* Single frame transfer trigger */
 
+/* HREF */
+#define HREF_VSTART_SHIFT	6	/* VSTART LSB */
+#define HREF_HSTART_SHIFT	4	/* HSTART 2 LSBs */
+#define HREF_VSIZE_SHIFT	2	/* VSIZE LSB */
+#define HREF_HSIZE_SHIFT	0	/* HSIZE 2 LSBs */
+
 /* EXHCH */
-#define VSIZE_LSB       0x04	/* Vertical data output size LSB */
+#define EXHCH_VSIZE_SHIFT	2	/* VOUTSIZE LSB */
+#define EXHCH_HSIZE_SHIFT	0	/* HOUTSIZE 2 LSBs */
 
 /* DSP_CTRL1 */
 #define FIFO_ON         0x80	/* FIFO enable/disable selection */
@@ -344,30 +351,6 @@
 #define DSP_OFMT_RAW8	0x02
 #define DSP_OFMT_RAW10	0x03
 
-/* HSTART */
-#define HST_VGA         0x23
-#define HST_QVGA        0x3F
-
-/* HSIZE */
-#define HSZ_VGA         0xA0
-#define HSZ_QVGA        0x50
-
-/* VSTART */
-#define VST_VGA         0x07
-#define VST_QVGA        0x03
-
-/* VSIZE */
-#define VSZ_VGA         0xF0
-#define VSZ_QVGA        0x78
-
-/* HOUTSIZE */
-#define HOSZ_VGA        0xA0
-#define HOSZ_QVGA       0x50
-
-/* VOUTSIZE */
-#define VOSZ_VGA        0xF0
-#define VOSZ_QVGA       0x78
-
 /* DSPAUTO (DSP Auto Function ON/OFF Control) */
 #define AWB_ACTRL       0x80 /* AWB auto threshold control */
 #define DENOISE_ACTRL   0x40 /* De-noise auto threshold control */
@@ -386,10 +369,6 @@
 /*
  * struct
  */
-struct regval_list {
-	unsigned char reg_num;
-	unsigned char value;
-};
 
 struct ov772x_color_format {
 	enum v4l2_mbus_pixelcode code;
@@ -402,10 +381,8 @@ struct ov772x_color_format {
 
 struct ov772x_win_size {
 	char                     *name;
-	__u32                     width;
-	__u32                     height;
 	unsigned char             com7_bit;
-	const struct regval_list *regs;
+	struct v4l2_rect	  rect;
 };
 
 struct ov772x_priv {
@@ -421,31 +398,6 @@ struct ov772x_priv {
 	unsigned short                    band_filter;
 };
 
-#define ENDMARKER { 0xff, 0xff }
-
-/*
- * register setting for window size
- */
-static const struct regval_list ov772x_qvga_regs[] = {
-	{ HSTART,   HST_QVGA },
-	{ HSIZE,    HSZ_QVGA },
-	{ VSTART,   VST_QVGA },
-	{ VSIZE,    VSZ_QVGA  },
-	{ HOUTSIZE, HOSZ_QVGA },
-	{ VOUTSIZE, VOSZ_QVGA },
-	ENDMARKER,
-};
-
-static const struct regval_list ov772x_vga_regs[] = {
-	{ HSTART,   HST_VGA },
-	{ HSIZE,    HSZ_VGA },
-	{ VSTART,   VST_VGA },
-	{ VSIZE,    VSZ_VGA },
-	{ HOUTSIZE, HOSZ_VGA },
-	{ VOUTSIZE, VOSZ_VGA },
-	ENDMARKER,
-};
-
 /*
  * general function
  */
@@ -465,18 +417,6 @@ static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
 	return i2c_smbus_write_byte_data(client, addr, value);
 }
 
-static int ov772x_write_array(struct i2c_client        *client,
-			      const struct regval_list *vals)
-{
-	while (vals->reg_num != 0xff) {
-		int ret = ov772x_write(client, vals->reg_num, vals->value);
-		if (ret < 0)
-			return ret;
-		vals++;
-	}
-	return 0;
-}
-
 static int ov772x_mask_set(struct i2c_client *client, u8  command, u8  mask,
 			   u8  set)
 {
@@ -574,24 +514,26 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 
 #define VGA_WIDTH   640
 #define VGA_HEIGHT  480
-#define QVGA_WIDTH  320
-#define QVGA_HEIGHT 240
-#define MAX_WIDTH   VGA_WIDTH
-#define MAX_HEIGHT  VGA_HEIGHT
 
 static const struct ov772x_win_size ov772x_win_sizes[] = {
 	{
 		.name     = "VGA",
-		.width    = VGA_WIDTH,
-		.height   = VGA_HEIGHT,
 		.com7_bit = SLCT_VGA,
-		.regs     = ov772x_vga_regs,
+		.rect = {
+			.left = 140,
+			.top = 14,
+			.width = 640,
+			.height = 480,
+		},
 	}, {
 		.name     = "QVGA",
-		.width    = QVGA_WIDTH,
-		.height   = QVGA_HEIGHT,
 		.com7_bit = SLCT_QVGA,
-		.regs     = ov772x_qvga_regs,
+		.rect = {
+			.left = 252,
+			.top = 6,
+			.width = 320,
+			.height = 240,
+		},
 	},
 };
 
@@ -602,8 +544,8 @@ static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
 	u32 best_diff = (u32)-1;
 
 	for (i = 0; i < ARRAY_SIZE(ov772x_win_sizes); ++i) {
-		u32 diff = abs(width - ov772x_win_sizes[i].width)
-			 + abs(height - ov772x_win_sizes[i].height);
+		u32 diff = abs(width - ov772x_win_sizes[i].rect.width)
+			 + abs(height - ov772x_win_sizes[i].rect.height);
 		if (diff < best_diff) {
 			best_diff = diff;
 			win = &ov772x_win_sizes[i];
@@ -693,10 +635,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 			goto ov772x_set_fmt_error;
 	}
 
-	/*
-	 * set size format
-	 */
-	ret = ov772x_write_array(client, win->regs);
+	/* Format and window size */
+	ret = ov772x_write(client, HSTART, win->rect.left >> 2);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, HSIZE, win->rect.width >> 2);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, VSTART, win->rect.top >> 1);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, VSIZE, win->rect.height >> 1);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, HOUTSIZE, win->rect.width >> 2);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, VOUTSIZE, win->rect.height >> 1);
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, HREF,
+			   ((win->rect.top & 1) << HREF_VSTART_SHIFT) |
+			   ((win->rect.left & 3) << HREF_HSTART_SHIFT) |
+			   ((win->rect.height & 1) << HREF_VSIZE_SHIFT) |
+			   ((win->rect.width & 3) << HREF_HSIZE_SHIFT));
+	if (ret < 0)
+		goto ov772x_set_fmt_error;
+	ret = ov772x_write(client, EXHCH,
+			   ((win->rect.height & 1) << EXHCH_VSIZE_SHIFT) |
+			   ((win->rect.width & 3) << EXHCH_HSIZE_SHIFT));
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
@@ -899,8 +866,8 @@ static int ov772x_g_fmt(struct v4l2_subdev *sd,
 {
 	struct ov772x_priv *priv = to_ov772x(sd);
 
-	mf->width	= priv->win->width;
-	mf->height	= priv->win->height;
+	mf->width	= priv->win->rect.width;
+	mf->height	= priv->win->rect.height;
 	mf->code	= priv->cfmt->code;
 	mf->colorspace	= priv->cfmt->colorspace;
 	mf->field	= V4L2_FIELD_NONE;
@@ -925,8 +892,8 @@ static int ov772x_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf)
 	priv->cfmt = cfmt;
 
 	mf->code = cfmt->code;
-	mf->width = win->width;
-	mf->height = win->height;
+	mf->width = win->rect.width;
+	mf->height = win->rect.height;
 	mf->field = V4L2_FIELD_NONE;
 	mf->colorspace = cfmt->colorspace;
 
@@ -942,8 +909,8 @@ static int ov772x_try_fmt(struct v4l2_subdev *sd,
 	ov772x_select_params(mf, &cfmt, &win);
 
 	mf->code = cfmt->code;
-	mf->width = win->width;
-	mf->height = win->height;
+	mf->width = win->rect.width;
+	mf->height = win->rect.height;
 	mf->field = V4L2_FIELD_NONE;
 	mf->colorspace = cfmt->colorspace;
 
-- 
1.7.8.6

[PATCH 10/10] ov772x: Stop sensor readout right after reset

[Laurent Pinchart]

The sensor starts streaming video as soon as it gets powered or is
reset. Disable the output in the reset function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/ov772x.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 98b1bdf..6e08bae 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -432,9 +432,15 @@ static int ov772x_mask_set(struct i2c_client *client, u8  command, u8  mask,
 
 static int ov772x_reset(struct i2c_client *client)
 {
-	int ret = ov772x_write(client, COM7, SCCB_RESET);
+	int ret;
+
+	ret = ov772x_write(client, COM7, SCCB_RESET);
+	if (ret < 0)
+		return ret;
+
 	msleep(1);
-	return ret;
+
+	return ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
 }
 
 /* -----------------------------------------------------------------------------
-- 
1.7.8.6

Re: [PATCH 03/10] ov772x: Select the default format at probe time

[Guennadi Liakhovetski]

On Fri, 6 Jul 2012, Laurent Pinchart wrote:

> The format and window size are only initialized during the first g_fmt
> call. This leaves the device in an inconsistent state after
> initialization, which will cause problems when implementing pad
> operations. Move the format and window size initialization to probe
> time.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/ov772x.c |   63 ++++++++++++++++++++---------------------
>  1 files changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index 066bac6..576780a 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c
> @@ -547,37 +547,36 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
>  #define MAX_WIDTH   VGA_WIDTH
>  #define MAX_HEIGHT  VGA_HEIGHT
>  
> -static const struct ov772x_win_size ov772x_win_vga = {
> -	.name     = "VGA",
> -	.width    = VGA_WIDTH,
> -	.height   = VGA_HEIGHT,
> -	.com7_bit = SLCT_VGA,
> -	.regs     = ov772x_vga_regs,
> -};
> -
> -static const struct ov772x_win_size ov772x_win_qvga = {
> -	.name     = "QVGA",
> -	.width    = QVGA_WIDTH,
> -	.height   = QVGA_HEIGHT,
> -	.com7_bit = SLCT_QVGA,
> -	.regs     = ov772x_qvga_regs,
> +static const struct ov772x_win_size ov772x_win_sizes[] = {
> +	{
> +		.name     = "VGA",
> +		.width    = VGA_WIDTH,
> +		.height   = VGA_HEIGHT,
> +		.com7_bit = SLCT_VGA,
> +		.regs     = ov772x_vga_regs,
> +	}, {
> +		.name     = "QVGA",
> +		.width    = QVGA_WIDTH,
> +		.height   = QVGA_HEIGHT,
> +		.com7_bit = SLCT_QVGA,
> +		.regs     = ov772x_qvga_regs,
> +	},
>  };
>  
>  static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
>  {
> -	__u32 diff;
> -	const struct ov772x_win_size *win;
> -
> -	/* default is QVGA */
> -	diff = abs(width - ov772x_win_qvga.width) +
> -		abs(height - ov772x_win_qvga.height);
> -	win = &ov772x_win_qvga;
> -
> -	/* VGA */
> -	if (diff >
> -	    abs(width  - ov772x_win_vga.width) +
> -	    abs(height - ov772x_win_vga.height))
> -		win = &ov772x_win_vga;
> +	const struct ov772x_win_size *win = &ov772x_win_sizes[0];
> +	unsigned int i;
> +	u32 best_diff = (u32)-1;

Not a reason enough for a new version, if you don't mind and ig I don't 
forget, I might

#include <linux/kernel.h>
+	u32 best_diff = UINT_MAX;

> +
> +	for (i = 0; i < ARRAY_SIZE(ov772x_win_sizes); ++i) {
> +		u32 diff = abs(width - ov772x_win_sizes[i].width)
> +			 + abs(height - ov772x_win_sizes[i].height);
> +		if (diff < best_diff) {
> +			best_diff = diff;
> +			win = &ov772x_win_sizes[i];
> +		}
> +	}
>  
>  	return win;
>  }

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

Re: [PATCH 09/10] ov772x: Compute window size registers at runtime

[Guennadi Liakhovetski]

Hi Laurent

On Fri, 6 Jul 2012, Laurent Pinchart wrote:

> Instead of hardcoding register arrays, compute the values at runtime.

Great to see this register-array magic go! Just one nitpick:

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/ov772x.c |  149 ++++++++++++++++-------------------------
>  1 files changed, 58 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index 07ff709..98b1bdf 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c

[snip]

> @@ -574,24 +514,26 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
>  
>  #define VGA_WIDTH   640
>  #define VGA_HEIGHT  480
> -#define QVGA_WIDTH  320
> -#define QVGA_HEIGHT 240
> -#define MAX_WIDTH   VGA_WIDTH
> -#define MAX_HEIGHT  VGA_HEIGHT

You removed QVGA_* macros, because they only were used at one location, 
but you kept VGA_*.

>  
>  static const struct ov772x_win_size ov772x_win_sizes[] = {
>  	{
>  		.name     = "VGA",
> -		.width    = VGA_WIDTH,
> -		.height   = VGA_HEIGHT,
>  		.com7_bit = SLCT_VGA,
> -		.regs     = ov772x_vga_regs,
> +		.rect = {
> +			.left = 140,
> +			.top = 14,
> +			.width = 640,
> +			.height = 480,

...but here you hard-code .width and .height. I'd propose to use some 
symbolic names for all these sizes.

> +		},
>  	}, {
>  		.name     = "QVGA",
> -		.width    = QVGA_WIDTH,
> -		.height   = QVGA_HEIGHT,
>  		.com7_bit = SLCT_QVGA,
> -		.regs     = ov772x_qvga_regs,
> +		.rect = {
> +			.left = 252,
> +			.top = 6,
> +			.width = 320,
> +			.height = 240,
> +		},
>  	},
>  };
>  

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