[PATCH 00/18] staging: comedi: amplc_pci224: remove legacy stuff

[PATCH 00/18] staging: comedi: amplc_pci224: remove legacy stuff

[Ian Abbott]

The "amplc_pci224" driver retains a "legacy" attach mechanism via the
`COMEDI_DEVCONFIG` ioctl and the comedi driver "attach" hook, but
usually attaches PCI devices automatically via the comedi driver's
"auto_attach" hook.  The legacy mechanism is only retained so that
options can be passed via the ioctl to indicate how some hardware
jumpers are set on the boards, which is used to set up the range tables.
That's no use if the PCI device is attached automatically as the range
tables will be set up according to the factory default jumper positions.

Change the range tables to include all possible ranges, regardless of
jumper positions.  Then there is no need to have options to control
setting up the range tables and the "legacy" attach mechanism can be
removed.

Also, tidy the code up a bit.

01) staging: comedi: amplc_pci224: reformat some comments
02) staging: comedi: amplc_pci224: fix checkpatch line over 80
    characters
03) staging: comedi: amplc_pci224: blank lines aren't necessary before a
    close brace '}'
04) staging: comedi: amplc_pci224: multiple assignments should be
    avoided
05) staging: comedi: amplc_pci224: fix spinlock_t definition without
    comment
06) staging: comedi: amplc_pci224: add whitespace to pci224_boards[]
07) staging: comedi: amplc_pci224: set a more descriptive
    MODULE_DESCRIPTION()
08) staging: comedi: amplc_pci224: omit '!= 0' from logical expressions
09) staging: comedi: amplc_pci224: remove some unnecessary parentheses
10) staging: comedi: amplc_pci224: reduce leading whitespace in a few
    places
11) staging: comedi: amplc_pci224: no need for '&function'
12) staging: comedi: amplc_pci224: remove options to select output
    ranges
13) staging: comedi: amplc_pci224: remove "legacy" attach mechanism
14) staging: comedi: amplc_pci224: no need to manipulate PCI ref count
15) staging: comedi: amplc_pci224: put board indices in PCI driver_data
16) staging: comedi: amplc_pci224: remove PCI_DEVICE_ID_... macros
17) staging: comedi: amplc_pci224: absorb pci224_attach_common()
18) staging: comedi: amplc_pci224: no need to comedi_set_hw_dev() here

 drivers/staging/comedi/drivers/amplc_pci224.c | 661 ++++++++++----------------
 1 file changed, 254 insertions(+), 407 deletions(-)

[PATCH 01/18] staging: comedi: amplc_pci224: reformat some comments

[Ian Abbott]

Reformat comments to fit in with the preferred coding style, including
the copyright and comedi driver description comments at the start of the
file.  Also, remove a boiler-plate comment for the comedi device private
data structure.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 214 +++++++++++++-------------
 1 file changed, 109 insertions(+), 105 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 45aba1f..72506bf 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1,102 +1,103 @@
 /*
-    comedi/drivers/amplc_pci224.c
-    Driver for Amplicon PCI224 and PCI234 AO boards.
-
-    Copyright (C) 2005 MEV Ltd. <http://www.mev.co.uk/>
-
-    COMEDI - Linux Control and Measurement Device Interface
-    Copyright (C) 1998,2000 David A. Schleef <ds@schleef.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
+ * comedi/drivers/amplc_pci224.c
+ * Driver for Amplicon PCI224 and PCI234 AO boards.
+ *
+ * Copyright (C) 2005 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1998,2000 David A. Schleef <ds@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-*/
 /*
-Driver: amplc_pci224
-Description: Amplicon PCI224, PCI234
-Author: Ian Abbott <abbotti@mev.co.uk>
-Devices: [Amplicon] PCI224 (amplc_pci224 or pci224),
-  PCI234 (amplc_pci224 or pci234)
-Updated: Wed, 22 Oct 2008 12:25:08 +0100
-Status: works, but see caveats
-
-Supports:
-
-  - ao_insn read/write
-  - ao_do_cmd mode with the following sources:
-
-    - start_src         TRIG_INT        TRIG_EXT
-    - scan_begin_src    TRIG_TIMER      TRIG_EXT
-    - convert_src       TRIG_NOW
-    - scan_end_src      TRIG_COUNT
-    - stop_src          TRIG_COUNT      TRIG_EXT        TRIG_NONE
-
-    The channel list must contain at least one channel with no repeated
-    channels.  The scan end count must equal the number of channels in
-    the channel list.
-
-    There is only one external trigger source so only one of start_src,
-    scan_begin_src or stop_src may use TRIG_EXT.
-
-Configuration options - PCI224:
-  [0] - PCI bus of device (optional).
-  [1] - PCI slot of device (optional).
-          If bus/slot is not specified, the first available PCI device
-          will be used.
-  [2] - Select available ranges according to jumper LK1.  All channels
-        are set to the same range:
-        0=Jumper position 1-2 (factory default), 4 software-selectable
-          internal voltage references, giving 4 bipolar and 4 unipolar
-          ranges:
-            [-10V,+10V], [-5V,+5V], [-2.5V,+2.5V], [-1.25V,+1.25V],
-            [0,+10V], [0,+5V], [0,+2.5V], [0,1.25V].
-        1=Jumper position 2-3, 1 external voltage reference, giving
-          1 bipolar and 1 unipolar range:
-            [-Vext,+Vext], [0,+Vext].
-
-Configuration options - PCI234:
-  [0] - PCI bus of device (optional).
-  [1] - PCI slot of device (optional).
-          If bus/slot is not specified, the first available PCI device
-          will be used.
-  [2] - Select internal or external voltage reference according to
-        jumper LK1.  This affects all channels:
-        0=Jumper position 1-2 (factory default), Vref=5V internal.
-        1=Jumper position 2-3, Vref=Vext external.
-  [3] - Select channel 0 range according to jumper LK2:
-        0=Jumper position 2-3 (factory default), range [-2*Vref,+2*Vref]
-          (10V bipolar when options[2]=0).
-        1=Jumper position 1-2, range [-Vref,+Vref]
-          (5V bipolar when options[2]=0).
-  [4] - Select channel 1 range according to jumper LK3: cf. options[3].
-  [5] - Select channel 2 range according to jumper LK4: cf. options[3].
-  [6] - Select channel 3 range according to jumper LK5: cf. options[3].
-
-Passing a zero for an option is the same as leaving it unspecified.
-
-Caveats:
-
-  1) All channels on the PCI224 share the same range.  Any change to the
-     range as a result of insn_write or a streaming command will affect
-     the output voltages of all channels, including those not specified
-     by the instruction or command.
-
-  2) For the analog output command,  the first scan may be triggered
-     falsely at the start of acquisition.  This occurs when the DAC scan
-     trigger source is switched from 'none' to 'timer' (scan_begin_src =
-     TRIG_TIMER) or 'external' (scan_begin_src == TRIG_EXT) at the start
-     of acquisition and the trigger source is at logic level 1 at the
-     time of the switch.  This is very likely for TRIG_TIMER.  For
-     TRIG_EXT, it depends on the state of the external line and whether
-     the CR_INVERT flag has been set.  The remaining scans are triggered
-     correctly.
-*/
+ * Driver: amplc_pci224
+ * Description: Amplicon PCI224, PCI234
+ * Author: Ian Abbott <abbotti@mev.co.uk>
+ * Devices: [Amplicon] PCI224 (amplc_pci224 or pci224),
+ *   PCI234 (amplc_pci224 or pci234)
+ * Updated: Wed, 22 Oct 2008 12:25:08 +0100
+ * Status: works, but see caveats
+ *
+ * Supports:
+ *
+ *   - ao_insn read/write
+ *   - ao_do_cmd mode with the following sources:
+ *
+ *     - start_src         TRIG_INT        TRIG_EXT
+ *     - scan_begin_src    TRIG_TIMER      TRIG_EXT
+ *     - convert_src       TRIG_NOW
+ *     - scan_end_src      TRIG_COUNT
+ *     - stop_src          TRIG_COUNT      TRIG_EXT        TRIG_NONE
+ *
+ *     The channel list must contain at least one channel with no repeated
+ *     channels.  The scan end count must equal the number of channels in
+ *     the channel list.
+ *
+ *     There is only one external trigger source so only one of start_src,
+ *     scan_begin_src or stop_src may use TRIG_EXT.
+ *
+ * Configuration options - PCI224:
+ *   [0] - PCI bus of device (optional).
+ *   [1] - PCI slot of device (optional).
+ *           If bus/slot is not specified, the first available PCI device
+ *           will be used.
+ *   [2] - Select available ranges according to jumper LK1.  All channels
+ *         are set to the same range:
+ *         0=Jumper position 1-2 (factory default), 4 software-selectable
+ *           internal voltage references, giving 4 bipolar and 4 unipolar
+ *           ranges:
+ *             [-10V,+10V], [-5V,+5V], [-2.5V,+2.5V], [-1.25V,+1.25V],
+ *             [0,+10V], [0,+5V], [0,+2.5V], [0,1.25V].
+ *         1=Jumper position 2-3, 1 external voltage reference, giving
+ *           1 bipolar and 1 unipolar range:
+ *             [-Vext,+Vext], [0,+Vext].
+ *
+ * Configuration options - PCI234:
+ *   [0] - PCI bus of device (optional).
+ *   [1] - PCI slot of device (optional).
+ *           If bus/slot is not specified, the first available PCI device
+ *           will be used.
+ *   [2] - Select internal or external voltage reference according to
+ *         jumper LK1.  This affects all channels:
+ *         0=Jumper position 1-2 (factory default), Vref=5V internal.
+ *         1=Jumper position 2-3, Vref=Vext external.
+ *   [3] - Select channel 0 range according to jumper LK2:
+ *         0=Jumper position 2-3 (factory default), range [-2*Vref,+2*Vref]
+ *           (10V bipolar when options[2]=0).
+ *         1=Jumper position 1-2, range [-Vref,+Vref]
+ *           (5V bipolar when options[2]=0).
+ *   [4] - Select channel 1 range according to jumper LK3: cf. options[3].
+ *   [5] - Select channel 2 range according to jumper LK4: cf. options[3].
+ *   [6] - Select channel 3 range according to jumper LK5: cf. options[3].
+ *
+ * Passing a zero for an option is the same as leaving it unspecified.
+ *
+ * Caveats:
+ *
+ *   1) All channels on the PCI224 share the same range.  Any change to the
+ *      range as a result of insn_write or a streaming command will affect
+ *      the output voltages of all channels, including those not specified
+ *      by the instruction or command.
+ *
+ *   2) For the analog output command,  the first scan may be triggered
+ *      falsely at the start of acquisition.  This occurs when the DAC scan
+ *      trigger source is switched from 'none' to 'timer' (scan_begin_src =
+ *      TRIG_TIMER) or 'external' (scan_begin_src == TRIG_EXT) at the start
+ *      of acquisition and the trigger source is at logic level 1 at the
+ *      time of the switch.  This is very likely for TRIG_TIMER.  For
+ *      TRIG_EXT, it depends on the state of the external line and whether
+ *      the CR_INVERT flag has been set.  The remaining scans are triggered
+ *      correctly.
+ */
 
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -299,16 +300,20 @@ static const unsigned short hwrange_pci224_external[2] = {
 	PCI224_DACCON_POLAR_UNI,
 };
 
