All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] media: atomisp-ov2680: initialize return var
@ 2021-11-10 10:59 ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-10 10:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging, Hans Verkuil

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 1/2] media: atomisp-ov2680: initialize return var
@ 2021-11-10 10:59 ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-10 10:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging, Hans Verkuil

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] media: atomisp-ov2680: properly set the vts value
  2021-11-10 10:59 ` Mauro Carvalho Chehab
@ 2021-11-10 10:59   ` Mauro Carvalho Chehab
  -1 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-10 10:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging, Hans Verkuil

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] media: atomisp-ov2680: properly set the vts value
@ 2021-11-10 10:59   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-10 10:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging, Hans Verkuil

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] media: atomisp-ov2680: initialize return var
  2021-11-10 10:59 ` Mauro Carvalho Chehab
  (?)
  (?)
@ 2021-11-10 13:13 ` Hans de Goede
  -1 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-11-10 13:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Deepak R Varma, Greg Kroah-Hartman,
	Hans Verkuil, Mauro Carvalho Chehab, Sakari Ailus,
	Tomi Valkeinen, linux-kernel, linux-media, linux-staging,
	Hans Verkuil

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);
>  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-10 13:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10 10:59 [PATCH 1/2] media: atomisp-ov2680: initialize return var Mauro Carvalho Chehab
2021-11-10 10:59 ` Mauro Carvalho Chehab
2021-11-10 10:59 ` [PATCH 2/2] media: atomisp-ov2680: properly set the vts value Mauro Carvalho Chehab
2021-11-10 10:59   ` Mauro Carvalho Chehab
2021-11-10 13:13 ` [PATCH 1/2] media: atomisp-ov2680: initialize return var Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.