[PATCH 1/2] media: atomisp-ov2680: initialize return var

[PATCH 1/2] media: atomisp-ov2680: initialize return var

[Mauro Carvalho Chehab]

As the settings are only applied when the device is powered on,
it should return 0 when the device is not powered.

Not doing that causes a warning:

	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c: In function 'ov2680_ioctl':
	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:390:16: warning: 'ret' may be used uninitialized in this
	function [-Wmaybe-uninitialized]
	  390 |         return ov2680_set_exposure(sd, coarse_itg, analog_gain, digital_gain);
	      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:359:13: note: 'ret' was declared here
	  359 |         int ret;
	      |             ^~~

Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: 6b5b60687ada ("media: atomisp-ov2680: Save/restore exposure and gain over sensor power-down")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 34d008236bd9..497884d332e1 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -356,7 +356,7 @@ static int ov2680_set_exposure(struct v4l2_subdev *sd, int exposure,
 			       int gain, int digitgain)
 {
 	struct ov2680_device *dev = to_ov2680_sensor(sd);
-	int ret;
+	int ret = 0;
 
 	mutex_lock(&dev->input_lock);
 
-- 
2.33.1

[PATCH 2/2] media: atomisp-ov2680: properly set the vts value

[Mauro Carvalho Chehab]

The vts value should be set before being checked, as otherwise a
warning will arise:

	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c: In function 'ov2680_set_fmt':
	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:873:33: warning: 'vts' may be used uninitialized
	[-Wmaybe-uninitialized]
	  873 |         if (dev->exposure > vts - OV2680_INTEGRATION_TIME_MARGIN)

Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Hans de Goede <hdegoede@redhat.com>
Fixes: 62b984359b6f ("media: atomisp-ov2680: Fix ov2680_set_fmt() messing up high exposure settings")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 497884d332e1..d24f8830fd94 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -869,11 +869,11 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
 		dev_err(&client->dev,
 			"ov2680 write resolution register err: %d\n", ret);
 
+	vts = dev->res->lines_per_frame;
+
 	/* If necessary increase the VTS to match exposure + MARGIN */
 	if (dev->exposure > vts - OV2680_INTEGRATION_TIME_MARGIN)
 		vts = dev->exposure + OV2680_INTEGRATION_TIME_MARGIN;
-	else
-		vts = dev->res->lines_per_frame;
 
 	ret = ov2680_write_reg(client, 2, OV2680_TIMING_VTS_H, vts);
 	if (ret)
-- 
2.33.1

Re: [PATCH 1/2] media: atomisp-ov2680: initialize return var

[Hans de Goede]

Hi,

On 11/10/21 11:59, Mauro Carvalho Chehab wrote:
> As the settings are only applied when the device is powered on,
> it should return 0 when the device is not powered.
> 
> Not doing that causes a warning:
> 
> 	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c: In function 'ov2680_ioctl':
> 	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:390:16: warning: 'ret' may be used uninitialized in this
> 	function [-Wmaybe-uninitialized]
> 	  390 |         return ov2680_set_exposure(sd, coarse_itg, analog_gain, digital_gain);
> 	      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 	drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:359:13: note: 'ret' was declared here
> 	  359 |         int ret;
> 	      |             ^~~
> 
> Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Fixes: 6b5b60687ada ("media: atomisp-ov2680: Save/restore exposure and gain over sensor power-down")
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Thanks, the series looks good to me (weird that my compiler version
did not complain):

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

For the series.

Regards,

Hans

> ---
>  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index 34d008236bd9..497884d332e1 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -356,7 +356,7 @@ static int ov2680_set_exposure(struct v4l2_subdev *sd, int exposure,
>  			       int gain, int digitgain)
>  {
>  	struct ov2680_device *dev = to_ov2680_sensor(sd);
> -	int ret;
> +	int ret = 0;
>  
>  	mutex_lock(&dev->input_lock);
>  
>