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 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@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).

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@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
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