diff mbox series

[v2,08/11] media: i2c: ov5645: Switch to {enable,disable}_streams

Message ID 20240910170610.226189-9-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State New
Headers show
Series media: ov5645: Add support for streams | expand

Commit Message

Lad, Prabhakar Sept. 10, 2024, 5:06 p.m. UTC 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Switch from s_stream to enable_streams and disable_streams callbacks.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/i2c/ov5645.c | 90 +++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 44 deletions(-)

Comments

Laurent Pinchart Sept. 24, 2024, 10:55 p.m. UTC | #1 
Hi Prabhakar,

Thank you for the patch.

On Tue, Sep 10, 2024 at 06:06:07PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Switch from s_stream to enable_streams and disable_streams callbacks.

Here too you should explain why. The "why" is the most important part of
any commit message.

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/i2c/ov5645.c | 90 +++++++++++++++++++-------------------
>  1 file changed, 46 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index 9497ec737cb7..dc93514608ee 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -923,71 +923,71 @@ static int ov5645_get_selection(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> -static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable)
> +static int ov5645_enable_streams(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state, u32 pad,
> +				 u64 streams_mask)
>  {
> -	struct ov5645 *ov5645 = to_ov5645(subdev);
> -	struct v4l2_subdev_state *state;
> +	struct ov5645 *ov5645 = to_ov5645(sd);
>  	int ret;
>  
> -	state = v4l2_subdev_lock_and_get_active_state(&ov5645->sd);
> -
> -	if (enable) {
> -		ret = pm_runtime_resume_and_get(ov5645->dev);
> -		if (ret < 0) {
> -			v4l2_subdev_unlock_state(state);
> -			return ret;
> -		}
> +	ret = pm_runtime_resume_and_get(ov5645->dev);
> +	if (ret < 0)
> +		return ret;
>  
> -		ret = ov5645_set_register_array(ov5645,
> +	ret = ov5645_set_register_array(ov5645,
>  					ov5645->current_mode->data,
>  					ov5645->current_mode->data_size);
> -		if (ret < 0) {
> -			dev_err(ov5645->dev, "could not set mode %dx%d\n",
> -				ov5645->current_mode->width,
> -				ov5645->current_mode->height);
> -			goto err_rpm_put;
> -		}
> -		ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls);
> -		if (ret < 0) {
> -			dev_err(ov5645->dev, "could not sync v4l2 controls\n");
> -			goto err_rpm_put;
> -		}
> -
> -		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45);
> -		if (ret < 0)
> -			goto err_rpm_put;
> -
> -		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
> -				       OV5645_SYSTEM_CTRL0_START);
> -		if (ret < 0)
> -			goto err_rpm_put;
> -	} else {
> -		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40);
> -		if (ret < 0)
> -			goto stream_off_rpm_put;
> +	if (ret < 0) {
> +		dev_err(ov5645->dev, "could not set mode %dx%d\n",
> +			ov5645->current_mode->width,
> +			ov5645->current_mode->height);
> +		goto err_rpm_put;
> +	}
> +	ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls);
> +	if (ret < 0) {
> +		dev_err(ov5645->dev, "could not sync v4l2 controls\n");
> +		goto err_rpm_put;
> +	}
>  
> -		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
> -				       OV5645_SYSTEM_CTRL0_STOP);
> +	ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45);
> +	if (ret < 0)
> +		goto err_rpm_put;
>  
> -		goto stream_off_rpm_put;
> -	}
> +	ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
> +			       OV5645_SYSTEM_CTRL0_START);
> +	if (ret < 0)
> +		goto err_rpm_put;
>  
> -	v4l2_subdev_unlock_state(state);
>  	return 0;
>  
>  err_rpm_put:
>  	pm_runtime_put_sync(ov5645->dev);
>  	return ret;
> +}
> +
> +static int ov5645_disable_streams(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *state, u32 pad,
> +				  u64 streams_mask)
> +{
> +	struct ov5645 *ov5645 = to_ov5645(sd);
> +	int ret;
> +
> +	ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40);
> +	if (ret < 0)
> +		goto rpm_put;
> +
> +	ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
> +			       OV5645_SYSTEM_CTRL0_STOP);
>  
> -stream_off_rpm_put:
> +rpm_put:
>  	pm_runtime_mark_last_busy(ov5645->dev);
>  	pm_runtime_put_autosuspend(ov5645->dev);
> -	v4l2_subdev_unlock_state(state);
> +
>  	return ret;
>  }
>  
>  static const struct v4l2_subdev_video_ops ov5645_video_ops = {
> -	.s_stream = ov5645_s_stream,
> +	.s_stream = v4l2_subdev_s_stream_helper,
>  };
>  
>  static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = {
> @@ -996,6 +996,8 @@ static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = {
>  	.get_fmt = v4l2_subdev_get_fmt,
>  	.set_fmt = ov5645_set_format,
>  	.get_selection = ov5645_get_selection,
> +	.enable_streams = ov5645_enable_streams,
> +	.disable_streams = ov5645_disable_streams,
>  };
>  
>  static const struct v4l2_subdev_core_ops ov5645_core_ops = {
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 9497ec737cb7..dc93514608ee 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -923,71 +923,71 @@  static int ov5645_get_selection(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable)
+static int ov5645_enable_streams(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state, u32 pad,
+				 u64 streams_mask)
 {
-	struct ov5645 *ov5645 = to_ov5645(subdev);
-	struct v4l2_subdev_state *state;
+	struct ov5645 *ov5645 = to_ov5645(sd);
 	int ret;
 
-	state = v4l2_subdev_lock_and_get_active_state(&ov5645->sd);
-
-	if (enable) {
-		ret = pm_runtime_resume_and_get(ov5645->dev);
-		if (ret < 0) {
-			v4l2_subdev_unlock_state(state);
-			return ret;
-		}
+	ret = pm_runtime_resume_and_get(ov5645->dev);
+	if (ret < 0)
+		return ret;
 
-		ret = ov5645_set_register_array(ov5645,
+	ret = ov5645_set_register_array(ov5645,
 					ov5645->current_mode->data,
 					ov5645->current_mode->data_size);
-		if (ret < 0) {
-			dev_err(ov5645->dev, "could not set mode %dx%d\n",
-				ov5645->current_mode->width,
-				ov5645->current_mode->height);
-			goto err_rpm_put;
-		}
-		ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls);
-		if (ret < 0) {
-			dev_err(ov5645->dev, "could not sync v4l2 controls\n");
-			goto err_rpm_put;
-		}
-
-		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45);
-		if (ret < 0)
-			goto err_rpm_put;
-
-		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
-				       OV5645_SYSTEM_CTRL0_START);
-		if (ret < 0)
-			goto err_rpm_put;
-	} else {
-		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40);
-		if (ret < 0)
-			goto stream_off_rpm_put;
+	if (ret < 0) {
+		dev_err(ov5645->dev, "could not set mode %dx%d\n",
+			ov5645->current_mode->width,
+			ov5645->current_mode->height);
+		goto err_rpm_put;
+	}
+	ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls);
+	if (ret < 0) {
+		dev_err(ov5645->dev, "could not sync v4l2 controls\n");
+		goto err_rpm_put;
+	}
 
