Re: [PATCH v2 2/2] media: i2c: ov02c10: Check for errors in disable_streams

[Hans de Goede <hansg@kernel.org>]

Hi,

On 25-Jan-26 18:17, Saikiran wrote:
> The ov02c10_disable_streams() function ignores the return value from
> cci_write() when stopping the sensor. If the I2C write fails (e.g.,
> due to CCI timeout, power management race, or device removal), the
> error is silently lost.
> 
> While we still need to return 0 and call pm_runtime_put() regardless
> of hardware state (to prevent PM reference leaks and pipeline lock
> issues), we should at least log when the hardware stop fails.
> 
> This change:
> 1. Captures the cci_write() return value
> 2. Logs an error if the write fails
> 3. Still returns 0 to ensure proper cleanup
> 
> Returning an error from disable_streams would cause the camss driver's
> video_stop_streaming() to exit early without releasing the pipeline
> lock, permanently locking the camera.
> 
> Fixes: 0e98938b0157 ("media: i2c: add OmniVision OV02C10 sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saikiran <bjsaikiran@gmail.com>
> ---
>  drivers/media/i2c/ov02c10.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index b86cae3d2b74..743d8544ac53 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -629,8 +629,12 @@ static int ov02c10_disable_streams(struct v4l2_subdev *sd,
>  				   u32 pad, u64 streams_mask)
>  {
>  	struct ov02c10 *ov02c10 = to_ov02c10(sd);
> +	int ret;
> +
> +	ret = cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);
> +	if (ret)
> +		dev_err(ov02c10->dev, "failed to stop streaming: %d\n", ret);
>  
> -	cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);

cci_write() already logs a message on errors itself, so this is
undesirable as it will lead to duplicate log messages.

NACK, please drop this patch.

Regards,

Hans