Hi Dan,
On Thu, Mar 01, 2018 at 12:59:54PM +0300, Dan Carpenter wrote:
> [ I know you're just copying files, but you might have a fix for these
> since you're looking at the code. - dan ]
According to the static analyzer I should simply substitute all those
expressions with 0s. I would instead keep them for sake of readability
and accordance with register description in the video decoder manual.
Thanks
j
>
> Hello Jacopo Mondi,
>
> The patch e0d76c3842ee: "media: i2c: Copy tw9910 soc_camera sensor
> driver" from Feb 21, 2018, leads to the following static checker
> warning:
>
> drivers/media/i2c/tw9910.c:395 tw9910_set_hsync()
> warn: odd binop '0x260 & 0x7'
>
> drivers/media/i2c/tw9910.c:396 tw9910_set_hsync()
> warn: odd binop '0x300 & 0x7'
>
> drivers/media/i2c/tw9910.c:538 tw9910_s_std()
> warn: odd binop '0x0 & 0xc'
>
> drivers/media/i2c/tw9910.c
> 374 static int tw9910_set_hsync(struct i2c_client *client)
> 375 {
> 376 struct tw9910_priv *priv = to_tw9910(client);
> 377 int ret;
> 378
> 379 /* bit 10 - 3 */
> 380 ret = i2c_smbus_write_byte_data(client, HSBEGIN,
> 381 (HSYNC_START & 0x07F8) >> 3);
> 382 if (ret < 0)
> 383 return ret;
> 384
> 385 /* bit 10 - 3 */
> 386 ret = i2c_smbus_write_byte_data(client, HSEND,
> 387 (HSYNC_END & 0x07F8) >> 3);
> 388 if (ret < 0)
> 389 return ret;
> 390
> 391 /* So far only revisions 0 and 1 have been seen */
> 392 /* bit 2 - 0 */
> 393 if (priv->revision == 1)
> 394 ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
> 395 (HSYNC_START & 0x0007) << 4 |
> ^^^^^^^^^^^^^^^^^^^^
> 396 (HSYNC_END & 0x0007));
> ^^^^^^^^^^^^^^^^^^^^
> These always mask to zero.
>
> 397
> 398 return ret;
> 399 }
>
> [ snip ]
>
> 511 static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
> 512 {
> 513 struct i2c_client *client = v4l2_get_subdevdata(sd);
> 514 struct tw9910_priv *priv = to_tw9910(client);
> 515 const unsigned int hact = 720;
> 516 const unsigned int hdelay = 15;
> ^^^^^^^^^^^
> 517 unsigned int vact;
> 518 unsigned int vdelay;
> 519 int ret;
> 520
> 521 if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
> 522 return -EINVAL;
> 523
> 524 priv->norm = norm;
> 525 if (norm & V4L2_STD_525_60) {
> 526 vact = 240;
> 527 vdelay = 18;
> 528 ret = tw9910_mask_set(client, VVBI, 0x10, 0x10);
> 529 } else {
> 530 vact = 288;
> 531 vdelay = 24;
> 532 ret = tw9910_mask_set(client, VVBI, 0x10, 0x00);
> 533 }
> 534 if (!ret)
> 535 ret = i2c_smbus_write_byte_data(client, CROP_HI,
> 536 ((vdelay >> 2) & 0xc0) |
> 537 ((vact >> 4) & 0x30) |
> 538 ((hdelay >> 6) & 0x0c) |
> ^^^^^^^^^^^
> 15 >> 6 is zero.
>
> 539 ((hact >> 8) & 0x03));
> 540 if (!ret)
> 541 ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
> 542 vdelay & 0xff);
> 543 if (!ret)
> 544 ret = i2c_smbus_write_byte_data(client, VACTIVE_LO,
> 545 vact & 0xff);
> 546
> 547 return ret;
> 548 }
>
> regards,
> dan carpenter
