[PATCH v2 08/12] staging: comedi: rtd520: fix rtd_ao_winsn()

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

 



The "comedi range and offset conversions" in this function is
incorrect. According to the hardware manual, the DAC registers
are defined as:

    | D15  | D14-D03     | D02 | | D01 | D00 |
    | Sign | 12-bit data | (see manual)      |

The extended 'Sign' bit is only used for bipolar ranges to indicate
negative 12-bit data values.

The current code actually flips the sign for all bipolar data
values instead of just munging the value to 2's complement and
extending the sign bit.

Fix the function to correctly munge the comedi offset binary
values into 2's complement and extended the sign when bipolar
ranges are specified.

For aesthetics, rename this function and tidy it up a bit.

Save the readback value after the conversion is complete.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
 drivers/staging/comedi/drivers/rtd520.c | 37 +++++++++++++++++----------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/comedi/drivers/rtd520.c b/drivers/staging/comedi/drivers/rtd520.c
index 5fe92c4..e02ec51 100644
--- a/drivers/staging/comedi/drivers/rtd520.c
+++ b/drivers/staging/comedi/drivers/rtd520.c
@@ -1046,42 +1046,43 @@ static int rtd_ao_eoc(struct comedi_device *dev,
 	return -EBUSY;
 }
 
-static int rtd_ao_winsn(struct comedi_device *dev,
-			struct comedi_subdevice *s, struct comedi_insn *insn,
-			unsigned int *data)
+static int rtd_ao_insn_write(struct comedi_device *dev,
+			     struct comedi_subdevice *s,
+			     struct comedi_insn *insn,
+			     unsigned int *data)
 {
 	struct rtd_private *devpriv = dev->private;
-	int i;
-	int chan = CR_CHAN(insn->chanspec);
-	int range = CR_RANGE(insn->chanspec);
+	unsigned int chan = CR_CHAN(insn->chanspec);
+	unsigned int range = CR_RANGE(insn->chanspec);
 	int ret;
+	int i;
 
 	/* Configure the output range (table index matches the range values) */
 	writew(range & 7, dev->mmio + LAS0_DAC_CTRL(chan));
 
 	for (i = 0; i < insn->n; ++i) {
-		int val = data[i] << 3;
-
-		/* VERIFY: comedi range and offset conversions */
+		unsigned int val = data[i];
 
-		if ((range > 1) && (data[i] < 2048)) {	/* bipolar */
-			/* offset and sign extend */
-			val = (((int)data[i]) - 2048) << 3;
-		} else {	/* unipolor */
-			val = data[i] << 3;
+		/* bipolar uses 2's complement values with an extended sign */
+		if (comedi_range_is_bipolar(s, range)) {
+			val = comedi_offset_munge(s, val);
+			val |= (val & ((s->maxdata + 1) >> 1)) << 1;
 		}
 
+		/* shift the 12-bit data (+ sign) to match the register */
+		val <<= 3;
+
 		writew(val, devpriv->las1 + LAS1_DAC_FIFO(chan));
 		writew(0, dev->mmio + LAS0_UPDATE_DAC(chan));
 
-		s->readback[chan] = data[i];
-
 		ret = comedi_timeout(dev, s, insn, rtd_ao_eoc, 0);
 		if (ret)
 			return ret;
+
+		s->readback[chan] = data[i];
 	}
 
-	return i;
+	return insn->n;
 }
 
 static int rtd_dio_insn_bits(struct comedi_device *dev,
@@ -1240,7 +1241,7 @@ static int rtd_auto_attach(struct comedi_device *dev,
 	s->n_chan	= 2;
 	s->maxdata	= 0x0fff;
 	s->range_table	= &rtd_ao_range;
-	s->insn_write	= rtd_ao_winsn;
+	s->insn_write	= rtd_ao_insn_write;
 
 	ret = comedi_alloc_subdev_readback(s);
 	if (ret)
-- 
2.5.1

_______________________________________________
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