Skip to contentSkip to conversation

staging: comedi: das16: remove a duplicate condition

PATCH In review Dan Carpenter 4 participants 5 messages Jul 29, 2015 – Jul 30, 2015

Subsystems (2)
Staging Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 staging: comedi: das16: remove a duplicate condition

We checked that "it->options[3]" was non-zero on the line before so there is no need to check again.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/das16.c +1 -2
index d7cf4b1..056bca9 100644
@@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 
 	/*  check that clock setting is valid */
 	if (it->options[3]) {
-		if (it->options[3] != 0 &&
-		    it->options[3] != 1 && it->options[3] != 10) {
+		if (it->options[3] != 1 && it->options[3] != 10) {
 			dev_err(dev->class_dev,
 				"Invalid option. Master clock must be set to 1 or 10 (MHz)\n");
 			return -EINVAL;
Reviewed-by H Hartley Sweeten on v1 1/1
#1 lore raw
Quoted 4 lines · Dan Carpenter

We checked that "it->options[3]" was non-zero on the line before so there is no need to check again.

Signed-off-by Dan Carpenter <[email protected]>

v

Quoted 5 lines
drivers/staging/comedi/drivers/das16.c +0 -0
index d7cf4b1..056bca9 100644

drivers/staging/comedi/drivers/das16.c static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it) · line 1038

Quoted 9 lines
drivers/staging/comedi/drivers/das16.c +1 -2
@@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 
 	/*  check that clock setting is valid */
 	if (it->options[3]) {
-		if (it->options[3] != 0 &&
-		    it->options[3] != 1 && it->options[3] != 10) {
+		if (it->options[3] != 1 && it->options[3] != 10) {
 			dev_err(dev->class_dev,
 				"Invalid option. Master clock must be set to 1 or 10 (MHz)\n");
 			return -EINVAL;

Thanks,

Reviewed-by H Hartley Sweeten <[email protected]>
#2 lore raw
In reply to Dan Carpenter

Am 29.07.2015 23:36, schrieb Dan Carpenter:

Quoted 10 lines

We checked that "it->options[3]" was non-zero on the line before so there is no need to check again.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/das16.c +0 -0
index d7cf4b1..056bca9 100644

drivers/staging/comedi/drivers/das16.c static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it) · line 1038

Quoted 9 lines
drivers/staging/comedi/drivers/das16.c +1 -2
@@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 
 	/*  check that clock setting is valid */
 	if (it->options[3]) {
-		if (it->options[3] != 0 &&
-		    it->options[3] != 1 && it->options[3] != 10) {
+		if (it->options[3] != 1 && it->options[3] != 10) {
 			dev_err(dev->class_dev,
 				"Invalid option. Master clock must be set to 1 or 10 (MHz)\n");
 			return -EINVAL;

mmh, acording to the error messages 1 or 10 is allowed, obviously 0 is/was also valid. I would suggest to put that into one if () to make things more obvious. It 0 is also valid a minor tweak to the error msg would be nice.

re,
wh

author
#3 lore raw
In reply to Dan Carpenter
Quoted 14 lines · walter harms

Am 29.07.2015 23:36, schrieb Dan Carpenter:

Quoted 11 lines

We checked that "it->options[3]" was non-zero on the line before so there is no need to check again.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/das16.c +0 -0
index d7cf4b1..056bca9 100644
@@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it)
Quoted 12 lines

drivers/staging/comedi/drivers/das16.c static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it) · line 1038

Quoted 8 lines
drivers/staging/comedi/drivers/das16.c +1 -2
 	/*  check that clock setting is valid */
 	if (it->options[3]) {
-		if (it->options[3] != 0 &&
-		    it->options[3] != 1 && it->options[3] != 10) {
+		if (it->options[3] != 1 && it->options[3] != 10) {
 			dev_err(dev->class_dev,
 				"Invalid option. Master clock must be set to 1 or 10 (MHz)\n");
 			return -EINVAL;

mmh, acording to the error messages 1 or 10 is allowed, obviously 0 is/was also valid. I would suggest to put that into one if () to make things more obvious. It 0 is also valid a minor tweak to the error msg would be nice.

Zero values are not used. If you are going to set it then it has to be 1 or 10. The message makes sense and matches the logic of the code.

regards,
dan carpenter

Reviewed-by on v1 1/1
#4 lore raw
In reply to Dan Carpenter
Quoted 11 lines · Dan Carpenter

We checked that "it->options[3]" was non-zero on the line before so there is no need to check again.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/das16.c +0 -0
index d7cf4b1..056bca9 100644

drivers/staging/comedi/drivers/das16.c static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it) · line 1038

Quoted 9 lines
drivers/staging/comedi/drivers/das16.c +1 -2
@@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 
  	/*  check that clock setting is valid */
  	if (it->options[3]) {
-		if (it->options[3] != 0 &&
-		    it->options[3] != 1 && it->options[3] != 10) {
+		if (it->options[3] != 1 && it->options[3] != 10) {
  			dev_err(dev->class_dev,
  				"Invalid option. Master clock must be set to 1 or 10 (MHz)\n");
  			return -EINVAL;
Reviewed-by Ian Abbott <[email protected]>
Signature · 2 lines
-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]> )=-
-=(                          Web: http://www.mev.co.uk/  )=-