[PATCH 15/22] staging: comedi: adv_pci1710: tidy up analog output subdev_flags

[Ian Abbott]

On 05/11/15 18:56, Hartley Sweeten wrote:
> On Thursday, November 05, 2015 6:01 AM, Ian Abbott wrote:
>> On 04/11/15 16:55, H Hartley Sweeten wrote:
>>> The SDF_GROUND and SDF_COMMON flags are not needed for the analog output
>>> subdevice. Just remove them.
>>>
>>> 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/adv_pci1710.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
>>> index 339130b..0511f26 100644
>>> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
>>> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
>>> @@ -817,7 +817,7 @@ static int pci1710_auto_attach(struct comedi_device *dev,
>>>    	if (board->has_ao) {
>>>    		s = &dev->subdevices[subdev];
>>>    		s->type		= COMEDI_SUBD_AO;
>>> -		s->subdev_flags	= SDF_WRITABLE | SDF_GROUND | SDF_COMMON;
>>> +		s->subdev_flags	= SDF_WRITABLE;
>>>    		s->n_chan	= 2;
>>>    		s->maxdata	= 0x0fff;
>>>    		s->range_table	= &pci171x_ao_range;
>>>
>>
>> The signal connector has separate pins for AIGND, AOGND and DGND,
>> although ultimately they are referenced to the same voltage.  I'd just
>> leave the SDF_GROUND and SDF_COMMON flags set, same as for the analog
>> input subdevice.
>
> They just don't make any sense here. The SDF_* aref flags are used with the
> analog input subdevices to specify which AREF_* options are supported. The
> AREF_* options are then used to program the hardware for the desired
> analog aref.
>
> The connector does have the separate pins for the analog input, analog output
> and digital grounds but, like you said, they all go to a common reference. The
> user might specify a AREF_* selection but it doesn't actually do anything.
>
> If the analog aref is not programmable I think it's a bit overkill to specify
> the SDF_* aref in the subdev_flags. If anything just the SDF_GROUND should
> be specified which is the default aref (AREF_GROUND = 0x00).

Most drivers' AO subdevices seem to set include SDF_GROUND and/or 
SDF_COMMON.  I think analog subdevices should specify some sort of 
reference for information purposes.

The AI subdevice also sets both SDF_GROUND and SDF_COMMON, and SDF_DIFF 
if it supports differential.

> For the analog inputs, the user's manual for the PCI-1710/1710HG show a
> couple  wiring options
> 1) Single-ended (AI[x] to AIGND)
> 2) Differential - ground reference (AI[x]  to AI[x+1]. AI[x+1] to GND)
> 3) Differential - AIGND reference (AI[x]  to AI[x+1]. pull-down resistors to AIGND)
>
> Again, these are wiring options not programmable options. I feel that
> the subdev_flags should either not specify an SDF_* aref option or just
> specify the SDF_GROUND option.

I don't mind removing SDF_COMMON, but in that case it should be removed 
from the AI subdevice as well, as ultimately the AI and AO share a 
common ground reference.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-