[PATCH v3 0/3] media: ov7670: Add entity init and power operation

[PATCH v3 0/3] media: ov7670: Add entity init and power operation

[Wenyou Yang]

This patch set is to add the media entity pads initialization,
the s_power operation and get_fmt callback support.

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

Wenyou Yang (3):
  media: ov7670: Add entity pads initialization
  media: ov7670: Add the get_fmt callback
  media: ov7670: Add the s_power operation

 drivers/media/i2c/ov7670.c | 103 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 96 insertions(+), 7 deletions(-)

-- 
2.13.0

[PATCH v3 1/3] media: ov7670: Add entity pads initialization

[Wenyou Yang]

Add the media entity pads initialization.

Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/media/i2c/ov7670.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..5c8460ee65c3 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,7 @@ struct ov7670_devtype {
 struct ov7670_format_struct;  /* coming later */
 struct ov7670_info {
 	struct v4l2_subdev sd;
+	struct media_pad pad;
 	struct v4l2_ctrl_handler hdl;
 	struct {
 		/* gain cluster */
@@ -1688,14 +1689,23 @@ static int ov7670_probe(struct i2c_client *client,
 	v4l2_ctrl_auto_cluster(2, &info->auto_exposure,
 			       V4L2_EXPOSURE_MANUAL, false);
 	v4l2_ctrl_cluster(2, &info->saturation);
+
+	info->pad.flags = MEDIA_PAD_FL_SOURCE;
+	info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	ret = media_entity_pads_init(&info->sd.entity, 1, &info->pad);
+	if (ret < 0)
+		goto hdl_free;
+
 	v4l2_ctrl_handler_setup(&info->hdl);
 
 	ret = v4l2_async_register_subdev(&info->sd);
 	if (ret < 0)
-		goto hdl_free;
+		goto entity_cleanup;
 
 	return 0;
 
+entity_cleanup:
+	media_entity_cleanup(&info->sd.entity);
 hdl_free:
 	v4l2_ctrl_handler_free(&info->hdl);
 clk_disable:
@@ -1712,6 +1722,7 @@ static int ov7670_remove(struct i2c_client *client)
 	v4l2_device_unregister_subdev(sd);
 	v4l2_ctrl_handler_free(&info->hdl);
 	clk_disable_unprepare(info->clk);
+	media_entity_cleanup(&info->sd.entity);
 	return 0;
 }
 
-- 
2.13.0

Re: [PATCH v3 1/3] media: ov7670: Add entity pads initialization

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6692 bytes --]

Hi Wenyou,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.14-rc1 next-20170915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Wenyou-Yang/media-ov7670-Add-entity-init-and-power-operation/20170918-093913
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-x009-09180108 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/media/i2c/ov7670.c: In function 'ov7670_probe':
>> drivers/media/i2c/ov7670.c:1694:10: error: 'struct v4l2_subdev' has no member named 'entity'
     info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
             ^
   drivers/media/i2c/ov7670.c:1695:40: error: 'struct v4l2_subdev' has no member named 'entity'
     ret = media_entity_pads_init(&info->sd.entity, 1, &info->pad);
                                           ^
   drivers/media/i2c/ov7670.c:1708:32: error: 'struct v4l2_subdev' has no member named 'entity'
     media_entity_cleanup(&info->sd.entity);
                                   ^
   drivers/media/i2c/ov7670.c: In function 'ov7670_remove':
   drivers/media/i2c/ov7670.c:1725:32: error: 'struct v4l2_subdev' has no member named 'entity'
     media_entity_cleanup(&info->sd.entity);
                                   ^

vim +1694 drivers/media/i2c/ov7670.c

  1575	
  1576	static int ov7670_probe(struct i2c_client *client,
  1577				const struct i2c_device_id *id)
  1578	{
  1579		struct v4l2_fract tpf;
  1580		struct v4l2_subdev *sd;
  1581		struct ov7670_info *info;
  1582		int ret;
  1583	
  1584		info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
  1585		if (info == NULL)
  1586			return -ENOMEM;
  1587		sd = &info->sd;
  1588		v4l2_i2c_subdev_init(sd, client, &ov7670_ops);
  1589	
  1590		info->clock_speed = 30; /* default: a guess */
  1591		if (client->dev.platform_data) {
  1592			struct ov7670_config *config = client->dev.platform_data;
  1593	
  1594			/*
  1595			 * Must apply configuration before initializing device, because it
  1596			 * selects I/O method.
  1597			 */
  1598			info->min_width = config->min_width;
  1599			info->min_height = config->min_height;
  1600			info->use_smbus = config->use_smbus;
  1601	
  1602			if (config->clock_speed)
  1603				info->clock_speed = config->clock_speed;
  1604	
  1605			/*
  1606			 * It should be allowed for ov7670 too when it is migrated to
  1607			 * the new frame rate formula.
  1608			 */
  1609			if (config->pll_bypass && id->driver_data != MODEL_OV7670)
  1610				info->pll_bypass = true;
  1611	
  1612			if (config->pclk_hb_disable)
  1613				info->pclk_hb_disable = true;
  1614		}
  1615	
  1616		info->clk = devm_clk_get(&client->dev, "xclk");
  1617		if (IS_ERR(info->clk))
  1618			return PTR_ERR(info->clk);
  1619		ret = clk_prepare_enable(info->clk);
  1620		if (ret)
  1621			return ret;
  1622	
  1623		ret = ov7670_init_gpio(client, info);
  1624		if (ret)
  1625			goto clk_disable;
  1626	
  1627		info->clock_speed = clk_get_rate(info->clk) / 1000000;
  1628		if (info->clock_speed < 10 || info->clock_speed > 48) {
  1629			ret = -EINVAL;
  1630			goto clk_disable;
  1631		}
  1632	
  1633		/* Make sure it's an ov7670 */
  1634		ret = ov7670_detect(sd);
  1635		if (ret) {
  1636			v4l_dbg(1, debug, client,
  1637				"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
  1638				client->addr << 1, client->adapter->name);
  1639			goto clk_disable;
  1640		}
  1641		v4l_info(client, "chip found @ 0x%02x (%s)\n",
  1642				client->addr << 1, client->adapter->name);
  1643	
  1644		info->devtype = &ov7670_devdata[id->driver_data];
  1645		info->fmt = &ov7670_formats[0];
  1646		info->clkrc = 0;
  1647	
  1648		/* Set default frame rate to 30 fps */
  1649		tpf.numerator = 1;
  1650		tpf.denominator = 30;
  1651		info->devtype->set_framerate(sd, &tpf);
  1652	
  1653		if (info->pclk_hb_disable)
  1654			ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
  1655	
  1656		v4l2_ctrl_handler_init(&info->hdl, 10);
  1657		v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1658				V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
  1659		v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1660				V4L2_CID_CONTRAST, 0, 127, 1, 64);
  1661		v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1662				V4L2_CID_VFLIP, 0, 1, 1, 0);
  1663		v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1664				V4L2_CID_HFLIP, 0, 1, 1, 0);
  1665		info->saturation = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1666				V4L2_CID_SATURATION, 0, 256, 1, 128);
  1667		info->hue = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1668				V4L2_CID_HUE, -180, 180, 5, 0);
  1669		info->gain = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1670				V4L2_CID_GAIN, 0, 255, 1, 128);
  1671		info->auto_gain = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1672				V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
  1673		info->exposure = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
  1674				V4L2_CID_EXPOSURE, 0, 65535, 1, 500);
  1675		info->auto_exposure = v4l2_ctrl_new_std_menu(&info->hdl, &ov7670_ctrl_ops,
  1676				V4L2_CID_EXPOSURE_AUTO, V4L2_EXPOSURE_MANUAL, 0,
  1677				V4L2_EXPOSURE_AUTO);
  1678		sd->ctrl_handler = &info->hdl;
  1679		if (info->hdl.error) {
  1680			ret = info->hdl.error;
  1681	
  1682			goto hdl_free;
  1683		}
  1684		/*
  1685		 * We have checked empirically that hw allows to read back the gain
  1686		 * value chosen by auto gain but that's not the case for auto exposure.
  1687		 */
  1688		v4l2_ctrl_auto_cluster(2, &info->auto_gain, 0, true);
  1689		v4l2_ctrl_auto_cluster(2, &info->auto_exposure,
  1690				       V4L2_EXPOSURE_MANUAL, false);
  1691		v4l2_ctrl_cluster(2, &info->saturation);
  1692	
  1693		info->pad.flags = MEDIA_PAD_FL_SOURCE;
