[PATCH 0/2] staging: comedi: ni_daq_700: AI additions

[PATCH 0/2] staging: comedi: ni_daq_700: AI additions

[Ian Abbott]

The first patch reformats (and slightly changes) the driver comment to
avoid checkpatch warnings for the second patch.

The second patch adds functionality to the AI subdevice and is the work
of the original author, Fred Brooks.

1) staging: comedi: ni_daq_700: update driver comment
2) staging: comedi: ni_daq_700: add AI range and input mode

 drivers/staging/comedi/drivers/ni_daq_700.c | 98 +++++++++++++++++++----------
 1 file changed, 65 insertions(+), 33 deletions(-)

[PATCH 1/2] staging: comedi: ni_daq_700: update driver comment

[Ian Abbott]

Reformat the comment describing this comedi driver to use the usual
block comment format.

Also remove reference to digital I/O emulating an 8255, because it
doesn't, and remove "DIO only" from the "Description:" line as it also
supports analog inputs.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/ni_daq_700.c | 51 ++++++++++++++---------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
index 171a71d..55d80ef 100644
--- a/drivers/staging/comedi/drivers/ni_daq_700.c
+++ b/drivers/staging/comedi/drivers/ni_daq_700.c
@@ -18,32 +18,31 @@
  */
 
 /*
-Driver: ni_daq_700
-Description: National Instruments PCMCIA DAQCard-700 DIO only
-Author: Fred Brooks <nsaspook@nsaspook.com>,
-  based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
-Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
-Status: works
-Updated: Wed, 19 Sep 2012 12:07:20 +0000
-
-The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
-16 channels and a analog input subdevice (1) with 16 single-ended channels.
-
-Digital:  The channel 0 corresponds to the daqcard-700's output
-port, bit 0; channel 8 corresponds to the input port, bit 0.
-
-Digital direction configuration: channels 0-7 output, 8-15 input (8225 device
-emu as port A output, port B input, port C N/A).
-
-Analog: The input  range is 0 to 4095 for -10 to +10 volts
-IRQ is assigned but not used.
-
-Version 0.1	Original DIO only driver
-Version 0.2	DIO and basic AI analog input support on 16 se channels
-
-Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
-		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
-*/
+ * Driver: ni_daq_700
+ * Description: National Instruments PCMCIA DAQCard-700
+ * Author: Fred Brooks <nsaspook@nsaspook.com>,
+ *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
+ * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
+ * Status: works
+ * Updated: Wed, 19 Sep 2012 12:07:20 +0000
+ *
+ * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
+ * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
+ *
+ * Digital:  The channel 0 corresponds to the daqcard-700's output
+ * port, bit 0; channel 8 corresponds to the input port, bit 0.
+ *
+ * Digital direction configuration: channels 0-7 output, 8-15 input.
+ *
+ * Analog: The input  range is 0 to 4095 for -10 to +10 volts
+ * IRQ is assigned but not used.
+ *
+ * Version 0.1	Original DIO only driver
+ * Version 0.2	DIO and basic AI analog input support on 16 se channels
+ *
+ * Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
+ *		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
+ */
 
 #include <linux/module.h>
 #include <linux/delay.h>
-- 
1.9.2

RE: [PATCH 1/2] staging: comedi: ni_daq_700: update driver comment

[Hartley Sweeten]

On Thursday, May 29, 2014 4:16 AM, Ian Abbott wrote:
>
> Reformat the comment describing this comedi driver to use the usual
> block comment format.
>
> Also remove reference to digital I/O emulating an 8255, because it
> doesn't, and remove "DIO only" from the "Description:" line as it also
> supports analog inputs.
> 
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
>  drivers/staging/comedi/drivers/ni_daq_700.c | 51 ++++++++++++++---------------
>  1 file changed, 25 insertions(+), 26 deletions(-)

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

[PATCH 2/2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Ian Abbott]

I received a patch from the original author of the driver, Fred Brooks,
to add support for switching the input range and the
single-ended/differential input mode for the AI subdevice.

I used the patch as-is apart from minor whitespace fixes and
reformatting the changes to the driver description comment.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
---
 drivers/staging/comedi/drivers/ni_daq_700.c | 53 +++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
index 55d80ef..e66c0b9 100644
--- a/drivers/staging/comedi/drivers/ni_daq_700.c
+++ b/drivers/staging/comedi/drivers/ni_daq_700.c
@@ -24,21 +24,30 @@
  *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
  * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
  * Status: works
