Skip to content

Commit

Permalink
staging: comedi: addi_apci_3501: rewrite the analog output support
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
H Hartley Sweeten authored and Greg Kroah-Hartman committed Jan 25, 2013
1 parent 2627369 commit 298ab7d
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 116 deletions.
113 changes: 0 additions & 113 deletions drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 |
Expand Down
51 changes: 48 additions & 3 deletions drivers/staging/comedi/drivers/addi_apci_3501.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 298ab7d

Please sign in to comment.