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

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

 



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@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---
>>   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).

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.

Hartley

_______________________________________________
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