Re: [PATCH 2/3] staging: comedi: ni_mio_comon: adds finite regeneration to AO output

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

 



On 12/01/16 08:17, Spencer E. Olson wrote:
This patch implements for analog output the reinterpretation of stop_arg
when stop_src == TRIG_NONE to allow the user to specify the length of the
buffer that should be repeated.  The intent is to allow a user to have a
specific buffer repeated as-is indefinitely.  The contents of the DMA
buffer can be left static or changed by the user via mmap access to the DMA
buffer.  If the contents are changed by the user, additional munging is not
performed by the driver and only a single call to
comedi_mark_buffer_written should be done.

This patch implements ni_ao_cmd much more closely organized like NI MHDDK
examples and DAQ-STC pseudo-code.  Adds comments with some more specific
references to the DAQ-STC.

ni_ao_reset similarly cleaned up and made to appear more comparable to
DAC-STC.

Signed-off-by: Spencer E. Olson <olsonse@xxxxxxxxx>
---
  drivers/staging/comedi/drivers/ni_mio_common.c | 564 ++++++++++++++++++-------
  1 file changed, 405 insertions(+), 159 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
index 6cc304a..8941351 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c

It would be nice if this patch could be split up, e.g. reorganization of ni_ao_cmd(), cleaning up of ni_ao_reset(), reinterpretation of stop_arg when stop_src == TRIG_NONE. It's a bit hard to keep track of all the changes in this patch.

