media: imx: mipi csi-2: Don't fail if initial state times-out

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

Signed-off-by Ezequiel Garcia <[email protected]>

---
  drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
  1 file changed, 9 insertions(+), 24 deletions(-)

drivers/staging/media/imx/imx6-mipi-csi2.c +9 -24

index f29e28df36ed..10342434e797 100644
@@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct csi2_dev *csi2)
  }
 
  /* Waits for low-power LP-11 state on data and clock lanes. */
-static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
+static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
  {
  	u32 mask, reg;
  	int ret;

@@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
  	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  				 (reg & mask) == mask, 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
  }
 
  /* Wait for active clock on the clock lane. */
-static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
+static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
  {
  	u32 reg;
  	int ret;
 
  	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  				 (reg & PHY_RXCLKACTIVEHS), 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
-			 reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
+			  reg);
  }
 
  /* Setup the i.MX CSI2IPU Gasket */

@@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
  	csi2_enable(csi2, true);
 
  	/* Step 5 */
-	ret = csi2_dphy_wait_stopstate(csi2);
-	if (ret)
-		goto err_assert_reset;
+	csi2_dphy_wait_stopstate(csi2);
 
  	/* Step 6 */
  	ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);

@@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
  		goto err_assert_reset;
 
  	/* Step 7 */
-	ret = csi2_dphy_wait_clock_lane(csi2);
-	if (ret)
-		goto err_stop_upstream;
-
+	csi2_dphy_wait_clock_lane(csi2);
  	return 0;
 
-err_stop_upstream:
-	v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
  err_assert_reset:
  	csi2_enable(csi2, false);
  err_disable_clk:

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
 1 file changed, 9 insertions(+), 24 deletions(-)

drivers/staging/media/imx/imx6-mipi-csi2.c +9 -24

index f29e28df36ed..10342434e797 100644
@@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct csi2_dev *csi2)
 }
 
 /* Waits for low-power LP-11 state on data and clock lanes. */
-static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
+static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 {
 	u32 mask, reg;
 	int ret;

@@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & mask) == mask, 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
 }
 
 /* Wait for active clock on the clock lane. */
-static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
+static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
 {
 	u32 reg;
 	int ret;
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & PHY_RXCLKACTIVEHS), 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
-			 reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
+			  reg);
 }
 
 /* Setup the i.MX CSI2IPU Gasket */

@@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
 	csi2_enable(csi2, true);
 
 	/* Step 5 */
-	ret = csi2_dphy_wait_stopstate(csi2);
-	if (ret)
-		goto err_assert_reset;
+	csi2_dphy_wait_stopstate(csi2);
 
 	/* Step 6 */
 	ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);

@@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
 		goto err_assert_reset;
 
 	/* Step 7 */
-	ret = csi2_dphy_wait_clock_lane(csi2);
-	if (ret)
-		goto err_stop_upstream;
-
+	csi2_dphy_wait_clock_lane(csi2);
 	return 0;
 
-err_stop_upstream:
-	v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
 err_assert_reset:
 	csi2_enable(csi2, false);
 err_disable_clk:

I think the messages could use a note that they may be due to a sensor or sensor driver bug, and that there might be image artifacts or outright failure to capture as a consequence.

Hi Philipp,

Quoted 3 lines · Philipp Zabel

I think the messages could use a note that they may be due to a sensor or sensor driver bug, and that there might be image artifacts or outright failure to capture as a consequence.

Yes, this a good idea.

With this patch I could successfully test camera capture on a imx6dl-wandboard connected to a OV5645.

Without this patch the Gsteamer pipeline fails.

Tested-by Fabio Estevam <[email protected]>

Did you only get the LP-11 timeout warning message with this patch on the OV5645, or both the LP-11 timeout and clock lane timeout warnings?

Hi Steve,

Quoted 2 lines · Steve Longerbeam

Did you only get the LP-11 timeout warning message with this patch on the OV5645, or both the LP-11 timeout and clock lane timeout warnings?

With this patch applied I get only the LP-11 timeout warnings, not clock lane timeouts.

Hi Fabio,

Quoted 7 lines · Fabio Estevam

Hi Steve,

Quoted 2 lines · Steve Longerbeam

Did you only get the LP-11 timeout warning message with this patch on the OV5645, or both the LP-11 timeout and clock lane timeout warnings?

Quoted 2 lines

With this patch applied I get only the LP-11 timeout warnings, not clock lane timeouts.

Ok thanks for the confirmation that the imx6 CSI-2 receiver is able to successfully move to stream on without seeing the LP-11 state in this case.

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

Are there any visual artifacts in the first frame(s) in this case?

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

I agree.

Hi Philipp,

Quoted 1 line · Philipp Zabel

Are there any visual artifacts in the first frame(s) in this case?

I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! kmssink

Quoted 3 lines

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

Quoted 1 line

I agree.

Here is a reworked version of Ezequiel's patch as per the suggestions:
http://code.bulix.org/g5qap5-780475

Does this one look good?

Hi Fabio,

Quoted 11 lines · Fabio Estevam

