| Message ID | 81245e1273f2e0e96a520b9d00cd415f65d37b48.1636672052.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some W=1 clang Werror at staging/media | expand |
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 > >
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
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 >
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)
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(-)