[PATCH] [RFC PATCH] staging: media: atomisp: remove redundant ov2722_startup() helper

[Mohit Mishra]

In atomisp-ov2722.c, ov2722_startup() was invoked inside
ov2722_set_fmt() to program sensor hardware registers during format
negotiation, accompanied by a "TODO: remove it" comment.

Standard V4L2 subdevice drivers do not program hardware registers during
.set_fmt(), but defer hardware register configuration to stream start
(.s_stream(1)).

This patch:
1. Removes the ov2722_startup() helper function and its TODO comment.
2. Refactors register initialization into a static
   ov2722_startup_registers() helper.
3. Moves sensor software reset and resolution register array configuration
   into ov2722_s_stream() when enable == 1.
4. Preserves the PMIC power-cycle retry loop on startup failure inside
   ov2722_s_stream(1) to maintain hardware fault-tolerance for AtomISP
   tablets.
5. Updates ov2722_set_fmt() to purely update internal software state.

BEHAVIORAL RISK & UNTESTED NOTICE:
This is a functional timing change. Deferring hardware register writes to
.s_stream(1) has not been tested on physical Intel AtomISP hardware.
Submitted as RFC for review by AtomISP maintainers (Hans de Goede,
Mauro Carvalho Chehab).

Signed-off-by: Mohit Mishra <mishraloopmohit@xxxxxxxxx>
---
 .../media/atomisp/i2c/atomisp-ov2722.c        | 109 +++++++++---------
 1 file changed, 53 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 2c41c496daa6..5cbfbe5217a4 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -599,39 +599,14 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
 	return ret;
 }
 
-/* TODO: remove it. */
-static int ov2722_startup(struct v4l2_subdev *sd)
-{
-	struct ov2722_device *dev = to_ov2722_sensor(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	int ret = 0;
-
-	ret = ov2722_write_reg(client, OV2722_8BIT,
-			       OV2722_SW_RESET, 0x01);
-	if (ret) {
-		dev_err(&client->dev, "ov2722 reset err.\n");
-		return ret;
-	}
-
-	ret = ov2722_write_reg_array(client, dev->res->regs);
-	if (ret) {
-		dev_err(&client->dev, "ov2722 write register err.\n");
-		return ret;
-	}
-
-	return ret;
-}
-
 static int ov2722_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_state *sd_state,
 			  struct v4l2_subdev_format *format)
 {
 	struct v4l2_mbus_framefmt *fmt = &format->format;
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct ov2722_resolution *res;
 	struct camera_mipi_info *ov2722_info = NULL;
-	int ret = 0;
 
 	if (format->pad)
 		return -EINVAL;
@@ -662,38 +637,8 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
 	dev->pixels_per_line = dev->res->pixels_per_line;
 	dev->lines_per_frame = dev->res->lines_per_frame;
 
-	ret = ov2722_startup(sd);
-	if (ret) {
-		int i = 0;
-
-		dev_err(&client->dev, "ov2722 startup err, retry to power up\n");
-		for (i = 0; i < OV2722_POWER_UP_RETRY_NUM; i++) {
-			dev_err(&client->dev,
-				"ov2722 retry to power up %d/%d times, result: ",
-				i + 1, OV2722_POWER_UP_RETRY_NUM);
-			power_down(sd);
-			ret = power_up(sd);
-			if (ret) {
-				dev_err(&client->dev, "power up failed, continue\n");
-				continue;
-			}
-			ret = ov2722_startup(sd);
-			if (ret) {
-				dev_err(&client->dev, " startup FAILED!\n");
-			} else {
-				dev_err(&client->dev, " startup SUCCESS!\n");
-				break;
-			}
-		}
-		if (ret) {
-			dev_err(&client->dev, "ov2722 startup err\n");
-			goto err;
-		}
-	}
-
-err:
 	mutex_unlock(&dev->input_lock);
-	return ret;
+	return 0;
 }
 
 static int ov2722_get_fmt(struct v4l2_subdev *sd,
@@ -746,6 +691,26 @@ static int ov2722_detect(struct i2c_client *client)
 	return 0;
 }
 
+static int ov2722_startup_registers(struct v4l2_subdev *sd)
+{
+	struct ov2722_device *dev = to_ov2722_sensor(sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	int ret;
+
+	ret = ov2722_write_reg(client, OV2722_8BIT,
+			       OV2722_SW_RESET, 0x01);
+	if (ret) {
+		dev_err(&client->dev, "ov2722 reset err.\n");
+		return ret;
+	}
+
+	ret = ov2722_write_reg_array(client, dev->res->regs);
+	if (ret)
+		dev_err(&client->dev, "ov2722 write register err.\n");
+
+	return ret;
+}
+
 static int ov2722_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
@@ -754,10 +719,42 @@ static int ov2722_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&dev->input_lock);
 
+	if (enable) {
+		ret = ov2722_startup_registers(sd);
+		if (ret) {
+			int i;
+
+			dev_err(&client->dev, "ov2722 startup err, retry to power up\n");
+			for (i = 0; i < OV2722_POWER_UP_RETRY_NUM; i++) {
+				dev_err(&client->dev,
+					"ov2722 retry to power up %d/%d times, result: ",
+					i + 1, OV2722_POWER_UP_RETRY_NUM);
+				power_down(sd);
+				ret = power_up(sd);
+				if (ret) {
+					dev_err(&client->dev, "power up failed, continue\n");
+					continue;
+				}
+				ret = ov2722_startup_registers(sd);
+				if (ret) {
+					dev_err(&client->dev, " startup FAILED!\n");
+				} else {
+					dev_err(&client->dev, " startup SUCCESS!\n");
+					break;
+				}
+			}
+			if (ret) {
+				dev_err(&client->dev, "ov2722 startup err\n");
+				goto unlock;
+			}
+		}
+	}
+
 	ret = ov2722_write_reg(client, OV2722_8BIT, OV2722_SW_STREAM,
 			       enable ? OV2722_START_STREAMING :
 			       OV2722_STOP_STREAMING);
 
+unlock:
 	mutex_unlock(&dev->input_lock);
 	return ret;
 }
-- 
2.43.0