[PATCH 08/11] staging: comedi: rtd520: tidy up rtd_ao_winsn()

Ian Abbott abbotti at mev.co.uk
Thu Sep 24 10:59:59 UTC 2015


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 at visionengravers.com>
> Cc: Ian Abbott <abbotti at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> ---
>   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 at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-


More information about the devel mailing list