media: i2c: imx415: Fix control error handling

[PATCH] media: i2c: imx415: Fix control error handling

Posted by Narasimharao Vadlamudi 2 months ago

imx415_set_testpattern() accumulates failures from cci_write(), but drops
the value and always returns success. Return the accumulated error so V4L2
reports failures to userspace.

The VBLANK path also returned immediately when programming VMAX failed
after pm_runtime_get_if_in_use() had taken a runtime PM reference. Break
out of the switch instead so the common pm_runtime_put() path is used.

Fixes: d5df1c7f3f83 ("media: i2c: imx415: Convert to new CCI register access helpers")
Fixes: 3bcae55ab96a ("media: i2c: imx415: Add read/write control of VBLANK")
Signed-off-by: Narasimharao Vadlamudi <ahmisaranrao@gmail.com>
---
 drivers/media/i2c/imx415.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
index 0b424c17e880..c3b22b238ea0 100644
--- a/drivers/media/i2c/imx415.c
+++ b/drivers/media/i2c/imx415.c
@@ -686,7 +686,7 @@ static int imx415_set_testpattern(struct imx415 *sensor, int val)
 		cci_write(sensor->regmap, IMX415_DIG_CLP_MODE, 0x01, &ret);
 		cci_write(sensor->regmap, IMX415_WRJ_OPEN, 0x01, &ret);
 	}
-	return 0;
+	return ret;
 }
 
 static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -720,7 +720,7 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = cci_write(sensor->regmap, IMX415_VMAX,
 				format->height + ctrl->val, NULL);
 		if (ret)
-			return ret;
+			break;
 		/*
 		 * Exposure is set based on VMAX which has just changed, so
 		 * program exposure register as well
-- 
2.43.0

Re: [PATCH] media: i2c: imx415: Fix control error handling

Posted by Jacopo Mondi 1 month, 4 weeks ago

Hello Narasimharao

On Fri, Jul 31, 2026 at 06:25:42PM +0530, Narasimharao Vadlamudi wrote:
> imx415_set_testpattern() accumulates failures from cci_write(), but drops
> the value and always returns success. Return the accumulated error so V4L2
> reports failures to userspace.
>
> The VBLANK path also returned immediately when programming VMAX failed
> after pm_runtime_get_if_in_use() had taken a runtime PM reference. Break
> out of the switch instead so the common pm_runtime_put() path is used.
>
These seems to be two separate issue worth two differnt patches ?

> Fixes: d5df1c7f3f83 ("media: i2c: imx415: Convert to new CCI register access helpers")
> Fixes: 3bcae55ab96a ("media: i2c: imx415: Add read/write control of VBLANK")

The double Fixes tag seems to suggest that

Also, Fixes commit need

Cc: stable@vger.kernel.org

> Signed-off-by: Narasimharao Vadlamudi <ahmisaranrao@gmail.com>

Thanks
   j

> ---
>  drivers/media/i2c/imx415.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
> index 0b424c17e880..c3b22b238ea0 100644
> --- a/drivers/media/i2c/imx415.c
> +++ b/drivers/media/i2c/imx415.c
> @@ -686,7 +686,7 @@ static int imx415_set_testpattern(struct imx415 *sensor, int val)
>  		cci_write(sensor->regmap, IMX415_DIG_CLP_MODE, 0x01, &ret);
>  		cci_write(sensor->regmap, IMX415_WRJ_OPEN, 0x01, &ret);
>  	}
> -	return 0;
> +	return ret;
>  }
>
>  static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -720,7 +720,7 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
>  		ret = cci_write(sensor->regmap, IMX415_VMAX,
>  				format->height + ctrl->val, NULL);
>  		if (ret)
> -			return ret;
> +			break;
>  		/*
>  		 * Exposure is set based on VMAX which has just changed, so
>  		 * program exposure register as well
> --
> 2.43.0
>
>

Re: [PATCH] media: i2c: imx415: Fix control error handling

Posted by Michael Riesch 1 month, 4 weeks ago

Hi Narasimharao Vadlamudi,

Thanks for the patch!

On 7/31/26 14:55, Narasimharao Vadlamudi wrote:
> imx415_set_testpattern() accumulates failures from cci_write(), but drops
> the value and always returns success. Return the accumulated error so V4L2
> reports failures to userspace.
> 
> The VBLANK path also returned immediately when programming VMAX failed
> after pm_runtime_get_if_in_use() had taken a runtime PM reference. Break
> out of the switch instead so the common pm_runtime_put() path is used.
> 
> Fixes: d5df1c7f3f83 ("media: i2c: imx415: Convert to new CCI register access helpers")
> Fixes: 3bcae55ab96a ("media: i2c: imx415: Add read/write control of VBLANK")
> Signed-off-by: Narasimharao Vadlamudi <ahmisaranrao@gmail.com>

Reviewed-by: Michael Riesch <michael.riesch@collabora.com>

Best regards,
Michael

> ---
>  drivers/media/i2c/imx415.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
> index 0b424c17e880..c3b22b238ea0 100644
> --- a/drivers/media/i2c/imx415.c
> +++ b/drivers/media/i2c/imx415.c
> @@ -686,7 +686,7 @@ static int imx415_set_testpattern(struct imx415 *sensor, int val)
>  		cci_write(sensor->regmap, IMX415_DIG_CLP_MODE, 0x01, &ret);
>  		cci_write(sensor->regmap, IMX415_WRJ_OPEN, 0x01, &ret);
>  	}
> -	return 0;
> +	return ret;
>  }
>  
>  static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -720,7 +720,7 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
>  		ret = cci_write(sensor->regmap, IMX415_VMAX,
>  				format->height + ctrl->val, NULL);
>  		if (ret)
> -			return ret;
> +			break;
>  		/*
>  		 * Exposure is set based on VMAX which has just changed, so
>  		 * program exposure register as well