> 1694		info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
  1695		ret = media_entity_pads_init(&info->sd.entity, 1, &info->pad);
  1696		if (ret < 0)
  1697			goto hdl_free;
  1698	
  1699		v4l2_ctrl_handler_setup(&info->hdl);
  1700	
  1701		ret = v4l2_async_register_subdev(&info->sd);
  1702		if (ret < 0)
  1703			goto entity_cleanup;
  1704	
  1705		return 0;
  1706	
  1707	entity_cleanup:
  1708		media_entity_cleanup(&info->sd.entity);
  1709	hdl_free:
  1710		v4l2_ctrl_handler_free(&info->hdl);
  1711	clk_disable:
  1712		clk_disable_unprepare(info->clk);
  1713		return ret;
  1714	}
  1715	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28336 bytes --]

[PATCH v3 2/3] media: ov7670: Add the get_fmt callback

[Wenyou Yang]

Add the get_fmt callback, also enable V4L2_SUBDEV_FL_HAS_DEVNODE flag
to make this subdev has device node.

Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2: None

 drivers/media/i2c/ov7670.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 5c8460ee65c3..5d6f1859a39b 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -230,6 +230,7 @@ struct ov7670_info {
 		struct v4l2_ctrl *saturation;
 		struct v4l2_ctrl *hue;
 	};