- * Updated: Wed, 19 Sep 2012 12:07:20 +0000
+ * Updated: Wed, 21 May 2014 12:07:20 +0000
  *
  * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
- * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
+ * 16 channels and a analog input subdevice (1) with 16 single-ended channels
+ * or 8 differential channels, and three input ranges.
  *
  * Digital:  The channel 0 corresponds to the daqcard-700's output
  * port, bit 0; channel 8 corresponds to the input port, bit 0.
  *
  * Digital direction configuration: channels 0-7 output, 8-15 input.
  *
- * Analog: The input  range is 0 to 4095 for -10 to +10 volts
+ * Analog: The input  range is 0 to 4095 with a default of -10 to +10 volts.
+ * Valid ranges:
+ *       0 for -10 to 10V bipolar
+ *       1 for -5 to 5V bipolar
+ *       2 for -2.5 to 2.5V bipolar
+ *
  * IRQ is assigned but not used.
  *
  * Version 0.1	Original DIO only driver
  * Version 0.2	DIO and basic AI analog input support on 16 se channels
+ * Version 0.3	Add SE or DIFF mode and range switching +-10,+-5,+-2.5
+ *		Clear the FIFO of data before the conversion to handle card
+ *		mode switching glitches.
  *
  * Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
  *		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
@@ -68,6 +77,17 @@
 #define CDA_R2		0x0A	/* RW 8bit */
 #define CMO_R		0x0B	/* RO 8bit */
 #define TIC_R		0x06	/* WO 8bit */
