staging: comedi: usbduxfast: chanlist check is Step 5 of (*do_cmdtest)
authorH Hartley Sweeten <hsweeten@visionengravers.com>
Mon, 24 Aug 2015 17:13:54 +0000 (10:13 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 13 Sep 2015 01:24:23 +0000 (18:24 -0700)
The channel list should be checked in Step 5 of the (*do_cmdtest) not
as part of the (*do_cmd). Factor the check out of usbduxfast_ai_cmd().

Tidy up the factored out code. The channel number 'i' will never be
greater than NUMCHANNELS due to the subdevice setup and the checks
done in the code. The up/down of the semaphore is also not needed
because the (*do_cmdtest) never actually tries to access the hardware.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/staging/comedi/drivers/usbduxfast.c

index d90dc59982be6e146d412ac07fa369fcc42f9de1..16b28faa2ec2d29c499ca632ab1c0e06f330e84f 100644 (file)
@@ -332,6 +332,31 @@ static int usbduxfast_submit_urb(struct comedi_device *dev)
        return 0;
 }
 
+static int usbduxfast_ai_check_chanlist(struct comedi_device *dev,
+                                       struct comedi_subdevice *s,
+                                       struct comedi_cmd *cmd)
+{
+       unsigned int gain0 = CR_RANGE(cmd->chanlist[0]);
+       int i;
+
+       for (i = 0; i < cmd->chanlist_len; ++i) {
+               unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+               unsigned int gain = CR_RANGE(cmd->chanlist[i]);
+
+               if (chan != i) {
+                       dev_err(dev->class_dev,
+                               "channels are not consecutive\n");
+                       return -EINVAL;
+               }
+               if (gain != gain0 && cmd->chanlist_len > 3) {
+                       dev_err(dev->class_dev,
+                               "gain must be the same for all channels\n");
+                       return -EINVAL;
+               }
+       }
+       return 0;
+}
+
 static int usbduxfast_ai_cmdtest(struct comedi_device *dev,
                                 struct comedi_subdevice *s,
                                 struct comedi_cmd *cmd)
@@ -417,7 +442,13 @@ static int usbduxfast_ai_cmdtest(struct comedi_device *dev,
        if (err)
                return 3;
 
-       /* step 4: fix up any arguments */
+       /* Step 4: fix up any arguments */
+
+       /* Step 5: check channel list if it exists */
+       if (cmd->chanlist && cmd->chanlist_len > 0)
+               err |= usbduxfast_ai_check_chanlist(dev, s, cmd);
+       if (err)
+               return 5;
 
        return 0;
 }
@@ -460,8 +491,8 @@ static int usbduxfast_ai_cmd(struct comedi_device *dev,
 {
        struct usbduxfast_private *devpriv = dev->private;
        struct comedi_cmd *cmd = &s->async->cmd;
-       unsigned int chan, gain, rngmask = 0xff;
-       int i, j, ret;
+       unsigned int rngmask = 0xff;
+       int j, ret;
        int result;
        long steps, steps_tmp;
 
@@ -481,27 +512,6 @@ static int usbduxfast_ai_cmd(struct comedi_device *dev,
         */
        devpriv->ignore = PACKETS_TO_IGNORE;
 
-       gain = CR_RANGE(cmd->chanlist[0]);
-       for (i = 0; i < cmd->chanlist_len; ++i) {
-               chan = CR_CHAN(cmd->chanlist[i]);
-               if (chan != i) {
-                       dev_err(dev->class_dev,
-                               "channels are not consecutive\n");
-                       up(&devpriv->sem);
-                       return -EINVAL;
-               }
-               if ((gain != CR_RANGE(cmd->chanlist[i]))
-                       && (cmd->chanlist_len > 3)) {
-                       dev_err(dev->class_dev,
-                               "gain must be the same for all channels\n");
-                       up(&devpriv->sem);
-                       return -EINVAL;
-               }
-               if (i >= NUMCHANNELS) {
-                       dev_err(dev->class_dev, "chanlist too long\n");
-                       break;
-               }
-       }
        steps = 0;
        if (cmd->convert_src == TRIG_TIMER)
                steps = (cmd->convert_arg * 30) / 1000;