Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1733670 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/3] media: ov7670: Add entity init and power operation Wenyou Yang <wenyou.yang@microchip.com> - 2017-09-18 08:50 +0200
    [PATCH v4 1/3] media: ov7670: Add entity pads initialization Wenyou Yang <wenyou.yang@microchip.com> - 2017-09-18 08:50 +0200
    [PATCH v4 3/3] media: ov7670: Add the s_power operation Wenyou Yang <wenyou.yang@microchip.com> - 2017-09-18 08:50 +0200
      Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation Sakari Ailus <sakari.ailus@iki.fi> - 2017-09-18 09:40 +0200
        Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation "Yang, Wenyou" <Wenyou.Yang@Microchip.com> - 2017-09-18 11:30 +0200
          Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation Sakari Ailus <sakari.ailus@iki.fi> - 2017-09-18 15:30 +0200
    [PATCH v4 2/3] media: ov7670: Add the get_fmt callback Wenyou Yang <wenyou.yang@microchip.com> - 2017-09-18 08:50 +0200
      Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback Sakari Ailus <sakari.ailus@iki.fi> - 2017-09-18 09:40 +0200
        Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback "Yang, Wenyou" <Wenyou.Yang@Microchip.com> - 2017-09-18 11:30 +0200

#1733670 — [PATCH v4 0/3] media: ov7670: Add entity init and power operation

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

Changes in v4:
 - Fix the build error when not enabling Media Controller API option.
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

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 | 128 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 121 insertions(+), 7 deletions(-)

-- 
2.13.0

[toc] | [next] | [standalone]

#1733671 — [PATCH v4 1/3] media: ov7670: Add entity pads initialization

Add the media entity pads initialization.

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

Changes in v4:
 - Fix the build error when not enabling Media Controller API option.

Changes in v3: None
Changes in v2: None

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

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..553945d4ca28 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,9 @@ struct ov7670_devtype {
 struct ov7670_format_struct;  /* coming later */
 struct ov7670_info {
 	struct v4l2_subdev sd;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad pad;
+#endif
 	struct v4l2_ctrl_handler hdl;
 	struct {
 		/* gain cluster */
@@ -1688,14 +1691,27 @@ 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);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	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;
+#endif
+
 	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:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	media_entity_cleanup(&info->sd.entity);
+#endif
 hdl_free:
 	v4l2_ctrl_handler_free(&info->hdl);
 clk_disable:
@@ -1712,6 +1728,9 @@ static int ov7670_remove(struct i2c_client *client)
 	v4l2_device_unregister_subdev(sd);
 	v4l2_ctrl_handler_free(&info->hdl);
 	clk_disable_unprepare(info->clk);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	media_entity_cleanup(&info->sd.entity);
+#endif
 	return 0;
 }
 
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1733674 — [PATCH v4 3/3] media: ov7670: Add the s_power operation

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 v4: None
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 456f48057605..304abc769a67 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1542,6 +1542,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)
 {
@@ -1575,6 +1591,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 = {
@@ -1692,23 +1709,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);
@@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
 #endif
 hdl_free:
 	v4l2_ctrl_handler_free(&info->hdl);
+power_off:
+	ov7670_s_power(sd, 0);
 clk_disable:
 	clk_disable_unprepare(info->clk);
 	return ret;
@@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	media_entity_cleanup(&info->sd.entity);
 #endif
+	ov7670_s_power(sd, 0);
 	return 0;
 }
 
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1733699 — Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

Hi Wenyou,

Thanks for the update.

The driver exposes controls that are accessible through the sub-device node
even if the device hasn't been powered on.

Many ISP and bridge drivers will also power the sensor down once the last
user of the user space device nodes disappears. You could keep the device
powered at all times or change the behaviour so that controls can be
accessed when the power is off.

The best option would be to convert the driver to use runtime PM. An
example of this can be found in drivers/media/i2c/ov13858.c .

On Mon, Sep 18, 2017 at 02:45:14PM +0800, Wenyou Yang wrote:
> 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 v4: None
> 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 456f48057605..304abc769a67 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -1542,6 +1542,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)
>  {
> @@ -1575,6 +1591,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 = {
> @@ -1692,23 +1709,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);
> @@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
>  #endif
>  hdl_free:
>  	v4l2_ctrl_handler_free(&info->hdl);
> +power_off:
> +	ov7670_s_power(sd, 0);
>  clk_disable:
>  	clk_disable_unprepare(info->clk);
>  	return ret;
> @@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	media_entity_cleanup(&info->sd.entity);
>  #endif
> +	ov7670_s_power(sd, 0);
>  	return 0;
>  }
>  
> -- 
> 2.13.0
> 

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

[toc] | [prev] | [next] | [standalone]

#1733876 — Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

Hi Sakari,