+/* daqcard700 modes */
+#define CMD_R3_DIFF     0x04    /* diff mode */
+
+static const struct comedi_lrange range_daq700_ai = {
+	3,
+	{
+		BIP_RANGE(10),
+		BIP_RANGE(5),
+		BIP_RANGE(2.5)
+	}
+};
 
 static int daq700_dio_insn_bits(struct comedi_device *dev,
 				struct comedi_subdevice *s,
@@ -130,20 +150,34 @@ static int daq700_ai_rinsn(struct comedi_device *dev,
 			   struct comedi_subdevice *s,
 			   struct comedi_insn *insn, unsigned int *data)
 {
-	int n, chan;
+	int n;
 	int d;
 	int ret;
+	unsigned int chan	= CR_CHAN(insn->chanspec);
+	unsigned int aref	= CR_AREF(insn->chanspec);
+	unsigned int range	= CR_RANGE(insn->chanspec);
+	unsigned int r3_bits	= 0;
+
+	/* set channel input modes */
+	if (aref == AREF_DIFF)
+		r3_bits |= CMD_R3_DIFF;
+	/* write channel mode/range */
+	if (range >= 1)
+		range++;        /* convert range to hardware value */
+	outb(r3_bits | (range & 0x03), dev->iobase + CMD_R3);
 
-	chan = CR_CHAN(insn->chanspec);
 	/* write channel to multiplexer */
 	/* set mask scan bit high to disable scanning */
-	outb(chan | 0x80, dev->iobase + CMD_R1);
+	outb((chan & 0x0f) | 0x80, dev->iobase + CMD_R1);
 
 	/* convert n samples */
 	for (n = 0; n < insn->n; n++) {
 		/* trigger conversion with out0 L to H */
 		outb(0x00, dev->iobase + CMD_R2); /* enable ADC conversions */
 		outb(0x30, dev->iobase + CMO_R); /* mode 0 out0 L, from H */
+		outb(0x00, dev->iobase + ADCLEAR_R);	/* clear the ADC FIFO */
+		/* read 16bit junk from FIFO to clear */
+		inw(dev->iobase + ADFIFO_R);
 		/* mode 1 out0 H, L to H, start conversion */
 		outb(0x32, dev->iobase + CMO_R);
 
@@ -219,11 +253,10 @@ static int daq700_auto_attach(struct comedi_device *dev,
 	/* DAQCard-700 ai */
 	s = &dev->subdevices[1];
 	s->type = COMEDI_SUBD_AI;
-	/* we support single-ended (ground)  */
-	s->subdev_flags = SDF_READABLE | SDF_GROUND;
+	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF;
 	s->n_chan = 16;
 	s->maxdata = (1 << 12) - 1;
-	s->range_table = &range_bipolar10;
+	s->range_table = &range_daq700_ai;
 	s->insn_read = daq700_ai_rinsn;
 	daq700_ai_config(dev, s);
 
@@ -260,5 +293,5 @@ module_comedi_pcmcia_driver(daq700_driver, daq700_cs_driver);
 MODULE_AUTHOR("Fred Brooks <nsaspook@nsaspook.com>");
 MODULE_DESCRIPTION(
 	"Comedi driver for National Instruments PCMCIA DAQCard-700 DIO/AI");
-MODULE_VERSION("0.2.00");
+MODULE_VERSION("0.3.00");
 MODULE_LICENSE("GPL");
-- 
1.9.2

Re: [PATCH 2/2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Dan Carpenter]

These days, I don't barely look at comedi changes at all because you
guys do such a great job.  I just had a process/git comment on this one.
Normally, these changelogs look like:

From: Fred Brooks <frederick.brooks@microchip.com>

Add support for switching the input range and the
single-ended/differential input mode for the AI subdevice.  We needed to
clear the FIFO of data before the conversion to handle card mode
switching glitches.

[ I made some style changes.  - Ian ]

If the first line of the email is From: then git assigns authorship
credit automatically.

regards,
dan carpenter

Re: [PATCH 2/2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Ian Abbott]

On 2014/05/29 12:51 PM, Dan Carpenter wrote:
> These days, I don't barely look at comedi changes at all because you
> guys do such a great job.  I just had a process/git comment on this one.
> Normally, these changelogs look like:
> 
> From: Fred Brooks <frederick.brooks@microchip.com>
> 
> Add support for switching the input range and the
> single-ended/differential input mode for the AI subdevice.  We needed to
> clear the FIFO of data before the conversion to handle card mode
> switching glitches.
> 
> [ I made some style changes.  - Ian ]
> 
> If the first line of the email is From: then git assigns authorship
> credit automatically.

Okay, I'll repost it with a better comment.

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

[PATCH 2/2 v2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Ian Abbott]

From: Fred Brooks <frederick.brooks@microchip.com>

Add support for switching the input range and the single-ended/
differential input mode for the AI subdevice.  We needed to clear the
FIFO of data before the conversion to handle card mode switching
glitches.

[ Minor whitespace fixes and driver comment reformatting.  - Ian ]

Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: Set authorship to Fred Brooks and updated patch description as
suggested by Dan Carpenter.
---
 drivers/staging/comedi/drivers/ni_daq_700.c | 53 +++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
index 55d80ef..e66c0b9 100644
--- a/drivers/staging/comedi/drivers/ni_daq_700.c
+++ b/drivers/staging/comedi/drivers/ni_daq_700.c
@@ -24,21 +24,30 @@
  *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
  * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
  * Status: works
- * Updated: Wed, 19 Sep 2012 12:07:20 +0000
+ * Updated: Wed, 21 May 2014 12:07:20 +0000
  *
  * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
- * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
+ * 16 channels and a analog input subdevice (1) with 16 single-ended channels
+ * or 8 differential channels, and three input ranges.
  *
  * Digital:  The channel 0 corresponds to the daqcard-700's output
  * port, bit 0; channel 8 corresponds to the input port, bit 0.
  *
  * Digital direction configuration: channels 0-7 output, 8-15 input.
  *
- * Analog: The input  range is 0 to 4095 for -10 to +10 volts
+ * Analog: The input  range is 0 to 4095 with a default of -10 to +10 volts.
+ * Valid ranges:
+ *       0 for -10 to 10V bipolar
+ *       1 for -5 to 5V bipolar
+ *       2 for -2.5 to 2.5V bipolar
+ *
  * IRQ is assigned but not used.
  *
  * Version 0.1	Original DIO only driver
  * Version 0.2	DIO and basic AI analog input support on 16 se channels
+ * Version 0.3	Add SE or DIFF mode and range switching +-10,+-5,+-2.5
+ *		Clear the FIFO of data before the conversion to handle card
+ *		mode switching glitches.
  *
  * Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
  *		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
@@ -68,6 +77,17 @@
 #define CDA_R2		0x0A	/* RW 8bit */
 #define CMO_R		0x0B	/* RO 8bit */
 #define TIC_R		0x06	/* WO 8bit */
+/* daqcard700 modes */
+#define CMD_R3_DIFF     0x04    /* diff mode */
+
+static const struct comedi_lrange range_daq700_ai = {
+	3,
+	{
+		BIP_RANGE(10),
+		BIP_RANGE(5),
+		BIP_RANGE(2.5)
+	}
+};
 
 static int daq700_dio_insn_bits(struct comedi_device *dev,
 				struct comedi_subdevice *s,
@@ -130,20 +150,34 @@ static int daq700_ai_rinsn(struct comedi_device *dev,
 			   struct comedi_subdevice *s,
 			   struct comedi_insn *insn, unsigned int *data)
 {
-	int n, chan;
+	int n;
 	int d;
 	int ret;
+	unsigned int chan	= CR_CHAN(insn->chanspec);
+	unsigned int aref	= CR_AREF(insn->chanspec);
+	unsigned int range	= CR_RANGE(insn->chanspec);
+	unsigned int r3_bits	= 0;
+
+	/* set channel input modes */
+	if (aref == AREF_DIFF)
+		r3_bits |= CMD_R3_DIFF;
+	/* write channel mode/range */
+	if (range >= 1)
+		range++;        /* convert range to hardware value */
+	outb(r3_bits | (range & 0x03), dev->iobase + CMD_R3);
 
-	chan = CR_CHAN(insn->chanspec);
 	/* write channel to multiplexer */
 	/* set mask scan bit high to disable scanning */
-	outb(chan | 0x80, dev->iobase + CMD_R1);
+	outb((chan & 0x0f) | 0x80, dev->iobase + CMD_R1);
 
 	/* convert n samples */
 	for (n = 0; n < insn->n; n++) {
 		/* trigger conversion with out0 L to H */
 		outb(0x00, dev->iobase + CMD_R2); /* enable ADC conversions */
 		outb(0x30, dev->iobase + CMO_R); /* mode 0 out0 L, from H */
+		outb(0x00, dev->iobase + ADCLEAR_R);	/* clear the ADC FIFO */
+		/* read 16bit junk from FIFO to clear */
+		inw(dev->iobase + ADFIFO_R);
 		/* mode 1 out0 H, L to H, start conversion */
 		outb(0x32, dev->iobase + CMO_R);
 
@@ -219,11 +253,10 @@ static int daq700_auto_attach(struct comedi_device *dev,
 	/* DAQCard-700 ai */
 	s = &dev->subdevices[1];
 	s->type = COMEDI_SUBD_AI;
-	/* we support single-ended (ground)  */
-	s->subdev_flags = SDF_READABLE | SDF_GROUND;
+	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF;
 	s->n_chan = 16;
 	s->maxdata = (1 << 12) - 1;
-	s->range_table = &range_bipolar10;
+	s->range_table = &range_daq700_ai;
 	s->insn_read = daq700_ai_rinsn;
 	daq700_ai_config(dev, s);
 
@@ -260,5 +293,5 @@ module_comedi_pcmcia_driver(daq700_driver, daq700_cs_driver);
 MODULE_AUTHOR("Fred Brooks <nsaspook@nsaspook.com>");
 MODULE_DESCRIPTION(
 	"Comedi driver for National Instruments PCMCIA DAQCard-700 DIO/AI");
-MODULE_VERSION("0.2.00");
+MODULE_VERSION("0.3.00");
 MODULE_LICENSE("GPL");
-- 
1.9.2

RE: [PATCH 2/2 v2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Hartley Sweeten]

On Thursday, May 29, 2014 5:29 AM, Ian Abbott wrote:
> From: Fred Brooks <frederick.brooks@microchip.com>
>
> Add support for switching the input range and the single-ended/
> differential input mode for the AI subdevice.  We needed to clear the
> FIFO of data before the conversion to handle card mode switching
> glitches.
>
> [ Minor whitespace fixes and driver comment reformatting.  - Ian ]
>
> Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> v2: Set authorship to Fred Brooks and updated patch description as
> suggested by Dan Carpenter.
> ---
>  drivers/staging/comedi/drivers/ni_daq_700.c | 53 +++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 10 deletions(-)

Ian,

I couple nitpicks on this patch but nothing big. Ignore all of these
comments if you wish.

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

> diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
> index 55d80ef..e66c0b9 100644
> --- a/drivers/staging/comedi/drivers/ni_daq_700.c
> +++ b/drivers/staging/comedi/drivers/ni_daq_700.c
> @@ -24,21 +24,30 @@
>   *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
>   * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
>   * Status: works
> - * Updated: Wed, 19 Sep 2012 12:07:20 +0000
> + * Updated: Wed, 21 May 2014 12:07:20 +0000
>   *
>   * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
> - * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
> + * 16 channels and a analog input subdevice (1) with 16 single-ended channels
> + * or 8 differential channels, and three input ranges.
>   *
>   * Digital:  The channel 0 corresponds to the daqcard-700's output
>   * port, bit 0; channel 8 corresponds to the input port, bit 0.
>   *
>   * Digital direction configuration: channels 0-7 output, 8-15 input.
>   *
> - * Analog: The input  range is 0 to 4095 for -10 to +10 volts
> + * Analog: The input  range is 0 to 4095 with a default of -10 to +10 volts.
> + * Valid ranges:
> + *       0 for -10 to 10V bipolar
> + *       1 for -5 to 5V bipolar
> + *       2 for -2.5 to 2.5V bipolar
> + *
>   * IRQ is assigned but not used.
>   *
>   * Version 0.1	Original DIO only driver
>   * Version 0.2	DIO and basic AI analog input support on 16 se channels
> + * Version 0.3	Add SE or DIFF mode and range switching +-10,+-5,+-2.5
> + *		Clear the FIFO of data before the conversion to handle card
> + *		mode switching glitches.

Is this version information really necessary? 

>   *
>   * Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
>   *		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
> @@ -68,6 +77,17 @@
>  #define CDA_R2		0x0A	/* RW 8bit */
>  #define CMO_R		0x0B	/* RO 8bit */
>  #define TIC_R		0x06	/* WO 8bit */
> +/* daqcard700 modes */
> +#define CMD_R3_DIFF     0x04    /* diff mode */
> +
> +static const struct comedi_lrange range_daq700_ai = {
> +	3,
> +	{
> +		BIP_RANGE(10),
> +		BIP_RANGE(5),
> +		BIP_RANGE(2.5)
> +	}
> +};

The "normal" form for this in the comedi drivers is:

	3, {
 
>  static int daq700_dio_insn_bits(struct comedi_device *dev,
>  				struct comedi_subdevice *s,
> @@ -130,20 +150,34 @@ static int daq700_ai_rinsn(struct comedi_device *dev,
>  			   struct comedi_subdevice *s,
>  			   struct comedi_insn *insn, unsigned int *data)
>  {
> -	int n, chan;
> +	int n;
>  	int d;
>  	int ret;
> +	unsigned int chan	= CR_CHAN(insn->chanspec);
> +	unsigned int aref	= CR_AREF(insn->chanspec);
> +	unsigned int range	= CR_RANGE(insn->chanspec);
> +	unsigned int r3_bits	= 0;

Is the whitespace really necessary on the initialized variables?

Also, I think the uninitialized variables should be listed after the
initialized ones.

> +
> +	/* set channel input modes */
> +	if (aref == AREF_DIFF)
> +		r3_bits |= CMD_R3_DIFF;
> +	/* write channel mode/range */
> +	if (range >= 1)
> +		range++;        /* convert range to hardware value */
> +	outb(r3_bits | (range & 0x03), dev->iobase + CMD_R3);
>  
> -	chan = CR_CHAN(insn->chanspec);
>  	/* write channel to multiplexer */
>  	/* set mask scan bit high to disable scanning */
> -	outb(chan | 0x80, dev->iobase + CMD_R1);
> +	outb((chan & 0x0f) | 0x80, dev->iobase + CMD_R1);
>  
>  	/* convert n samples */
>  	for (n = 0; n < insn->n; n++) {
>  		/* trigger conversion with out0 L to H */
>  		outb(0x00, dev->iobase + CMD_R2); /* enable ADC conversions */
>  		outb(0x30, dev->iobase + CMO_R); /* mode 0 out0 L, from H */
> +		outb(0x00, dev->iobase + ADCLEAR_R);	/* clear the ADC FIFO */
> +		/* read 16bit junk from FIFO to clear */
> +		inw(dev->iobase + ADFIFO_R);
>  		/* mode 1 out0 H, L to H, start conversion */
>  		outb(0x32, dev->iobase + CMO_R);
>  
> @@ -219,11 +253,10 @@ static int daq700_auto_attach(struct comedi_device *dev,
>  	/* DAQCard-700 ai */
>  	s = &dev->subdevices[1];
>  	s->type = COMEDI_SUBD_AI;
> -	/* we support single-ended (ground)  */
> -	s->subdev_flags = SDF_READABLE | SDF_GROUND;
> +	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF;
>  	s->n_chan = 16;
>  	s->maxdata = (1 << 12) - 1;
> -	s->range_table = &range_bipolar10;
> +	s->range_table = &range_daq700_ai;
>  	s->insn_read = daq700_ai_rinsn;
>  	daq700_ai_config(dev, s);
>  
> @@ -260,5 +293,5 @@ module_comedi_pcmcia_driver(daq700_driver, daq700_cs_driver);
>  MODULE_AUTHOR("Fred Brooks <nsaspook@nsaspook.com>");
>  MODULE_DESCRIPTION(
>  	"Comedi driver for National Instruments PCMCIA DAQCard-700 DIO/AI");
> -MODULE_VERSION("0.2.00");
> +MODULE_VERSION("0.3.00");

Is the MODULE_VERSION necessary? The vmk80xx and this driver are the only
ones that have it.

>  MODULE_LICENSE("GPL");
> -- 
> 1.9.2

Regards,
Hartley

Re: [PATCH 2/2 v2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Greg Kroah-Hartman]

On Thu, May 29, 2014 at 04:47:00PM +0000, Hartley Sweeten wrote:
> On Thursday, May 29, 2014 5:29 AM, Ian Abbott wrote:
> > From: Fred Brooks <frederick.brooks@microchip.com>
> >
> > Add support for switching the input range and the single-ended/
> > differential input mode for the AI subdevice.  We needed to clear the
> > FIFO of data before the conversion to handle card mode switching
> > glitches.
> >
> > [ Minor whitespace fixes and driver comment reformatting.  - Ian ]
> >
> > Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
> > Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> > ---
> > v2: Set authorship to Fred Brooks and updated patch description as
> > suggested by Dan Carpenter.
> > ---
> >  drivers/staging/comedi/drivers/ni_daq_700.c | 53 +++++++++++++++++++++++------
> >  1 file changed, 43 insertions(+), 10 deletions(-)
> 
> Ian,
> 
> I couple nitpicks on this patch but nothing big. Ignore all of these
> comments if you wish.
> 
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> 
> > diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
> > index 55d80ef..e66c0b9 100644
> > --- a/drivers/staging/comedi/drivers/ni_daq_700.c
> > +++ b/drivers/staging/comedi/drivers/ni_daq_700.c
> > @@ -24,21 +24,30 @@
> >   *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
> >   * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
> >   * Status: works
> > - * Updated: Wed, 19 Sep 2012 12:07:20 +0000
> > + * Updated: Wed, 21 May 2014 12:07:20 +0000
> >   *
> >   * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
> > - * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
> > + * 16 channels and a analog input subdevice (1) with 16 single-ended channels
> > + * or 8 differential channels, and three input ranges.
> >   *
> >   * Digital:  The channel 0 corresponds to the daqcard-700's output
> >   * port, bit 0; channel 8 corresponds to the input port, bit 0.
> >   *
> >   * Digital direction configuration: channels 0-7 output, 8-15 input.
> >   *
> > - * Analog: The input  range is 0 to 4095 for -10 to +10 volts
> > + * Analog: The input  range is 0 to 4095 with a default of -10 to +10 volts.
> > + * Valid ranges:
> > + *       0 for -10 to 10V bipolar
> > + *       1 for -5 to 5V bipolar
> > + *       2 for -2.5 to 2.5V bipolar
> > + *
> >   * IRQ is assigned but not used.
> >   *
> >   * Version 0.1	Original DIO only driver
> >   * Version 0.2	DIO and basic AI analog input support on 16 se channels
> > + * Version 0.3	Add SE or DIFF mode and range switching +-10,+-5,+-2.5
> > + *		Clear the FIFO of data before the conversion to handle card
> > + *		mode switching glitches.
> 
> Is this version information really necessary? 

No, it should all be removed from in-kernel drivers.

> > @@ -260,5 +293,5 @@ module_comedi_pcmcia_driver(daq700_driver, daq700_cs_driver);
> >  MODULE_AUTHOR("Fred Brooks <nsaspook@nsaspook.com>");
> >  MODULE_DESCRIPTION(
> >  	"Comedi driver for National Instruments PCMCIA DAQCard-700 DIO/AI");
> > -MODULE_VERSION("0.2.00");
> > +MODULE_VERSION("0.3.00");
> 
> Is the MODULE_VERSION necessary? The vmk80xx and this driver are the only
> ones that have it.

It should be removed, it makes no sense for in-kernel modules.

I'll take this as-is, but further cleanups are always appreciated :)

thanks,

greg k-h

Re: [PATCH 2/2 v2] staging: comedi: ni_daq_700: add AI range and input mode switching

[Greg Kroah-Hartman]

On Thu, May 29, 2014 at 01:29:26PM +0100, Ian Abbott wrote:
> From: Fred Brooks <frederick.brooks@microchip.com>
> 
> Add support for switching the input range and the single-ended/
> differential input mode for the AI subdevice.  We needed to clear the
> FIFO of data before the conversion to handle card mode switching
> glitches.
> 
> [ Minor whitespace fixes and driver comment reformatting.  - Ian ]

This patch doesn't apply to the tree for some odd reason.  Care to
refresh it and resend?

thanks,

greg k-h

[PATCH 2/2 v3] staging: comedi: ni_daq_700: add AI range and input mode switching

[Ian Abbott]

From: Fred Brooks <frederick.brooks@microchip.com>

Add support for switching the input range and the single-ended/
differential input mode for the AI subdevice.  We needed to clear the
FIFO of data before the conversion to handle card mode switching
glitches.

[ Minor whitespace fixes and driver comment reformatting.  - Ian ]

Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: Set authorship to Fred Brooks and updated patch description as
suggested by Dan Carpenter.
v3: Rebased to fix merge conflict with "staging: comedi: ni_daq_700: add
mux settling delay" patch.  The channel number ('chan') doesn't need
ANDing with 0xf as it will be less than 16.
---
 drivers/staging/comedi/drivers/ni_daq_700.c | 51 ++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_daq_700.c b/drivers/staging/comedi/drivers/ni_daq_700.c
index 4eedefd..3d841d4 100644
--- a/drivers/staging/comedi/drivers/ni_daq_700.c
+++ b/drivers/staging/comedi/drivers/ni_daq_700.c
@@ -24,21 +24,30 @@
  *   based on ni_daq_dio24 by Daniel Vecino Castel <dvecino@able.es>
  * Devices: [National Instruments] PCMCIA DAQ-Card-700 (ni_daq_700)
  * Status: works
- * Updated: Wed, 19 Sep 2012 12:07:20 +0000
+ * Updated: Wed, 21 May 2014 12:07:20 +0000
  *
  * The daqcard-700 appears in Comedi as a  digital I/O subdevice (0) with
- * 16 channels and a analog input subdevice (1) with 16 single-ended channels.
+ * 16 channels and a analog input subdevice (1) with 16 single-ended channels
+ * or 8 differential channels, and three input ranges.
  *
  * Digital:  The channel 0 corresponds to the daqcard-700's output
  * port, bit 0; channel 8 corresponds to the input port, bit 0.
  *
  * Digital direction configuration: channels 0-7 output, 8-15 input.
  *
- * Analog: The input  range is 0 to 4095 for -10 to +10 volts
+ * Analog: The input  range is 0 to 4095 with a default of -10 to +10 volts.
+ * Valid ranges:
+ *       0 for -10 to 10V bipolar
+ *       1 for -5 to 5V bipolar
+ *       2 for -2.5 to 2.5V bipolar
+ *
  * IRQ is assigned but not used.
  *
  * Version 0.1	Original DIO only driver
  * Version 0.2	DIO and basic AI analog input support on 16 se channels
+ * Version 0.3	Add SE or DIFF mode and range switching +-10,+-5,+-2.5
+ *		Clear the FIFO of data before the conversion to handle card
+ *		mode switching glitches.
  *
  * Manuals:	Register level:	http://www.ni.com/pdf/manuals/340698.pdf
  *		User Manual:	http://www.ni.com/pdf/manuals/320676d.pdf
@@ -68,6 +77,17 @@
 #define CDA_R2		0x0A	/* RW 8bit */
 #define CMO_R		0x0B	/* RO 8bit */
 #define TIC_R		0x06	/* WO 8bit */
+/* daqcard700 modes */
+#define CMD_R3_DIFF     0x04    /* diff mode */
+
+static const struct comedi_lrange range_daq700_ai = {
+	3,
+	{
+		BIP_RANGE(10),
+		BIP_RANGE(5),
+		BIP_RANGE(2.5)
+	}
+};
 
 static int daq700_dio_insn_bits(struct comedi_device *dev,
 				struct comedi_subdevice *s,
@@ -130,11 +150,22 @@ static int daq700_ai_rinsn(struct comedi_device *dev,
 			   struct comedi_subdevice *s,
 			   struct comedi_insn *insn, unsigned int *data)
 {
-	int n, chan;
+	int n;
 	int d;
 	int ret;
+	unsigned int chan	= CR_CHAN(insn->chanspec);
+	unsigned int aref	= CR_AREF(insn->chanspec);
+	unsigned int range	= CR_RANGE(insn->chanspec);
+	unsigned int r3_bits	= 0;
+
+	/* set channel input modes */
+	if (aref == AREF_DIFF)
+		r3_bits |= CMD_R3_DIFF;
+	/* write channel mode/range */
+	if (range >= 1)
+		range++;        /* convert range to hardware value */
+	outb(r3_bits | (range & 0x03), dev->iobase + CMD_R3);
 
-	chan = CR_CHAN(insn->chanspec);
 	/* write channel to multiplexer */
 	/* set mask scan bit high to disable scanning */
 	outb(chan | 0x80, dev->iobase + CMD_R1);
@@ -146,6 +177,9 @@ static int daq700_ai_rinsn(struct comedi_device *dev,
 		/* trigger conversion with out0 L to H */
 		outb(0x00, dev->iobase + CMD_R2); /* enable ADC conversions */
 		outb(0x30, dev->iobase + CMO_R); /* mode 0 out0 L, from H */
+		outb(0x00, dev->iobase + ADCLEAR_R);	/* clear the ADC FIFO */
+		/* read 16bit junk from FIFO to clear */
+		inw(dev->iobase + ADFIFO_R);
 		/* mode 1 out0 H, L to H, start conversion */
 		outb(0x32, dev->iobase + CMO_R);
 
@@ -221,11 +255,10 @@ static int daq700_auto_attach(struct comedi_device *dev,
 	/* DAQCard-700 ai */
 	s = &dev->subdevices[1];
 	s->type = COMEDI_SUBD_AI;
-	/* we support single-ended (ground)  */
-	s->subdev_flags = SDF_READABLE | SDF_GROUND;
+	s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF;
 	s->n_chan = 16;
 	s->maxdata = (1 << 12) - 1;
-	s->range_table = &range_bipolar10;
+	s->range_table = &range_daq700_ai;
 	s->insn_read = daq700_ai_rinsn;
 	daq700_ai_config(dev, s);
 
@@ -262,5 +295,5 @@ module_comedi_pcmcia_driver(daq700_driver, daq700_cs_driver);
 MODULE_AUTHOR("Fred Brooks <nsaspook@nsaspook.com>");
 MODULE_DESCRIPTION(
 	"Comedi driver for National Instruments PCMCIA DAQCard-700 DIO/AI");
-MODULE_VERSION("0.2.00");
+MODULE_VERSION("0.3.00");
 MODULE_LICENSE("GPL");
-- 
2.0.0

Re: [PATCH 2/2 v3] staging: comedi: ni_daq_700: add AI range and input mode switching

[Ian Abbott]

On 2014-06-20 12:53, Ian Abbott wrote:
> From: Fred Brooks <frederick.brooks@microchip.com>
>
> Add support for switching the input range and the single-ended/
> differential input mode for the AI subdevice.  We needed to clear the
> FIFO of data before the conversion to handle card mode switching
> glitches.
>
> [ Minor whitespace fixes and driver comment reformatting.  - Ian ]
>
> Signed-off-by: Fred Brooks <frederick.brooks@microchip.com>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> v2: Set authorship to Fred Brooks and updated patch description as
> suggested by Dan Carpenter.
> v3: Rebased to fix merge conflict with "staging: comedi: ni_daq_700: add
> mux settling delay" patch.  The channel number ('chan') doesn't need
> ANDing with 0xf as it will be less than 16.

I forgot to add:

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

from his reply to the v2 patch.

I'll strip out the the module version information in a subsequent patch. 
  I don't think the minor code formatting niggles matter enough to worry 
about.

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