[PATCH 20/21] staging: comedi: ni_at_ao: fix analog output ranges

Ian Abbott abbotti at mev.co.uk
Tue Oct 1 10:26:56 UTC 2013


On 2013-10-01 01:55, H Hartley Sweeten wrote:
> Each analog output channel is individually configurable. The current
> analog output 'range_table' selection in this driver assumes that all
> the channels are configured as either bipolar or unipolar. User option[2]
> is then used during the attach to select the range_table to use.
>
> Add a comedi_lrange table to allow the user to specify the actual range
> to use for each channel.
>
> Make sure the approriate DAC2S bit is set in the CFG2 register when writing
> straight binary unipolar values to the DAC. Also, the data only needs to
> be munged to two's complement when writing bipolar values.

Since this bit only affects how the hardware interprets values written 
to the DAC, and all comedi data values are straight binary already, 
can't you just leave it initialized to straight binary (already done by 
atao_reset()) and write the comedi data values as is without munging?

>
> 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/ni_at_ao.c | 41 ++++++++++++++++++++++++++-----
>   1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_at_ao.c b/drivers/staging/comedi/drivers/ni_at_ao.c
> index 10e3e947..e483c7e 100644
> --- a/drivers/staging/comedi/drivers/ni_at_ao.c
> +++ b/drivers/staging/comedi/drivers/ni_at_ao.c
> @@ -29,9 +29,6 @@
>    *   [0] - I/O port base address
>    *   [1] - IRQ (unused)
>    *   [2] - DMA (unused)
> - *   [3] - analog output range, set by jumpers on hardware
> - *         0 for -10 to 10V bipolar
> - *         1 for 0V to 10V unipolar
>    */
>
>   #include <linux/module.h>
> @@ -99,6 +96,32 @@
>   #define ATAO_2_RTSISHFT_RSI	(1 << 0)
>   #define ATAO_2_RTSISTRB_REG	0x07
>
> +/*
> + * Each Analog Output channel can be configured indepentently to
> + * have either a bipolar (factory setting) or unipolar output.
> + * This selection is done with jumpers on the board.
> + *
> + * There are also jumpers to select if the channel uses the internal
> + * 10V reference voltage (factory default) or an exteral reference
> + * voltage applied to the EXTREFx pin on the I/O connector. Each
> + * EXTREFx signal is shared by two DACs that are in the same chip;
> + * that is DAC0 and DAC1 share EXTREF0, DAC2 and DAC3 share EXTREF2,
> + * etc. Each channel is individually configured.
> + *
> + * The following ranges cover all the configuration options. The user
> + * must select the correct range, based on the board configuration,
> + * to correctly calculate the values needed to produce a given output
> + * voltage.
> + */
> +static const struct comedi_lrange atao_range = {
> +	4, {
> +		BIP_RANGE(10),		/* internal bipolar */
> +		UNI_RANGE(10),		/* internal unipolar */
> +		RANGE_ext(-1, 1),	/* external bipolar */
> +		RANGE_ext(0, 1)		/* external unipolar */
> +	}
> +};
> +
>   struct atao_board {
>   	const char *name;
>   	int n_ao_chans;
> @@ -143,9 +166,14 @@ static int atao_ao_insn_write(struct comedi_device *dev,
>   {
>   	struct atao_private *devpriv = dev->private;
>   	unsigned int chan = CR_CHAN(insn->chanspec);
> +	unsigned int range = CR_RANGE(insn->chanspec);
>   	unsigned int val;
>   	int i;
>
> +	/* enable straight binary format for unipolar ranges */
> +	if (comedi_range_is_unipolar(s, range))
> +		outw(ATAO_CFG2_DACS(chan), dev->iobase + ATAO_CFG2_REG);
> +

I don't think you need this of the munging of bipolar range values 
below, but the above bit of code only seems to be half-working as it 
doesn't write anything to the register for bipolar ranges.

>   	if (chan == 0)
>   		atao_select_reg_group(dev, 1);
>
> @@ -153,8 +181,9 @@ static int atao_ao_insn_write(struct comedi_device *dev,
>   		val = data[i];
>   		devpriv->ao_readback[chan] = val;
>
> -		/* munge offset binary (unsigned) to two's complement */
> -		val = comedi_offset_munge(s, val);
> +		/* bipolar ranges use two's complement format */
> +		if (comedi_range_is_bipolar(s, range))
> +			val = comedi_offset_munge(s, val);
>   		outw(val, dev->iobase + ATAO_AO_REG(chan));
>   	}

Just remove the edits from atao_ao_insn_write() and the remainder of the 
patch is fine!

>
> @@ -361,7 +390,7 @@ static int atao_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>   	s->subdev_flags	= SDF_WRITABLE;
>   	s->n_chan	= board->n_ao_chans;
>   	s->maxdata	= 0x0fff;
> -	s->range_table	= it->options[3] ? &range_unipolar10 : &range_bipolar10;
> +	s->range_table	= &atao_range;
>   	s->insn_write	= atao_ao_insn_write;
>   	s->insn_read	= atao_ao_insn_read;

Since you mentioned it in your patch 00 message, I'm not bothered about 
removing the configuration option as the new way of handling the range 
is more flexible.  (The old way should probably have had one 
configuration option per channel anyway.)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-


More information about the devel mailing list