All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes
@ 2012-07-18 13:58 Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 1/9] ov772x: Fix memory leak in probe error path Laurent Pinchart
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

Hi,

Here's the second version of the ov772x cleanup and fixes patches. They apply
on top of my previous "Miscellaneous soc-camera patches" (v3) series.

Changes compared to v1:

- Dropped the "Reorganize the code in sections" patch.
- Use UINT_MAX instead of reinventing it (2/9)
- Add OV772X_DEFAULT_{WIDTH,HEIGHT} macros (8/9) instead of hardcoding values.

Laurent Pinchart (9):
  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 |  426 ++++++++++++++++++++----------------------
 1 files changed, 207 insertions(+), 219 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/9] ov772x: Fix memory leak in probe error path
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 2/9] ov772x: Select the default format at probe time Laurent Pinchart
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 641f6f4..0fede50d 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -1098,18 +1098,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/9] ov772x: Select the default format at probe time
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 1/9] ov772x: Fix memory leak in probe error path Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 3/9] ov772x: Don't fail in s_fmt if the requested format isn't supported Laurent Pinchart
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 |   64 +++++++++++++++++++++---------------------
 1 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 0fede50d..3f6e4bf 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
@@ -504,20 +505,20 @@ 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,
+	},
 };
 
 /*
@@ -693,19 +694,18 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
 
 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];
+	u32 best_diff = UINT_MAX;
+	unsigned int i;
+
+	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;
 }
@@ -890,11 +890,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;
@@ -1103,6 +1098,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/9] ov772x: Don't fail in s_fmt if the requested format isn't supported
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 1/9] ov772x: Fix memory leak in probe error path Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 2/9] ov772x: Select the default format at probe time Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 4/9] ov772x: try_fmt must not default to the current format Laurent Pinchart
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 3f6e4bf..c2bd087 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -581,11 +581,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",
@@ -710,31 +705,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
@@ -791,14 +788,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);
@@ -809,7 +806,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)
@@ -827,7 +824,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);
@@ -846,16 +843,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;
 }
@@ -899,18 +891,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/9] ov772x: try_fmt must not default to the current format
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (2 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 3/9] ov772x: Don't fail in s_fmt if the requested format isn't supported Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 5/9] ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv Laurent Pinchart
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 c2bd087..be3dfb5 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -919,38 +919,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/9] ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (3 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 4/9] ov772x: try_fmt must not default to the current format Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 6/9] ov772x: Add ov772x_read() and ov772x_write() functions Laurent Pinchart
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 be3dfb5..2b95dd4 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -525,10 +525,9 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
  * 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,
@@ -574,7 +573,7 @@ static int ov772x_reset(struct i2c_client *client)
 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);
@@ -638,7 +637,7 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
 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;
@@ -880,7 +879,7 @@ static int ov772x_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 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;
@@ -893,7 +892,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;
@@ -933,9 +932,9 @@ static int ov772x_try_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
-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;
@@ -1078,7 +1077,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;
 
@@ -1095,7 +1094,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/9] ov772x: Add ov772x_read() and ov772x_write() functions
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (4 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 5/9] ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 7/9] ov772x: Add support for SBGGR10 format Laurent Pinchart
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 2b95dd4..13f4688 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -530,13 +530,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++;
@@ -544,24 +552,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;
 }
@@ -656,7 +662,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_read(client, reg->reg);
 	if (ret < 0)
 		return ret;
 
@@ -674,7 +680,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
 
@@ -946,8 +952,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:
@@ -970,8 +976,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 7/9] ov772x: Add support for SBGGR10 format
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (5 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 6/9] ov772x: Add ov772x_read() and ov772x_write() functions Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 8/9] ov772x: Compute window size registers at runtime Laurent Pinchart
  2012-07-18 13:58 ` [PATCH v2 9/9] ov772x: Stop sensor readout right after reset Laurent Pinchart
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 13f4688..3874dbc 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -275,6 +275,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 */
@@ -338,6 +339,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
@@ -389,6 +396,7 @@ struct ov772x_color_format {
 	enum v4l2_mbus_pixelcode code;
 	enum v4l2_colorspace colorspace;
 	u8 dsp3;
+	u8 dsp4;
 	u8 com3;
 	u8 com7;
 };
@@ -447,6 +455,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,
 	},
@@ -454,6 +463,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,
 	},
@@ -461,6 +471,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,
 	},
@@ -468,6 +479,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,
 	},
@@ -475,6 +487,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,
 	},
@@ -482,6 +495,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,
 	},
@@ -489,9 +503,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,
+	},
 };
 
 
@@ -808,6 +835,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
 	 */
