diff mbox series

[08/10] media: i2c: Add pm_runtime support to ov7251

Message ID 20220215230737.1870630-9-djrscally@gmail.com (mailing list archive)
State New, archived
Headers show
Series Support OVTI7251 on Microsoft Surface line | expand

Commit Message

Daniel Scally Feb. 15, 2022, 11:07 p.m. UTC 
Add pm_runtime support to the ov7251 driver.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 drivers/media/i2c/ov7251.c | 78 ++++++++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 15 deletions(-)

Comments

Sakari Ailus Feb. 16, 2022, 4:05 p.m. UTC | #1 
Hi Daniel,

On Tue, Feb 15, 2022 at 11:07:35PM +0000, Daniel Scally wrote:
> Add pm_runtime support to the ov7251 driver.
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
>  drivers/media/i2c/ov7251.c | 78 ++++++++++++++++++++++++++++++--------
>  1 file changed, 63 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
> index f137a1e87537..d620ed6a4e42 100644
> --- a/drivers/media/i2c/ov7251.c
> +++ b/drivers/media/i2c/ov7251.c
> @@ -15,6 +15,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> @@ -884,6 +885,24 @@ static void ov7251_set_power_off(struct ov7251 *ov7251)
>  	ov7251_regulators_disable(ov7251);
>  }
>  
> +static int __maybe_unused ov7251_sensor_suspend(struct device *dev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct ov7251 *ov7251 = to_ov7251(sd);
> +
> +	ov7251_set_power_off(ov7251);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused ov7251_sensor_resume(struct device *dev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct ov7251 *ov7251 = to_ov7251(sd);
> +
> +	return ov7251_set_power_on(ov7251);

Could you change the argument type of ov7251_set_power_o{n,ff}() to struct
device * so you'd have just one set of these functions?

> +}
> +
>  static int ov7251_s_power(struct v4l2_subdev *sd, int on)
>  {
>  	struct ov7251 *ov7251 = to_ov7251(sd);
> @@ -985,7 +1004,7 @@ static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
>  
>  	/* v4l2_ctrl_lock() locks our mutex */
>  
> -	if (!ov7251->power_on)
> +	if (!pm_runtime_get_if_in_use(ov7251->dev))
>  		return 0;
>  
>  	switch (ctrl->id) {
> @@ -1009,6 +1028,8 @@ static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
>  		break;
>  	}
>  
> +	pm_runtime_put(ov7251->dev);
> +
>  	return ret;
>  }
>  
> @@ -1261,10 +1282,15 @@ static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
>  	mutex_lock(&ov7251->lock);
>  
>  	if (enable) {
> +		ret = pm_runtime_get_sync(ov7251->dev);
> +		if (ret < 0)
> +			return ret;
> +
>  		ret = ov7251_pll_configure(ov7251);
> -		if (ret)
> -			return dev_err_probe(ov7251->dev, ret,
> -					     "error configuring PLLs\n");
> +		if (ret) {
> +			dev_err(ov7251->dev, "error configuring PLLs\n");
> +			goto err_power_down;
> +		}
>  
>  		ret = ov7251_set_register_array(ov7251,
>  					ov7251->current_mode->data,
> @@ -1273,23 +1299,29 @@ static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
>  			dev_err(ov7251->dev, "could not set mode %dx%d\n",
>  				ov7251->current_mode->width,
>  				ov7251->current_mode->height);
> -			goto exit;
> +			goto err_power_down;
>  		}
>  		ret = __v4l2_ctrl_handler_setup(&ov7251->ctrls);
>  		if (ret < 0) {
>  			dev_err(ov7251->dev, "could not sync v4l2 controls\n");
> -			goto exit;
> +			goto err_power_down;
>  		}
>  		ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
>  				       OV7251_SC_MODE_SELECT_STREAMING);
> +		if (ret)
> +			goto err_power_down;
>  	} else {
>  		ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
>  				       OV7251_SC_MODE_SELECT_SW_STANDBY);
> +		pm_runtime_put(ov7251->dev);
>  	}
>  
> -exit:
>  	mutex_unlock(&ov7251->lock);
> +	return ret;
>  
> +err_power_down:
> +	pm_runtime_put_noidle(ov7251->dev);
> +	mutex_unlock(&ov7251->lock);
>  	return ret;
>  }
>  
> @@ -1604,23 +1636,24 @@ static int ov7251_probe(struct i2c_client *client)
>  		goto free_ctrl;
>  	}
>  
> -	ret = ov7251_s_power(&ov7251->sd, true);
> -	if (ret < 0) {
> -		dev_err(dev, "could not power up OV7251\n");
> +	ret = ov7251_set_power_on(ov7251);
> +	if (ret)
>  		goto free_entity;
> -	}
>  
>  	ret = ov7251_detect_chip(ov7251);
>  	if (ret)
>  		goto power_down;
>  
> +	pm_runtime_set_active(&client->dev);
> +	pm_runtime_get_noresume(&client->dev);
> +	pm_runtime_enable(&client->dev);
>  
>  	ret = ov7251_read_reg(ov7251, OV7251_PRE_ISP_00,
>  			      &ov7251->pre_isp_00);
>  	if (ret < 0) {
>  		dev_err(dev, "could not read test pattern value\n");
>  		ret = -ENODEV;
> -		goto power_down;
> +		goto err_pm_runtime;
>  	}
>  
>  	ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT1,
> @@ -1628,7 +1661,7 @@ static int ov7251_probe(struct i2c_client *client)
>  	if (ret < 0) {
>  		dev_err(dev, "could not read vflip value\n");
>  		ret = -ENODEV;
> -		goto power_down;
> +		goto err_pm_runtime;
>  	}
>  
>  	ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT2,
> @@ -1636,10 +1669,12 @@ static int ov7251_probe(struct i2c_client *client)
>  	if (ret < 0) {
>  		dev_err(dev, "could not read hflip value\n");
>  		ret = -ENODEV;
> -		goto power_down;
> +		goto err_pm_runtime;
>  	}
>  
> -	ov7251_s_power(&ov7251->sd, false);
> +	pm_runtime_set_autosuspend_delay(&client->dev, 1000);
> +	pm_runtime_use_autosuspend(&client->dev);
> +	pm_runtime_put_autosuspend(&client->dev);
>  
>  	ret = v4l2_async_register_subdev(&ov7251->sd);
>  	if (ret < 0) {
> @@ -1651,6 +1686,9 @@ static int ov7251_probe(struct i2c_client *client)
>  
>  	return 0;
>  
> +err_pm_runtime:
> +	pm_runtime_disable(ov7251->dev);
> +	pm_runtime_put_noidle(ov7251->dev);
>  power_down:
>  	ov7251_s_power(&ov7251->sd, false);
>  free_entity:
> @@ -1672,9 +1710,18 @@ static int ov7251_remove(struct i2c_client *client)
>  	v4l2_ctrl_handler_free(&ov7251->ctrls);
>  	mutex_destroy(&ov7251->lock);
>  
> +	pm_runtime_disable(ov7251->dev);
> +	if (!pm_runtime_status_suspended(ov7251->dev))
> +		ov7251_set_power_off(ov7251);
> +	pm_runtime_set_suspended(ov7251->dev);
> +
>  	return 0;
>  }
>  
> +static const struct dev_pm_ops ov7251_pm_ops = {
> +	SET_RUNTIME_PM_OPS(ov7251_sensor_suspend, ov7251_sensor_resume, NULL)
> +};
> +
>  static const struct of_device_id ov7251_of_match[] = {
>  	{ .compatible = "ovti,ov7251" },
>  	{ /* sentinel */ }
> @@ -1692,6 +1739,7 @@ static struct i2c_driver ov7251_i2c_driver = {
>  		.of_match_table = ov7251_of_match,
>  		.acpi_match_table = ov7251_acpi_match,
>  		.name  = "ov7251",
> +		.pm = &ov7251_pm_ops,
>  	},
>  	.probe_new  = ov7251_probe,
>  	.remove = ov7251_remove,
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index f137a1e87537..d620ed6a4e42 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -15,6 +15,7 @@ 
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -884,6 +885,24 @@  static void ov7251_set_power_off(struct ov7251 *ov7251)
 	ov7251_regulators_disable(ov7251);
 }
 
