staging: comedi: addi_apci_3501: rewrite the analog output support

Currently the analog output subdevice has two support functions:
  (*insn_config) - i_APCI3501_ConfigAnalogOutput()
  (*insn_write)  - i_APCI3501_WriteAnalogOutput()

The (*insn_config) function is used to configure the analog outputs
in either bipolar or unipolar mode. This function abuses the comedi
API since it treats the data[0] value as a parameter instead of as
the config "instruction".

The (*insn_write) function then writes a single value to the desired
analog output channel after doing some sanity checking on the channel
number. The sanity checking is not required since the comedi core has
already done it. Also, the (*insn_write) functions are supposed to
write all the data, indicated by insn->n, to the channel not just a
single value.

Rewrite the support code so it works properly with the comedi API.

The bipolar/unipolar configuration can be determine in the (*insn_write)
by checking the passed insn->chanspec.

Since the unipolar configuration only has 13-bit resolution, we need
to check that the data is in range because the subdevice 'maxdata' is
set to 14-bits for the bipolar mode. If the data is out of range,
output a dev_warn() and return -EINVAL.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c

@@ -46,10 +46,6 @@ You should also find the complete GPL in the COPYING file accompanying this sour
   +----------+-----------+------------------------------------------------+
 */
 
-/* Analog Output related Defines */
-#define MODE0				0
-#define MODE1				1
-
 /* Watchdog Related Defines */
 
 #define APCI3501_WATCHDOG		0x20
@@ -64,115 +60,6 @@ You should also find the complete GPL in the COPYING file accompanying this sour
 #define ADDIDATA_TIMER			0
 #define ADDIDATA_WATCHDOG		2
 
