Hi Ezequiel, On Tue, Jun 25, 2019 at 05:39:45PM -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. >
We hit this in the past with the ov5640 sensor, whose driver was not
properly initializing its data lanes in LP-11 state, so yes, I'm not
surprised other sensors might fail in the same way.
Do you get frames after you hit the error? I recall it was not
possible with ov5640, even with something similar to what you've done
here in the CSI-2 receiver driver.
> 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.
>
It seems you're also silencing errors related to clock lane detection.
I would mention that.
> Signed-off-by: Ezequiel Garcia <ezequ...@collabora.com>
> ---
> 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:
> --
> 2.20.1
>
signature.asc
Description: PGP signature
