On Wed, Jan 08, 2014 at 12:58:35PM +0100, Andrzej Hajda wrote:
> On 01/08/2014 10:58 AM, Dan Carpenter wrote:
> > Hello Andrzej Hajda,
> >
> > The patch 7d459937dc09: "[media] Add driver for Samsung S5K5BAF
> > camera sensor" from Dec 5, 2013, leads to the following
> > static checker warning:
> >
> > drivers/media/i2c/s5k5baf.c:1043 s5k5baf_set_power()
> > warn: add some parenthesis here?
> >
> > drivers/media/i2c/s5k5baf.c
> > 1036 static int s5k5baf_set_power(struct v4l2_subdev *sd, int on)
> > 1037 {
> > 1038 struct s5k5baf *state = to_s5k5baf(sd);
> > 1039 int ret = 0;
> > 1040
> > 1041 mutex_lock(&state->lock);
> > 1042
> > 1043 if (!on != state->power)
> > ^^^^^^^^^^^^^^^^^^^
> > This would be cleaner if it were "if (on == state->power)"
>
> This version works correctly only for 'on' equal 0 and 1, my version
> works for all ints. On the other side documentation says only 0 and 1 is
> allowed for s_power callbacks :)
> I would stay with my version, similar approach is in other drivers.
Even "if (!!on == state->power)" like you do in s5k5baf_s_stream() would
be more readable than the current code.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