-/* The hardware selectable Vref*2 external range for PCI234
- * (option[2] == 1, option[3+n] == 0). */
+/*
+ * The hardware selectable Vref*2 external range for PCI234
+ * (option[2] == 1, option[3+n] == 0).
+ */
 static const struct comedi_lrange range_pci234_ext2 = {
 	1, {
 		RANGE_ext(-2, 2)
 	}
 };
 
-/* The hardware selectable Vref external range for PCI234
- * (option[2] == 1, option[3+n] == 1). */
+/*
+ * The hardware selectable Vref external range for PCI234
+ * (option[2] == 1, option[3+n] == 1).
+ */
 static const struct comedi_lrange range_pci234_ext = {
 	1, {
 		RANGE_ext(-1, 1)
@@ -356,9 +361,6 @@ static const struct pci224_board pci224_boards[] = {
 	 },
 };
 
-/* this structure is for data unique to this hardware driver.  If
-   several hardware drivers keep similar information in this structure,
-   feel free to suggest moving the variable to the struct comedi_device struct.  */
 struct pci224_private {
 	const unsigned short *hwrange;
 	unsigned long iobase1;
@@ -428,8 +430,10 @@ pci224_ao_insn_write(struct comedi_device *dev, struct comedi_subdevice *s,
 	chan = CR_CHAN(insn->chanspec);
 	range = CR_RANGE(insn->chanspec);
 
-	/* Writing a list of values to an AO channel is probably not
-	 * very useful, but that's how the interface is defined. */
+	/*
+	 * Writing a list of values to an AO channel is probably not
+	 * very useful, but that's how the interface is defined.
+	 */
 	for (i = 0; i < insn->n; i++)
 		pci224_ao_set_data(dev, chan, range, data[i]);
 
-- 
2.0.0

[PATCH 02/18] staging: comedi: amplc_pci224: fix checkpatch line over 80 characters

[Ian Abbott]

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 72506bf..6a570ef 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -565,7 +565,8 @@ static void pci224_ao_handle_fifo(struct comedi_device *dev,
 	switch (dacstat & PCI224_DACCON_FIFOFL_MASK) {
 	case PCI224_DACCON_FIFOFL_EMPTY:
 		room = PCI224_FIFO_ROOM_EMPTY;
-		if (cmd->stop_src == TRIG_COUNT && devpriv->ao_stop_count == 0) {
+		if (cmd->stop_src == TRIG_COUNT &&
+		    devpriv->ao_stop_count == 0) {
 			/* FIFO empty at end of counted acquisition. */
 			s->async->events |= COMEDI_CB_EOA;
 			cfc_handle_events(dev, s);
-- 
2.0.0

[PATCH 03/18] staging: comedi: amplc_pci224: blank lines aren't necessary before a close brace '}'

[Ian Abbott]

Fix checkpatch issues: "CHECK: Blank lines aren't necessary before a
close brace '}'".

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 6a570ef..e34bc5e 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -557,7 +557,6 @@ static void pci224_ao_handle_fifo(struct comedi_device *dev,
 		/* Fixed number of scans. */
 		if (num_scans > devpriv->ao_stop_count)
 			num_scans = devpriv->ao_stop_count;
-
 	}
 
 	/* Determine how much room is in the FIFO (in samples). */
@@ -644,7 +643,6 @@ static void pci224_ao_handle_fifo(struct comedi_device *dev,
 				trig = PCI224_DACCON_TRIG_EXTN;
 			else
 				trig = PCI224_DACCON_TRIG_EXTP;
-
 		}
 		devpriv->daccon = COMBINE(devpriv->daccon, trig,
 					  PCI224_DACCON_TRIG_MASK);
@@ -908,7 +906,6 @@ static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		for (j = 0; j < cmd->chanlist_len; j++) {
 			if (CR_CHAN(cmd->chanlist[j]) < ch)
 				rank++;
-
 		}
 		devpriv->ao_scan_order[rank] = i;
 	}
@@ -1002,7 +999,6 @@ pci224_ao_munge(struct comedi_device *dev, struct comedi_subdevice *s,
 	/* Munge the data. */
 	for (i = 0; i < length; i++)
 		array[i] = (array[i] << shift) - offset;
-
 }
 
 /*
@@ -1038,11 +1034,9 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
 					pci224_ao_start(dev, s);
 				else if (cmd->stop_src == TRIG_EXT)
 					pci224_ao_stop(dev, s);
-
 			}
 			if (valid_intstat & PCI224_INTR_DAC)
 				pci224_ao_handle_fifo(dev, s);
-
 		}
 		/* Reenable interrupt sources. */
 		spin_lock_irqsave(&devpriv->ao_spinlock, flags);
-- 
2.0.0

[PATCH 04/18] staging: comedi: amplc_pci224: multiple assignments should be avoided

[Ian Abbott]

Fix checkpatch issue: "CHECK: multiple assignments should be avoided".

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index e34bc5e..8e5d94a 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1192,11 +1192,12 @@ static int pci224_attach_common(struct comedi_device *dev,
 		/* PCI234 range options. */
 		const struct comedi_lrange **range_table_list;
 
-		s->range_table_list = range_table_list =
+		range_table_list =
 		    kmalloc(sizeof(struct comedi_lrange *) * s->n_chan,
 			    GFP_KERNEL);
-		if (!s->range_table_list)
+		if (!range_table_list)
 			return -ENOMEM;
+		s->range_table_list = range_table_list;
 
 		if (options) {
 			for (n = 2; n < 3 + s->n_chan; n++) {
-- 
2.0.0

[PATCH 05/18] staging: comedi: amplc_pci224: fix spinlock_t definition without comment

[Ian Abbott]

Fix checkpatch issue: "CHECK: spinlock_t definition without comment".

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 8e5d94a..dfeb70c 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -365,7 +365,7 @@ struct pci224_private {
 	const unsigned short *hwrange;
 	unsigned long iobase1;
 	unsigned long state;
-	spinlock_t ao_spinlock;
+	spinlock_t ao_spinlock;	/* spinlock for AO command handling */
 	unsigned int *ao_readback;
 	unsigned short *ao_scan_vals;
 	unsigned char *ao_scan_order;
-- 
2.0.0

[PATCH 06/18] staging: comedi: amplc_pci224: add whitespace to pci224_boards[]

[Ian Abbott]

Add a bit of whitespace to the initializer of `pci224_boards[]` for
aesthetic reasons.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index dfeb70c..8ce0f4f 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -341,24 +341,24 @@ struct pci224_board {
 
 static const struct pci224_board pci224_boards[] = {
 	{
-	 .name = "pci224",
-	 .devid = PCI_DEVICE_ID_AMPLICON_PCI224,
-	 .model = pci224_model,
-	 .ao_chans = 16,
-	 .ao_bits = 12,
-	 },
+		.name		= "pci224",
+		.devid		= PCI_DEVICE_ID_AMPLICON_PCI224,
+		.model		= pci224_model,
+		.ao_chans	= 16,
+		.ao_bits	= 12,
+	},
 	{
-	 .name = "pci234",
-	 .devid = PCI_DEVICE_ID_AMPLICON_PCI234,
-	 .model = pci234_model,
-	 .ao_chans = 4,
-	 .ao_bits = 16,
-	 },
+		.name		= "pci234",
+		.devid		= PCI_DEVICE_ID_AMPLICON_PCI234,
+		.model		= pci234_model,
+		.ao_chans	= 4,
+		.ao_bits	= 16,
+	},
 	{
-	 .name = "amplc_pci224",
-	 .devid = PCI_DEVICE_ID_INVALID,
-	 .model = any_model,	/* wildcard */
-	 },
+		.name		= "amplc_pci224",
+		.devid		= PCI_DEVICE_ID_INVALID,
+		.model		= any_model,	/* wildcard */
+	},
 };
 
 struct pci224_private {
-- 
2.0.0

[PATCH 07/18] staging: comedi: amplc_pci224: set a more descriptive MODULE_DESCRIPTION()

[Ian Abbott]

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 8ce0f4f..f786237 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1362,5 +1362,5 @@ static struct pci_driver amplc_pci224_pci_driver = {
 module_comedi_pci_driver(amplc_pci224_driver, amplc_pci224_pci_driver);
 
 MODULE_AUTHOR("Comedi http://www.comedi.org");
-MODULE_DESCRIPTION("Comedi low-level driver");
+MODULE_DESCRIPTION("Comedi driver for Amplicon PCI224 and PCI234 AO boards");
 MODULE_LICENSE("GPL");
-- 
2.0.0

[PATCH 08/18] staging: comedi: amplc_pci224: omit '!= 0' from logical expressions

[Ian Abbott]

Since anything non-zero is logically "true", don't bother doing
"not-equal" comparisons with zero, except when testing for an explicit
number 0 (not as a result of bit tests for example).

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index f786237..0ac05fb 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -759,13 +759,13 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 		break;
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
-		if ((cmd->start_arg & ~CR_FLAGS_MASK) != 0) {
+		if (cmd->start_arg & ~CR_FLAGS_MASK) {
 			cmd->start_arg = COMBINE(cmd->start_arg, 0,
 						 ~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* The only flag allowed is CR_EDGE, which is ignored. */
-		if ((cmd->start_arg & CR_FLAGS_MASK & ~CR_EDGE) != 0) {
+		if (cmd->start_arg & CR_FLAGS_MASK & ~CR_EDGE) {
 			cmd->start_arg = COMBINE(cmd->start_arg, 0,
 						 CR_FLAGS_MASK & ~CR_EDGE);
 			err |= -EINVAL;
@@ -785,14 +785,14 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 		break;
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
-		if ((cmd->scan_begin_arg & ~CR_FLAGS_MASK) != 0) {
+		if (cmd->scan_begin_arg & ~CR_FLAGS_MASK) {
 			cmd->scan_begin_arg = COMBINE(cmd->scan_begin_arg, 0,
 						      ~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* Only allow flags CR_EDGE and CR_INVERT.  Ignore CR_EDGE. */
-		if ((cmd->scan_begin_arg & CR_FLAGS_MASK &
-		     ~(CR_EDGE | CR_INVERT)) != 0) {
+		if (cmd->scan_begin_arg & CR_FLAGS_MASK &
+		    ~(CR_EDGE | CR_INVERT)) {
 			cmd->scan_begin_arg = COMBINE(cmd->scan_begin_arg, 0,
 						      CR_FLAGS_MASK &
 						      ~(CR_EDGE | CR_INVERT));
@@ -810,13 +810,13 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 		break;
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
-		if ((cmd->stop_arg & ~CR_FLAGS_MASK) != 0) {
+		if (cmd->stop_arg & ~CR_FLAGS_MASK) {
 			cmd->stop_arg = COMBINE(cmd->stop_arg, 0,
 						~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* The only flag allowed is CR_EDGE, which is ignored. */
-		if ((cmd->stop_arg & CR_FLAGS_MASK & ~CR_EDGE) != 0) {
+		if (cmd->stop_arg & CR_FLAGS_MASK & ~CR_EDGE) {
 			cmd->stop_arg = COMBINE(cmd->stop_arg, 0,
 						CR_FLAGS_MASK & ~CR_EDGE);
 		}
@@ -1026,7 +1026,7 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
 		devpriv->intr_running = 1;
 		devpriv->intr_cpuid = THISCPU;
 		spin_unlock_irqrestore(&devpriv->ao_spinlock, flags);
-		if (valid_intstat != 0) {
+		if (valid_intstat) {
 			cmd = &s->async->cmd;
 			if (valid_intstat & PCI224_INTR_EXT) {
 				devpriv->intsce &= ~PCI224_INTR_EXT;
-- 
2.0.0

[PATCH 09/18] staging: comedi: amplc_pci224: remove some unnecessary parentheses

[Ian Abbott]

Remove some pairs of parentheses that don't really improve readability.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 0ac05fb..a7f3454 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -396,8 +396,8 @@ pci224_ao_set_data(struct comedi_device *dev, int chan, int range,
 	outw(1 << chan, dev->iobase + PCI224_DACCEN);
 	/* Set range and reset FIFO. */
 	devpriv->daccon = COMBINE(devpriv->daccon, devpriv->hwrange[range],
-				  (PCI224_DACCON_POLAR_MASK |
-				   PCI224_DACCON_VREF_MASK));
+				  PCI224_DACCON_POLAR_MASK |
+				  PCI224_DACCON_VREF_MASK);
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
 	     dev->iobase + PCI224_DACCON);
 	/*
@@ -685,7 +685,7 @@ static int pci224_ao_check_chanlist(struct comedi_device *dev,
 				__func__);
 			return -EINVAL;
 		}
-		chan_mask |= (1 << chan);
+		chan_mask |= 1 << chan;
 
 		if (range != range0) {
 			dev_dbg(dev->class_dev,
@@ -925,13 +925,13 @@ static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 * N.B. DAC FIFO interrupts are currently disabled.
 	 */
 	devpriv->daccon = COMBINE(devpriv->daccon,
-				  (devpriv->
-				   hwrange[range] | PCI224_DACCON_TRIG_NONE |
-				   PCI224_DACCON_FIFOINTR_NHALF),
-				  (PCI224_DACCON_POLAR_MASK |
-				   PCI224_DACCON_VREF_MASK |
-				   PCI224_DACCON_TRIG_MASK |
-				   PCI224_DACCON_FIFOINTR_MASK));
+				  devpriv->hwrange[range] |
+				  PCI224_DACCON_TRIG_NONE |
+				  PCI224_DACCON_FIFOINTR_NHALF,
+				  PCI224_DACCON_POLAR_MASK |
+				  PCI224_DACCON_VREF_MASK |
+				  PCI224_DACCON_TRIG_MASK |
+				  PCI224_DACCON_FIFOINTR_MASK);
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
 	     dev->iobase + PCI224_DACCON);
 
@@ -1161,9 +1161,8 @@ static int pci224_attach_common(struct comedi_device *dev,
 	outw(PCI224_DACCON_GLOBALRESET, dev->iobase + PCI224_DACCON);
 	outw(0, dev->iobase + PCI224_DACCEN);
 	outw(0, dev->iobase + PCI224_FIFOSIZ);
-	devpriv->daccon = (PCI224_DACCON_TRIG_SW | PCI224_DACCON_POLAR_BI |
-			   PCI224_DACCON_FIFOENAB |
-			   PCI224_DACCON_FIFOINTR_EMPTY);
+	devpriv->daccon = PCI224_DACCON_TRIG_SW | PCI224_DACCON_POLAR_BI |
+			  PCI224_DACCON_FIFOENAB | PCI224_DACCON_FIFOINTR_EMPTY;
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
 	     dev->iobase + PCI224_DACCON);
 
-- 
2.0.0

[PATCH 10/18] staging: comedi: amplc_pci224: reduce leading whitespace in a few places

[Ian Abbott]

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index a7f3454..68fc407 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -500,11 +500,10 @@ static void pci224_ao_stop(struct comedi_device *dev,
 	spin_unlock_irqrestore(&devpriv->ao_spinlock, flags);
 	/* Reconfigure DAC for insn_write usage. */
 	outw(0, dev->iobase + PCI224_DACCEN);	/* Disable channels. */
-	devpriv->daccon = COMBINE(devpriv->daccon,
-				  PCI224_DACCON_TRIG_SW |
-				  PCI224_DACCON_FIFOINTR_EMPTY,
-				  PCI224_DACCON_TRIG_MASK |
-				  PCI224_DACCON_FIFOINTR_MASK);
+	devpriv->daccon =
+	     COMBINE(devpriv->daccon,
+		     PCI224_DACCON_TRIG_SW | PCI224_DACCON_FIFOINTR_EMPTY,
+		     PCI224_DACCON_TRIG_MASK | PCI224_DACCON_FIFOINTR_MASK);
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
 	     dev->iobase + PCI224_DACCON);
 }
@@ -644,8 +643,8 @@ static void pci224_ao_handle_fifo(struct comedi_device *dev,
 			else
 				trig = PCI224_DACCON_TRIG_EXTP;
 		}
-		devpriv->daccon = COMBINE(devpriv->daccon, trig,
-					  PCI224_DACCON_TRIG_MASK);
+		devpriv->daccon =
+		    COMBINE(devpriv->daccon, trig, PCI224_DACCON_TRIG_MASK);
 		outw(devpriv->daccon, dev->iobase + PCI224_DACCON);
 	}
 
@@ -717,11 +716,11 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 
 	err |= cfc_check_trigger_src(&cmd->start_src, TRIG_INT | TRIG_EXT);
 	err |= cfc_check_trigger_src(&cmd->scan_begin_src,
-					TRIG_EXT | TRIG_TIMER);
+				     TRIG_EXT | TRIG_TIMER);
 	err |= cfc_check_trigger_src(&cmd->convert_src, TRIG_NOW);
 	err |= cfc_check_trigger_src(&cmd->scan_end_src, TRIG_COUNT);
 	err |= cfc_check_trigger_src(&cmd->stop_src,
-					TRIG_COUNT | TRIG_EXT | TRIG_NONE);
+				     TRIG_COUNT | TRIG_EXT | TRIG_NONE);
 
 	if (err)
 		return 1;
@@ -760,8 +759,8 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
 		if (cmd->start_arg & ~CR_FLAGS_MASK) {
-			cmd->start_arg = COMBINE(cmd->start_arg, 0,
-						 ~CR_FLAGS_MASK);
+			cmd->start_arg =
+			    COMBINE(cmd->start_arg, 0, ~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* The only flag allowed is CR_EDGE, which is ignored. */
@@ -786,16 +785,16 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
 		if (cmd->scan_begin_arg & ~CR_FLAGS_MASK) {
-			cmd->scan_begin_arg = COMBINE(cmd->scan_begin_arg, 0,
-						      ~CR_FLAGS_MASK);
+			cmd->scan_begin_arg =
+			    COMBINE(cmd->scan_begin_arg, 0, ~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* Only allow flags CR_EDGE and CR_INVERT.  Ignore CR_EDGE. */
 		if (cmd->scan_begin_arg & CR_FLAGS_MASK &
 		    ~(CR_EDGE | CR_INVERT)) {
-			cmd->scan_begin_arg = COMBINE(cmd->scan_begin_arg, 0,
-						      CR_FLAGS_MASK &
-						      ~(CR_EDGE | CR_INVERT));
+			cmd->scan_begin_arg =
+			    COMBINE(cmd->scan_begin_arg, 0,
+				    CR_FLAGS_MASK & ~(CR_EDGE | CR_INVERT));
 			err |= -EINVAL;
 		}
 		break;
@@ -811,14 +810,14 @@ pci224_ao_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s,
 	case TRIG_EXT:
 		/* Force to external trigger 0. */
 		if (cmd->stop_arg & ~CR_FLAGS_MASK) {
-			cmd->stop_arg = COMBINE(cmd->stop_arg, 0,
-						~CR_FLAGS_MASK);
+			cmd->stop_arg =
+			    COMBINE(cmd->stop_arg, 0, ~CR_FLAGS_MASK);
 			err |= -EINVAL;
 		}
 		/* The only flag allowed is CR_EDGE, which is ignored. */
 		if (cmd->stop_arg & CR_FLAGS_MASK & ~CR_EDGE) {
-			cmd->stop_arg = COMBINE(cmd->stop_arg, 0,
-						CR_FLAGS_MASK & ~CR_EDGE);
+			cmd->stop_arg =
+			    COMBINE(cmd->stop_arg, 0, CR_FLAGS_MASK & ~CR_EDGE);
 		}
 		break;
 	case TRIG_NONE:
@@ -924,14 +923,12 @@ static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 *
 	 * N.B. DAC FIFO interrupts are currently disabled.
 	 */
-	devpriv->daccon = COMBINE(devpriv->daccon,
-				  devpriv->hwrange[range] |
-				  PCI224_DACCON_TRIG_NONE |
-				  PCI224_DACCON_FIFOINTR_NHALF,
-				  PCI224_DACCON_POLAR_MASK |
-				  PCI224_DACCON_VREF_MASK |
-				  PCI224_DACCON_TRIG_MASK |
-				  PCI224_DACCON_FIFOINTR_MASK);
+	devpriv->daccon =
+	    COMBINE(devpriv->daccon,
+		    devpriv->hwrange[range] | PCI224_DACCON_TRIG_NONE |
+		    PCI224_DACCON_FIFOINTR_NHALF,
+		    PCI224_DACCON_POLAR_MASK | PCI224_DACCON_VREF_MASK |
+		    PCI224_DACCON_TRIG_MASK | PCI224_DACCON_FIFOINTR_MASK);
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
 	     dev->iobase + PCI224_DACCON);
 
-- 
2.0.0

[PATCH 11/18] staging: comedi: amplc_pci224: no need for '&function'

[Ian Abbott]

Remove the "address-of" operator when the operand is a function.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 68fc407..b8b86ab 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1173,15 +1173,14 @@ static int pci224_attach_common(struct comedi_device *dev,
 	s->subdev_flags = SDF_WRITABLE | SDF_GROUND | SDF_CMD_WRITE;
 	s->n_chan = thisboard->ao_chans;
 	s->maxdata = (1 << thisboard->ao_bits) - 1;
-	s->insn_write = &pci224_ao_insn_write;
-	s->insn_read = &pci224_ao_insn_read;
+	s->insn_write = pci224_ao_insn_write;
+	s->insn_read = pci224_ao_insn_read;
 	s->len_chanlist = s->n_chan;
-
 	dev->write_subdev = s;
-	s->do_cmd = &pci224_ao_cmd;
-	s->do_cmdtest = &pci224_ao_cmdtest;
-	s->cancel = &pci224_ao_cancel;
-	s->munge = &pci224_ao_munge;
+	s->do_cmd = pci224_ao_cmd;
+	s->do_cmdtest = pci224_ao_cmdtest;
+	s->cancel = pci224_ao_cancel;
+	s->munge = pci224_ao_munge;
 
 	/* Sort out channel range options. */
 	if (thisboard->model == pci234_model) {
-- 
2.0.0

[PATCH 12/18] staging: comedi: amplc_pci224: remove options to select output ranges

[Ian Abbott]

When attaching a PCI224 or PCI234 manually via the `COMEDI_DEVCONFIG`
ioctl, there are several options the user can supply that describe the
state of the hardware jumpers (LK1 for PCI224, LK1 thru LK5 for PCI234).
These options control how the driver sets up the AO range tables for the
device.  Those options are useless when the board is attached
automatically via the PCI driver probe function
`amplc_pci225_pci_probe()`, `comedi_pci_auto_config()`, and the
comedi driver "auto_attach" handler `pci224_auto_attach()`.

Rip out the range table selection options and use a single, static range
table per board type, containing all the software- and
hardware-selectable ranges for that board.  The PCI234 used to have a
per-channel `range_table_list` rather than an all-channel `range_table`,
as the jumpers selected different ranges for all channels.  Now that the
channels are using a unified range table, use an all-channel
`range_table` instead.

When checking the channel list for an asynchronous command in
`pci224_ao_check_chanlist()` make sure the ranges specified in the list
have compatible jumper settings.  We don't know how the jumpers are
actually set, but we can at least avoid conflicting settings.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 243 +++++++++++---------------
 1 file changed, 106 insertions(+), 137 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index b8b86ab..fba0198 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -24,7 +24,7 @@
  * Author: Ian Abbott <abbotti@mev.co.uk>
  * Devices: [Amplicon] PCI224 (amplc_pci224 or pci224),
  *   PCI234 (amplc_pci224 or pci234)
- * Updated: Wed, 22 Oct 2008 12:25:08 +0100
+ * Updated: Wed, 30 Jul 2014 18:08:43 +0000
  * Status: works, but see caveats
  *
  * Supports:
@@ -45,42 +45,48 @@
  *     There is only one external trigger source so only one of start_src,
  *     scan_begin_src or stop_src may use TRIG_EXT.
  *
- * Configuration options - PCI224:
+ * Configuration options:
  *   [0] - PCI bus of device (optional).
  *   [1] - PCI slot of device (optional).
  *           If bus/slot is not specified, the first available PCI device
  *           will be used.
- *   [2] - Select available ranges according to jumper LK1.  All channels
- *         are set to the same range:
- *         0=Jumper position 1-2 (factory default), 4 software-selectable
- *           internal voltage references, giving 4 bipolar and 4 unipolar
- *           ranges:
- *             [-10V,+10V], [-5V,+5V], [-2.5V,+2.5V], [-1.25V,+1.25V],
- *             [0,+10V], [0,+5V], [0,+2.5V], [0,1.25V].
- *         1=Jumper position 2-3, 1 external voltage reference, giving
- *           1 bipolar and 1 unipolar range:
- *             [-Vext,+Vext], [0,+Vext].
- *
- * Configuration options - PCI234:
- *   [0] - PCI bus of device (optional).
- *   [1] - PCI slot of device (optional).
- *           If bus/slot is not specified, the first available PCI device
- *           will be used.
- *   [2] - Select internal or external voltage reference according to
- *         jumper LK1.  This affects all channels:
- *         0=Jumper position 1-2 (factory default), Vref=5V internal.
- *         1=Jumper position 2-3, Vref=Vext external.
- *   [3] - Select channel 0 range according to jumper LK2:
- *         0=Jumper position 2-3 (factory default), range [-2*Vref,+2*Vref]
- *           (10V bipolar when options[2]=0).
- *         1=Jumper position 1-2, range [-Vref,+Vref]
- *           (5V bipolar when options[2]=0).
- *   [4] - Select channel 1 range according to jumper LK3: cf. options[3].
- *   [5] - Select channel 2 range according to jumper LK4: cf. options[3].
- *   [6] - Select channel 3 range according to jumper LK5: cf. options[3].
  *
  * Passing a zero for an option is the same as leaving it unspecified.
  *
+ * Output range selection - PCI224:
+ *
+ *   Output ranges on PCI224 are partly software-selectable and partly
+ *   hardware-selectable according to jumper LK1.  All channels are set
+ *   to the same range:
+ *
+ *   - LK1 position 1-2 (factory default) corresponds to the following
+ *     comedi ranges:
+ *
+ *       0: [-10V,+10V]; 1: [-5V,+5V]; 2: [-2.5V,+2.5V], 3: [-1.25V,+1.25V],
+ *       4: [0,+10V],    5: [0,+5V],   6: [0,+2.5V],     7: [0,+1.25V]
+ *
+ *   - LK1 position 2-3 corresponds to the following Comedi ranges, using
+ *     an external voltage reference:
+ *
+ *       0: [-Vext,+Vext],
+ *       1: [0,+Vext]
+ *
+ * Output range selection - PCI234:
+ *
+ *   Output ranges on PCI234 are hardware-selectable according to jumper
+ *   LK1 which affects all channels, and jumpers LK2, LK3, LK4 and LK5
+ *   which affect channels 0, 1, 2 and 3 individually.  LK1 chooses between
+ *   an internal 5V reference and an external voltage reference (Vext).
+ *   LK2/3/4/5 choose (per channel) to double the reference or not according
+ *   to the following table:
+ *
+ *     LK1 position   LK2/3/4/5 pos  Comedi range
+ *     -------------  -------------  --------------
+ *     2-3 (factory)  1-2 (factory)  0: [-10V,+10V]
+ *     2-3 (factory)  2-3            1: [-5V,+5V]
+ *     1-2            1-2 (factory)  2: [-2*Vext,+2*Vext]
+ *     1-2            2-3            3: [-Vext,+Vext]
+ *
  * Caveats:
  *
  *   1) All channels on the PCI224 share the same range.  Any change to the
@@ -262,9 +268,17 @@
  * Range tables.
  */
 
-/* The software selectable internal ranges for PCI224 (option[2] == 0). */
-static const struct comedi_lrange range_pci224_internal = {
-	8, {
+/*
+ * The ranges for PCI224.
+ *
+ * These are partly hardware-selectable by jumper LK1 and partly
+ * software-selectable.
+ *
+ * All channels share the same hardware range.
+ */
+static const struct comedi_lrange range_pci224 = {
+	10, {
+		/* jumper LK1 in position 1-2 (factory default) */
 		BIP_RANGE(10),
 		BIP_RANGE(5),
 		BIP_RANGE(2.5),
@@ -272,11 +286,15 @@ static const struct comedi_lrange range_pci224_internal = {
 		UNI_RANGE(10),
 		UNI_RANGE(5),
 		UNI_RANGE(2.5),
-		UNI_RANGE(1.25)
+		UNI_RANGE(1.25),
+		/* jumper LK1 in position 2-3 */
+		RANGE_ext(-1, 1),	/* bipolar [-Vext,+Vext] */
+		RANGE_ext(0, 1),	/* unipolar [0,+Vext] */
 	}
 };
 
-static const unsigned short hwrange_pci224_internal[8] = {
+static const unsigned short hwrange_pci224[10] = {
+	/* jumper LK1 in position 1-2 (factory default) */
 	PCI224_DACCON_POLAR_BI | PCI224_DACCON_VREF_10,
 	PCI224_DACCON_POLAR_BI | PCI224_DACCON_VREF_5,
 	PCI224_DACCON_POLAR_BI | PCI224_DACCON_VREF_2_5,
@@ -285,44 +303,47 @@ static const unsigned short hwrange_pci224_internal[8] = {
 	PCI224_DACCON_POLAR_UNI | PCI224_DACCON_VREF_5,
 	PCI224_DACCON_POLAR_UNI | PCI224_DACCON_VREF_2_5,
 	PCI224_DACCON_POLAR_UNI | PCI224_DACCON_VREF_1_25,
-};
-
-/* The software selectable external ranges for PCI224 (option[2] == 1). */
-static const struct comedi_lrange range_pci224_external = {
-	2, {
-		RANGE_ext(-1, 1),	/* bipolar [-Vref,+Vref] */
-		RANGE_ext(0, 1)		/* unipolar [0,+Vref] */
-	}
-};
-
-static const unsigned short hwrange_pci224_external[2] = {
+	/* jumper LK1 in position 2-3 */
 	PCI224_DACCON_POLAR_BI,
 	PCI224_DACCON_POLAR_UNI,
 };
 
-/*
- * The hardware selectable Vref*2 external range for PCI234
- * (option[2] == 1, option[3+n] == 0).
- */
-static const struct comedi_lrange range_pci234_ext2 = {
-	1, {
-		RANGE_ext(-2, 2)
-	}
+/* Used to check all channels set to the same range on PCI224. */
+static const unsigned char range_check_pci224[10] = {
+	0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
 };
 
 /*
- * The hardware selectable Vref external range for PCI234
- * (option[2] == 1, option[3+n] == 1).
+ * The ranges for PCI234.
+ *
+ * These are all hardware-selectable by jumper LK1 affecting all channels,
+ * and jumpers LK2, LK3, LK4 and LK5 affecting channels 0, 1, 2 and 3
+ * individually.
  */
-static const struct comedi_lrange range_pci234_ext = {
-	1, {
-		RANGE_ext(-1, 1)
+static const struct comedi_lrange range_pci234 = {
+	4, {
+		/* LK1: 1-2 (fact def), LK2/3/4/5: 2-3 (fac def) */
+		BIP_RANGE(10),
+		/* LK1: 1-2 (fact def), LK2/3/4/5: 1-2 */
+		BIP_RANGE(5),
+		/* LK1: 2-3, LK2/3/4/5: 2-3 (fac def) */
+		RANGE_ext(-2, 2),	/* bipolar [-2*Vext,+2*Vext] */
+		/* LK1: 2-3, LK2/3/4/5: 1-2 */
+		RANGE_ext(-1, 1),	/* bipolar [-Vext,+Vext] */
 	}
 };
 
-/* This serves for all the PCI234 ranges. */
-static const unsigned short hwrange_pci234[1] = {
-	PCI224_DACCON_POLAR_BI,	/* bipolar - hardware ignores it! */
+/* N.B. PCI234 ignores the polarity bit, but software uses it. */
+static const unsigned short hwrange_pci234[4] = {
+	PCI224_DACCON_POLAR_BI,
+	PCI224_DACCON_POLAR_BI,
+	PCI224_DACCON_POLAR_BI,
+	PCI224_DACCON_POLAR_BI,
+};
+
+/* Used to check all channels use same LK1 setting on PCI234. */
+static const unsigned char range_check_pci234[4] = {
+	0, 0, 1, 1,
 };
 
 /*
@@ -337,6 +358,9 @@ struct pci224_board {
 	enum pci224_model model;
 	unsigned int ao_chans;
 	unsigned int ao_bits;
+	const struct comedi_lrange *ao_range;
+	const unsigned short *ao_hwrange;
+	const unsigned char *ao_range_check;
 };
 
 static const struct pci224_board pci224_boards[] = {
@@ -346,6 +370,9 @@ static const struct pci224_board pci224_boards[] = {
 		.model		= pci224_model,
 		.ao_chans	= 16,
 		.ao_bits	= 12,
+		.ao_range	= &range_pci224,
+		.ao_hwrange	= &hwrange_pci224[0],
+		.ao_range_check	= &range_check_pci224[0],
 	},
 	{
 		.name		= "pci234",
@@ -353,6 +380,9 @@ static const struct pci224_board pci224_boards[] = {
 		.model		= pci234_model,
 		.ao_chans	= 4,
 		.ao_bits	= 16,
+		.ao_range	= &range_pci234,
+		.ao_hwrange	= &hwrange_pci234[0],
+		.ao_range_check	= &range_check_pci234[0],
 	},
 	{
 		.name		= "amplc_pci224",
@@ -362,7 +392,6 @@ static const struct pci224_board pci224_boards[] = {
 };
 
 struct pci224_private {
-	const unsigned short *hwrange;
 	unsigned long iobase1;
 	unsigned long state;
 	spinlock_t ao_spinlock;	/* spinlock for AO command handling */
@@ -395,7 +424,7 @@ pci224_ao_set_data(struct comedi_device *dev, int chan, int range,
 	/* Enable the channel. */
 	outw(1 << chan, dev->iobase + PCI224_DACCEN);
 	/* Set range and reset FIFO. */
-	devpriv->daccon = COMBINE(devpriv->daccon, devpriv->hwrange[range],
+	devpriv->daccon = COMBINE(devpriv->daccon, thisboard->ao_hwrange[range],
 				  PCI224_DACCON_POLAR_MASK |
 				  PCI224_DACCON_VREF_MASK);
 	outw(devpriv->daccon | PCI224_DACCON_FIFORESET,
@@ -670,13 +699,14 @@ static int pci224_ao_check_chanlist(struct comedi_device *dev,
 				    struct comedi_subdevice *s,
 				    struct comedi_cmd *cmd)
 {
-	unsigned int range0 = CR_RANGE(cmd->chanlist[0]);
+	const struct pci224_board *thisboard = comedi_board(dev);
+	unsigned int range_check_0;
 	unsigned int chan_mask = 0;
 	int i;
 
+	range_check_0 = thisboard->ao_range_check[CR_RANGE(cmd->chanlist[0])];
 	for (i = 0; i < cmd->chanlist_len; i++) {
 		unsigned int chan = CR_CHAN(cmd->chanlist[i]);
-		unsigned int range = CR_RANGE(cmd->chanlist[i]);
 
 		if (chan_mask & (1 << chan)) {
 			dev_dbg(dev->class_dev,
@@ -686,9 +716,10 @@ static int pci224_ao_check_chanlist(struct comedi_device *dev,
 		}
 		chan_mask |= 1 << chan;
 
-		if (range != range0) {
+		if (thisboard->ao_range_check[CR_RANGE(cmd->chanlist[i])] !=
+		    range_check_0) {
 			dev_dbg(dev->class_dev,
-				"%s: entries in chanlist must all have the same range index\n",
+				"%s: entries in chanlist have incompatible ranges\n",
 				__func__);
 			return -EINVAL;
 		}
@@ -882,6 +913,7 @@ static void pci224_ao_start_pacer(struct comedi_device *dev,
 
 static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 {
+	const struct pci224_board *thisboard = comedi_board(dev);
 	struct pci224_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
 	int range;
@@ -925,7 +957,7 @@ static int pci224_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 */
 	devpriv->daccon =
 	    COMBINE(devpriv->daccon,
-		    devpriv->hwrange[range] | PCI224_DACCON_TRIG_NONE |
+		    thisboard->ao_hwrange[range] | PCI224_DACCON_TRIG_NONE |
 		    PCI224_DACCON_FIFOINTR_NHALF,
 		    PCI224_DACCON_POLAR_MASK | PCI224_DACCON_VREF_MASK |
 		    PCI224_DACCON_TRIG_MASK | PCI224_DACCON_FIFOINTR_MASK);
@@ -974,7 +1006,6 @@ pci224_ao_munge(struct comedi_device *dev, struct comedi_subdevice *s,
 		void *data, unsigned int num_bytes, unsigned int chan_index)
 {
 	const struct pci224_board *thisboard = comedi_board(dev);
-	struct pci224_private *devpriv = dev->private;
 	struct comedi_cmd *cmd = &s->async->cmd;
 	unsigned short *array = data;
 	unsigned int length = num_bytes / sizeof(*array);
@@ -985,7 +1016,7 @@ pci224_ao_munge(struct comedi_device *dev, struct comedi_subdevice *s,
 	/* The hardware expects 16-bit numbers. */
 	shift = 16 - thisboard->ao_bits;
 	/* Channels will be all bipolar or all unipolar. */
-	if ((devpriv->hwrange[CR_RANGE(cmd->chanlist[0])] &
+	if ((thisboard->ao_hwrange[CR_RANGE(cmd->chanlist[0])] &
 	     PCI224_DACCON_POLAR_MASK) == PCI224_DACCON_POLAR_UNI) {
 		/* Unipolar */
 		offset = 0;
@@ -1108,13 +1139,12 @@ static struct pci_dev *pci224_find_pci_dev(struct comedi_device *dev,
  * Common part of attach and auto_attach.
  */
 static int pci224_attach_common(struct comedi_device *dev,
-				struct pci_dev *pci_dev, int *options)
+				struct pci_dev *pci_dev)
 {
 	const struct pci224_board *thisboard = comedi_board(dev);
 	struct pci224_private *devpriv = dev->private;
 	struct comedi_subdevice *s;
 	unsigned int irq;
-	unsigned n;
 	int ret;
 
 	comedi_set_hw_dev(dev, &pci_dev->dev);
@@ -1173,6 +1203,7 @@ static int pci224_attach_common(struct comedi_device *dev,
 	s->subdev_flags = SDF_WRITABLE | SDF_GROUND | SDF_CMD_WRITE;
 	s->n_chan = thisboard->ao_chans;
 	s->maxdata = (1 << thisboard->ao_bits) - 1;
+	s->range_table = thisboard->ao_range;
 	s->insn_write = pci224_ao_insn_write;
 	s->insn_read = pci224_ao_insn_read;
 	s->len_chanlist = s->n_chan;
@@ -1182,61 +1213,6 @@ static int pci224_attach_common(struct comedi_device *dev,
 	s->cancel = pci224_ao_cancel;
 	s->munge = pci224_ao_munge;
 
-	/* Sort out channel range options. */
-	if (thisboard->model == pci234_model) {
-		/* PCI234 range options. */
-		const struct comedi_lrange **range_table_list;
-
-		range_table_list =
-		    kmalloc(sizeof(struct comedi_lrange *) * s->n_chan,
-			    GFP_KERNEL);
-		if (!range_table_list)
-			return -ENOMEM;
-		s->range_table_list = range_table_list;
-
-		if (options) {
-			for (n = 2; n < 3 + s->n_chan; n++) {
-				if (options[n] < 0 || options[n] > 1) {
-					dev_warn(dev->class_dev,
-						 "warning! bad options[%u]=%d\n",
-						 n, options[n]);
-				}
-			}
-		}
-		for (n = 0; n < s->n_chan; n++) {
-			if (n < COMEDI_NDEVCONFOPTS - 3 && options &&
-			    options[3 + n] == 1) {
-				if (options[2] == 1)
-					range_table_list[n] = &range_pci234_ext;
-				else
-					range_table_list[n] = &range_bipolar5;
-
-			} else {
-				if (options && options[2] == 1) {
-					range_table_list[n] =
-					    &range_pci234_ext2;
-				} else {
-					range_table_list[n] = &range_bipolar10;
-				}
-			}
-		}
-		devpriv->hwrange = hwrange_pci234;
-	} else {
-		/* PCI224 range options. */
-		if (options && options[2] == 1) {
-			s->range_table = &range_pci224_external;
-			devpriv->hwrange = hwrange_pci224_external;
-		} else {
-			if (options && options[2] != 0) {
-				dev_warn(dev->class_dev,
-					 "warning! bad options[2]=%d\n",
-					 options[2]);
-			}
-			s->range_table = &range_pci224_internal;
-			devpriv->hwrange = hwrange_pci224_internal;
-		}
-	}
-
 	dev->board_name = thisboard->name;
 
 	if (irq) {
@@ -1268,7 +1244,7 @@ static int pci224_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 	if (!pci_dev)
 		return -EIO;
 
-	return pci224_attach_common(dev, pci_dev, it->options);
+	return pci224_attach_common(dev, pci_dev);
 }
 
 static int
@@ -1296,7 +1272,7 @@ pci224_auto_attach(struct comedi_device *dev, unsigned long context_unused)
 	 * has been removed.
 	 */
 	pci_dev_get(pci_dev);
-	return pci224_attach_common(dev, pci_dev, NULL);
+	return pci224_attach_common(dev, pci_dev);
 }
 
 static void pci224_detach(struct comedi_device *dev)
@@ -1306,13 +1282,6 @@ static void pci224_detach(struct comedi_device *dev)
 
 	if (dev->irq)
 		free_irq(dev->irq, dev);
-	if (dev->subdevices) {
-		struct comedi_subdevice *s;
-
-		s = &dev->subdevices[0];
-		/* AO subdevice */
-		kfree(s->range_table_list);
-	}
 	if (devpriv) {
 		kfree(devpriv->ao_readback);
 		kfree(devpriv->ao_scan_vals);
-- 
2.0.0

[PATCH 13/18] staging: comedi: amplc_pci224: remove "legacy" attach mechanism

[Ian Abbott]

Since the driver no longer supports options in its "legacy" attach
mechanism to describe the jumper settings (or any options beyond
restricting a PCI search to a particular bus and/or slot), there is no
need to retain this mechanism in the driver.  Remove the comedi driver
"attach" handler `pci224_attach()`, and the now unused
`pci224_find_pci_dev()`.  Also, remove the "wildcard" entry from the
board table `pci224_boards[]` as it is no longer needed.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 82 ++-------------------------
 1 file changed, 6 insertions(+), 76 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index fba0198..0bf5e68 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -22,9 +22,8 @@
  * Driver: amplc_pci224
  * Description: Amplicon PCI224, PCI234
  * Author: Ian Abbott <abbotti@mev.co.uk>
- * Devices: [Amplicon] PCI224 (amplc_pci224 or pci224),
- *   PCI234 (amplc_pci224 or pci234)
- * Updated: Wed, 30 Jul 2014 18:08:43 +0000
+ * Devices: [Amplicon] PCI224 (amplc_pci224), PCI234
+ * Updated: Thu, 31 Jul 2014 11:08:03 +0000
  * Status: works, but see caveats
  *
  * Supports:
@@ -46,12 +45,10 @@
  *     scan_begin_src or stop_src may use TRIG_EXT.
  *
  * Configuration options:
- *   [0] - PCI bus of device (optional).
- *   [1] - PCI slot of device (optional).
- *           If bus/slot is not specified, the first available PCI device
- *           will be used.
+ *   none
  *
- * Passing a zero for an option is the same as leaving it unspecified.
+ * Manual configuration of PCI cards is not supported; they are configured
+ * automatically.
  *
  * Output range selection - PCI224:
  *
@@ -350,7 +347,7 @@ static const unsigned char range_check_pci234[4] = {
  * Board descriptions.
  */
 
-enum pci224_model { any_model, pci224_model, pci234_model };
+enum pci224_model { pci224_model, pci234_model };
 
 struct pci224_board {
 	const char *name;
@@ -384,11 +381,6 @@ static const struct pci224_board pci224_boards[] = {
 		.ao_hwrange	= &hwrange_pci234[0],
 		.ao_range_check	= &range_check_pci234[0],
 	},
-	{
-		.name		= "amplc_pci224",
-		.devid		= PCI_DEVICE_ID_INVALID,
-		.model		= any_model,	/* wildcard */
-	},
 };
 
 struct pci224_private {
@@ -1093,49 +1085,6 @@ static const struct pci224_board
 }
 
 /*
- * This function looks for a PCI device matching the requested board name,
- * bus and slot.
- */
-static struct pci_dev *pci224_find_pci_dev(struct comedi_device *dev,
-					   struct comedi_devconfig *it)
-{
-	const struct pci224_board *thisboard = comedi_board(dev);
-	struct pci_dev *pci_dev = NULL;
-	int bus = it->options[0];
-	int slot = it->options[1];
-
-	for_each_pci_dev(pci_dev) {
-		if (bus || slot) {
-			if (bus != pci_dev->bus->number ||
-			    slot != PCI_SLOT(pci_dev->devfn))
-				continue;
-		}
-		if (pci_dev->vendor != PCI_VENDOR_ID_AMPLICON)
-			continue;
-
-		if (thisboard->model == any_model) {
-			/* Match any supported model. */
-			const struct pci224_board *board_ptr;
-
-			board_ptr = pci224_find_pci_board(pci_dev);
-			if (board_ptr == NULL)
-				continue;
-			/* Change board_ptr to matched board. */
-			dev->board_ptr = board_ptr;
-		} else {
-			/* Match specific model name. */
-			if (thisboard->devid != pci_dev->device)
-				continue;
-		}
-		return pci_dev;
-	}
-	dev_err(dev->class_dev,
-		"No supported board found! (req. bus %d, slot %d)\n",
-		bus, slot);
-	return NULL;
-}
-
-/*
  * Common part of attach and auto_attach.
  */
 static int pci224_attach_common(struct comedi_device *dev,
@@ -1229,24 +1178,6 @@ static int pci224_attach_common(struct comedi_device *dev,
 	return 0;
 }
 
-static int pci224_attach(struct comedi_device *dev, struct comedi_devconfig *it)
-{
-	struct pci224_private *devpriv;
-	struct pci_dev *pci_dev;
-
-	dev_info(dev->class_dev, "attach\n");
-
-	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
-	if (!devpriv)
-		return -ENOMEM;
-
-	pci_dev = pci224_find_pci_dev(dev, it);
-	if (!pci_dev)
-		return -EIO;
-
-	return pci224_attach_common(dev, pci_dev);
-}
-
 static int
 pci224_auto_attach(struct comedi_device *dev, unsigned long context_unused)
 {
@@ -1295,7 +1226,6 @@ static void pci224_detach(struct comedi_device *dev)
 static struct comedi_driver amplc_pci224_driver = {
 	.driver_name	= "amplc_pci224",
 	.module		= THIS_MODULE,
-	.attach		= pci224_attach,
 	.detach		= pci224_detach,
 	.auto_attach	= pci224_auto_attach,
 	.board_name	= &pci224_boards[0].name,
-- 
2.0.0

[PATCH 14/18] staging: comedi: amplc_pci224: no need to manipulate PCI ref count

[Ian Abbott]

This driver no longer supports a "legacy" attach mechanism that searches
for a suitable PCI device and increments it's reference count, but since
the common "detach" handler `pci224_detach()` still has a left-over
`pci_dev_put()`, a matching `pci_dev_get()` is needed in the
"auto_attach" handler `pci224_auto_attach()`.  There is no longer any
reason to "get" and "put" the PCI device, so those calls can be removed.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 0bf5e68..bcef9e6 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1196,20 +1196,12 @@ pci224_auto_attach(struct comedi_device *dev, unsigned long context_unused)
 			"BUG! cannot determine board type!\n");
 		return -EINVAL;
 	}
-	/*
-	 * Need to 'get' the PCI device to match the 'put' in pci224_detach().
-	 * TODO: Remove the pci_dev_get() and matching pci_dev_put() once
-	 * support for manual attachment of PCI devices via pci224_attach()
-	 * has been removed.
-	 */
-	pci_dev_get(pci_dev);
 	return pci224_attach_common(dev, pci_dev);
 }
 
 static void pci224_detach(struct comedi_device *dev)
 {
 	struct pci224_private *devpriv = dev->private;
-	struct pci_dev *pcidev = comedi_to_pci_dev(dev);
 
 	if (dev->irq)
 		free_irq(dev->irq, dev);
@@ -1219,8 +1211,6 @@ static void pci224_detach(struct comedi_device *dev)
 		kfree(devpriv->ao_scan_order);
 	}
 	comedi_pci_disable(dev);
-	if (pcidev)
-		pci_dev_put(pcidev);
 }
 
 static struct comedi_driver amplc_pci224_driver = {
-- 
2.0.0

[PATCH 15/18] staging: comedi: amplc_pci224: put board indices in PCI driver_data

[Ian Abbott]

The `driver_data` member value from the matched entry of the PCI module
device table `amplc_pci224_pci_table[]` is passed through to our comedi
"auto_attach" handler, `pci224_auto_attach()`.  Use that to index
directly into our static board data array `pci224_boards[]` instead of
calling `pci224_find_pci_board()` to search for the entry matching the
PCI device ID.  That function can be removed.  The `devid` and `model`
members of `struct pci224_board` are no longer needed either and can be
removed.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 52 ++++++++++-----------------
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index bcef9e6..977894d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -351,8 +351,6 @@ enum pci224_model { pci224_model, pci234_model };
 
 struct pci224_board {
 	const char *name;
-	unsigned short devid;
-	enum pci224_model model;
 	unsigned int ao_chans;
 	unsigned int ao_bits;
 	const struct comedi_lrange *ao_range;
@@ -361,20 +359,16 @@ struct pci224_board {
 };
 
 static const struct pci224_board pci224_boards[] = {
-	{
+	[pci224_model] {
 		.name		= "pci224",
-		.devid		= PCI_DEVICE_ID_AMPLICON_PCI224,
-		.model		= pci224_model,
 		.ao_chans	= 16,
 		.ao_bits	= 12,
 		.ao_range	= &range_pci224,
 		.ao_hwrange	= &hwrange_pci224[0],
 		.ao_range_check	= &range_check_pci224[0],
 	},
-	{
+	[pci234_model] {
 		.name		= "pci234",
-		.devid		= PCI_DEVICE_ID_AMPLICON_PCI234,
-		.model		= pci234_model,
 		.ao_chans	= 4,
 		.ao_bits	= 16,
 		.ao_range	= &range_pci234,
@@ -1071,20 +1065,6 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
 }
 
 /*
- * This function looks for a board matching the supplied PCI device.
- */
-static const struct pci224_board
-*pci224_find_pci_board(struct pci_dev *pci_dev)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(pci224_boards); i++)
-		if (pci_dev->device == pci224_boards[i].devid)
-			return &pci224_boards[i];
-	return NULL;
-}
-
-/*
  * Common part of attach and auto_attach.
  */
 static int pci224_attach_common(struct comedi_device *dev,
@@ -1162,8 +1142,6 @@ static int pci224_attach_common(struct comedi_device *dev,
 	s->cancel = pci224_ao_cancel;
 	s->munge = pci224_ao_munge;
 
-	dev->board_name = thisboard->name;
-
 	if (irq) {
 		ret = request_irq(irq, pci224_interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -1179,23 +1157,29 @@ static int pci224_attach_common(struct comedi_device *dev,
 }
 
 static int
-pci224_auto_attach(struct comedi_device *dev, unsigned long context_unused)
+pci224_auto_attach(struct comedi_device *dev, unsigned long context_model)
 {
 	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
+	const struct pci224_board *thisboard = NULL;
 	struct pci224_private *devpriv;
 
-	dev_info(dev->class_dev, "attach pci %s\n", pci_name(pci_dev));
+	if (context_model < ARRAY_SIZE(pci224_boards))
+		thisboard = &pci224_boards[context_model];
+	if (!thisboard || !thisboard->name) {
+		dev_err(dev->class_dev,
+			"amplc_pci224: BUG! cannot determine board type!\n");
+		return -EINVAL;
+	}
+	dev->board_ptr = thisboard;
+	dev->board_name = thisboard->name;
+
+	dev_info(dev->class_dev, "amplc_pci224: attach pci %s - %s\n",
+		 pci_name(pci_dev), dev->board_name);
 
 	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
 	if (!devpriv)
 		return -ENOMEM;
 
-	dev->board_ptr = pci224_find_pci_board(pci_dev);
-	if (dev->board_ptr == NULL) {
-		dev_err(dev->class_dev,
-			"BUG! cannot determine board type!\n");
-		return -EINVAL;
-	}
 	return pci224_attach_common(dev, pci_dev);
 }
 
@@ -1231,8 +1215,8 @@ static int amplc_pci224_pci_probe(struct pci_dev *dev,
 }
 
 static const struct pci_device_id amplc_pci224_pci_table[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI224) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI234) },
+	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI224), pci224_model },
+	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI234), pci234_model },
 	{ 0 }
 };
 MODULE_DEVICE_TABLE(pci, amplc_pci224_pci_table);
-- 
2.0.0

[PATCH 16/18] staging: comedi: amplc_pci224: remove PCI_DEVICE_ID_... macros

[Ian Abbott]

The macros `PCI_DEVICE_ID_AMPLICON_PCI224` and
`PCI_DEVICE_ID_AMPLICON_PCI234` are only used in the PCI module device
table `amplc_pci224_pci_table[]`.  Just expand the macros where they are
used and remove them.  The macro `PCI_DEVICE_ID_INVALID` is no longer
used either, so remove it.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 977894d..4fceb7c 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -113,13 +113,6 @@
 #include "8253.h"
 
 /*
- * PCI IDs.
- */
-#define PCI_DEVICE_ID_AMPLICON_PCI224 0x0007
-#define PCI_DEVICE_ID_AMPLICON_PCI234 0x0008
-#define PCI_DEVICE_ID_INVALID 0xffff
-
-/*
  * PCI224/234 i/o space 1 (PCIBAR2) registers.
  */
 #define PCI224_Z2_CT0	0x14	/* 82C54 counter/timer 0 */
@@ -1215,8 +1208,8 @@ static int amplc_pci224_pci_probe(struct pci_dev *dev,
 }
 
 static const struct pci_device_id amplc_pci224_pci_table[] = {
-	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI224), pci224_model },
-	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI234), pci234_model },
+	{ PCI_VDEVICE(AMPLICON, 0x0007), pci224_model },
+	{ PCI_VDEVICE(AMPLICON, 0x0008), pci234_model },
 	{ 0 }
 };
 MODULE_DEVICE_TABLE(pci, amplc_pci224_pci_table);
-- 
2.0.0

[PATCH 17/18] staging: comedi: amplc_pci224: absorb pci224_attach_common()

[Ian Abbott]

`pci224_attach_common()` is now only called from `pci225_auto_attach()`,
so absorb it into that function.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 56 +++++++++++----------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 4fceb7c..4b45319 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1057,18 +1057,33 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
 	return IRQ_RETVAL(retval);
 }
 
-/*
- * Common part of attach and auto_attach.
- */
-static int pci224_attach_common(struct comedi_device *dev,
-				struct pci_dev *pci_dev)
+static int
+pci224_auto_attach(struct comedi_device *dev, unsigned long context_model)
 {
-	const struct pci224_board *thisboard = comedi_board(dev);
-	struct pci224_private *devpriv = dev->private;
+	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
+	const struct pci224_board *thisboard = NULL;
+	struct pci224_private *devpriv;
 	struct comedi_subdevice *s;
 	unsigned int irq;
 	int ret;
 
+	if (context_model < ARRAY_SIZE(pci224_boards))
+		thisboard = &pci224_boards[context_model];
+	if (!thisboard || !thisboard->name) {
+		dev_err(dev->class_dev,
+			"amplc_pci224: BUG! cannot determine board type!\n");
+		return -EINVAL;
+	}
+	dev->board_ptr = thisboard;
+	dev->board_name = thisboard->name;
+
+	dev_info(dev->class_dev, "amplc_pci224: attach pci %s - %s\n",
+		 pci_name(pci_dev), dev->board_name);
+
+	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
+	if (!devpriv)
+		return -ENOMEM;
+
 	comedi_set_hw_dev(dev, &pci_dev->dev);
 
 	ret = comedi_pci_enable(dev);
@@ -1149,33 +1164,6 @@ static int pci224_attach_common(struct comedi_device *dev,
 	return 0;
 }
 
-static int
-pci224_auto_attach(struct comedi_device *dev, unsigned long context_model)
-{
-	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
-	const struct pci224_board *thisboard = NULL;
-	struct pci224_private *devpriv;
-
-	if (context_model < ARRAY_SIZE(pci224_boards))
-		thisboard = &pci224_boards[context_model];
-	if (!thisboard || !thisboard->name) {
-		dev_err(dev->class_dev,
-			"amplc_pci224: BUG! cannot determine board type!\n");
-		return -EINVAL;
-	}
-	dev->board_ptr = thisboard;
-	dev->board_name = thisboard->name;
-
-	dev_info(dev->class_dev, "amplc_pci224: attach pci %s - %s\n",
-		 pci_name(pci_dev), dev->board_name);
-
-	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
-	if (!devpriv)
-		return -ENOMEM;
-
-	return pci224_attach_common(dev, pci_dev);
-}
-
 static void pci224_detach(struct comedi_device *dev)
 {
 	struct pci224_private *devpriv = dev->private;
-- 
2.0.0

[PATCH 18/18] staging: comedi: amplc_pci224: no need to comedi_set_hw_dev() here

[Ian Abbott]

The comedi core module calls `comedi_set_hw_dev()` to associate the
hardware `struct device` with the `struct comedi_device` before it calls
the comedi driver's "auto_attach" hook `pci224_auto_attach()`.  There is
no need for `pci224_auto_attach()` to call `comedi_set_hw_dev()` itself,
so remove the call.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 4b45319..6fed6b8 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1084,8 +1084,6 @@ pci224_auto_attach(struct comedi_device *dev, unsigned long context_model)
 	if (!devpriv)
 		return -ENOMEM;
 
-	comedi_set_hw_dev(dev, &pci_dev->dev);
-
 	ret = comedi_pci_enable(dev);
 	if (ret)
 		return ret;
-- 
2.0.0

RE: [PATCH 00/18] staging: comedi: amplc_pci224: remove legacy stuff

[Hartley Sweeten]

On Thursday, July 31, 2014 6:48 AM, Ian Abbott wrote:
> The "amplc_pci224" driver retains a "legacy" attach mechanism via the
> `COMEDI_DEVCONFIG` ioctl and the comedi driver "attach" hook, but
> usually attaches PCI devices automatically via the comedi driver's
> "auto_attach" hook.  The legacy mechanism is only retained so that
> options can be passed via the ioctl to indicate how some hardware
> jumpers are set on the boards, which is used to set up the range tables.
> That's no use if the PCI device is attached automatically as the range
> tables will be set up according to the factory default jumper positions.
>
> Change the range tables to include all possible ranges, regardless of
> jumper positions.  Then there is no need to have options to control
> setting up the range tables and the "legacy" attach mechanism can be
> removed.
>
> Also, tidy the code up a bit.

Looks good.

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

Side note:

Now that the manual attach has been removed, you could also remove
the board information from the comedi_driver declaration.

Regards,
Hartley

RE: [PATCH 00/18] staging: comedi: amplc_pci224: remove legacy stuff

[Hartley Sweeten]

On Thursday, July 31, 2014 1:28 PM, Hartley Sweeten wrote:
> On Thursday, July 31, 2014 6:48 AM, Ian Abbott wrote:
>> The "amplc_pci224" driver retains a "legacy" attach mechanism via the
>> `COMEDI_DEVCONFIG` ioctl and the comedi driver "attach" hook, but
>> usually attaches PCI devices automatically via the comedi driver's
>> "auto_attach" hook.  The legacy mechanism is only retained so that
>> options can be passed via the ioctl to indicate how some hardware
>> jumpers are set on the boards, which is used to set up the range tables.
>> That's no use if the PCI device is attached automatically as the range
>> tables will be set up according to the factory default jumper positions.
>>
>> Change the range tables to include all possible ranges, regardless of
>> jumper positions.  Then there is no need to have options to control
>> setting up the range tables and the "legacy" attach mechanism can be
>> removed.
>>
>> Also, tidy the code up a bit.
>
> Looks good.
>
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> Side note:
>
> Now that the manual attach has been removed, you could also remove
> the board information from the comedi_driver declaration.

Oops.. Just did a sparse build and get this:

drivers/staging/comedi/drivers/amplc_pci224.c:355:24: warning: obsolete array initializer, use C99 syntax
drivers/staging/comedi/drivers/amplc_pci224.c:363:24: warning: obsolete array initializer, use C99 syntax

The following patch fixes it, or you fold it into patch 15.

Regards,
Hartley

---

From: H Hartley Sweeten <hsweeten@visionengravers.com>
Date: Wed, 30 Jul 2014 10:50:11 -0700
Subject: [PATCH] staging: comedi: amplc_pci224: fix obsolete array initializer

Fix the sparse warnings:
warning: obsolete array initializer, use C99 syntax

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index 21f06bb..2170e5e 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -352,7 +352,7 @@ struct pci224_board {
 };
 
 static const struct pci224_board pci224_boards[] = {
-	[pci224_model] {
+	[pci224_model] = {
 		.name		= "pci224",
 		.ao_chans	= 16,
 		.ao_bits	= 12,
@@ -360,7 +360,7 @@ static const struct pci224_board pci224_boards[] = {
 		.ao_hwrange	= &hwrange_pci224[0],
 		.ao_range_check	= &range_check_pci224[0],
 	},
-	[pci234_model] {
+	[pci234_model] = {
 		.name		= "pci234",
 		.ao_chans	= 4,
 		.ao_bits	= 16,

Re: [PATCH 00/18] staging: comedi: amplc_pci224: remove legacy stuff

[Ian Abbott]

On 2014-07-31 21:28, Hartley Sweeten wrote:
> On Thursday, July 31, 2014 6:48 AM, Ian Abbott wrote:
>> The "amplc_pci224" driver retains a "legacy" attach mechanism via the
>> `COMEDI_DEVCONFIG` ioctl and the comedi driver "attach" hook, but
>> usually attaches PCI devices automatically via the comedi driver's
>> "auto_attach" hook.  The legacy mechanism is only retained so that
>> options can be passed via the ioctl to indicate how some hardware
>> jumpers are set on the boards, which is used to set up the range tables.
>> That's no use if the PCI device is attached automatically as the range
>> tables will be set up according to the factory default jumper positions.
>>
>> Change the range tables to include all possible ranges, regardless of
>> jumper positions.  Then there is no need to have options to control
>> setting up the range tables and the "legacy" attach mechanism can be
>> removed.
>>
>> Also, tidy the code up a bit.
>
> Looks good.
>
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> Side note:
>
> Now that the manual attach has been removed, you could also remove
> the board information from the comedi_driver declaration.

Good point.  I can do that as a follow-up patch.  It only affects the 
output of /proc/comedi I think.

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

[PATCH 15/18 v2] staging: comedi: amplc_pci224: put board indices in PCI driver_data

[Ian Abbott]

The `driver_data` member value from the matched entry of the PCI module
device table `amplc_pci224_pci_table[]` is passed through to our comedi
"auto_attach" handler, `pci224_auto_attach()`.  Use that to index
directly into our static board data array `pci224_boards[]` instead of
calling `pci224_find_pci_board()` to search for the entry matching the
PCI device ID.  That function can be removed.  The `devid` and `model`
members of `struct pci224_board` are no longer needed either and can be
removed.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
v2: Use C99 syntax for amplc_pci224_pci_table[] initializer.  Thanks to
Hartley for finding that!
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 52 ++++++++++-----------------
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
index bcef9e6..675bbe8 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -351,8 +351,6 @@ enum pci224_model { pci224_model, pci234_model };
 
 struct pci224_board {
 	const char *name;
-	unsigned short devid;
-	enum pci224_model model;
 	unsigned int ao_chans;
 	unsigned int ao_bits;
 	const struct comedi_lrange *ao_range;
@@ -361,20 +359,16 @@ struct pci224_board {
 };
 
 static const struct pci224_board pci224_boards[] = {
-	{
+	[pci224_model] = {
 		.name		= "pci224",
-		.devid		= PCI_DEVICE_ID_AMPLICON_PCI224,
-		.model		= pci224_model,
 		.ao_chans	= 16,
 		.ao_bits	= 12,
 		.ao_range	= &range_pci224,
 		.ao_hwrange	= &hwrange_pci224[0],
 		.ao_range_check	= &range_check_pci224[0],
 	},
-	{
+	[pci234_model] = {
 		.name		= "pci234",
-		.devid		= PCI_DEVICE_ID_AMPLICON_PCI234,
-		.model		= pci234_model,
 		.ao_chans	= 4,
 		.ao_bits	= 16,
 		.ao_range	= &range_pci234,
@@ -1071,20 +1065,6 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
 }
 
 /*
- * This function looks for a board matching the supplied PCI device.
- */
-static const struct pci224_board
-*pci224_find_pci_board(struct pci_dev *pci_dev)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(pci224_boards); i++)
-		if (pci_dev->device == pci224_boards[i].devid)
-			return &pci224_boards[i];
-	return NULL;
-}
-
-/*
  * Common part of attach and auto_attach.
  */
 static int pci224_attach_common(struct comedi_device *dev,
@@ -1162,8 +1142,6 @@ static int pci224_attach_common(struct comedi_device *dev,
 	s->cancel = pci224_ao_cancel;
 	s->munge = pci224_ao_munge;
 
-	dev->board_name = thisboard->name;
-
 	if (irq) {
 		ret = request_irq(irq, pci224_interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
@@ -1179,23 +1157,29 @@ static int pci224_attach_common(struct comedi_device *dev,
 }
 
 static int
-pci224_auto_attach(struct comedi_device *dev, unsigned long context_unused)
+pci224_auto_attach(struct comedi_device *dev, unsigned long context_model)
 {
 	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
+	const struct pci224_board *thisboard = NULL;
 	struct pci224_private *devpriv;
 
-	dev_info(dev->class_dev, "attach pci %s\n", pci_name(pci_dev));
+	if (context_model < ARRAY_SIZE(pci224_boards))
+		thisboard = &pci224_boards[context_model];
+	if (!thisboard || !thisboard->name) {
+		dev_err(dev->class_dev,
+			"amplc_pci224: BUG! cannot determine board type!\n");
+		return -EINVAL;
+	}
+	dev->board_ptr = thisboard;
+	dev->board_name = thisboard->name;
+
+	dev_info(dev->class_dev, "amplc_pci224: attach pci %s - %s\n",
+		 pci_name(pci_dev), dev->board_name);
 
 	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
 	if (!devpriv)
 		return -ENOMEM;
 
-	dev->board_ptr = pci224_find_pci_board(pci_dev);
-	if (dev->board_ptr == NULL) {
-		dev_err(dev->class_dev,
-			"BUG! cannot determine board type!\n");
-		return -EINVAL;
-	}
 	return pci224_attach_common(dev, pci_dev);
 }
 
@@ -1231,8 +1215,8 @@ static int amplc_pci224_pci_probe(struct pci_dev *dev,
 }
 
 static const struct pci_device_id amplc_pci224_pci_table[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI224) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI234) },
+	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI224), pci224_model },
+	{ PCI_VDEVICE(AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI234), pci234_model },
 	{ 0 }
 };
 MODULE_DEVICE_TABLE(pci, amplc_pci224_pci_table);
-- 
2.0.0

RE: [PATCH 15/18 v2] staging: comedi: amplc_pci224: put board indices in PCI driver_data

[Hartley Sweeten]

On Monday, August 04, 2014 4:14 AM, Ian Abbott wrote:
> The `driver_data` member value from the matched entry of the PCI module
> device table `amplc_pci224_pci_table[]` is passed through to our comedi
> "auto_attach" handler, `pci224_auto_attach()`.  Use that to index
> directly into our static board data array `pci224_boards[]` instead of
> calling `pci224_find_pci_board()` to search for the entry matching the
> PCI device ID.  That function can be removed.  The `devid` and `model`
> members of `struct pci224_board` are no longer needed either and can be
> removed.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> v2: Use C99 syntax for amplc_pci224_pci_table[] initializer.  Thanks to
> Hartley for finding that!

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

Thanks,
Hartley