Re: [PATCH 16/49] staging: comedi: me4000: cleanup ai_check_chanlist()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2014-04-15 18:37, H Hartley Sweeten wrote:
This is the Step 5 helper function of the (*do_cmdtest). It validates that
the chanlist, chan/range/aref/etc, is compatible with the actual hardware.
At this point in the (*do_cmdtest) the chanlist is valid, due to checks in
the core, so any sanity checks are not needed.

For aesthetics, rename this function so it has namespace associated with
the driver and move it closer to the (*do_cmdtest) function.

Tidy it up to function.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  drivers/staging/comedi/drivers/me4000.c | 96 ++++++++++++++-------------------
  1 file changed, 40 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c
index 2657e5c..717965b 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -598,58 +598,6 @@ static int me4000_ai_cancel(struct comedi_device *dev,
  	return 0;
  }

-static int ai_check_chanlist(struct comedi_device *dev,
-			     struct comedi_subdevice *s, struct comedi_cmd *cmd)
-{
-	const struct me4000_board *thisboard = comedi_board(dev);
-	int aref;
-	int i;
-
-	/* Check whether aref is equal for all entries */
-	aref = CR_AREF(cmd->chanlist[0]);
-	for (i = 0; i < cmd->chanlist_len; i++) {
-		if (CR_AREF(cmd->chanlist[i]) != aref) {
-			dev_err(dev->class_dev,
-				"Mode is not equal for all entries\n");
-			return -EINVAL;
-		}
-	}
-
-	/* Check whether channels are available for this ending */
-	if (aref == SDF_DIFF) {
-		for (i = 0; i < cmd->chanlist_len; i++) {
-			if (CR_CHAN(cmd->chanlist[i]) >=
-			    thisboard->ai_diff_nchan) {
-				dev_err(dev->class_dev,
-					"Channel number to high\n");
-				return -EINVAL;
-			}
-		}
-	} else {
-		for (i = 0; i < cmd->chanlist_len; i++) {
-			if (CR_CHAN(cmd->chanlist[i]) >= thisboard->ai_nchan) {
-				dev_err(dev->class_dev,
-					"Channel number to high\n");
-				return -EINVAL;
-			}
-		}
-	}
-
-	/* Check if bipolar is set for all entries when in differential mode */
-	if (aref == SDF_DIFF) {
-		for (i = 0; i < cmd->chanlist_len; i++) {
-			if (CR_RANGE(cmd->chanlist[i]) != 1 &&
-			    CR_RANGE(cmd->chanlist[i]) != 2) {
-				dev_err(dev->class_dev,
-				       "Bipolar is not selected in differential mode\n");
-				return -EINVAL;
-			}
-		}
-	}
-
-	return 0;
-}
-
  static int ai_round_cmd_args(struct comedi_device *dev,
  			     struct comedi_subdevice *s,
  			     struct comedi_cmd *cmd,
@@ -855,6 +803,41 @@ static int me4000_ai_do_cmd(struct comedi_device *dev,
  	return 0;
  }

+static int me4000_ai_check_chanlist(struct comedi_device *dev,
+				    struct comedi_subdevice *s,
+				    struct comedi_cmd *cmd)
+{
+	const struct me4000_board *board = comedi_board(dev);
+	unsigned int aref0 = CR_AREF(cmd->chanlist[0]);
+	int i;
+
+	for (i = 0; i < cmd->chanlist_len; i++) {
+		unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+		unsigned int range = CR_RANGE(cmd->chanlist[i]);
+		unsigned int aref = CR_AREF(cmd->chanlist[i]);
+
+		if (aref != aref0) {
+			dev_err(dev->class_dev,
+				"Mode is not equal for all entries\n");
+			return -EINVAL;
+		}
+
+		if (aref == SDF_DIFF) {
+			if (chan >= board->ai_diff_nchan) {
+				dev_err(dev->class_dev,
+					"Channel number to high\n");
+				return -EINVAL;
+			}
+			if (!comedi_range_is_bipolar(s, range)) {
+				dev_err(dev->class_dev,
+				       "Bipolar is not selected in differential mode\n");
+				return -EINVAL;
+			}
+		}
+	}
+	return 0;
+}
+
  static int me4000_ai_do_cmd_test(struct comedi_device *dev,
  				 struct comedi_subdevice *s,
  				 struct comedi_cmd *cmd)
@@ -1069,10 +1052,11 @@ static int me4000_ai_do_cmd_test(struct comedi_device *dev,
  	if (err)
  		return 4;

-	/*
-	 * Stage 5. Check the channel list.
-	 */
-	if (ai_check_chanlist(dev, s, cmd))
+	/* Step 5: check channel list */
+
+	err |= me4000_ai_check_chanlist(dev, s, cmd);
+
+	if (err)
  		return 5;

  	return 0;


For patch 02 I mentioned that ai_check_chanlist() should only be called if chanlist is non-NULL and chanlist_len is non-0, so the same applies to the renamed function.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux