Skip to contentSkip to conversation

staging: comedi: pcl812: missing break statement

PATCH In review Dan Carpenter 2 participants 4 messages Aug 21, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: comedi: pcl812: missing break statement

Smatch complains that there is a missing break statement here.

---
This looks correct to me, but please double check.

drivers/staging/comedi/drivers/pcl812.c +1 -0
index 35251e7..03a0989 100644
@@ -1404,6 +1404,7 @@ no_dma:
 		if (it->options[3] > 0)
 						/*  we use external trigger */
 			devpriv->use_ext_trg = 1;
+		break;
 	case boardA821:
 		devpriv->max_812_ai_mode0_rangewait = 1;
 		devpriv->mode_reg_int = (irq << 4) & 0xf0;
#1 lore raw
Quoted 12 lines · Dan Carpenter

Smatch complains that there is a missing break statement here.

---
This looks correct to me, but please double check.

drivers/staging/comedi/drivers/pcl812.c +0 -0
index 35251e7..03a0989 100644

drivers/staging/comedi/drivers/pcl812.c no_dma: · line 1410

Quoted 7 lines
drivers/staging/comedi/drivers/pcl812.c +1 -0
@@ -1404,6 +1404,7 @@ no_dma:
  		if (it->options[3] > 0)
  						/*  we use external trigger */
  			devpriv->use_ext_trg = 1;
+		break;
  	case boardA821:
  		devpriv->max_812_ai_mode0_rangewait = 1;
  		devpriv->mode_reg_int = (irq << 4) & 0xf0;

The missing break might be deliberate; I'll need to track down the manuals for the affected cards. case boardACL8216 in the same switch also has no break.

Signature · 2 lines
-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
Acked-by Reviewed-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter
Quoted 16 lines · Ian Abbott
Quoted 15 lines · Dan Carpenter

Smatch complains that there is a missing break statement here.

---
This looks correct to me, but please double check.

drivers/staging/comedi/drivers/pcl812.c +1 -0
index 35251e7..03a0989 100644
@@ -1404,6 +1404,7 @@ no_dma:
   		if (it->options[3] > 0)
   						/*  we use external trigger */
   			devpriv->use_ext_trg = 1;
+		break;
Quoted 8 lines

drivers/staging/comedi/drivers/pcl812.c no_dma: · line 1410

Quoted 4 lines
drivers/staging/comedi/drivers/pcl812.c +0 -0
   	case boardA821:
   		devpriv->max_812_ai_mode0_rangewait = 1;
   		devpriv->mode_reg_int = (irq << 4) & 0xf0;

The missing break might be deliberate; I'll need to track down the manuals for the affected cards. case boardACL8216 in the same switch also has no break.

After examining the manuals for the affected boards (the formatting of the manual for PCL-812 and PCL-812PG is dreadful!), the 'break' before 'case boardA821' does seem to be missing, although the bug is harmless as it only affects some "don't care" bits in a hardware register (PCL812_MODE at offset 11) for the cases that fall through to here, and they assign devpriv->max_812_ai_mode0_rangewait to the same value (1).

The other fall through from 'case boardACL8216' to 'case boardPCL812PG' in this switch seems to be deliberate.

(whichever is appropriate)

Signature · 2 lines
-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
#3 lore raw
In reply to Dan Carpenter
Quoted 5 lines · Ian Abbott

(whichever is appropriate)

To me Acked-by is more emphatic. Since you're the maintainer it's appropriate. I normally use "Reviewed-by" because I don't have the authority to approve anything. Also if it's a bug fix that I approve of or if I previously complained about the patch then I'll use Acked-by.

I don't know how other people do it. :P

regards,
dan carpenter