@@ -826,13 +860,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 8/9] ov772x: Compute window size registers at runtime
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (6 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 7/9] ov772x: Add support for SBGGR10 format Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  2012-07-20 13:58   ` Guennadi Liakhovetski
  2012-07-18 13:58 ` [PATCH v2 9/9] ov772x: Stop sensor readout right after reset Laurent Pinchart
  8 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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

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

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 3874dbc..aa2ba9e 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -318,8 +318,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 */
@@ -345,30 +352,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 */
@@ -377,6 +360,9 @@
 #define SCAL0_ACTRL     0x08 /* Auto scaling factor control */
 #define SCAL1_2_ACTRL   0x04 /* Auto scaling factor control */
 
+#define OV772X_DEFAULT_WIDTH	640
+#define OV772X_DEFAULT_HEIGHT	480
+
 /*
  * ID
  */
@@ -387,10 +373,6 @@
 /*
  * struct
  */
-struct regval_list {
-	unsigned char reg_num;
-	unsigned char value;
-};
 
 struct ov772x_color_format {
 	enum v4l2_mbus_pixelcode code;
@@ -403,10 +385,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 {
@@ -422,31 +402,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,
-};
-
 /*
  * supported color format list
  */
@@ -525,26 +480,26 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
 /*
  * window size list
  */
-#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,
+		},
 	},
 };
 
@@ -567,18 +522,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)
 {
@@ -726,8 +669,8 @@ static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
 	unsigned int i;
 
 	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];
@@ -817,10 +760,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;
 
@@ -890,8 +858,8 @@ static int ov772x_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 {
 	a->c.left	= 0;
 	a->c.top	= 0;
-	a->c.width	= VGA_WIDTH;
-	a->c.height	= VGA_HEIGHT;
+	a->c.width	= OV772X_DEFAULT_WIDTH;
+	a->c.height	= OV772X_DEFAULT_HEIGHT;
 	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
 	return 0;
@@ -901,8 +869,8 @@ 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->bounds.width			= OV772X_DEFAULT_WIDTH;
+	a->bounds.height		= OV772X_DEFAULT_HEIGHT;
 	a->defrect			= a->bounds;
 	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	a->pixelaspect.numerator	= 1;
@@ -916,8 +884,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;
@@ -942,8 +910,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;
 
@@ -959,8 +927,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 9/9] ov772x: Stop sensor readout right after reset
  2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
                   ` (7 preceding siblings ...)
  2012-07-18 13:58 ` [PATCH v2 8/9] ov772x: Compute window size registers at runtime Laurent Pinchart
@ 2012-07-18 13:58 ` Laurent Pinchart
  8 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-18 13:58 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

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 aa2ba9e..e78e0e1 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -537,9 +537,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 8/9] ov772x: Compute window size registers at runtime
  2012-07-18 13:58 ` [PATCH v2 8/9] ov772x: Compute window size registers at runtime Laurent Pinchart
@ 2012-07-20 13:58   ` Guennadi Liakhovetski
  2012-07-20 14:14     ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-20 13:58 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media

Hi Laurent

Thanks for the patch

On Wed, 18 Jul 2012, Laurent Pinchart wrote:

> Instead of hardcoding register arrays, compute the values at runtime.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/ov772x.c |  162 +++++++++++++++++-------------------------
>  1 files changed, 65 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index 3874dbc..aa2ba9e 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c

I'm afraid, I still don't quite agree with your changes to size macros. 
This is not a huge deal, but I'd preserve the current (Q)VGA_* naming for 
now until we find a better solution. How about this patch merged with yours:

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index a2dde04..76a80b6 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -360,8 +360,12 @@
 #define SCAL0_ACTRL     0x08 /* Auto scaling factor control */
 #define SCAL1_2_ACTRL   0x04 /* Auto scaling factor control */
 
-#define OV772X_DEFAULT_WIDTH	640
-#define OV772X_DEFAULT_HEIGHT	480
+#define VGA_WIDTH		640
+#define VGA_HEIGHT		480
+#define QVGA_WIDTH		320
+#define QVGA_HEIGHT		240
+#define OV772X_MAX_WIDTH	VGA_WIDTH
+#define OV772X_MAX_HEIGHT	VGA_HEIGHT
 
 /*
  * ID
@@ -488,8 +492,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
 		.rect = {
 			.left = 140,
 			.top = 14,
-			.width = 640,
-			.height = 480,
+			.width = VGA_WIDTH,
+			.height = VGA_HEIGHT,
 		},
 	}, {
 		.name     = "QVGA",
@@ -497,8 +501,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
 		.rect = {
 			.left = 252,
 			.top = 6,
-			.width = 320,
-			.height = 240,
+			.width = QVGA_WIDTH,
+			.height = QVGA_HEIGHT,
 		},
 	},
 };
@@ -858,8 +862,8 @@ static int ov772x_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 {
 	a->c.left	= 0;
 	a->c.top	= 0;
-	a->c.width	= OV772X_DEFAULT_WIDTH;
-	a->c.height	= OV772X_DEFAULT_HEIGHT;
+	a->c.width	= VGA_WIDTH;
+	a->c.height	= VGA_HEIGHT;
 	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
 	return 0;
@@ -869,8 +873,8 @@ static int ov772x_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 {
 	a->bounds.left			= 0;
 	a->bounds.top			= 0;
-	a->bounds.width			= OV772X_DEFAULT_WIDTH;
-	a->bounds.height		= OV772X_DEFAULT_HEIGHT;
+	a->bounds.width			= OV772X_MAX_WIDTH;
+	a->bounds.height		= OV772X_MAX_HEIGHT;
 	a->defrect			= a->bounds;
 	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	a->pixelaspect.numerator	= 1;

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 8/9] ov772x: Compute window size registers at runtime
  2012-07-20 13:58   ` Guennadi Liakhovetski
