[PATCH 0/3] Fix some W=1 clang Werror at staging/media

[PATCH 0/3] Fix some W=1 clang Werror at staging/media

[Mauro Carvalho Chehab]

Currently, there are three errors produced when building with clang
with CONFIG_WERROR and W=1. Address them.

Mauro Carvalho Chehab (3):
  media: ipu3: drop an unused variable
  media: atomisp-gc2235: drop an unused var
  media: atomisp: handle errors at sh_css_create_isp_params()

 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 8 +++++---
 drivers/staging/media/atomisp/pci/sh_css_params.c  | 4 ++++
 drivers/staging/media/ipu3/ipu3-css-params.c       | 3 ---
 3 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.33.1

[PATCH 1/3] media: ipu3: drop an unused variable

[Mauro Carvalho Chehab]

Fix this clang Werror with W=1:

	drivers/staging/media/ipu3/ipu3-css-params.c:774:8: error: variable 'pin_scale' set but not used [-Werror,-Wunused-but-set-variable]
	                        int pin_scale = 0;
	                            ^

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/

 drivers/staging/media/ipu3/ipu3-css-params.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
index e9d6bd9e9332..d9e3c3785075 100644
--- a/drivers/staging/media/ipu3/ipu3-css-params.c
+++ b/drivers/staging/media/ipu3/ipu3-css-params.c
@@ -771,7 +771,6 @@ static int imgu_css_osys_calc_frame_and_stripe_params(
 		 */
 		{
 			unsigned int i;
-			int pin_scale = 0;
 			/*Input resolution */
 
 			stripe_params[s].input_width = stripe_input_width_y;
@@ -791,8 +790,6 @@ static int imgu_css_osys_calc_frame_and_stripe_params(
 						reso.pin_height[i];
 					stripe_params[s].output_offset[i] =
 						stripe_offset_out_y;
-
-					pin_scale += frame_params[i].scaled;
 				} else {
 					/* Unscaled pin */
 					stripe_params[s].output_width[i] =
-- 
2.33.1

[PATCH 2/3] media: atomisp-gc2235: drop an unused var

[Mauro Carvalho Chehab]

Fix this clang Werror with W=1:

drivers/staging/media/atomisp/i2c/atomisp-gc2235.c:573:6: error: variable 'ret' set but not used [-Werror,-Wunused-but-set-variable]
        int ret = -1;
            ^

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 5e7085264189..0e6b2e6100d1 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -570,14 +570,16 @@ static int power_ctrl(struct v4l2_subdev *sd, bool flag)
 static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 {
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
-	int ret = -1;
+	int ret;
 
 	if (!dev || !dev->platform_data)
 		return -ENODEV;
 
-	ret |= dev->platform_data->gpio1_ctrl(sd, !flag);
+	ret = dev->platform_data->gpio1_ctrl(sd, !flag);
 	usleep_range(60, 90);
-	return dev->platform_data->gpio0_ctrl(sd, flag);
+	ret |= dev->platform_data->gpio0_ctrl(sd, flag);
+
+	return ret;
 }
 
 static int power_up(struct v4l2_subdev *sd)
-- 
2.33.1

[PATCH 3/3] media: atomisp: handle errors at sh_css_create_isp_params()

[Mauro Carvalho Chehab]

The succ var tracks memory allocation erros on this function.

Fix it, in order to stop this W=1 Werror in clang:

drivers/staging/media/atomisp/pci/sh_css_params.c:2430:7: error: variable 'succ' set but not used [-Werror,-Wunused-but-set-variable]
        bool succ = true;
             ^

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/pci/sh_css_params.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index dbd3bfe3d343..3cc5b82a8a31 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -2473,6 +2473,10 @@ sh_css_create_isp_params(struct ia_css_stream *stream,
 	succ &= (ddr_ptrs->macc_tbl != mmgr_NULL);
 
 	*isp_params_out = params;
+
+	if (!succ)
+		return -ENOMEM;
+
 	return err;
 }
 
-- 
2.33.1

Re: [PATCH 1/3] media: ipu3: drop an unused variable

[Nathan Chancellor]

On Thu, Nov 11, 2021 at 11:08:51PM +0000, Mauro Carvalho Chehab wrote:
> Fix this clang Werror with W=1:
> 
> 	drivers/staging/media/ipu3/ipu3-css-params.c:774:8: error: variable 'pin_scale' set but not used [-Werror,-Wunused-but-set-variable]
> 	                        int pin_scale = 0;
> 	                            ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/
> 
>  drivers/staging/media/ipu3/ipu3-css-params.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> index e9d6bd9e9332..d9e3c3785075 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> @@ -771,7 +771,6 @@ static int imgu_css_osys_calc_frame_and_stripe_params(
>  		 */
>  		{
>  			unsigned int i;
> -			int pin_scale = 0;
>  			/*Input resolution */
>  
>  			stripe_params[s].input_width = stripe_input_width_y;
> @@ -791,8 +790,6 @@ static int imgu_css_osys_calc_frame_and_stripe_params(
>  						reso.pin_height[i];
>  					stripe_params[s].output_offset[i] =
>  						stripe_offset_out_y;
> -
> -					pin_scale += frame_params[i].scaled;
>  				} else {
>  					/* Unscaled pin */
>  					stripe_params[s].output_width[i] =
> -- 
> 2.33.1
> 
> 

Re: [PATCH 2/3] media: atomisp-gc2235: drop an unused var

[Nathan Chancellor]

On Thu, Nov 11, 2021 at 11:08:52PM +0000, Mauro Carvalho Chehab wrote:
> Fix this clang Werror with W=1:
> 
> drivers/staging/media/atomisp/i2c/atomisp-gc2235.c:573:6: error: variable 'ret' set but not used [-Werror,-Wunused-but-set-variable]
>         int ret = -1;
>             ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/
> 
>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index 5e7085264189..0e6b2e6100d1 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -570,14 +570,16 @@ static int power_ctrl(struct v4l2_subdev *sd, bool flag)
>  static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>  {
>  	struct gc2235_device *dev = to_gc2235_sensor(sd);
> -	int ret = -1;
> +	int ret;
>  
>  	if (!dev || !dev->platform_data)
>  		return -ENODEV;
>  
> -	ret |= dev->platform_data->gpio1_ctrl(sd, !flag);
> +	ret = dev->platform_data->gpio1_ctrl(sd, !flag);
>  	usleep_range(60, 90);
> -	return dev->platform_data->gpio0_ctrl(sd, flag);
> +	ret |= dev->platform_data->gpio0_ctrl(sd, flag);
> +
> +	return ret;
>  }
>  
>  static int power_up(struct v4l2_subdev *sd)
> -- 
> 2.33.1
> 
> 

Re: [PATCH 3/3] media: atomisp: handle errors at sh_css_create_isp_params()

[Nathan Chancellor]

On Thu, Nov 11, 2021 at 11:08:53PM +0000, Mauro Carvalho Chehab wrote:
> The succ var tracks memory allocation erros on this function.
> 
> Fix it, in order to stop this W=1 Werror in clang:
> 
> drivers/staging/media/atomisp/pci/sh_css_params.c:2430:7: error: variable 'succ' set but not used [-Werror,-Wunused-but-set-variable]
>         bool succ = true;
>              ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
> To mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/
> 
>  drivers/staging/media/atomisp/pci/sh_css_params.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index dbd3bfe3d343..3cc5b82a8a31 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -2473,6 +2473,10 @@ sh_css_create_isp_params(struct ia_css_stream *stream,
>  	succ &= (ddr_ptrs->macc_tbl != mmgr_NULL);
>  
>  	*isp_params_out = params;
> +
> +	if (!succ)
> +		return -ENOMEM;
> +
>  	return err;
>  }
>  
> -- 
> 2.33.1
> 
> 

Re: [PATCH 2/3] media: atomisp-gc2235: drop an unused var

[Dan Carpenter]

On Thu, Nov 11, 2021 at 11:08:52PM +0000, Mauro Carvalho Chehab wrote:
> Fix this clang Werror with W=1:
> 
> drivers/staging/media/atomisp/i2c/atomisp-gc2235.c:573:6: error: variable 'ret' set but not used [-Werror,-Wunused-but-set-variable]
>         int ret = -1;
>             ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> 

Looks good.

There should be warning for "int x = -1;" followed by |= because that's
a no-op OR assignment.

I'm surprised that clang doesn't print errors about the other
implementations of power_ctrl().

drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
   448  static int power_ctrl(struct v4l2_subdev *sd, bool flag)
   449  {
   450          int ret;
   451          struct mt9m114_device *dev = to_mt9m114_sensor(sd);
   452  
   453          if (!dev || !dev->platform_data)
   454                  return -ENODEV;
   455  
   456          if (flag) {
   457                  ret = dev->platform_data->v2p8_ctrl(sd, 1);
   458                  if (ret == 0) {
   459                          ret = dev->platform_data->v1p8_ctrl(sd, 1);
   460                          if (ret)
   461                                  ret = dev->platform_data->v2p8_ctrl(sd, 0);
   462                  }
   463          } else {
   464                  ret = dev->platform_data->v2p8_ctrl(sd, 0);
   465                  ret = dev->platform_data->v1p8_ctrl(sd, 0);
                        ^^^^^^

   466          }
   467          return ret;
   468  }
   469  
   470  static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
   471  {
   472          int ret;
   473          struct mt9m114_device *dev = to_mt9m114_sensor(sd);
   474  
   475          if (!dev || !dev->platform_data)
   476                  return -ENODEV;
   477  
   478          /*
   479           * Note: current modules wire only one GPIO signal (RESET#),
   480           * but the schematic wires up two to the connector.  BIOS
   481           * versions have been unfortunately inconsistent with which
   482           * ACPI index RESET# is on, so hit both
   483           */
   484  
   485          if (flag) {
   486                  ret = dev->platform_data->gpio0_ctrl(sd, 0);
   487                  ret = dev->platform_data->gpio1_ctrl(sd, 0);
                        ^^^^^^^

   488                  msleep(60);
   489                  ret |= dev->platform_data->gpio0_ctrl(sd, 1);
   490                  ret |= dev->platform_data->gpio1_ctrl(sd, 1);
   491          } else {

regards,
dan carpenter

Re: [PATCH 3/3] media: atomisp: handle errors at sh_css_create_isp_params()

[Dan Carpenter]

On Thu, Nov 11, 2021 at 11:08:53PM +0000, Mauro Carvalho Chehab wrote:
> The succ var tracks memory allocation erros on this function.
> 
> Fix it, in order to stop this W=1 Werror in clang:
> 
> drivers/staging/media/atomisp/pci/sh_css_params.c:2430:7: error: variable 'succ' set but not used [-Werror,-Wunused-but-set-variable]
>         bool succ = true;
>              ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> 
> To mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636672052.git.mchehab+huawei@kernel.org/
> 
>  drivers/staging/media/atomisp/pci/sh_css_params.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index dbd3bfe3d343..3cc5b82a8a31 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -2473,6 +2473,10 @@ sh_css_create_isp_params(struct ia_css_stream *stream,
>  	succ &= (ddr_ptrs->macc_tbl != mmgr_NULL);
>  
>  	*isp_params_out = params;
> +
> +	if (!succ)
> +		return -ENOMEM;
> +
>  	return err;

Better to "return 0;"


regards,
dan carpenter

Re: [PATCH 2/3] media: atomisp-gc2235: drop an unused var

[Nick Desaulniers]

On Mon, Nov 15, 2021 at 1:29 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, Nov 11, 2021 at 11:08:52PM +0000, Mauro Carvalho Chehab wrote:
> > Fix this clang Werror with W=1:
> >
> > drivers/staging/media/atomisp/i2c/atomisp-gc2235.c:573:6: error: variable 'ret' set but not used [-Werror,-Wunused-but-set-variable]
> >         int ret = -1;
> >             ^
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >
>
> Looks good.
>
> There should be warning for "int x = -1;" followed by |= because that's
> a no-op OR assignment.
>
> I'm surprised that clang doesn't print errors about the other
> implementations of power_ctrl().

clang-tidy and scan-build will report dead stores.  You may have seen
patches from reports from Abaci Robot which is running those checks
continuously.

>
> drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
>    448  static int power_ctrl(struct v4l2_subdev *sd, bool flag)
>    449  {
>    450          int ret;
>    451          struct mt9m114_device *dev = to_mt9m114_sensor(sd);
>    452
>    453          if (!dev || !dev->platform_data)
>    454                  return -ENODEV;
>    455
>    456          if (flag) {
>    457                  ret = dev->platform_data->v2p8_ctrl(sd, 1);
>    458                  if (ret == 0) {
>    459                          ret = dev->platform_data->v1p8_ctrl(sd, 1);
>    460                          if (ret)
>    461                                  ret = dev->platform_data->v2p8_ctrl(sd, 0);
>    462                  }
>    463          } else {
>    464                  ret = dev->platform_data->v2p8_ctrl(sd, 0);
>    465                  ret = dev->platform_data->v1p8_ctrl(sd, 0);
>                         ^^^^^^
>
>    466          }
>    467          return ret;
>    468  }
>    469
>    470  static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>    471  {
>    472          int ret;
>    473          struct mt9m114_device *dev = to_mt9m114_sensor(sd);
>    474
>    475          if (!dev || !dev->platform_data)
>    476                  return -ENODEV;
>    477
>    478          /*
>    479           * Note: current modules wire only one GPIO signal (RESET#),
>    480           * but the schematic wires up two to the connector.  BIOS
>    481           * versions have been unfortunately inconsistent with which
>    482           * ACPI index RESET# is on, so hit both
>    483           */
>    484
>    485          if (flag) {
>    486                  ret = dev->platform_data->gpio0_ctrl(sd, 0);
>    487                  ret = dev->platform_data->gpio1_ctrl(sd, 0);
>                         ^^^^^^^
>
>    488                  msleep(60);
>    489                  ret |= dev->platform_data->gpio0_ctrl(sd, 1);
>    490                  ret |= dev->platform_data->gpio1_ctrl(sd, 1);
>    491          } else {
>
> regards,
> dan carpenter
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/3] media: ipu3: drop an unused variable

[Sakari Ailus]

On Thu, Nov 11, 2021 at 11:08:51PM +0000, Mauro Carvalho Chehab wrote:
> Fix this clang Werror with W=1:
> 
> 	drivers/staging/media/ipu3/ipu3-css-params.c:774:8: error: variable 'pin_scale' set but not used [-Werror,-Wunused-but-set-variable]
> 	                        int pin_scale = 0;
> 	                            ^
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Thanks!

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

I.e. feel free to merge through your tree, I don't think there's a chance
for conflicts here.

-- 
Sakari Ailus