Skip to contentSkip to conversation

Add driver for Samsung S5K5BAF camera sensor

media Dan Carpenter 2 participants 3 messages Jan 8, 2014 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

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

  1044                  goto out;
  1045  
  1046          if (on) {

regards,
dan carpenter

#1 lore raw
Quoted 18 lines · Dan Carpenter

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)

Quoted 2 lines
                    ^^^^^^^^^^^^^^^^^^^

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.

Regards
Andrzej

Quoted 8 lines
  1044                  goto out;
  1045  
  1046          if (on) {

regards,
dan carpenter

#2 lore raw
Quoted 22 lines · Andrzej Hajda
Quoted 18 lines · Dan Carpenter

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)

Quoted 2 lines
                    ^^^^^^^^^^^^^^^^^^^

This would be cleaner if it were "if (on == state->power)"

Quoted 4 lines

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