-		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
-				       OV5645_SYSTEM_CTRL0_STOP);
+	ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45);
+	if (ret < 0)
+		goto err_rpm_put;
 
-		goto stream_off_rpm_put;
-	}
+	ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
+			       OV5645_SYSTEM_CTRL0_START);
+	if (ret < 0)
+		goto err_rpm_put;
 
-	v4l2_subdev_unlock_state(state);
 	return 0;
 
 err_rpm_put:
 	pm_runtime_put_sync(ov5645->dev);
 	return ret;
+}
+
+static int ov5645_disable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state, u32 pad,
+				  u64 streams_mask)
+{
+	struct ov5645 *ov5645 = to_ov5645(sd);
+	int ret;
+
+	ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40);
+	if (ret < 0)
+		goto rpm_put;
+
+	ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
+			       OV5645_SYSTEM_CTRL0_STOP);
 
-stream_off_rpm_put:
+rpm_put:
 	pm_runtime_mark_last_busy(ov5645->dev);
 	pm_runtime_put_autosuspend(ov5645->dev);
-	v4l2_subdev_unlock_state(state);
+
 	return ret;
 }
 
 static const struct v4l2_subdev_video_ops ov5645_video_ops = {
-	.s_stream = ov5645_s_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
 static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = {
@@ -996,6 +996,8 @@  static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = {
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = ov5645_set_format,
 	.get_selection = ov5645_get_selection,
+	.enable_streams = ov5645_enable_streams,
+	.disable_streams = ov5645_disable_streams,
 };
 
 static const struct v4l2_subdev_core_ops ov5645_core_ops = {