[snip]
+static void ni_ao_cmd_set_counters(struct comedi_device *dev,
+				   const struct comedi_cmd *cmd)
+{
+	struct ni_private *devpriv = dev->private;
+	/* Not supporting 'waveform staging' or 'local buffer with pauses' */
+
+	ni_stc_writew(dev, NISTC_RESET_AO_CFG_START, NISTC_RESET_REG);
+	/*
+	 * This relies on ao_mode1/(Trigger_Once | Continuous) being set in
+	 * set_trigger above.  It is unclear whether we really need to re-write
+	 * this register with these values.  The mhddk examples for e-series
+	 * show writing this in both places, but the examples for m-series show
+	 * a single write in the set_counters function (here).
+	 */
  	ni_stc_writew(dev, devpriv->ao_mode1, NISTC_AO_MODE1_REG);
+
+	/* sync (upload number of buffer iterations -1) */
+	/* indicate that we want to use BC_Load_A_Register as the source */
  	devpriv->ao_mode2 &= ~NISTC_AO_MODE2_BC_INIT_LOAD_SRC;
  	ni_stc_writew(dev, devpriv->ao_mode2, NISTC_AO_MODE2_REG);
-	if (cmd->stop_src == TRIG_NONE)
-		ni_stc_writel(dev, 0xffffff, NISTC_AO_BC_LOADA_REG);
-	else
-		ni_stc_writel(dev, 0, NISTC_AO_BC_LOADA_REG);
+
+	/*
+	 * if the BC_TC interrupt is still issued in spite of UC, BC, UI
+	 * ignoring BC_TC, then we will need to find a way to ignore that
+	 * interrupt in continuous mode.
+	 */
+	ni_stc_writel(dev, 0, NISTC_AO_BC_LOADA_REG); /* iter once */
+
+	/* sync (issue command to load number of buffer iterations -1) */
  	ni_stc_writew(dev, NISTC_AO_CMD1_BC_LOAD, NISTC_AO_CMD1_REG);
+
+	/* sync (upload number of updates in buffer) */
+	/* indicate that we want to use UC_Load_A_Register as the source */
  	devpriv->ao_mode2 &= ~NISTC_AO_MODE2_UC_INIT_LOAD_SRC;
  	ni_stc_writew(dev, devpriv->ao_mode2, NISTC_AO_MODE2_REG);
-	switch (cmd->stop_src) {
-	case TRIG_COUNT:
+
+	/*
+	 * if a user specifies '0', this automatically assumes the entire 24bit
+	 * address space is available for the (multiple iterations of single
+	 * buffer) MISB.  Otherwise, stop_arg specifies the MISB length that
+	 * will be used, regardless of whether we are in continuous mode or not.
+	 * In continuous mode, the output will just iterate indefinitely over
+	 * the MISB.
+	 */
+	{
+		unsigned int stop_arg = cmd->stop_arg > 0 ?
+			(cmd->stop_arg & 0xffffff) : 0xffffff;
+
  		if (devpriv->is_m_series) {
-			/*  this is how the NI example code does it for m-series boards, verified correct with 6259 */
-			ni_stc_writel(dev, cmd->stop_arg - 1,
-				      NISTC_AO_UC_LOADA_REG);
+			/*
+			 * this is how the NI example code does it for m-series
+			 * boards, verified correct with 6259
+			 */
+			ni_stc_writel(dev, stop_arg - 1, NISTC_AO_UC_LOADA_REG);
+
+			/* sync (issue cmd to load number of updates in MISB) */
  			ni_stc_writew(dev, NISTC_AO_CMD1_UC_LOAD,
  				      NISTC_AO_CMD1_REG);
  		} else {
-			ni_stc_writel(dev, cmd->stop_arg,
-				      NISTC_AO_UC_LOADA_REG);
+			ni_stc_writel(dev, stop_arg, NISTC_AO_UC_LOADA_REG);
+
+			/* sync (issue cmd to load number of updates in MISB) */
  			ni_stc_writew(dev, NISTC_AO_CMD1_UC_LOAD,
  				      NISTC_AO_CMD1_REG);
-			ni_stc_writel(dev, cmd->stop_arg - 1,
-				      NISTC_AO_UC_LOADA_REG);
+
+			/*
+			 * sync (upload number of updates-1 in MISB)
+			 * --eseries only?
+			 */
+			ni_stc_writel(dev, stop_arg - 1, NISTC_AO_UC_LOADA_REG);
  		}
-		break;
-	case TRIG_NONE:
-		ni_stc_writel(dev, 0xffffff, NISTC_AO_UC_LOADA_REG);
-		ni_stc_writew(dev, NISTC_AO_CMD1_UC_LOAD, NISTC_AO_CMD1_REG);
-		ni_stc_writel(dev, 0xffffff, NISTC_AO_UC_LOADA_REG);
-		break;
-	default:
-		ni_stc_writel(dev, 0, NISTC_AO_UC_LOADA_REG);
-		ni_stc_writew(dev, NISTC_AO_CMD1_UC_LOAD, NISTC_AO_CMD1_REG);
-		ni_stc_writel(dev, cmd->stop_arg, NISTC_AO_UC_LOADA_REG);
  	}

-	devpriv->ao_mode1 &= ~(NISTC_AO_MODE1_UPDATE_SRC_MASK |
-			       NISTC_AO_MODE1_UI_SRC_MASK |
-			       NISTC_AO_MODE1_UPDATE_SRC_POLARITY |
-			       NISTC_AO_MODE1_UI_SRC_POLARITY);
+	ni_stc_writew(dev, NISTC_RESET_AO_CFG_END, NISTC_RESET_REG);
+}

I'm not entirely sure what this function is doing with cmd->stop_arg, since its value is used two different things now, depending on cmd->stop_src, but the above code doesn't seem to take account of that. (Admittedly, I don't know much about how the NI hardware works.)

[snip]
+static void ni_ao_cmd_set_channels(struct comedi_device *dev,
+				   struct comedi_subdevice *s)
+{
+	struct ni_private *devpriv = dev->private;
+	const struct comedi_cmd *cmd = &s->async->cmd;
+	unsigned bits = 0;
+
+	ni_stc_writew(dev, NISTC_RESET_AO_CFG_START, NISTC_RESET_REG);
+
+	if (devpriv->is_6xxx) {
+		unsigned int i;
+

The original code has this call here:

		ni_ao_win_outw(dev, NI611X_AO_MISC_CLEAR_WG,
			       NI611X_AO_MISC_REG);

Is that redundant?

+		bits = 0;
+		for (i = 0; i < cmd->chanlist_len; ++i) {
+			int chan = CR_CHAN(cmd->chanlist[i]);
+
+			bits |= 1 << chan;
+			ni_ao_win_outw(dev, chan, NI611X_AO_WAVEFORM_GEN_REG);
+		}
+		ni_ao_win_outw(dev, bits, NI611X_AO_TIMED_REG);
+	}
+
+	ni_ao_config_chanlist(dev, s, cmd->chanlist, cmd->chanlist_len, 1);
+
[snip]

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
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