Hi Philipp,

Quoted 1 line · Philipp Zabel

Are there any visual artifacts in the first frame(s) in this case?

I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! kmssink

Quoted 3 lines

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

Quoted 6 lines

Quoted 1 line

I agree.

Here is a reworked version of Ezequiel's patch as per the suggestions:
http://code.bulix.org/g5qap5-780475

Does this one look good?

Limiting the change to wait_stopstate is fine, the actual message makes assumptions that could be misleading. How about:

"Timeout waiting for LP-11 state on all active lanes.
This is most likely caused by a bug in the sensor driver. Capture might fail or contain visual artifacts."

I'd like to keep the phy_state register output though, if only as dev_dbg(). It contains useful output for debugging, for example if only some of the lanes are in stop state, which could indicate an issue with connections or lane configuration.

I think Philipp's suggestions looks very good, both the text and keeping the phy state. I think both should be kept in the warning.

Fabio: feel free to submit a v2, or let me know so I'll add it to my TODO.

Hi Fabio,

Quoted 13 lines · Fabio Estevam

Hi Philipp,

Quoted 1 line · Philipp Zabel

Are there any visual artifacts in the first frame(s) in this case?

I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! kmssink

Quoted 3 lines

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

I agree.

Here is a reworked version of Ezequiel's patch as per the suggestions:
http://code.bulix.org/g5qap5-780475

Quoted 1 line

Does this one look good?

Limiting the change to wait_stopstate is fine, the actual message makes assumptions that could be misleading. How about:

"Timeout waiting for LP-11 state on all active lanes.
This is most likely caused by a bug in the sensor driver. Capture might fail or contain visual artifacts."

I'd like to keep the phy_state register output though, if only as dev_dbg(). It contains useful output for debugging, for example if only some of the lanes are in stop state, which could indicate an issue with connections or lane configuration.

Quoted 24 lines · Philipp Zabel

Hi Fabio,

Quoted 11 lines · Fabio Estevam

Hi Philipp,

Quoted 1 line · Philipp Zabel

Are there any visual artifacts in the first frame(s) in this case?

I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! kmssink

Quoted 3 lines

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

Quoted 6 lines

Quoted 1 line

I agree.

Here is a reworked version of Ezequiel's patch as per the suggestions:
http://code.bulix.org/g5qap5-780475

Does this one look good?

Limiting the change to wait_stopstate is fine, the actual message makes assumptions that could be misleading. How about:

Quoted 3 lines

"Timeout waiting for LP-11 state on all active lanes.
This is most likely caused by a bug in the sensor driver. Capture might fail or contain visual artifacts."

Yes I agree that is more descriptive, if a bit wordy for a kernel error message.

I think it could be reduced, something like:

"LP-11 wait timeout on all lanes, likely a sensor driver bug, expect capture failures."

Quoted 10 lines · Steve Longerbeam

Hi Fabio,

Quoted 7 lines · Fabio Estevam

Hi Steve,

Quoted 2 lines · Steve Longerbeam

Did you only get the LP-11 timeout warning message with this patch on the OV5645, or both the LP-11 timeout and clock lane timeout warnings?

Quoted 6 lines

Quoted 2 lines

With this patch applied I get only the LP-11 timeout warnings, not clock lane timeouts.

Ok thanks for the confirmation that the imx6 CSI-2 receiver is able to successfully move to stream on without seeing the LP-11 state in this case.

Are there any visual artifacts in the first frame(s) in this case?

Quoted 2 lines

So in my opinion the next version of this patch should make LP-11 timeout a warning only, but keep the error return on clock lane timeouts.

I agree.

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
 1 file changed, 9 insertions(+), 24 deletions(-)

drivers/staging/media/imx/imx6-mipi-csi2.c +9 -24

index f29e28df36ed..10342434e797 100644
@@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct csi2_dev *csi2)
 }
 
 /* Waits for low-power LP-11 state on data and clock lanes. */
-static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
+static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 {
 	u32 mask, reg;
 	int ret;

@@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & mask) == mask, 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
 }
 
 /* Wait for active clock on the clock lane. */
-static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
+static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
 {
 	u32 reg;
 	int ret;
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & PHY_RXCLKACTIVEHS), 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
-			 reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
+			  reg);
 }
 
 /* Setup the i.MX CSI2IPU Gasket */

@@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
 	csi2_enable(csi2, true);
 
 	/* Step 5 */
-	ret = csi2_dphy_wait_stopstate(csi2);
-	if (ret)
-		goto err_assert_reset;
+	csi2_dphy_wait_stopstate(csi2);
 
 	/* Step 6 */
 	ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);

@@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
 		goto err_assert_reset;
 
 	/* Step 7 */
-	ret = csi2_dphy_wait_clock_lane(csi2);
-	if (ret)
-		goto err_stop_upstream;
-
+	csi2_dphy_wait_clock_lane(csi2);
 	return 0;
 
-err_stop_upstream:
-	v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
 err_assert_reset:
 	csi2_enable(csi2, false);
 err_disable_clk:

