From 298ab7de9ced3ecbe7961360ece58d5aa17aa3c1 Mon Sep 17 00:00:00 2001 From: H Hartley Sweeten Date: Wed, 23 Jan 2013 12:45:21 -0700 Subject: [PATCH] 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 Cc: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- .../comedi/drivers/addi-data/hwdrv_apci3501.c | 113 --------------------- drivers/staging/comedi/drivers/addi_apci_3501.c | 51 +++++++++- 2 files changed, 48 insertions(+), 116 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c index b057906..7d8fbdf 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c +++ b/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 @@ -66,115 +62,6 @@ You should also find the complete GPL in the COPYING file accompanying this sour /* +----------------------------------------------------------------------------+ -| 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 | | (struct comedi_device *dev,struct comedi_subdevice *s, | | struct comedi_insn *insn,unsigned int *data) | diff --git a/drivers/staging/comedi/drivers/addi_apci_3501.c b/drivers/staging/comedi/drivers/addi_apci_3501.c index d687081..7fedf15 100644 --- a/drivers/staging/comedi/drivers/addi_apci_3501.c +++ b/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; } -- 2.7.4