+	struct v4l2_mbus_framefmt format;
 	struct ov7670_format_struct *fmt;  /* Current format */
 	struct clk *clk;
 	struct gpio_desc *resetb_gpio;
@@ -973,6 +974,9 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
 	fmt->width = wsize->width;
 	fmt->height = wsize->height;
 	fmt->colorspace = ov7670_formats[index].colorspace;
+
+	info->format = *fmt;
+
 	return 0;
 }
 
@@ -986,6 +990,7 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
 	struct ov7670_format_struct *ovfmt;
 	struct ov7670_win_size *wsize;
 	struct ov7670_info *info = to_state(sd);
+	struct v4l2_mbus_framefmt *mbus_fmt;
 	unsigned char com7;
 	int ret;
 
@@ -996,7 +1001,8 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
 		ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
 		if (ret)
 			return ret;
-		cfg->try_fmt = format->format;
+		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+		*mbus_fmt = format->format;
 		return 0;
 	}
 
@@ -1039,6 +1045,23 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int ov7670_get_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *format)
+{
+	struct ov7670_info *info = to_state(sd);
+	struct v4l2_mbus_framefmt *mbus_fmt;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+		format->format = *mbus_fmt;
+	} else {
+		format->format = info->format;
+	}
+
+	return 0;
+}
+
 /*
  * Implement G/S_PARM.  There is a "high quality" mode we could try
  * to do someday; for now, we just do the frame rate tweak.
@@ -1506,6 +1529,28 @@ static int ov7670_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_regis
 }
 #endif
 
+static void ov7670_get_default_format(struct v4l2_subdev *sd,
+				      struct v4l2_mbus_framefmt *format)
+{
+	struct ov7670_info *info = to_state(sd);
+
+	format->width = info->devtype->win_sizes[0].width;
+	format->height = info->devtype->win_sizes[0].height;
+	format->colorspace = info->fmt->colorspace;
+	format->code = info->fmt->mbus_code;
+	format->field = V4L2_FIELD_NONE;
+}
+
+static int ov7670_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct v4l2_mbus_framefmt *format =
+				v4l2_subdev_get_try_format(sd, fh->pad, 0);
+
+	ov7670_get_default_format(sd, format);
+
+	return 0;
+}
+
 /* ----------------------------------------------------------------------- */
 
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
@@ -1526,6 +1571,7 @@ static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
 	.enum_frame_interval = ov7670_enum_frame_interval,
 	.enum_frame_size = ov7670_enum_frame_size,
 	.enum_mbus_code = ov7670_enum_mbus_code,
+	.get_fmt = ov7670_get_fmt,
 	.set_fmt = ov7670_set_fmt,
 };
 
@@ -1535,6 +1581,10 @@ static const struct v4l2_subdev_ops ov7670_ops = {
 	.pad = &ov7670_pad_ops,
 };
 
