[PATCH v2] staging: comedi: ni_mio_common: split out ao arming from ni_ao_inttrig

Spencer E. Olson olsonse at umich.edu
Mon Oct 10 14:14:19 UTC 2016


AO device arming was previously done as a part of ni_ao_inttrig which is
called as a result of the user calling comedi_internal_trigger.  For
start_src == TRIG_EXT, this does not make very much sense since external
triggers should not conceptually need to be software triggered also.  This
patch splits out the arming functionality to allow arming to specifically
and separately be done via the CONFIG_INSN_ARM ioctl command.

In order to provide backwards compatibility, this patch also provides
automatic arming if ni_ao_inttrig is simply called.

Signed-off-by: Spencer E. Olson <olsonse at umich.edu>
---
Changes in v2:
  - reordered ni_ao_arm to avoid forward declaration
  - changed /** comments to basic block comments
  - changed "needs_arming" member of ni_private to ao_needs_arming to be clearer
    that this is only for the ao subdevice.

 drivers/staging/comedi/drivers/ni_mio_common.c | 150 ++++++++++++++++---------
 drivers/staging/comedi/drivers/ni_stc.h        |  14 +++
 2 files changed, 112 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
index ff7640b5..32ca4ca 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -2729,66 +2729,36 @@ static int ni_ao_insn_write(struct comedi_device *dev,
 	return insn->n;
 }
 
-static int ni_ao_insn_config(struct comedi_device *dev,
-			     struct comedi_subdevice *s,
-			     struct comedi_insn *insn, unsigned int *data)
-{
-	const struct ni_board_struct *board = dev->board_ptr;
-	struct ni_private *devpriv = dev->private;
-	unsigned int nbytes;
-
-	switch (data[0]) {
-	case INSN_CONFIG_GET_HARDWARE_BUFFER_SIZE:
-		switch (data[1]) {
-		case COMEDI_OUTPUT:
-			nbytes = comedi_samples_to_bytes(s,
-							 board->ao_fifo_depth);
-			data[2] = 1 + nbytes;
-			if (devpriv->mite)
-				data[2] += devpriv->mite->fifo_size;
-			break;
-		case COMEDI_INPUT:
-			data[2] = 0;
-			break;
-		default:
-			return -EINVAL;
-		}
-		return 0;
-	default:
-		break;
-	}
-
-	return -EINVAL;
-}
-
-static int ni_ao_inttrig(struct comedi_device *dev,
-			 struct comedi_subdevice *s,
-			 unsigned int trig_num)
+/*
+ * Arms the AO device in preparation for a trigger event.
+ * This function also allocates and prepares a DMA channel (or FIFO if DMA is
+ * not used).  As a part of this preparation, this function preloads the DAC
+ * registers with the first values of the output stream.  This ensures that the
+ * first clock cycle after the trigger can be used for output.
+ *
+ * Note that this function _must_ happen after a user has written data to the
+ * output buffers via either mmap or write(fileno,...).
+ */
+static int ni_ao_arm(struct comedi_device *dev,
+		     struct comedi_subdevice *s)
 {
 	struct ni_private *devpriv = dev->private;
-	struct comedi_cmd *cmd = &s->async->cmd;
 	int ret;
 	int interrupt_b_bits;
 	int i;
 	static const int timeout = 1000;
 
 	/*
-	 * Require trig_num == cmd->start_arg when cmd->start_src == TRIG_INT.
-	 * For backwards compatibility, also allow trig_num == 0 when
-	 * cmd->start_src != TRIG_INT (i.e. when cmd->start_src == TRIG_EXT);
-	 * in that case, the internal trigger is being used as a pre-trigger
-	 * before the external trigger.
+	 * Prevent ao from doing things like trying to allocate the ao dma
+	 * channel multiple times.
 	 */
-	if (!(trig_num == cmd->start_arg ||
-	      (trig_num == 0 && cmd->start_src != TRIG_INT)))
+	if (!devpriv->ao_needs_arming) {
+		dev_dbg(dev->class_dev, "%s: device does not need arming!\n",
+			__func__);
 		return -EINVAL;
+	}
 
-	/*
-	 * Null trig at beginning prevent ao start trigger from executing more
-	 * than once per command (and doing things like trying to allocate the
-	 * ao dma channel multiple times).
-	 */
-	s->async->inttrig = NULL;
+	devpriv->ao_needs_arming = 0;
 
 	ni_set_bits(dev, NISTC_INTB_ENA_REG,
 		    NISTC_INTB_ENA_AO_FIFO | NISTC_INTB_ENA_AO_ERR, 0);
@@ -2840,6 +2810,75 @@ static int ni_ao_inttrig(struct comedi_device *dev,
 			   devpriv->ao_cmd1,
 		      NISTC_AO_CMD1_REG);
 
+	return 0;
+}
+
+static int ni_ao_insn_config(struct comedi_device *dev,
+			     struct comedi_subdevice *s,
+			     struct comedi_insn *insn, unsigned int *data)
+{
+	const struct ni_board_struct *board = dev->board_ptr;
+	struct ni_private *devpriv = dev->private;
+	unsigned int nbytes;
+
+	switch (data[0]) {
+	case INSN_CONFIG_GET_HARDWARE_BUFFER_SIZE:
+		switch (data[1]) {
+		case COMEDI_OUTPUT:
+			nbytes = comedi_samples_to_bytes(s,
+							 board->ao_fifo_depth);
+			data[2] = 1 + nbytes;
+			if (devpriv->mite)
+				data[2] += devpriv->mite->fifo_size;
+			break;
+		case COMEDI_INPUT:
+			data[2] = 0;
+			break;
+		default:
+			return -EINVAL;
+		}
+		return 0;
+	case INSN_CONFIG_ARM:
+		return ni_ao_arm(dev, s);
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int ni_ao_inttrig(struct comedi_device *dev,
+			 struct comedi_subdevice *s,
+			 unsigned int trig_num)
+{
+	struct ni_private *devpriv = dev->private;
+	struct comedi_cmd *cmd = &s->async->cmd;
+	int ret;
+
+	/*
+	 * Require trig_num == cmd->start_arg when cmd->start_src == TRIG_INT.
+	 * For backwards compatibility, also allow trig_num == 0 when
+	 * cmd->start_src != TRIG_INT (i.e. when cmd->start_src == TRIG_EXT);
+	 * in that case, the internal trigger is being used as a pre-trigger
+	 * before the external trigger.
+	 */
+	if (!(trig_num == cmd->start_arg ||
+	      (trig_num == 0 && cmd->start_src != TRIG_INT)))
+		return -EINVAL;
+
+	/*
+	 * Null trig at beginning prevent ao start trigger from executing more
+	 * than once per command.
+	 */
+	s->async->inttrig = NULL;
+
+	if (devpriv->ao_needs_arming) {
+		/* only arm this device if it still needs arming */
+		ret = ni_ao_arm(dev, s);
+		if (ret)
+			return ret;
+	}
+
 	ni_stc_writew(dev, NISTC_AO_CMD2_START1_PULSE | devpriv->ao_cmd2,
 		      NISTC_AO_CMD2_REG);
 
@@ -3227,10 +3266,17 @@ static int ni_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	ni_ao_cmd_set_interrupts(dev, s);
 
 	/*
-	 * arm(ing) and star(ting) happen in ni_ao_inttrig, which _must_ be
-	 * called for ao commands since 1) TRIG_NOW is not supported and 2) DMA
-	 * must be setup and initially written to before arm/start happen.
+	 * arm(ing) must happen later so that DMA can be setup and DACs
+	 * preloaded with the actual output buffer before starting.
+	 *
+	 * start(ing) must happen _after_ arming is completed.  Starting can be
+	 * done either via ni_ao_inttrig, or via an external trigger.
+	 *
+	 * **Currently, ni_ao_inttrig will automatically attempt a call to
+	 * ni_ao_arm if the device still needs arming at that point.  This
+	 * allows backwards compatibility.
 	 */
+	devpriv->ao_needs_arming = 1;
 	return 0;
 }
 
diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
index 1966519..f27b545 100644
--- a/drivers/staging/comedi/drivers/ni_stc.h
+++ b/drivers/staging/comedi/drivers/ni_stc.h
@@ -1053,6 +1053,20 @@ struct ni_private {
 	unsigned int is_67xx:1;
 	unsigned int is_6711:1;
 	unsigned int is_6713:1;
+
+	/*
+	 * Boolean value of whether device needs to be armed.
+	 *
+	 * Currently, only NI AO devices are known to be needing arming, since
+	 * the DAC registers must be preloaded before triggering.
+	 * This variable should only be set true during a command operation
+	 * (e.g ni_ao_cmd) and should then be set false by the arming
+	 * function (e.g. ni_ao_arm).
+	 *
+	 * This variable helps to ensure that multiple DMA allocations are not
+	 * possible.
+	 */
+	unsigned int ao_needs_arming:1;
 };
 
 static const struct comedi_lrange range_ni_E_ao_ext;
-- 
1.9.1



More information about the devel mailing list