@ 2012-07-20 14:14     ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2012-07-20 14:14 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

Hi Guennadi,

On Friday 20 July 2012 15:58:25 Guennadi Liakhovetski wrote:
> On Wed, 18 Jul 2012, Laurent Pinchart wrote:
> > Instead of hardcoding register arrays, compute the values at runtime.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/video/ov772x.c |  162
> >  +++++++++++++++++------------------------- 1 files changed, 65
> >  insertions(+), 97 deletions(-)
> > 
> > diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> > index 3874dbc..aa2ba9e 100644
> > --- a/drivers/media/video/ov772x.c
> > +++ b/drivers/media/video/ov772x.c
> 
> I'm afraid, I still don't quite agree with your changes to size macros.
> This is not a huge deal, but I'd preserve the current (Q)VGA_* naming for
> now until we find a better solution. How about this patch merged with yours:

I still like my solution better, but you're the driver maintainer :-)

> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index a2dde04..76a80b6 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c
> @@ -360,8 +360,12 @@
>  #define SCAL0_ACTRL     0x08 /* Auto scaling factor control */
>  #define SCAL1_2_ACTRL   0x04 /* Auto scaling factor control */
> 
> -#define OV772X_DEFAULT_WIDTH	640
> -#define OV772X_DEFAULT_HEIGHT	480
> +#define VGA_WIDTH		640
> +#define VGA_HEIGHT		480
> +#define QVGA_WIDTH		320
> +#define QVGA_HEIGHT		240
> +#define OV772X_MAX_WIDTH	VGA_WIDTH
> +#define OV772X_MAX_HEIGHT	VGA_HEIGHT
> 
>  /*
>   * ID
> @@ -488,8 +492,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] =
> { .rect = {
>  			.left = 140,
>  			.top = 14,
> -			.width = 640,
> -			.height = 480,
> +			.width = VGA_WIDTH,
> +			.height = VGA_HEIGHT,
>  		},
>  	}, {
>  		.name     = "QVGA",
> @@ -497,8 +501,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] =
> { .rect = {
>  			.left = 252,
>  			.top = 6,
> -			.width = 320,
> -			.height = 240,
> +			.width = QVGA_WIDTH,
> +			.height = QVGA_HEIGHT,
>  		},
>  	},
>  };
> @@ -858,8 +862,8 @@ static int ov772x_g_crop(struct v4l2_subdev *sd, struct
> v4l2_crop *a) {
>  	a->c.left	= 0;
>  	a->c.top	= 0;
> -	a->c.width	= OV772X_DEFAULT_WIDTH;
> -	a->c.height	= OV772X_DEFAULT_HEIGHT;
> +	a->c.width	= VGA_WIDTH;
> +	a->c.height	= VGA_HEIGHT;
>  	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> 
>  	return 0;
> @@ -869,8 +873,8 @@ static int ov772x_cropcap(struct v4l2_subdev *sd, struct
> v4l2_cropcap *a) {
>  	a->bounds.left			= 0;
>  	a->bounds.top			= 0;
> -	a->bounds.width			= OV772X_DEFAULT_WIDTH;
> -	a->bounds.height		= OV772X_DEFAULT_HEIGHT;
> +	a->bounds.width			= OV772X_MAX_WIDTH;
> +	a->bounds.height		= OV772X_MAX_HEIGHT;
>  	a->defrect			= a->bounds;
>  	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	a->pixelaspect.numerator	= 1;

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-07-20 14:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-18 13:58 [PATCH v2 0/9] Miscellaneous ov772x cleanups and fixes Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 1/9] ov772x: Fix memory leak in probe error path Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 2/9] ov772x: Select the default format at probe time Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 3/9] ov772x: Don't fail in s_fmt if the requested format isn't supported Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 4/9] ov772x: try_fmt must not default to the current format Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 5/9] ov772x: Make to_ov772x convert from v4l2_subdev to ov772x_priv Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 6/9] ov772x: Add ov772x_read() and ov772x_write() functions Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 7/9] ov772x: Add support for SBGGR10 format Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 8/9] ov772x: Compute window size registers at runtime Laurent Pinchart
2012-07-20 13:58   ` Guennadi Liakhovetski
2012-07-20 14:14     ` Laurent Pinchart
2012-07-18 13:58 ` [PATCH v2 9/9] ov772x: Stop sensor readout right after reset Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.