On 2017/9/18 15:35, Sakari Ailus wrote:
> Hi Wenyou,
>
> Thanks for the update.
>
> The driver exposes controls that are accessible through the sub-device node
> even if the device hasn't been powered on.
>
> Many ISP and bridge drivers will also power the sensor down once the last
> user of the user space device nodes disappears. You could keep the device
> powered at all times or change the behaviour so that controls can be
> accessed when the power is off.
>
> The best option would be to convert the driver to use runtime PM.
Yes, I agree with you.
I also noticed there are a lot of things needed to improve except for 
it, such as the platform data via device tree.
I would like do it in another patch set. I will continue to work on it.
> An example of this can be found in drivers/media/i2c/ov13858.c .
A good example, thank you for your providing.
>
> On Mon, Sep 18, 2017 at 02:45:14PM +0800, Wenyou Yang wrote:
>> 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 v4: None
>> 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 456f48057605..304abc769a67 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -1542,6 +1542,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)
>>   {
>> @@ -1575,6 +1591,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 = {
>> @@ -1692,23 +1709,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);
>> @@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
>>   #endif
>>   hdl_free:
>>   	v4l2_ctrl_handler_free(&info->hdl);
>> +power_off:
>> +	ov7670_s_power(sd, 0);
>>   clk_disable:
>>   	clk_disable_unprepare(info->clk);
>>   	return ret;
>> @@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
>>   #if defined(CONFIG_MEDIA_CONTROLLER)
>>   	media_entity_cleanup(&info->sd.entity);
>>   #endif
>> +	ov7670_s_power(sd, 0);
>>   	return 0;
>>   }
>>   
>> -- 
>> 2.13.0
>>

Best Regards,
Wenyou Yang

[toc] | [prev] | [next] | [standalone]

#1734114 — Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

Hi Wenyou,

On Mon, Sep 18, 2017 at 05:26:16PM +0800, Yang, Wenyou wrote:
> Hi Sakari,
> 
> 
> On 2017/9/18 15:35, Sakari Ailus wrote:
> > Hi Wenyou,
> > 
> > Thanks for the update.
> > 
> > The driver exposes controls that are accessible through the sub-device node
> > even if the device hasn't been powered on.
> > 
> > Many ISP and bridge drivers will also power the sensor down once the last
> > user of the user space device nodes disappears. You could keep the device
> > powered at all times or change the behaviour so that controls can be
> > accessed when the power is off.
> > 
> > The best option would be to convert the driver to use runtime PM.
> Yes, I agree with you.
> I also noticed there are a lot of things needed to improve except for it,
> such as the platform data via device tree.
> I would like do it in another patch set. I will continue to work on it.

Adding runtime PM support later on sound good to me.

> > An example of this can be found in drivers/media/i2c/ov13858.c .
> A good example, thank you for your providing.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

[toc] | [prev] | [next] | [standalone]

#1733679 — [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

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 v4:
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

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 | 75 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 553945d4ca28..456f48057605 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -232,6 +232,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;
@@ -975,6 +976,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;
 }
 
@@ -998,8 +1002,15 @@ 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;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+		struct v4l2_mbus_framefmt *mbus_fmt;
+
+		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+		*mbus_fmt = format->format;
 		return 0;
+#else
+		return -ENOTTY;
+#endif
 	}
 
 	ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);
@@ -1041,6 +1052,29 @@ 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);
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+		struct v4l2_mbus_framefmt *mbus_fmt;
+
+		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+		format->format = *mbus_fmt;
+		return 0;
+#else
+		return -ENOTTY;
+#endif
+	} 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.
@@ -1508,6 +1542,30 @@ 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;
+}
+
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+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;
+}
+#endif
+
 /* ----------------------------------------------------------------------- */
 
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
@@ -1528,6 +1586,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,
 };
 
@@ -1537,6 +1596,12 @@ static const struct v4l2_subdev_ops ov7670_ops = {
 	.pad = &ov7670_pad_ops,
 };
 
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+static const struct v4l2_subdev_internal_ops ov7670_subdev_internal_ops = {
+	.open = ov7670_open,
+};
+#endif
+
 /* ----------------------------------------------------------------------- */
 
 static const struct ov7670_devtype ov7670_devdata[] = {
@@ -1589,6 +1654,11 @@ static int ov7670_probe(struct i2c_client *client,
 	sd = &info->sd;
 	v4l2_i2c_subdev_init(sd, client, &ov7670_ops);
 
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+	sd->internal_ops = &ov7670_subdev_internal_ops;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+#endif
+
 	info->clock_speed = 30; /* default: a guess */
 	if (client->dev.platform_data) {
 		struct ov7670_config *config = client->dev.platform_data;
@@ -1645,6 +1715,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

[toc] | [prev] | [next] | [standalone]

#1733700 — Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

Hi Wenyou,

On Mon, Sep 18, 2017 at 02:45:13PM +0800, Wenyou Yang wrote:
> @@ -998,8 +1002,15 @@ 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;
> +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> +		struct v4l2_mbus_framefmt *mbus_fmt;

This will emit a compiler warning at least.

> +
> +		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
> +		*mbus_fmt = format->format;
>  		return 0;
> +#else
> +		return -ENOTTY;
> +#endif
>  	}
>  
>  	ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

[toc] | [prev] | [next] | [standalone]

#1733897 — Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

Hi Sakari,


On 2017/9/18 15:36, Sakari Ailus wrote:
> Hi Wenyou,
>
> On Mon, Sep 18, 2017 at 02:45:13PM +0800, Wenyou Yang wrote:
>> @@ -998,8 +1002,15 @@ 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;
>> +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
>> +		struct v4l2_mbus_framefmt *mbus_fmt;
> This will emit a compiler warning at least.
Thank you for your review.
Will be fixed in next version.
>
>> +
>> +		mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
>> +		*mbus_fmt = format->format;
>>   		return 0;
>> +#else
>> +		return -ENOTTY;
>> +#endif
>>   	}
>>   
>>   	ret = ov7670_try_fmt_internal(sd, &format->format, &ovfmt, &wsize);

Best Regards,
Wenyou Yang

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web