On Tue, 2019-06-25 at 17:39 -0300, Ezequiel Garcia wrote:
> 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.

Do you have a real world example for this?

> 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 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.

> Signed-off-by: Ezequiel Garcia <ezequ...@collabora.com>

Reviewed-by: Philipp Zabel <p.za...@pengutronix.de>

regards
Philipp

> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++++++----------------
>  1 file changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
> b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index f29e28df36ed..10342434e797 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -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:

Reply via email to