-/*
-+----------------------------------------------------------------------------+
-| Function   Name   : int i_APCI3501_ConfigAnalogOutput                      |
-|			  (struct comedi_device *dev,struct comedi_subdevice *s,               |
-|                      struct comedi_insn *insn,unsigned int *data)                     |
-+----------------------------------------------------------------------------+
-| Task              : Configures The Analog Output Subdevice                 |
-+----------------------------------------------------------------------------+
-| Input Parameters  : struct comedi_device *dev      : Driver handle                |
-|                     struct comedi_subdevice *s     : Subdevice Pointer            |
-|                     struct comedi_insn *insn       : Insn Structure Pointer       |
-|                     unsigned int *data          : Data Pointer contains        |
-|                                          configuration parameters as below |
-|                                                                            |
-|					data[0]            : Voltage Mode                |
-|                                                0:Mode 0                    |
-|                                                1:Mode 1                    |
-|                                                                            |
-+----------------------------------------------------------------------------+
-| Output Parameters :	--													 |
-+----------------------------------------------------------------------------+
-| Return Value      : TRUE  : No error occur                                 |
-|		            : FALSE : Error occur. Return the error          |
-|			                                                         |
-+----------------------------------------------------------------------------+
-*/
-static int i_APCI3501_ConfigAnalogOutput(struct comedi_device *dev,
-					 struct comedi_subdevice *s,
-					 struct comedi_insn *insn,
-					 unsigned int *data)
-{
-	struct apci3501_private *devpriv = dev->private;
-
-	outl(data[0], dev->iobase + APCI3501_AO_CTRL_STATUS_REG);
-
-	if (data[0]) {
-		devpriv->b_InterruptMode = MODE1;
-	} else {
-		devpriv->b_InterruptMode = MODE0;
-	}
-	return insn->n;
-}
-
-/*
-+----------------------------------------------------------------------------+
-| Function   Name   : int i_APCI3501_WriteAnalogOutput                       |
-|			  (struct comedi_device *dev,struct comedi_subdevice *s,               |
-|                      struct comedi_insn *insn,unsigned int *data)                     |
-+----------------------------------------------------------------------------+
-| Task              : Writes To the Selected Anlog Output Channel            |
-+----------------------------------------------------------------------------+
-| Input Parameters  : struct comedi_device *dev      : Driver handle                |
-|                     struct comedi_subdevice *s     : Subdevice Pointer            |
-|                     struct comedi_insn *insn       : Insn Structure Pointer       |
-|                     unsigned int *data          : Data Pointer contains        |
-|                                          configuration parameters as below |
-|                                                                            |
-|                                                                            |
-+----------------------------------------------------------------------------+
-| Output Parameters :	--													 |
-+----------------------------------------------------------------------------+
-| Return Value      : TRUE  : No error occur                                 |
-|		            : FALSE : Error occur. Return the error          |
-|			                                                         |
-+----------------------------------------------------------------------------+
-*/
-static int i_APCI3501_WriteAnalogOutput(struct comedi_device *dev,
-					struct comedi_subdevice *s,
-					struct comedi_insn *insn,
-					unsigned int *data)
-{
-	struct apci3501_private *devpriv = dev->private;
-	unsigned int ul_Command1 = 0, ul_Channel_no, ul_Polarity;
-	int ret;
-
-	ul_Channel_no = CR_CHAN(insn->chanspec);
-
-	if (devpriv->b_InterruptMode == MODE1) {
-		ul_Polarity = 0x80000000;
-		if ((*data < 0) || (*data > 16384)) {
-			printk("\nIn WriteAnalogOutput :: Not Valid Data\n");
-		}
-
-	}			/*  end if(devpriv->b_InterruptMode==MODE1) */
-	else {
-		ul_Polarity = 0;
-		if ((*data < 0) || (*data > 8192)) {
-			printk("\nIn WriteAnalogOutput :: Not Valid Data\n");
-		}
-
-	}			/*  end else */
-
-	if ((ul_Channel_no < 0) || (ul_Channel_no > 7)) {
-		printk("\nIn WriteAnalogOutput :: Not Valid Channel\n");
-	}			/*  end if((ul_Channel_no<0)||(ul_Channel_no>7)) */
-
-	ret = apci3501_wait_for_dac(dev);
-	if (ret)
-		return ret;
-
-	/* Output the Value on the output channels. */
-	ul_Command1 = (unsigned int) ((unsigned int) (ul_Channel_no & 0xFF) |
-			(unsigned int) ((*data << 0x8) & 0x7FFFFF00L) |
-			(unsigned int) (ul_Polarity));
-	outl(ul_Command1, dev->iobase + APCI3501_AO_DATA_REG);
-
-	return insn->n;
-}
-
 /*
 +----------------------------------------------------------------------------+
 | Function   Name   : int i_APCI3501_ConfigTimerCounterWatchdog              |

drivers/staging/comedi/drivers/addi_apci_3501.c

@@ -40,7 +40,6 @@ struct apci3501_private {
 	int i_IobaseAmcc;
 	struct task_struct *tsk_Current;
 	unsigned char b_TimerSelectMode;
-	unsigned char b_InterruptMode;
 };
 
 static struct comedi_lrange apci3501_ao_range = {
@@ -61,6 +60,53 @@ static int apci3501_wait_for_dac(struct comedi_device *dev)
 	return 0;
 }
 
+static int apci3501_ao_insn_write(struct comedi_device *dev,
+				  struct comedi_subdevice *s,
+				  struct comedi_insn *insn,
+				  unsigned int *data)
+{
+	unsigned int chan = CR_CHAN(insn->chanspec);
+	unsigned int range = CR_RANGE(insn->chanspec);
+	unsigned int val = 0;
+	int i;
+	int ret;
+
+	/*
+	 * All analog output channels have the same output range.
+	 *	14-bit bipolar: 0-10V
+	 *	13-bit unipolar: +/-10V
+	 * Changing the range of one channel changes all of them!
+	 */
+	if (range) {
+		outl(0, dev->iobase + APCI3501_AO_CTRL_STATUS_REG);
+	} else {
+		val |= APCI3501_AO_DATA_BIPOLAR;
+		outl(APCI3501_AO_CTRL_BIPOLAR,
+		     dev->iobase + APCI3501_AO_CTRL_STATUS_REG);
+	}
+
+	val |= APCI3501_AO_DATA_CHAN(chan);
+
+	for (i = 0; i < insn->n; i++) {
+		if (range == 1) {
+			if (data[i] > 0x1fff) {
+				dev_err(dev->class_dev,
+					"Unipolar resolution is only 13-bits\n");
+				return -EINVAL;
+			}
+		}
+
+		ret = apci3501_wait_for_dac(dev);
+		if (ret)
+			return ret;
+
+		outl(val | APCI3501_AO_DATA_VAL(data[i]),
+		     dev->iobase + APCI3501_AO_DATA_REG);
+	}
+
+	return insn->n;
+}
+
 #include "addi-data/hwdrv_apci3501.c"
 
 static int apci3501_di_insn_bits(struct comedi_device *dev,
@@ -294,8 +340,7 @@ static int apci3501_auto_attach(struct comedi_device *dev,
 		s->n_chan	= ao_n_chan;
 		s->maxdata	= 0x3fff;
 		s->range_table	= &apci3501_ao_range;
-		s->insn_config	= i_APCI3501_ConfigAnalogOutput;
-		s->insn_write	= i_APCI3501_WriteAnalogOutput;
+		s->insn_write	= apci3501_ao_insn_write;
 	} else {
 		s->type		= COMEDI_SUBD_UNUSED;
 	}