+static int __maybe_unused ov7251_sensor_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov7251 *ov7251 = to_ov7251(sd);
+
+	ov7251_set_power_off(ov7251);
+
+	return 0;
+}
+
+static int __maybe_unused ov7251_sensor_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov7251 *ov7251 = to_ov7251(sd);
+
+	return ov7251_set_power_on(ov7251);
+}
+
 static int ov7251_s_power(struct v4l2_subdev *sd, int on)
 {
 	struct ov7251 *ov7251 = to_ov7251(sd);
@@ -985,7 +1004,7 @@  static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
 
 	/* v4l2_ctrl_lock() locks our mutex */
 
-	if (!ov7251->power_on)
+	if (!pm_runtime_get_if_in_use(ov7251->dev))
 		return 0;
 
 	switch (ctrl->id) {
@@ -1009,6 +1028,8 @@  static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
+	pm_runtime_put(ov7251->dev);
+
 	return ret;
 }
 
@@ -1261,10 +1282,15 @@  static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
 	mutex_lock(&ov7251->lock);
 
 	if (enable) {
+		ret = pm_runtime_get_sync(ov7251->dev);
+		if (ret < 0)
+			return ret;
+
 		ret = ov7251_pll_configure(ov7251);
-		if (ret)
-			return dev_err_probe(ov7251->dev, ret,
-					     "error configuring PLLs\n");
+		if (ret) {
+			dev_err(ov7251->dev, "error configuring PLLs\n");
+			goto err_power_down;
+		}
 
 		ret = ov7251_set_register_array(ov7251,
 					ov7251->current_mode->data,
@@ -1273,23 +1299,29 @@  static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
 			dev_err(ov7251->dev, "could not set mode %dx%d\n",
 				ov7251->current_mode->width,
 				ov7251->current_mode->height);
-			goto exit;
+			goto err_power_down;
 		}
 		ret = __v4l2_ctrl_handler_setup(&ov7251->ctrls);
 		if (ret < 0) {
 			dev_err(ov7251->dev, "could not sync v4l2 controls\n");
-			goto exit;
+			goto err_power_down;
 		}
 		ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
 				       OV7251_SC_MODE_SELECT_STREAMING);
+		if (ret)
+			goto err_power_down;
 	} else {
 		ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
 				       OV7251_SC_MODE_SELECT_SW_STANDBY);