Hi Ezequiel,

Thank you for the patch.

Quoted 11 lines · Ezequiel Garcia

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

I'm not sure this is a very good idea. Failure to detect the LP-11 state may mean that the sensor is completely powered off, but it may also mean that it is already streaming data. I don't know how the CSI-2 receiver state machine will operate in the first case, but in the second case it will not be able to synchronise to the incoming stream, so it won't work anyway.

I think you should instead fix the problem in the sensor driver, as you hinted.

Relaxing the requirement here will only make it more confusing, it's a hack, and isn't portable across CSI-2 receivers.

The same buggy
sensor driver won't work with other CSI-2 receivers whose internal state machine require starting in the LP-11 state.

Which sensor are you using ?

Hi Ezequiel,

Thank you for the patch.

Quoted 11 lines · Ezequiel Garcia

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

I'm not sure this is a very good idea. Failure to detect the LP-11 state may mean that the sensor is completely powered off, but it may also mean that it is already streaming data. I don't know how the CSI-2 receiver state machine will operate in the first case, but in the second case it will not be able to synchronise to the incoming stream, so it won't work anyway.

I think you should instead fix the problem in the sensor driver, as you hinted. Relaxing the requirement here will only make it more confusing, it's a hack, and isn't portable across CSI-2 receivers. The same buggy sensor driver won't work with other CSI-2 receivers whose internal state machine require starting in the LP-11 state.

Which sensor are you using ?

Quoted 81 lines

Signed-off-by Ezequiel Garcia <[email protected]>

---
  drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
  1 file changed, 9 insertions(+), 24 deletions(-)

drivers/staging/media/imx/imx6-mipi-csi2.c +9 -24

index f29e28df36ed..10342434e797 100644
@@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct csi2_dev *csi2)
  }
 
  /* Waits for low-power LP-11 state on data and clock lanes. */
-static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
+static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
  {
  	u32 mask, reg;
  	int ret;

@@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
  	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  				 (reg & mask) == mask, 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
  }
 
  /* Wait for active clock on the clock lane. */
-static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
+static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
  {
  	u32 reg;
  	int ret;
 
  	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
  				 (reg & PHY_RXCLKACTIVEHS), 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
-			 reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
+			  reg);
  }
 
  /* Setup the i.MX CSI2IPU Gasket */

@@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
  	csi2_enable(csi2, true);
 
  	/* Step 5 */
-	ret = csi2_dphy_wait_stopstate(csi2);
-	if (ret)
-		goto err_assert_reset;
+	csi2_dphy_wait_stopstate(csi2);
 
  	/* Step 6 */
  	ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);

@@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
  		goto err_assert_reset;
 
  	/* Step 7 */
-	ret = csi2_dphy_wait_clock_lane(csi2);
-	if (ret)
-		goto err_stop_upstream;
-
+	csi2_dphy_wait_clock_lane(csi2);
  	return 0;
 
-err_stop_upstream:
-	v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
  err_assert_reset:
  	csi2_enable(csi2, false);
  err_disable_clk:

Not all sensors will be able to guarantee a proper initial state. This may be either because the driver is not properly written, or (probably unlikely) because the hardware won't support it.

While the right solution in the former case is to fix the sensor driver, the real world not always allows right solutions, due to lack of available documentation and support on these sensors.

Let's relax this requirement, and allow the driver to support stream start, even if the sensor initial sequence wasn't the expected.
A warning is still emitted, so users should be hinted that something is off.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
 1 file changed, 9 insertions(+), 24 deletions(-)

drivers/staging/media/imx/imx6-mipi-csi2.c +9 -25

index f29e28df36ed..10342434e797 100644
@@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct csi2_dev *csi2)
 }
 
 /* Waits for low-power LP-11 state on data and clock lanes. */
-static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
+static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 {
 	u32 mask, reg;
 	int ret;

@@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & mask) == mask, 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
 }
 
 /* Wait for active clock on the clock lane. */
-static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
+static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
 {
 	u32 reg;
 	int ret;
 
 	ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
 				 (reg & PHY_RXCLKACTIVEHS), 0, 500000);
-	if (ret) {
-		v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
-			 reg);
-		return ret;
-	}
-
-	return 0;
+	if (ret)
+		v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
+			  reg);
 }
 
 /* Setup the i.MX CSI2IPU Gasket */

@@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
 	csi2_enable(csi2, true);
 
 	/* Step 5 */
-	ret = csi2_dphy_wait_stopstate(csi2);
-	if (ret)
-		goto err_assert_reset;
+	csi2_dphy_wait_stopstate(csi2);
 
 	/* Step 6 */
 	ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);

@@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
 		goto err_assert_reset;
 
 	/* Step 7 */
-	ret = csi2_dphy_wait_clock_lane(csi2);
-	if (ret)
-		goto err_stop_upstream;
-
+	csi2_dphy_wait_clock_lane(csi2);
 	return 0;
 
-err_stop_upstream:
-	v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
 err_assert_reset:
 	csi2_enable(csi2, false);
 err_disable_clk:
--

2.20.1