[Linux-stm32] [PATCH] media: ov5640: fix MIPI power sequence

[Sakari Ailus]

Hi Hugues,

On Wed, Oct 07, 2020 at 04:43:18PM +0200, Hugues Fruchet wrote:
> fixes: b1751ae652fb ("media: i2c: ov5640: Separate out mipi configuration from s_power")

Fixes: ...

And preferrably before Sob line.

> 
> Fix ov5640_write_reg()return value unchecked at power off.
> Reformat code to keep register access below the register description.

If there's an error, I wouldn't stop turning things off, but just proceed.
The caller will ignore the error in that case anyway. This changes with the
patch.

> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet at st.com>
> Change-Id: I12db67c416c3d63eadee400a3c89aaf48c5b1469

This was probably not intended to be here.

> ---
>  drivers/media/i2c/ov5640.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 8d0254d..f34e62e 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1940,14 +1940,6 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>  {
>  	int ret;
>  
> -	if (!on) {
> -		/* Reset MIPI bus settings to their default values. */
> -		ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x58);
> -		ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x04);
> -		ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x00);
> -		return 0;
> -	}
> -
>  	/*
>  	 * Power up MIPI HS Tx and LS Rx; 2 data lanes mode
>  	 *
> @@ -1958,7 +1950,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>  	 * [3] = 0	: Power up MIPI LS Rx
>  	 * [2] = 0	: MIPI interface disabled
>  	 */
> -	ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x40);
> +	ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
> +			       on ? 0x40 : 0x58);
>  	if (ret)
>  		return ret;
>  
> @@ -1969,7 +1962,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>  	 * [5] = 1	: Gate clock when 'no packets'
>  	 * [2] = 1	: MIPI bus in LP11 when 'no packets'
>  	 */
> -	ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x24);
> +	ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00,
> +			       on ? 0x24 : 0x04);
>  	if (ret)
>  		return ret;
>  
> @@ -1981,7 +1975,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>  	 * [5] = 1	: MIPI data lane 1 in LP11 when 'sleeping'
>  	 * [4] = 1	: MIPI clock lane in LP11 when 'sleeping'
>  	 */
> -	ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x70);
> +	ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
> +			       on ? 0x70 : 0x00);
>  	if (ret)
>  		return ret;
>  

-- 
Kind regards,

Sakari Ailus