+		pm_runtime_put(ov7251->dev);
 	}
 
-exit:
 	mutex_unlock(&ov7251->lock);
+	return ret;
 
+err_power_down:
+	pm_runtime_put_noidle(ov7251->dev);
+	mutex_unlock(&ov7251->lock);
 	return ret;
 }
 
@@ -1604,23 +1636,24 @@  static int ov7251_probe(struct i2c_client *client)
 		goto free_ctrl;
 	}
 
-	ret = ov7251_s_power(&ov7251->sd, true);
-	if (ret < 0) {
-		dev_err(dev, "could not power up OV7251\n");
+	ret = ov7251_set_power_on(ov7251);
+	if (ret)
 		goto free_entity;
-	}
 
 	ret = ov7251_detect_chip(ov7251);
 	if (ret)
 		goto power_down;
 
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_get_noresume(&client->dev);
+	pm_runtime_enable(&client->dev);
 
 	ret = ov7251_read_reg(ov7251, OV7251_PRE_ISP_00,
 			      &ov7251->pre_isp_00);
 	if (ret < 0) {
 		dev_err(dev, "could not read test pattern value\n");
 		ret = -ENODEV;
-		goto power_down;
+		goto err_pm_runtime;
 	}
 
 	ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT1,
@@ -1628,7 +1661,7 @@  static int ov7251_probe(struct i2c_client *client)
 	if (ret < 0) {
 		dev_err(dev, "could not read vflip value\n");
 		ret = -ENODEV;
-		goto power_down;
+		goto err_pm_runtime;
 	}
 
 	ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT2,
@@ -1636,10 +1669,12 @@  static int ov7251_probe(struct i2c_client *client)
 	if (ret < 0) {
 		dev_err(dev, "could not read hflip value\n");
 		ret = -ENODEV;
-		goto power_down;
+		goto err_pm_runtime;
 	}
 
-	ov7251_s_power(&ov7251->sd, false);
+	pm_runtime_set_autosuspend_delay(&client->dev, 1000);
+	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_put_autosuspend(&client->dev);
 
 	ret = v4l2_async_register_subdev(&ov7251->sd);
 	if (ret < 0) {
@@ -1651,6 +1686,9 @@  static int ov7251_probe(struct i2c_client *client)
 
 	return 0;
 
+err_pm_runtime:
+	pm_runtime_disable(ov7251->dev);
+	pm_runtime_put_noidle(ov7251->dev);
 power_down:
 	ov7251_s_power(&ov7251->sd, false);
 free_entity:
@@ -1672,9 +1710,18 @@  static int ov7251_remove(struct i2c_client *client)
 	v4l2_ctrl_handler_free(&ov7251->ctrls);
 	mutex_destroy(&ov7251->lock);
 
+	pm_runtime_disable(ov7251->dev);
+	if (!pm_runtime_status_suspended(ov7251->dev))
+		ov7251_set_power_off(ov7251);
+	pm_runtime_set_suspended(ov7251->dev);
+
 	return 0;
 }
 
+static const struct dev_pm_ops ov7251_pm_ops = {
+	SET_RUNTIME_PM_OPS(ov7251_sensor_suspend, ov7251_sensor_resume, NULL)
+};
+
 static const struct of_device_id ov7251_of_match[] = {
 	{ .compatible = "ovti,ov7251" },
 	{ /* sentinel */ }
@@ -1692,6 +1739,7 @@  static struct i2c_driver ov7251_i2c_driver = {
 		.of_match_table = ov7251_of_match,
 		.acpi_match_table = ov7251_acpi_match,
 		.name  = "ov7251",
+		.pm = &ov7251_pm_ops,
 	},
 	.probe_new  = ov7251_probe,
 	.remove = ov7251_remove,