On 23/09/15 20:16, H Hartley Sweeten wrote:
For aesthetics, rename this function and tidy it up a bit.
Use the comedi_range_is_bipolar() and comedi_offset_munge() helpers
to handle the 2's complement munging.
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 | 35 ++++++++++++++++-----------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/rtd520.c b/drivers/staging/comedi/drivers/rtd520.c
index 5fe92c4..2df8494 100644
--- a/drivers/staging/comedi/drivers/rtd520.c
+++ b/drivers/staging/comedi/drivers/rtd520.c
@@ -1046,42 +1046,41 @@ 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;
+ unsigned int val = data[i];
- /* VERIFY: comedi range and offset conversions */
+ /* bipolar ranges use 2's complement values */
+ if (comedi_range_is_bipolar(s, range))
+ val = comedi_offset_munge(s, val);
- if ((range > 1) && (data[i] < 2048)) { /* bipolar */
- /* offset and sign extend */
- val = (((int)data[i]) - 2048) << 3;
- } else { /* unipolor */
- val = data[i] << 3;
- }
+ /* the 12-bit data is left justified to 16-bits */
+ val <<= 3;
12 + 3 = 15. The original mapping of values for bipolar ranges looks a
bit peculiar, and is different to the new mapping. Let's see:
Comedi Old raw << 3 New raw << 3
------------- --------------- ---------------
[0x000,0x7ff] [0xc000,0xfff8] [0x4000,0x7ff8]
[0x800,0xfff] [0x4000,0x7ff8] [0x0000,0x3ff8]
Section 4.3.3.1 of the manual
<http://www.rtd.com/manuals/DM/DM7520_DM7530_PCI4520_Hardware_manual_v20.pdf>
says, "A write programs the D/A1 FIFO in the format shown below.
Because of the extended sign bit in a bipolar and in unipolar mode also
the data format is two's complement."
| D15 | D14 - D03 | D02 | D01 | D00 |
| Sign | 12-bit data | (see manual) |
The "extended sign bit" and "two's complement" suggests that the mapping
of values for bipolar ranges ought to be:
Comedi Raw << 3
------------- ---------------
[0x000,0x7ff] [0xc000,0xfff8]
[0x800,0xfff] [0x0000,0x3ff8]
I guess the original mapping works (assuming it does) because of the the
redundancy. Perhaps bit 14 gets ignored in favour of bit 15 in two's
complement mode. In fact bit 14 seems to be always '1' in the original
mapping!
I think it's safest to stick with the original mapping.
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 +1239,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)
--
-=( 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