+static const struct v4l2_subdev_internal_ops ov7670_subdev_internal_ops = {
+	.open = ov7670_open,
+};
+
 /* ----------------------------------------------------------------------- */
 
 static const struct ov7670_devtype ov7670_devdata[] = {
@@ -1587,6 +1637,9 @@ static int ov7670_probe(struct i2c_client *client,
 	sd = &info->sd;
 	v4l2_i2c_subdev_init(sd, client, &ov7670_ops);
 
+	sd->internal_ops = &ov7670_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
 	info->clock_speed = 30; /* default: a guess */
 	if (client->dev.platform_data) {
 		struct ov7670_config *config = client->dev.platform_data;
@@ -1643,6 +1696,9 @@ static int ov7670_probe(struct i2c_client *client,
 
 	info->devtype = &ov7670_devdata[id->driver_data];
 	info->fmt = &ov7670_formats[0];
+
+	ov7670_get_default_format(sd, &info->format);
+
 	info->clkrc = 0;
 
 	/* Set default frame rate to 30 fps */
-- 
2.13.0

Re: [PATCH v3 2/3] media: ov7670: Add the get_fmt callback

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5039 bytes --]

Hi Wenyou,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.14-rc1 next-20170915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Wenyou-Yang/media-ov7670-Add-entity-init-and-power-operation/20170918-093913
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-x009-09180108 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/media//i2c/ov7670.c: In function 'ov7670_set_fmt':
>> drivers/media//i2c/ov7670.c:1004:14: error: implicit declaration of function 'v4l2_subdev_get_try_format' [-Werror=implicit-function-declaration]
      mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/media//i2c/ov7670.c:1004:12: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
               ^
   drivers/media//i2c/ov7670.c: In function 'ov7670_get_fmt':
   drivers/media//i2c/ov7670.c:1056:12: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
               ^
   drivers/media//i2c/ov7670.c: In function 'ov7670_open':
>> drivers/media//i2c/ov7670.c:1547:38: error: 'struct v4l2_subdev_fh' has no member named 'pad'
        v4l2_subdev_get_try_format(sd, fh->pad, 0);
                                         ^~
   drivers/media//i2c/ov7670.c: In function 'ov7670_probe':
   drivers/media//i2c/ov7670.c:1750:10: error: 'struct v4l2_subdev' has no member named 'entity'
     info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
             ^
   drivers/media//i2c/ov7670.c:1751:40: error: 'struct v4l2_subdev' has no member named 'entity'
     ret = media_entity_pads_init(&info->sd.entity, 1, &info->pad);
                                           ^
   drivers/media//i2c/ov7670.c:1764:32: error: 'struct v4l2_subdev' has no member named 'entity'
     media_entity_cleanup(&info->sd.entity);
                                   ^
   drivers/media//i2c/ov7670.c: In function 'ov7670_remove':
   drivers/media//i2c/ov7670.c:1781:32: error: 'struct v4l2_subdev' has no member named 'entity'
     media_entity_cleanup(&info->sd.entity);
                                   ^
   cc1: some warnings being treated as errors

vim +/v4l2_subdev_get_try_format +1004 drivers/media//i2c/ov7670.c

   982	
   983	/*
   984	 * Set a format.
   985	 */
   986	static int ov7670_set_fmt(struct v4l2_subdev *sd,
   987			struct v4l2_subdev_pad_config *cfg,
   988			struct v4l2_subdev_format *format)
   989	{
   990		struct ov7670_format_struct *ovfmt;
   991		struct ov7670_win_size *wsize;
   992		struct ov7670_info *info = to_state(sd);
   993		struct v4l2_mbus_framefmt *mbus_fmt;
   994		unsigned char com7;
   995		int ret;
   996	
   997		if (format->pad)
   998			return -EINVAL;
   999	
  1000		if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
  1001			ret = ov7670_try_fmt_internal(sd, &format->format, NULL, NULL);
  1002			if (ret)
  1003				return ret;
> 1004			mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
  1005			*mbus_fmt = format->format;
  1006			return 0;
  1007		}
  1008	
  1009		ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
  1010	
  1011		if (ret)
  1012			return ret;
  1013		/*
  1014		 * COM7 is a pain in the ass, it doesn't like to be read then
  1015		 * quickly written afterward.  But we have everything we need
  1016		 * to set it absolutely here, as long as the format-specific
  1017		 * register sets list it first.
  1018		 */
  1019		com7 = ovfmt->regs[0].value;
  1020		com7 |= wsize->com7_bit;
  1021		ov7670_write(sd, REG_COM7, com7);
  1022		/*
  1023		 * Now write the rest of the array.  Also store start/stops
  1024		 */
  1025		ov7670_write_array(sd, ovfmt->regs + 1);
  1026		ov7670_set_hw(sd, wsize->hstart, wsize->hstop, wsize->vstart,
  1027				wsize->vstop);
  1028		ret = 0;
  1029		if (wsize->regs)
  1030			ret = ov7670_write_array(sd, wsize->regs);
  1031		info->fmt = ovfmt;
  1032	
  1033		/*
  1034		 * If we're running RGB565, we must rewrite clkrc after setting
  1035		 * the other parameters or the image looks poor.  If we're *not*
  1036		 * doing RGB565, we must not rewrite clkrc or the image looks
  1037		 * *really* poor.
  1038		 *
  1039		 * (Update) Now that we retain clkrc state, we should be able
  1040		 * to write it unconditionally, and that will make the frame
  1041		 * rate persistent too.
  1042		 */
  1043		if (ret == 0)
  1044			ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
  1045		return 0;
  1046	}
  1047	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28336 bytes --]

[PATCH v3 3/3] media: ov7670: Add the s_power operation

[Wenyou Yang]

Add the s_power operation which is responsible for manipulating the
power dowm mode through the PWDN pin and the reset operation through
the RESET pin.

Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

 drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 5d6f1859a39b..66b9bf2111f7 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1529,6 +1529,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, const struct v4l2_dbg_regis
 }
 #endif
 
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+	struct ov7670_info *info = to_state(sd);
+
+	if (info->pwdn_gpio)
+		gpiod_direction_output(info->pwdn_gpio, !on);
+	if (on && info->resetb_gpio) {
+		gpiod_set_value(info->resetb_gpio, 1);
+		usleep_range(500, 1000);
+		gpiod_set_value(info->resetb_gpio, 0);
+		usleep_range(3000, 5000);
+	}
+
+	return 0;
+}
+
 static void ov7670_get_default_format(struct v4l2_subdev *sd,
 				      struct v4l2_mbus_framefmt *format)
 {
@@ -1560,6 +1576,7 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops = {
 	.g_register = ov7670_g_register,
 	.s_register = ov7670_s_register,
 #endif
+	.s_power = ov7670_s_power,
 };
 
 static const struct v4l2_subdev_video_ops ov7670_video_ops = {
@@ -1673,23 +1690,25 @@ static int ov7670_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = ov7670_init_gpio(client, info);
-	if (ret)
-		goto clk_disable;
-
 	info->clock_speed = clk_get_rate(info->clk) / 1000000;
 	if (info->clock_speed < 10 || info->clock_speed > 48) {
 		ret = -EINVAL;
 		goto clk_disable;
 	}
 
+	ret = ov7670_init_gpio(client, info);
+	if (ret)
+		goto clk_disable;
+
+	ov7670_s_power(sd, 1);
+
 	/* Make sure it's an ov7670 */
 	ret = ov7670_detect(sd);
 	if (ret) {
 		v4l_dbg(1, debug, client,
 			"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
 			client->addr << 1, client->adapter->name);
-		goto clk_disable;
+		goto power_off;
 	}
 	v4l_info(client, "chip found @ 0x%02x (%s)\n",
 			client->addr << 1, client->adapter->name);
@@ -1764,6 +1783,8 @@ static int ov7670_probe(struct i2c_client *client,
 	media_entity_cleanup(&info->sd.entity);
 hdl_free:
 	v4l2_ctrl_handler_free(&info->hdl);
+power_off:
+	ov7670_s_power(sd, 0);
 clk_disable:
 	clk_disable_unprepare(info->clk);
 	return ret;
@@ -1779,6 +1800,7 @@ static int ov7670_remove(struct i2c_client *client)
 	v4l2_ctrl_handler_free(&info->hdl);
 	clk_disable_unprepare(info->clk);
 	media_entity_cleanup(&info->sd.entity);
+	ov7670_s_power(sd, 0);
 	return 0;
 }
 
-- 
2.13.0