[PATCH 00/28] staging: comedi: more clean-up and remove legacy attach

[PATCH 00/28] staging: comedi: more clean-up and remove legacy attach

[Ian Abbott]

Continue to clean up the amplc_pci230 driver code and remove the legacy
attach mechanism, since it isn't very useful for this driver (see PATCH
05/28).

01) staging: comedi: amplc_pci230: update MODULE_DESCRIPTION()
02) staging: comedi: amplc_pci230: don't use multiple blank lines
03) staging: comedi: amplc_pci230: remove some unnecessary parentheses
04) staging: comedi: amplc_pci230: collapse some 'else { if' chains
05) staging: comedi: amplc_pci230: remove "legacy" attach mechanism
06) staging: comedi: amplc_pci230: no need to manipulate PCI ref count
07) staging: comedi: amplc_pci230: set detach handler to
    comedi_pci_detach()
08) staging: comedi: amplc_pci230: absorb pci230_attach_common()
09) staging: comedi: amplc_pci230: no need to comedi_set_hw_dev() here
10) staging: comedi: amplc_pci230: absorb pci230_alloc_private()
11) staging: comedi: amplc_pci230: remove ai_chans member
12) staging: comedi: amplc_pci230: remove ao_chans member
13) staging: comedi: amplc_pci230: shrink struct pci230_board
14) staging: comedi: amplc_pci230: simplify pci230_ao_mangle_datum()
15) staging: comedi: amplc_pci230: simplify pci230_ai_read()
16) staging: comedi: amplc_pci230: remove 'inline'
17) staging: comedi: amplc_pci230: rename pci230_ai_rinsn()
18) staging: comedi: amplc_pci230: add `pci230_` prefix to functions
19) staging: comedi: amplc_pci230: use comedi_range_is_bipolar()
20) staging: comedi: amplc_pci230: make `intr_running` a bitfield
21) staging: comedi: amplc_pci230: replace `state` member with bitfields
22) staging: comedi: amplc_pci230: rewrite shared resource handling
23) staging: comedi: amplc_pci230: reduce indentation in
    pci230_ao_inttrig_scan_begin()
24) staging: comedi: amplc_pci230: reduce indentation in
    pci230_ao_start()
25) staging: comedi: amplc_pci230: reduce indentation in
    pci230_ai_inttrig_convert()
26) staging: comedi: amplc_pci230: reduce indentation in
    pci230_ai_start()
27) staging: comedi: amplc_pci230: change pci230_handle_ao_fifo() return
    type
28) staging: comedi: amplc_pci230: simplify interrupt enable handling

 drivers/staging/comedi/drivers/amplc_pci230.c | 977 ++++++++++----------------
 1 file changed, 384 insertions(+), 593 deletions(-)

[PATCH 01/28] staging: comedi: amplc_pci230: update MODULE_DESCRIPTION()

[Ian Abbott]

Replace the generic, Comedi low-level driver module description string
with something more specific.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index cd63706..d077254 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2869,5 +2869,5 @@ static struct pci_driver amplc_pci230_pci_driver = {
 module_comedi_pci_driver(amplc_pci230_driver, amplc_pci230_pci_driver);
 
 MODULE_AUTHOR("Comedi http://www.comedi.org");
-MODULE_DESCRIPTION("Comedi low-level driver");
+MODULE_DESCRIPTION("Comedi driver for Amplicon PCI230(+) and PCI260(+)");
 MODULE_LICENSE("GPL");
-- 
2.0.4

[PATCH 02/28] staging: comedi: amplc_pci230: don't use multiple blank lines

[Ian Abbott]

Fix checkpatch issues: "CHECK: Please don't use multiple blank lines".

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index d077254..09b392b 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -864,7 +864,6 @@ static int pci230_ai_rinsn(struct comedi_device *dev,
 	else
 		adccon |= PCI230_ADC_IR_UNI;
 
-
 	/*
 	 * Enable only this channel in the scan list - otherwise by default
 	 * we'll get one sample from each channel.
@@ -2295,7 +2294,6 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	if (!get_resources(dev, res_mask, OWNER_AICMD))
 		return -EBUSY;
 
-
 	/* Get number of scans required. */
 	if (cmd->stop_src == TRIG_COUNT)
 		devpriv->ai_scan_count = cmd->stop_arg;
@@ -2487,7 +2485,6 @@ static irqreturn_t pci230_interrupt(int irq, void *d)
 	if (status_int == PCI230_INT_DISABLE)
 		return IRQ_NONE;
 
-
 	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
 	valid_status_int = devpriv->int_en & status_int;
 	/*
-- 
2.0.4

[PATCH 03/28] staging: comedi: amplc_pci230: remove some unnecessary parentheses

[Ian Abbott]

Remove some pairs of parentheses that don't really improve readability.
Also, reduce the amount of leading whitespace in a few places.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 09b392b..0ba06f7 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -650,7 +650,7 @@ static int get_resources(struct comedi_device *dev, unsigned int res_mask,
 	ok = 1;
 	claimed = 0;
 	spin_lock_irqsave(&devpriv->res_spinlock, irqflags);
-	for (b = 1, i = 0; (i < NUM_RESOURCES) && res_mask; b <<= 1, i++) {
+	for (b = 1, i = 0; i < NUM_RESOURCES && res_mask; b <<= 1, i++) {
 		if (res_mask & b) {
 			res_mask &= ~b;
 			if (devpriv->res_owner[i] == OWNER_NONE) {
@@ -676,7 +676,7 @@ static int get_resources(struct comedi_device *dev, unsigned int res_mask,
 static inline int get_one_resource(struct comedi_device *dev,
 				   unsigned int resource, unsigned char owner)
 {
-	return get_resources(dev, (1U << resource), owner);
+	return get_resources(dev, 1U << resource, owner);
 }
 
 static void put_resources(struct comedi_device *dev, unsigned int res_mask,
@@ -688,7 +688,7 @@ static void put_resources(struct comedi_device *dev, unsigned int res_mask,
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&devpriv->res_spinlock, irqflags);
-	for (b = 1, i = 0; (i < NUM_RESOURCES) && res_mask; b <<= 1, i++) {
+	for (b = 1, i = 0; i < NUM_RESOURCES && res_mask; b <<= 1, i++) {
 		if (res_mask & b) {
 			res_mask &= ~b;
 			if (devpriv->res_owner[i] == owner)
@@ -701,7 +701,7 @@ static void put_resources(struct comedi_device *dev, unsigned int res_mask,
 static inline void put_one_resource(struct comedi_device *dev,
 				    unsigned int resource, unsigned char owner)
 {
-	put_resources(dev, (1U << resource), owner);
+	put_resources(dev, 1U << resource, owner);
 }
 
 static inline void put_all_resources(struct comedi_device *dev,
@@ -743,7 +743,7 @@ static unsigned int pci230_choose_clk_count(uint64_t ns, unsigned int *count,
 
 	for (clk_src = CLK_10MHZ;; clk_src++) {
 		cnt = divide_ns(ns, pci230_timebase[clk_src], flags);
-		if ((cnt <= 65536) || (clk_src == CLK_1KHZ))
+		if (cnt <= 65536 || clk_src == CLK_1KHZ)
 			break;
 	}
 	*count = cnt;
@@ -973,7 +973,7 @@ static int pci230_ao_cmdtest(struct comedi_device *dev,
 	err |= cfc_check_trigger_src(&cmd->start_src, TRIG_INT);
 
 	tmp = TRIG_TIMER | TRIG_INT;
-	if ((thisboard->min_hwver > 0) && (devpriv->hwver >= 2)) {
+	if (thisboard->min_hwver > 0 && devpriv->hwver >= 2) {
 		/*
 		 * For PCI230+ hardware version 2 onwards, allow external
 		 * trigger from EXTTRIG/EXTCONVCLK input (PCI230+ pin 25).
@@ -1041,11 +1041,11 @@ static int pci230_ao_cmdtest(struct comedi_device *dev,
 		 * The only flags allowed are CR_EDGE and CR_INVERT.
 		 * The CR_EDGE flag is ignored.
 		 */
-		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));
+		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));
 			err |= -EINVAL;
 		}
 		break;
@@ -1215,8 +1215,8 @@ static int pci230_handle_ao_fifo(struct comedi_device *dev,
 		 * (otherwise there will be loads of "DAC FIFO not half full"
 		 * interrupts).
 		 */
-		if ((num_scans == 0) &&
-		    ((dacstat & PCI230P2_DAC_FIFO_HALF) == 0)) {
+		if (num_scans == 0 &&
+		    (dacstat & PCI230P2_DAC_FIFO_HALF) == 0) {
 			dev_err(dev->class_dev, "AO buffer underrun\n");
 			events |= COMEDI_CB_OVERFLOW | COMEDI_CB_ERROR;
 		}
@@ -1515,8 +1515,8 @@ static int pci230_ai_check_scan_period(struct comedi_cmd *cmd)
 		chanlist_len = 1;
 
 	min_scan_period = chanlist_len * cmd->convert_arg;
-	if ((min_scan_period < chanlist_len) ||
-	    (min_scan_period < cmd->convert_arg)) {
+	if (min_scan_period < chanlist_len ||
+	    min_scan_period < cmd->convert_arg) {
 		/* Arithmetic overflow. */
 		min_scan_period = UINT_MAX;
 		err++;
@@ -1645,7 +1645,7 @@ static int pci230_ai_cmdtest(struct comedi_device *dev,
 	err |= cfc_check_trigger_src(&cmd->start_src, TRIG_NOW | TRIG_INT);
 
 	tmp = TRIG_FOLLOW | TRIG_TIMER | TRIG_INT;
-	if ((thisboard->have_dio) || (thisboard->min_hwver > 0)) {
+	if (thisboard->have_dio || thisboard->min_hwver > 0) {
 		/*
 		 * Unfortunately, we cannot trigger a scan off an external
 		 * source on the PCI260 board, since it uses the PPIC0 (DIO)
@@ -1676,8 +1676,8 @@ static int pci230_ai_cmdtest(struct comedi_device *dev,
 	 * If scan_begin_src is not TRIG_FOLLOW, then a monostable will be
 	 * set up to generate a fixed number of timed conversion pulses.
 	 */
-	if ((cmd->scan_begin_src != TRIG_FOLLOW) &&
-	    (cmd->convert_src != TRIG_TIMER))
+	if (cmd->scan_begin_src != TRIG_FOLLOW &&
+	    cmd->convert_src != TRIG_TIMER)
 		err |= -EINVAL;
 
 	if (err)
@@ -1704,7 +1704,7 @@ static int pci230_ai_cmdtest(struct comedi_device *dev,
 			 * PCI230 or PCI260.  Max speed depends whether
 			 * single-ended or pseudo-differential.
 			 */
-			if (cmd->chanlist && (cmd->chanlist_len > 0)) {
+			if (cmd->chanlist && cmd->chanlist_len > 0) {
 				/* Peek analogue reference of first channel. */
 				if (CR_AREF(cmd->chanlist[0]) == AREF_DIFF)
 					max_speed_ai = MAX_SPEED_AI_DIFF;
@@ -1744,13 +1744,12 @@ static int pci230_ai_cmdtest(struct comedi_device *dev,
 			 * The only flags allowed are CR_INVERT and CR_EDGE.
 			 * CR_EDGE is required.
 			 */
-			if ((cmd->convert_arg &
-			     (CR_FLAGS_MASK & ~CR_INVERT)) != CR_EDGE) {
+			if ((cmd->convert_arg & CR_FLAGS_MASK & ~CR_INVERT) !=
+			    CR_EDGE) {
 				/* Set CR_EDGE, preserve CR_INVERT. */
-				cmd->convert_arg = COMBINE(cmd->start_arg,
-							   (CR_EDGE | 0),
-							   CR_FLAGS_MASK &
-							   ~CR_INVERT);
+				cmd->convert_arg =
+				    COMBINE(cmd->start_arg, CR_EDGE | 0,
+					    CR_FLAGS_MASK & ~CR_INVERT);
 				err |= -EINVAL;
 			}
 		} else {
@@ -1854,14 +1853,14 @@ static void pci230_ai_update_fifo_trigger_level(struct comedi_device *dev,
 		    scanlen >= PCI230_ADC_FIFOLEVEL_HALFFULL) {
 			wake = PCI230_ADC_FIFOLEVEL_HALFFULL;
 		} else {
-			wake = (devpriv->ai_scan_count * scanlen) -
+			wake = devpriv->ai_scan_count * scanlen -
 			       devpriv->ai_scan_pos;
 		}
 	}
 	if (wake >= PCI230_ADC_FIFOLEVEL_HALFFULL) {
 		triglev = PCI230_ADC_INT_FIFO_HALF;
 	} else {
-		if ((wake > 1) && (devpriv->hwver > 0)) {
+		if (wake > 1 && devpriv->hwver > 0) {
 			/* PCI230+/260+ programmable FIFO interrupt level. */
 			if (devpriv->adcfifothresh != wake) {
 				devpriv->adcfifothresh = wake;
@@ -1908,8 +1907,8 @@ static int pci230_ai_inttrig_convert(struct comedi_device *dev,
 		 * bit has a different meaning when FIFO enabled (and when
 		 * FIFO not enabled, it only works for software triggers).
 		 */
-		if (((devpriv->adccon & PCI230_ADC_IM_MASK) ==
-		     PCI230_ADC_IM_DIF) && (devpriv->hwver == 0)) {
+		if ((devpriv->adccon & PCI230_ADC_IM_MASK) ==
+		    PCI230_ADC_IM_DIF && devpriv->hwver == 0) {
 			/* PCI230/260 in differential mode */
 			delayus = 8;
 		} else {
@@ -2183,12 +2182,11 @@ static void pci230_handle_ai(struct comedi_device *dev,
 		todo = PCI230_ADC_FIFOLEVEL_HALFFULL;
 	} else if (devpriv->ai_scan_count == 0) {
 		todo = 0;
-	} else if ((devpriv->ai_scan_count > PCI230_ADC_FIFOLEVEL_HALFFULL) ||
-		   (scanlen > PCI230_ADC_FIFOLEVEL_HALFFULL)) {
+	} else if (devpriv->ai_scan_count > PCI230_ADC_FIFOLEVEL_HALFFULL ||
+		   scanlen > PCI230_ADC_FIFOLEVEL_HALFFULL) {
 		todo = PCI230_ADC_FIFOLEVEL_HALFFULL;
 	} else {
-		todo = (devpriv->ai_scan_count * scanlen) -
-		       devpriv->ai_scan_pos;
+		todo = devpriv->ai_scan_count * scanlen - devpriv->ai_scan_pos;
 		if (todo > PCI230_ADC_FIFOLEVEL_HALFFULL)
 			todo = PCI230_ADC_FIFOLEVEL_HALFFULL;
 	}
@@ -2281,13 +2279,13 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 * Need Z2-CT2 to supply a conversion trigger source at a high
 	 * logic level, even if not doing timed conversions.
 	 */
-	res_mask |= (1U << RES_Z2CT2);
+	res_mask |= 1U << RES_Z2CT2;
 	if (cmd->scan_begin_src != TRIG_FOLLOW) {
 		/* Using Z2-CT0 monostable to gate Z2-CT2 conversion timer */
-		res_mask |= (1U << RES_Z2CT0);
+		res_mask |= 1U << RES_Z2CT0;
 		if (cmd->scan_begin_src == TRIG_TIMER) {
 			/* Using Z2-CT1 for scan frequency */
-			res_mask |= (1U << RES_Z2CT1);
+			res_mask |= 1U << RES_Z2CT1;
 		}
 	}
 	/* Claim resources. */
@@ -2360,7 +2358,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 				adcen |= 1 << gainshift;
 			}
 		} else {
-			gainshift = (chan & ~1);
+			gainshift = chan & ~1;
 			adcen |= 1 << chan;
 		}
 		devpriv->adcg = (devpriv->adcg & ~(3 << gainshift)) |
@@ -2676,8 +2674,7 @@ static int pci230_attach_common(struct comedi_device *dev,
 				 */
 				extfunc |= PCI230P_EXTFUNC_GAT_EXTTRIG;
 			}
-			if ((thisboard->ao_chans > 0) &&
-			    (devpriv->hwver >= 2)) {
+			if (thisboard->ao_chans > 0 && devpriv->hwver >= 2) {
 				/* Enable DAC FIFO functionality. */
 				extfunc |= PCI230P2_EXTFUNC_DACFIFO;
 			}
-- 
2.0.4

[PATCH 04/28] staging: comedi: amplc_pci230: collapse some 'else { if' chains

[Ian Abbott]

Where the only thing in an `else { ... }` block is another `if`
statement, collapse it to an `else if {` block where it makes sense to
do so.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 0ba06f7..56141de 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1844,32 +1844,25 @@ static void pci230_ai_update_fifo_trigger_level(struct comedi_device *dev,
 	unsigned short triglev;
 	unsigned short adccon;
 
-	if (cmd->flags & TRIG_WAKE_EOS) {
-		/* Wake at end of scan. */
+	if (cmd->flags & TRIG_WAKE_EOS)
 		wake = scanlen - devpriv->ai_scan_pos;
-	} else {
-		if (cmd->stop_src != TRIG_COUNT ||
-		    devpriv->ai_scan_count >= PCI230_ADC_FIFOLEVEL_HALFFULL ||
-		    scanlen >= PCI230_ADC_FIFOLEVEL_HALFFULL) {
-			wake = PCI230_ADC_FIFOLEVEL_HALFFULL;
-		} else {
-			wake = devpriv->ai_scan_count * scanlen -
-			       devpriv->ai_scan_pos;
-		}
-	}
+	else if (cmd->stop_src != TRIG_COUNT ||
+		 devpriv->ai_scan_count >= PCI230_ADC_FIFOLEVEL_HALFFULL ||
+		 scanlen >= PCI230_ADC_FIFOLEVEL_HALFFULL)
+		wake = PCI230_ADC_FIFOLEVEL_HALFFULL;
+	else
+		wake = devpriv->ai_scan_count * scanlen - devpriv->ai_scan_pos;
 	if (wake >= PCI230_ADC_FIFOLEVEL_HALFFULL) {
 		triglev = PCI230_ADC_INT_FIFO_HALF;
-	} else {
-		if (wake > 1 && devpriv->hwver > 0) {
-			/* PCI230+/260+ programmable FIFO interrupt level. */
-			if (devpriv->adcfifothresh != wake) {
-				devpriv->adcfifothresh = wake;
-				outw(wake, devpriv->daqio + PCI230P_ADCFFTH);
-			}
-			triglev = PCI230P_ADC_INT_FIFO_THRESH;
-		} else {
-			triglev = PCI230_ADC_INT_FIFO_NEMPTY;
+	} else if (wake > 1 && devpriv->hwver > 0) {
+		/* PCI230+/260+ programmable FIFO interrupt level. */
+		if (devpriv->adcfifothresh != wake) {
+			devpriv->adcfifothresh = wake;
+			outw(wake, devpriv->daqio + PCI230P_ADCFFTH);
 		}
+		triglev = PCI230P_ADC_INT_FIFO_THRESH;
+	} else {
+		triglev = PCI230_ADC_INT_FIFO_NEMPTY;
 	}
 	adccon = (devpriv->adccon & ~PCI230_ADC_INT_FIFO_MASK) | triglev;
 	if (adccon != devpriv->adccon) {
@@ -2211,19 +2204,15 @@ static void pci230_handle_ai(struct comedi_device *dev,
 			} else if (status_fifo & PCI230_ADC_FIFO_HALF) {
 				/* FIFO half full. */
 				fifoamount = PCI230_ADC_FIFOLEVEL_HALFFULL;
+			} else if (devpriv->hwver > 0) {
+				/* Read PCI230+/260+ ADC FIFO level. */
+				fifoamount = inw(devpriv->daqio +
+						 PCI230P_ADCFFLEV);
+				if (fifoamount == 0)
+					break;	/* Shouldn't happen. */
 			} else {
 				/* FIFO not empty. */
-				if (devpriv->hwver > 0) {
-					/* Read PCI230+/260+ ADC FIFO level. */
-					fifoamount = inw(devpriv->daqio +
-							 PCI230P_ADCFFLEV);
-					if (fifoamount == 0) {
-						/* Shouldn't happen. */
-						break;
-					}
-				} else {
-					fifoamount = 1;
-				}
+				fifoamount = 1;
 			}
 		}
 		/* Read sample and store in Comedi's circular buffer. */
-- 
2.0.4

[PATCH 05/28] staging: comedi: amplc_pci230: remove "legacy" attach mechanism

[Ian Abbott]

The "amplc_pci230" driver currently retains the legacy attach mechanism
to allow devices to be attached manually via the `COMEDI_DEVCONFIG`
ioctl.  The only real use for this is to pretend that a PCI230+ or
PCI260+ is a PCI230 or PCI260 for backwards compatibility, as they have
different number of bits of resolution on the AI subdevice.  Since the
card would be automatically configured as a PCI230+ or PCI260+ at PCI
probe time anyway, hopefully any users who want it to appear as a PCI230
or PCI260 would have got tired of removing the automatically configured
device and configuring it manually by now and will have updated their
software to cope with the PCI230+ or PCI260+.

Get rid of the legacy attach mechanism by removing the Comedi driver
"attach" handler `pci230_attach()` and associated code.  Also remove the
"wildcard" entry from the board table `pci230_boards[]` as it is no
longer needed.  Don't bother initializing the `board_name`, `offset`,
and `num_names` members of `struct comedi_driver amplc_pci230_driver`
any longer as they are only needed when configuring the device manually.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 56141de..393712c 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -24,24 +24,19 @@
  * Author: Allan Willcox <allanwillcox@ozemail.com.au>,
  *   Steve D Sharples <steve.sharples@nottingham.ac.uk>,
  *   Ian Abbott <abbotti@mev.co.uk>
- * Updated: Wed, 22 Oct 2008 12:34:49 +0100
- * Devices: [Amplicon] PCI230 (pci230 or amplc_pci230),
- *   PCI230+ (pci230+ or amplc_pci230),
- *   PCI260 (pci260 or amplc_pci230), PCI260+ (pci260+ or amplc_pci230)
+ * Updated: Mon, 01 Sep 2014 10:09:16 +0000
+ * Devices: [Amplicon] PCI230 (amplc_pci230), PCI230+, PCI260, PCI260+
  * Status: works
  *
  * 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
  *
- * Configuring a "amplc_pci230" will match any supported card and it will
- * choose the best match, picking the "+" models if possible.  Configuring
- * a "pci230" will match a PCI230 or PCI230+ card and it will be treated as
- * a PCI230.  Configuring a "pci260" will match a PCI260 or PCI260+ card
- * and it will be treated as a PCI260.  Configuring a "pci230+" will match
- * a PCI230+ card.  Configuring a "pci260+" will match a PCI260+ card.
+ * Manual configuration of PCI cards is not supported; they are configured
+ * automatically.
+ *
+ * The PCI230+ and PCI260+ have the same PCI device IDs as the PCI230 and
+ * PCI260, but can be distinguished by the the size of the PCI regions.  A
+ * card will be configured as a "+" model if detected as such.
  *
  * Subdevices:
  *
@@ -201,7 +196,6 @@
  */
 #define PCI_DEVICE_ID_PCI230 0x0000
 #define PCI_DEVICE_ID_PCI260 0x0006
-#define PCI_DEVICE_ID_INVALID 0xffff
 
 /*
  * PCI230 i/o space 1 registers.
@@ -501,11 +495,6 @@ static const struct pci230_board pci230_boards[] = {
 		.ai_chans	= 16,
 		.ai_bits	= 12,
 	},
-	{
-		/* Wildcard matches any above */
-		.name		= "amplc_pci230",
-		.id		= PCI_DEVICE_ID_INVALID,
-	},
 };
 
 struct pci230_private {
@@ -2553,46 +2542,6 @@ static const struct pci230_board *pci230_find_pci_board(struct pci_dev *pci_dev)
 	return NULL;
 }
 
-/* Look for PCI device matching requested board name, bus and slot. */
-static struct pci_dev *pci230_find_pci_dev(struct comedi_device *dev,
-					   struct comedi_devconfig *it)
-{
-	const struct pci230_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) {
-		/* Check vendor ID (same for all supported PCI boards). */
-		if (pci_dev->vendor != PCI_VENDOR_ID_AMPLICON)
-			continue;
-		/* If bus/slot specified, check them. */
-		if ((bus || slot) &&
-		    (bus != pci_dev->bus->number ||
-		     slot != PCI_SLOT(pci_dev->devfn)))
-			continue;
-		if (thisboard->id == PCI_DEVICE_ID_INVALID) {
-			/* Wildcard board matches any supported PCI board. */
-			const struct pci230_board *foundboard;
-
-			foundboard = pci230_find_pci_board(pci_dev);
-			if (foundboard == NULL)
-				continue;
-			/* Replace wildcard board_ptr. */
-			dev->board_ptr = foundboard;
-		} else {
-			/* Need to match a specific board. */
-			if (!pci230_match_pci_board(thisboard, pci_dev))
-				continue;
-		}
-		return pci_dev;
-	}
-	dev_err(dev->class_dev,
-		"No supported board found! (req. bus %d, slot %d)\n",
-		bus, slot);
-	return NULL;
-}
-
 static int pci230_alloc_private(struct comedi_device *dev)
 {
 	struct pci230_private *devpriv;
@@ -2761,25 +2710,6 @@ static int pci230_attach_common(struct comedi_device *dev,
 	return 0;
 }
 
-static int pci230_attach(struct comedi_device *dev, struct comedi_devconfig *it)
-{
-	const struct pci230_board *thisboard = comedi_board(dev);
-	struct pci_dev *pci_dev;
-	int rc;
-
-	dev_info(dev->class_dev, "amplc_pci230: attach %s %d,%d\n",
-		 thisboard->name, it->options[0], it->options[1]);
-
-	rc = pci230_alloc_private(dev);
-	if (rc)
-		return rc;
-
-	pci_dev = pci230_find_pci_dev(dev, it);
-	if (!pci_dev)
-		return -EIO;
-	return pci230_attach_common(dev, pci_dev);
-}
-
 static int pci230_auto_attach(struct comedi_device *dev,
 			      unsigned long context_unused)
 {
@@ -2821,12 +2751,8 @@ static void pci230_detach(struct comedi_device *dev)
 static struct comedi_driver amplc_pci230_driver = {
 	.driver_name	= "amplc_pci230",
 	.module		= THIS_MODULE,
-	.attach		= pci230_attach,
 	.auto_attach	= pci230_auto_attach,
 	.detach		= pci230_detach,
-	.board_name	= &pci230_boards[0].name,
-	.offset		= sizeof(pci230_boards[0]),
-	.num_names	= ARRAY_SIZE(pci230_boards),
 };
 
 static int amplc_pci230_pci_probe(struct pci_dev *dev,
-- 
2.0.4

[PATCH 06/28] staging: comedi: amplc_pci230: 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 `pci230_detach()` still has a left-over
`pci_dev_put()`, a matching `pci_dev_get()` is needed in the
"auto_attach" handler `pci230_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_pci230.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 393712c..39fa2f6 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2729,23 +2729,12 @@ static int pci230_auto_attach(struct comedi_device *dev,
 			"amplc_pci230: BUG! cannot determine board type!\n");
 		return -EINVAL;
 	}
-	/*
-	 * Need to 'get' the PCI device to match the 'put' in pci230_detach().
-	 * TODO: Remove the pci_dev_get() and matching pci_dev_put() once
-	 * support for manual attachment of PCI devices via pci230_attach()
-	 * has been removed.
-	 */
-	pci_dev_get(pci_dev);
 	return pci230_attach_common(dev, pci_dev);
 }
 
 static void pci230_detach(struct comedi_device *dev)
 {
-	struct pci_dev *pcidev = comedi_to_pci_dev(dev);
-
 	comedi_pci_detach(dev);
-	if (pcidev)
-		pci_dev_put(pcidev);
 }
 
 static struct comedi_driver amplc_pci230_driver = {
-- 
2.0.4

[PATCH 07/28] staging: comedi: amplc_pci230: set detach handler to comedi_pci_detach()

[Ian Abbott]

Since the comedi driver's "detach`" handler `pci230_detach()` now merely
calls `comedi_pci_detach()` with the same parameter, use
`comedi_pci_detach()` itself as the "detach" handler.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 39fa2f6..35d7f0c 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2732,16 +2732,11 @@ static int pci230_auto_attach(struct comedi_device *dev,
 	return pci230_attach_common(dev, pci_dev);
 }
 
-static void pci230_detach(struct comedi_device *dev)
-{
-	comedi_pci_detach(dev);
-}
-
 static struct comedi_driver amplc_pci230_driver = {
 	.driver_name	= "amplc_pci230",
 	.module		= THIS_MODULE,
 	.auto_attach	= pci230_auto_attach,
-	.detach		= pci230_detach,
+	.detach		= comedi_pci_detach,
 };
 
 static int amplc_pci230_pci_probe(struct pci_dev *dev,
-- 
2.0.4

[PATCH 08/28] staging: comedi: amplc_pci230: absorb pci230_attach_common()

[Ian Abbott]

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

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 35d7f0c..46977e5 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2557,15 +2557,31 @@ static int pci230_alloc_private(struct comedi_device *dev)
 	return 0;
 }
 
-/* Common part of attach and auto_attach. */
-static int pci230_attach_common(struct comedi_device *dev,
-				struct pci_dev *pci_dev)
+static int pci230_auto_attach(struct comedi_device *dev,
+			      unsigned long context_unused)
 {
-	const struct pci230_board *thisboard = comedi_board(dev);
-	struct pci230_private *devpriv = dev->private;
+	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
+	const struct pci230_board *thisboard;
+	struct pci230_private *devpriv;
 	struct comedi_subdevice *s;
 	int rc;
 
+	dev_info(dev->class_dev, "amplc_pci230: attach pci %s\n",
+		 pci_name(pci_dev));
+
+	rc = pci230_alloc_private(dev);
+	if (rc)
+		return rc;
+
+	dev->board_ptr = pci230_find_pci_board(pci_dev);
+	if (dev->board_ptr == NULL) {
+		dev_err(dev->class_dev,
+			"amplc_pci230: BUG! cannot determine board type!\n");
+		return -EINVAL;
+	}
+	thisboard = comedi_board(dev);
+	devpriv = dev->private;
+
 	comedi_set_hw_dev(dev, &pci_dev->dev);
 
 	dev->board_name = thisboard->name;
@@ -2710,28 +2726,6 @@ static int pci230_attach_common(struct comedi_device *dev,
 	return 0;
 }
 
-static int pci230_auto_attach(struct comedi_device *dev,
-			      unsigned long context_unused)
-{
-	struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
-	int rc;
-
-	dev_info(dev->class_dev, "amplc_pci230: attach pci %s\n",
-		 pci_name(pci_dev));
-
-	rc = pci230_alloc_private(dev);
-	if (rc)
-		return rc;
-
-	dev->board_ptr = pci230_find_pci_board(pci_dev);
-	if (dev->board_ptr == NULL) {
-		dev_err(dev->class_dev,
-			"amplc_pci230: BUG! cannot determine board type!\n");
-		return -EINVAL;
-	}
-	return pci230_attach_common(dev, pci_dev);
-}
-
 static struct comedi_driver amplc_pci230_driver = {
 	.driver_name	= "amplc_pci230",
 	.module		= THIS_MODULE,
-- 
2.0.4

[PATCH 09/28] staging: comedi: amplc_pci230: 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 `pci230_auto_attach()`.  There is
no need for `pci230_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_pci230.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 46977e5..bd2e648 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2582,8 +2582,6 @@ static int pci230_auto_attach(struct comedi_device *dev,
 	thisboard = comedi_board(dev);
 	devpriv = dev->private;
 
-	comedi_set_hw_dev(dev, &pci_dev->dev);
-
 	dev->board_name = thisboard->name;
 
 	rc = comedi_pci_enable(dev);
-- 
2.0.4

[PATCH 10/28] staging: comedi: amplc_pci230: absorb pci230_alloc_private()

[Ian Abbott]

`pci230_alloc_private()` is now only called from `pci230_auto_attach()`
to allocate private device storage and initialize various spin-lock
members therein.  Absorb the body of `pci230_alloc_private()` into
`pci230_auto_attach()` itself.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index bd2e648..794e0ca 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2542,21 +2542,6 @@ static const struct pci230_board *pci230_find_pci_board(struct pci_dev *pci_dev)
 	return NULL;
 }
 
-static int pci230_alloc_private(struct comedi_device *dev)
-{
-	struct pci230_private *devpriv;
-
-	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
-	if (!devpriv)
-		return -ENOMEM;
-
-	spin_lock_init(&devpriv->isr_spinlock);
-	spin_lock_init(&devpriv->res_spinlock);
-	spin_lock_init(&devpriv->ai_stop_spinlock);
-	spin_lock_init(&devpriv->ao_stop_spinlock);
-	return 0;
-}
-
 static int pci230_auto_attach(struct comedi_device *dev,
 			      unsigned long context_unused)
 {
@@ -2569,9 +2554,14 @@ static int pci230_auto_attach(struct comedi_device *dev,
 	dev_info(dev->class_dev, "amplc_pci230: attach pci %s\n",
 		 pci_name(pci_dev));
 
-	rc = pci230_alloc_private(dev);
-	if (rc)
-		return rc;
+	devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
+	if (!devpriv)
+		return -ENOMEM;
+
+	spin_lock_init(&devpriv->isr_spinlock);
+	spin_lock_init(&devpriv->res_spinlock);
+	spin_lock_init(&devpriv->ai_stop_spinlock);
+	spin_lock_init(&devpriv->ao_stop_spinlock);
 
 	dev->board_ptr = pci230_find_pci_board(pci_dev);
 	if (dev->board_ptr == NULL) {
@@ -2580,8 +2570,6 @@ static int pci230_auto_attach(struct comedi_device *dev,
 		return -EINVAL;
 	}
 	thisboard = comedi_board(dev);
-	devpriv = dev->private;
-
 	dev->board_name = thisboard->name;
 
 	rc = comedi_pci_enable(dev);
-- 
2.0.4

[PATCH 11/28] staging: comedi: amplc_pci230: remove ai_chans member

[Ian Abbott]

All boards supported by the "amplc_pci230" driver have 16 AI channels,
so the `ai_chans` member of `struct pci230_board` is superfluous and can
be removed.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 794e0ca..42a79d2 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -454,7 +454,6 @@ enum {
 struct pci230_board {
 	const char *name;
 	unsigned short id;
-	int ai_chans;
 	int ai_bits;
 	int ao_chans;
 	int ao_bits;
@@ -466,7 +465,6 @@ static const struct pci230_board pci230_boards[] = {
 	{
 		.name		= "pci230+",
 		.id		= PCI_DEVICE_ID_PCI230,
-		.ai_chans	= 16,
 		.ai_bits	= 16,
 		.ao_chans	= 2,
 		.ao_bits	= 12,
@@ -476,14 +474,12 @@ static const struct pci230_board pci230_boards[] = {
 	{
 		.name		= "pci260+",
 		.id		= PCI_DEVICE_ID_PCI260,
-		.ai_chans	= 16,
 		.ai_bits	= 16,
 		.min_hwver	= 1,
 	},
 	{
 		.name		= "pci230",
 		.id		= PCI_DEVICE_ID_PCI230,
-		.ai_chans	= 16,
 		.ai_bits	= 12,
 		.ao_chans	= 2,
 		.ao_bits	= 12,
@@ -492,7 +488,6 @@ static const struct pci230_board pci230_boards[] = {
 	{
 		.name		= "pci260",
 		.id		= PCI_DEVICE_ID_PCI260,
-		.ai_chans	= 16,
 		.ai_bits	= 12,
 	},
 };
@@ -2660,7 +2655,7 @@ static int pci230_auto_attach(struct comedi_device *dev,
 	/* analog input subdevice */
 	s->type = COMEDI_SUBD_AI;
 	s->subdev_flags = SDF_READABLE | SDF_DIFF | SDF_GROUND;
-	s->n_chan = thisboard->ai_chans;
+	s->n_chan = 16;
 	s->maxdata = (1 << thisboard->ai_bits) - 1;
 	s->range_table = &pci230_ai_range;
 	s->insn_read = pci230_ai_rinsn;
-- 
2.0.4

[PATCH 12/28] staging: comedi: amplc_pci230: remove ao_chans member

[Ian Abbott]

The PCI230(+) has an AO subdevice with 2 channels, but the PCI260(+) has
none.

The `ao_chans` member of `struct pci230_board` indicates whether the
board has an AO subdevice and the number of AO channels.  The
`ao_bits` member indicates the AO sample width in bits and will only be
non-zero for boards with an AO subdevice.

Use `ao_bits` to indicate whether the board has an AO subdevice.  If it
has, assume the the number of AO channels is 2.  Then the `ao_chans`
member becomes redundant and can be removed.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 42a79d2..75b0e79 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -455,7 +455,6 @@ struct pci230_board {
 	const char *name;
 	unsigned short id;
 	int ai_bits;
-	int ao_chans;
 	int ao_bits;
 	int have_dio;
 	unsigned int min_hwver;	/* Minimum hardware version supported. */
@@ -466,7 +465,6 @@ static const struct pci230_board pci230_boards[] = {
 		.name		= "pci230+",
 		.id		= PCI_DEVICE_ID_PCI230,
 		.ai_bits	= 16,
-		.ao_chans	= 2,
 		.ao_bits	= 12,
 		.have_dio	= 1,
 		.min_hwver	= 1,
@@ -481,7 +479,6 @@ static const struct pci230_board pci230_boards[] = {
 		.name		= "pci230",
 		.id		= PCI_DEVICE_ID_PCI230,
 		.ai_bits	= 12,
-		.ao_chans	= 2,
 		.ao_bits	= 12,
 		.have_dio	= 1,
 	},
@@ -2609,7 +2606,7 @@ static int pci230_auto_attach(struct comedi_device *dev,
 				 */
 				extfunc |= PCI230P_EXTFUNC_GAT_EXTTRIG;
 			}
-			if (thisboard->ao_chans > 0 && devpriv->hwver >= 2) {
+			if (thisboard->ao_bits && devpriv->hwver >= 2) {
 				/* Enable DAC FIFO functionality. */
 				extfunc |= PCI230P2_EXTFUNC_DACFIFO;
 			}
@@ -2670,15 +2667,15 @@ static int pci230_auto_attach(struct comedi_device *dev,
 
 	s = &dev->subdevices[1];
 	/* analog output subdevice */
-	if (thisboard->ao_chans > 0) {
+	if (thisboard->ao_bits) {
 		s->type = COMEDI_SUBD_AO;
 		s->subdev_flags = SDF_WRITABLE | SDF_GROUND;
-		s->n_chan = thisboard->ao_chans;
+		s->n_chan = 2;
 		s->maxdata = (1 << thisboard->ao_bits) - 1;
 		s->range_table = &pci230_ao_range;
 		s->insn_write = pci230_ao_insn_write;
 		s->insn_read = comedi_readback_insn_read;
-		s->len_chanlist = thisboard->ao_chans;
+		s->len_chanlist = 2;
 		if (dev->irq) {
 			dev->write_subdev = s;
 			s->subdev_flags |= SDF_CMD_WRITE;
-- 
2.0.4

[PATCH 13/28] staging: comedi: amplc_pci230: shrink struct pci230_board

[Ian Abbott]

The `ai_bits`, `ao_bits`, and `min_hwver` members of `struct
pci230_board` are only set to small, non-negative values, so make them
`unsigned char`.  The `have_dio` member is used as a boolean so change
it to a bitfield of type `bool`.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 75b0e79..7fa9c70 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -454,10 +454,10 @@ enum {
 struct pci230_board {
 	const char *name;
 	unsigned short id;
-	int ai_bits;
-	int ao_bits;
-	int have_dio;
-	unsigned int min_hwver;	/* Minimum hardware version supported. */
+	unsigned char ai_bits;
+	unsigned char ao_bits;
+	unsigned char min_hwver; /* Minimum hardware version supported. */
+	bool have_dio:1;
 };
 
 static const struct pci230_board pci230_boards[] = {
@@ -466,7 +466,7 @@ static const struct pci230_board pci230_boards[] = {
 		.id		= PCI_DEVICE_ID_PCI230,
 		.ai_bits	= 16,
 		.ao_bits	= 12,
-		.have_dio	= 1,
+		.have_dio	= true,
 		.min_hwver	= 1,
 	},
 	{
@@ -480,7 +480,7 @@ static const struct pci230_board pci230_boards[] = {
 		.id		= PCI_DEVICE_ID_PCI230,
 		.ai_bits	= 12,
 		.ao_bits	= 12,
-		.have_dio	= 1,
+		.have_dio	= true,
 	},
 	{
 		.name		= "pci260",
-- 
2.0.4

[PATCH 14/28] staging: comedi: amplc_pci230: simplify pci230_ao_mangle_datum()

[Ian Abbott]

`pci230_ao_mangle_datum()` converts comedi sample values for the AO
subdevice to hardware register values.  The comedi sample value will be
an unsigned value in the range 0 to 4095 (assuming 12-bit resolution).
The hardware wants the value shifted so the m.s. bit of the sample in in
bit 15.  If set to a bipolar range, it also expects a 2's complement
value, so the top bit of the sample value needs to be inverted in that
case.

Simplify the existing code by doing the 2's complement conversion after
the shift.  That way, it is always bit 15 that is inverted regardless of
the resolution.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 7fa9c70..e4151d7 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -583,17 +583,16 @@ static inline unsigned short pci230_ao_mangle_datum(struct comedi_device *dev,
 	struct pci230_private *devpriv = dev->private;
 
 	/*
-	 * If a bipolar range was specified, mangle it
-	 * (straight binary->twos complement).
-	 */
-	if (devpriv->ao_bipolar)
-		datum ^= 1 << (thisboard->ao_bits - 1);
-
-	/*
 	 * PCI230 is 12 bit - stored in upper bits of 16 bit register (lower
 	 * four bits reserved for expansion).  PCI230+ is also 12 bit AO.
 	 */
 	datum <<= (16 - thisboard->ao_bits);
+	/*
+	 * If a bipolar range was specified, mangle it
+	 * (straight binary->twos complement).
+	 */
+	if (devpriv->ao_bipolar)
+		datum ^= 0x8000;
 	return datum;
 }
 
-- 
2.0.4

[PATCH 15/28] staging: comedi: amplc_pci230: simplify pci230_ai_read()

[Ian Abbott]

`pci230_ai_read()` reads a sample from the ADC data register and
converts it to a comedi sample value.  The AI sample may have 12 or 16
bits of resolution, depending on the board type, but 12-bit sample
values are in bits 15 to 4 of the register.  The hardware value is
signed, 2's complement if set to a bipolar mode, or unsigned, straight
binary if set to a unipolar mode.  To convert to a Comedi sample value
it may need shifting right by 4 bits, and the top bit of the sample
value may need to be toggled.

Simplify the existing code by doing the 2's complement to straight
binary conversion before the shift.  That way, it is always bit 15 that
is inverted regardless of the resolution.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index e4151d7..70b210b 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -563,16 +563,13 @@ static unsigned short pci230_ai_read(struct comedi_device *dev)
 	/*
 	 * PCI230 is 12 bit - stored in upper bits of 16 bit register
 	 * (lower four bits reserved for expansion).  PCI230+ is 16 bit AI.
-	 */
-	data = data >> (16 - thisboard->ai_bits);
-
-	/*
+	 *
 	 * If a bipolar range was specified, mangle it
 	 * (twos complement->straight binary).
 	 */
 	if (devpriv->ai_bipolar)
-		data ^= 1 << (thisboard->ai_bits - 1);
-
+		data ^= 0x8000;
+	data >>= (16 - thisboard->ai_bits);
 	return data;
 }
 
-- 
2.0.4

[PATCH 16/28] staging: comedi: amplc_pci230: remove 'inline'

[Ian Abbott]

Some functions in "amplc_pci230.c" are declared `inline`.  Remove the
`inline` specifiers and let the compiler do what it wants with them.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 70b210b..d832268 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -573,8 +573,8 @@ static unsigned short pci230_ai_read(struct comedi_device *dev)
 	return data;
 }
 
-static inline unsigned short pci230_ao_mangle_datum(struct comedi_device *dev,
-						    unsigned short datum)
+static unsigned short pci230_ao_mangle_datum(struct comedi_device *dev,
+					     unsigned short datum)
 {
 	const struct pci230_board *thisboard = comedi_board(dev);
 	struct pci230_private *devpriv = dev->private;
@@ -593,9 +593,8 @@ static inline unsigned short pci230_ao_mangle_datum(struct comedi_device *dev,
 	return datum;
 }
 
-static inline void pci230_ao_write_nofifo(struct comedi_device *dev,
-					  unsigned short datum,
-					  unsigned int chan)
+static void pci230_ao_write_nofifo(struct comedi_device *dev,
+				   unsigned short datum, unsigned int chan)
 {
 	struct pci230_private *devpriv = dev->private;
 
@@ -604,8 +603,8 @@ static inline void pci230_ao_write_nofifo(struct comedi_device *dev,
 	     devpriv->daqio + ((chan == 0) ? PCI230_DACOUT1 : PCI230_DACOUT2));
 }
 
-static inline void pci230_ao_write_fifo(struct comedi_device *dev,
-					unsigned short datum, unsigned int chan)
+static void pci230_ao_write_fifo(struct comedi_device *dev,
+				 unsigned short datum, unsigned int chan)
 {
 	struct pci230_private *devpriv = dev->private;
 
@@ -650,8 +649,8 @@ static int get_resources(struct comedi_device *dev, unsigned int res_mask,
 	return ok;
 }
 
-static inline int get_one_resource(struct comedi_device *dev,
-				   unsigned int resource, unsigned char owner)
+static int get_one_resource(struct comedi_device *dev,
+			    unsigned int resource, unsigned char owner)
 {
 	return get_resources(dev, 1U << resource, owner);
 }
@@ -675,14 +674,13 @@ static void put_resources(struct comedi_device *dev, unsigned int res_mask,
 	spin_unlock_irqrestore(&devpriv->res_spinlock, irqflags);
 }
 
-static inline void put_one_resource(struct comedi_device *dev,
-				    unsigned int resource, unsigned char owner)
+static void put_one_resource(struct comedi_device *dev,
+			     unsigned int resource, unsigned char owner)
 {
 	put_resources(dev, 1U << resource, owner);
 }
 
-static inline void put_all_resources(struct comedi_device *dev,
-				     unsigned char owner)
+static void put_all_resources(struct comedi_device *dev, unsigned char owner)
 {
 	put_resources(dev, (1U << NUM_RESOURCES) - 1, owner);
 }
-- 
2.0.4

[PATCH 17/28] staging: comedi: amplc_pci230: rename pci230_ai_rinsn()

[Ian Abbott]

Rename the AI subdevice "insn_read" handler function `pci230_ai_rinsn()`
to `pci230_ai_insn_read()` for consistency.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index d832268..72da4b6 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -774,9 +774,9 @@ static int pci230_ai_eoc(struct comedi_device *dev,
 	return -EBUSY;
 }
 
-static int pci230_ai_rinsn(struct comedi_device *dev,
-			   struct comedi_subdevice *s, struct comedi_insn *insn,
-			   unsigned int *data)
+static int pci230_ai_insn_read(struct comedi_device *dev,
+			       struct comedi_subdevice *s,
+			       struct comedi_insn *insn, unsigned int *data)
 {
 	struct pci230_private *devpriv = dev->private;
 	unsigned int n;
@@ -2649,7 +2649,7 @@ static int pci230_auto_attach(struct comedi_device *dev,
 	s->n_chan = 16;
 	s->maxdata = (1 << thisboard->ai_bits) - 1;
 	s->range_table = &pci230_ai_range;
-	s->insn_read = pci230_ai_rinsn;
+	s->insn_read = pci230_ai_insn_read;
 	s->len_chanlist = 256;	/* but there are restrictions. */
 	if (dev->irq) {
 		dev->read_subdev = s;
-- 
2.0.4

[PATCH 18/28] staging: comedi: amplc_pci230: add `pci230_` prefix to functions

[Ian Abbott]

Most functions in "amplc_pci230" are named with the prefix `pci230_`,
apart from one or two that have the prefix `amplc_pci230_` and a few
odd-balls with no particular prefix.  Rename the ones without a prefix
for consistency.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 72da4b6..6cd235d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -613,8 +613,8 @@ static void pci230_ao_write_fifo(struct comedi_device *dev,
 	     devpriv->daqio + PCI230P2_DACDATA);
 }
 
-static int get_resources(struct comedi_device *dev, unsigned int res_mask,
-			 unsigned char owner)
+static int pci230_get_resources(struct comedi_device *dev,
+				unsigned int res_mask, unsigned char owner)
 {
 	struct pci230_private *devpriv = dev->private;
 	int ok;
@@ -649,14 +649,14 @@ static int get_resources(struct comedi_device *dev, unsigned int res_mask,
 	return ok;
 }
 
-static int get_one_resource(struct comedi_device *dev,
-			    unsigned int resource, unsigned char owner)
+static int pci230_get_one_resource(struct comedi_device *dev,
+				   unsigned int resource, unsigned char owner)
 {
-	return get_resources(dev, 1U << resource, owner);
+	return pci230_get_resources(dev, 1U << resource, owner);
 }
 
-static void put_resources(struct comedi_device *dev, unsigned int res_mask,
-			  unsigned char owner)
+static void pci230_put_resources(struct comedi_device *dev,
+				 unsigned int res_mask, unsigned char owner)
 {
 	struct pci230_private *devpriv = dev->private;
 	unsigned int i;
@@ -674,19 +674,20 @@ static void put_resources(struct comedi_device *dev, unsigned int res_mask,
 	spin_unlock_irqrestore(&devpriv->res_spinlock, irqflags);
 }
 
-static void put_one_resource(struct comedi_device *dev,
-			     unsigned int resource, unsigned char owner)
+static void pci230_put_one_resource(struct comedi_device *dev,
+				    unsigned int resource, unsigned char owner)
 {
-	put_resources(dev, 1U << resource, owner);
+	pci230_put_resources(dev, 1U << resource, owner);
 }
 
-static void put_all_resources(struct comedi_device *dev, unsigned char owner)
+static void pci230_put_all_resources(struct comedi_device *dev,
+				     unsigned char owner)
 {
-	put_resources(dev, (1U << NUM_RESOURCES) - 1, owner);
+	pci230_put_resources(dev, (1U << NUM_RESOURCES) - 1, owner);
 }
 
-static unsigned int divide_ns(uint64_t ns, unsigned int timebase,
-			      unsigned int flags)
+static unsigned int pci230_divide_ns(uint64_t ns, unsigned int timebase,
+				     unsigned int flags)
 {
 	uint64_t div;
 	unsigned int rem;
@@ -717,7 +718,7 @@ static unsigned int pci230_choose_clk_count(uint64_t ns, unsigned int *count,
 	unsigned int clk_src, cnt;
 
 	for (clk_src = CLK_10MHZ;; clk_src++) {
-		cnt = divide_ns(ns, pci230_timebase[clk_src], flags);
+		cnt = pci230_divide_ns(ns, pci230_timebase[clk_src], flags);
 		if (cnt <= 65536 || clk_src == CLK_1KHZ)
 			break;
 	}
@@ -1112,7 +1113,7 @@ static void pci230_ao_stop(struct comedi_device *dev,
 		     devpriv->daqio + PCI230_DACCON);
 	}
 	/* Release resources. */
-	put_all_resources(dev, OWNER_AOCMD);
+	pci230_put_all_resources(dev, OWNER_AOCMD);
 }
 
 static void pci230_handle_ao_nofifo(struct comedi_device *dev,
@@ -1408,7 +1409,7 @@ static int pci230_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 
 	if (cmd->scan_begin_src == TRIG_TIMER) {
 		/* Claim Z2-CT1. */
-		if (!get_one_resource(dev, RES_Z2CT1, OWNER_AOCMD))
+		if (!pci230_get_one_resource(dev, RES_Z2CT1, OWNER_AOCMD))
 			return -EBUSY;
 	}
 
@@ -1963,7 +1964,7 @@ static void pci230_ai_stop(struct comedi_device *dev,
 	outw(devpriv->adccon | PCI230_ADC_FIFO_RESET,
 	     devpriv->daqio + PCI230_ADCCON);
 	/* Release resources. */
-	put_all_resources(dev, OWNER_AICMD);
+	pci230_put_all_resources(dev, OWNER_AICMD);
 }
 
 static void pci230_ai_start(struct comedi_device *dev,
@@ -2112,7 +2113,7 @@ static void pci230_ai_start(struct comedi_device *dev,
 			}
 		} else if (cmd->convert_src != TRIG_INT) {
 			/* No longer need Z2-CT2. */
-			put_one_resource(dev, RES_Z2CT2, OWNER_AICMD);
+			pci230_put_one_resource(dev, RES_Z2CT2, OWNER_AICMD);
 		}
 	}
 }
@@ -2253,7 +2254,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		}
 	}
 	/* Claim resources. */
-	if (!get_resources(dev, res_mask, OWNER_AICMD))
+	if (!pci230_get_resources(dev, res_mask, OWNER_AICMD))
 		return -EBUSY;
 
 	/* Get number of scans required. */
-- 
2.0.4

[PATCH 19/28] staging: comedi: amplc_pci230: use comedi_range_is_bipolar()

[Ian Abbott]

Use the inline `comedi_range_is_bipolar()` function from "comedidev.h"
to decide whether a range is bipolar or unipolar instead of using the
local static arrays `pci230_ai_bipolar[]` and `pci230_ao_bipolar[]`
which can then be removed.

Change the types of the `ai_bipolar` and `ao_bipolar` members of `struct
pci230_private` to `bool` to match the return value of
`comedi_range_is_bipolar()` and change them into single-bit bitfields to
save a bit of space.  Also change the type and name of some local
variables in `pci230_ai_check_chanlist()` that hold the result of
`comedi_range_is_bipolar()`.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 6cd235d..8959820 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -506,11 +506,11 @@ struct pci230_private {
 	unsigned short adcfifothresh;	/* ADC FIFO threshold (PCI230+/260+) */
 	unsigned short adcg;		/* ADCG register value */
 	unsigned char int_en;		/* Interrupt enable bits */
-	unsigned char ai_bipolar;	/* Flag AI range is bipolar */
-	unsigned char ao_bipolar;	/* Flag AO range is bipolar */
 	unsigned char ier;		/* Copy of interrupt enable register */
 	unsigned char intr_running;	/* Flag set in interrupt routine */
 	unsigned char res_owner[NUM_RESOURCES]; /* Shared resource owners */
+	bool ai_bipolar:1;		/* Flag AI range is bipolar */
+	bool ao_bipolar:1;		/* Flag AO range is bipolar */
 };
 
 /* PCI230 clock source periods in ns */
@@ -538,9 +538,6 @@ static const struct comedi_lrange pci230_ai_range = {
 /* PCI230 analogue gain bits for each input range. */
 static const unsigned char pci230_ai_gain[7] = { 0, 1, 2, 3, 1, 2, 3 };
 
-/* PCI230 adccon bipolar flag for each analogue input range. */
-static const unsigned char pci230_ai_bipolar[7] = { 1, 1, 1, 1, 0, 0, 0 };
-
 /* PCI230 analogue output range table */
 static const struct comedi_lrange pci230_ao_range = {
 	2, {
@@ -549,9 +546,6 @@ static const struct comedi_lrange pci230_ao_range = {
 	}
 };
 
-/* PCI230 daccon bipolar flag for each analogue output range. */
-static const unsigned char pci230_ao_bipolar[2] = { 0, 1 };
-
 static unsigned short pci230_ai_read(struct comedi_device *dev)
 {
 	const struct pci230_board *thisboard = comedi_board(dev);
@@ -809,7 +803,7 @@ static int pci230_ai_insn_read(struct comedi_device *dev,
 	adccon = PCI230_ADC_TRIG_Z2CT2 | PCI230_ADC_FIFO_EN;
 	/* Set Z2-CT2 output low to avoid any false triggers. */
 	i8254_set_mode(dev->iobase + PCI230_Z2_CT_BASE, 0, 2, I8254_MODE0);
-	devpriv->ai_bipolar = pci230_ai_bipolar[range];
+	devpriv->ai_bipolar = comedi_range_is_bipolar(s, range);
 	if (aref == AREF_DIFF) {
 		/* Differential. */
 		gainshift = chan * 2;
@@ -892,7 +886,7 @@ static int pci230_ao_insn_write(struct comedi_device *dev,
 	 * Set range - see analogue output range table; 0 => unipolar 10V,
 	 * 1 => bipolar +/-10V range scale
 	 */
-	devpriv->ao_bipolar = pci230_ao_bipolar[range];
+	devpriv->ao_bipolar = comedi_range_is_bipolar(s, range);
 	outw(range, devpriv->daqio + PCI230_DACCON);
 
 	for (i = 0; i < insn->n; i++) {
@@ -1424,7 +1418,7 @@ static int pci230_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 * 1 => bipolar +/-10V range scale
 	 */
 	range = CR_RANGE(cmd->chanlist[0]);
-	devpriv->ao_bipolar = pci230_ao_bipolar[range];
+	devpriv->ao_bipolar = comedi_range_is_bipolar(s, range);
 	daccon = devpriv->ao_bipolar ? PCI230_DAC_OR_BIP : PCI230_DAC_OR_UNI;
 	/* Use DAC FIFO for hardware version 2 onwards. */
 	if (devpriv->hwver >= 2) {
@@ -1514,7 +1508,7 @@ static int pci230_ai_check_chanlist(struct comedi_device *dev,
 	unsigned int prev_chan = 0;
 	unsigned int prev_range = 0;
 	unsigned int prev_aref = 0;
-	unsigned int prev_polarity = 0;
+	bool prev_bipolar = false;
 	unsigned int subseq_len = 0;
 	int i;
 
@@ -1523,7 +1517,7 @@ static int pci230_ai_check_chanlist(struct comedi_device *dev,
 		unsigned int chan = CR_CHAN(chanspec);
 		unsigned int range = CR_RANGE(chanspec);
 		unsigned int aref = CR_AREF(chanspec);
-		unsigned int polarity = pci230_ai_bipolar[range];
+		bool bipolar = comedi_range_is_bipolar(s, range);
 
 		if (aref == AREF_DIFF && chan >= max_diff_chan) {
 			dev_dbg(dev->class_dev,
@@ -1555,7 +1549,7 @@ static int pci230_ai_check_chanlist(struct comedi_device *dev,
 				return -EINVAL;
 			}
 
-			if (polarity != prev_polarity) {
+			if (bipolar != prev_bipolar) {
 				dev_dbg(dev->class_dev,
 					"%s: channel sequence ranges must be all bipolar or all unipolar\n",
 					__func__);
@@ -1573,7 +1567,7 @@ static int pci230_ai_check_chanlist(struct comedi_device *dev,
 		prev_chan = chan;
 		prev_range = range;
 		prev_aref = aref;
-		prev_polarity = polarity;
+		prev_bipolar = bipolar;
 	}
 
 	if (subseq_len == 0)
@@ -2296,7 +2290,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	}
 
 	range = CR_RANGE(cmd->chanlist[0]);
-	devpriv->ai_bipolar = pci230_ai_bipolar[range];
+	devpriv->ai_bipolar = comedi_range_is_bipolar(s, range);
 	if (devpriv->ai_bipolar)
 		adccon |= PCI230_ADC_IR_BIP;
 	else
-- 
2.0.4

[PATCH 20/28] staging: comedi: amplc_pci230: make `intr_running` a bitfield

[Ian Abbott]

Change the `intr_running` member of `struct pci230_private` into a
single-bit bitfield of type `bool` to save a bit of space.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 8959820..29e6016 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -507,8 +507,8 @@ struct pci230_private {
 	unsigned short adcg;		/* ADCG register value */
 	unsigned char int_en;		/* Interrupt enable bits */
 	unsigned char ier;		/* Copy of interrupt enable register */
-	unsigned char intr_running;	/* Flag set in interrupt routine */
 	unsigned char res_owner[NUM_RESOURCES]; /* Shared resource owners */
+	bool intr_running:1;		/* Flag set in interrupt routine */
 	bool ai_bipolar:1;		/* Flag AI range is bipolar */
 	bool ao_bipolar:1;		/* Flag AO range is bipolar */
 };
@@ -2451,7 +2451,7 @@ static irqreturn_t pci230_interrupt(int irq, void *d)
 	 */
 	devpriv->ier = devpriv->int_en & ~status_int;
 	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
-	devpriv->intr_running = 1;
+	devpriv->intr_running = true;
 	devpriv->intr_cpuid = THISCPU;
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 
@@ -2487,7 +2487,7 @@ static irqreturn_t pci230_interrupt(int irq, void *d)
 		devpriv->ier = devpriv->int_en;
 		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 	}
-	devpriv->intr_running = 0;
+	devpriv->intr_running = false;
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 
 	return IRQ_HANDLED;
-- 
2.0.4

[PATCH 21/28] staging: comedi: amplc_pci230: replace `state` member with bitfields

[Ian Abbott]

The `state` member of `struct pci230_private` is used with the atomic
bit-op functions and has a couple of bits defined, `AI_CMD_STARTED` and
`AO_CMD_STARTED`.  Spin-locks are used to protect the clearing of these
bits and other stuff.  No special protection is used for setting these
bits.  Replace the `state` member with a couple of new, single-bit
bitfield members, `ai_cmd_started` and `ao_cmd_started` to save some
space.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 29e6016..7a8291a 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -443,10 +443,6 @@ enum {
 /* Current CPU.  XXX should this be hard_smp_processor_id()? */
 #define THISCPU		smp_processor_id()
 
-/* State flags for atomic bit operations */
-#define AI_CMD_STARTED	0
-#define AO_CMD_STARTED	1
-
 /*
  * Board descriptions for the two boards supported.
  */
@@ -495,7 +491,6 @@ struct pci230_private {
 	spinlock_t ai_stop_spinlock;	/* Spin lock for stopping AI command */
 	spinlock_t ao_stop_spinlock;	/* Spin lock for stopping AO command */
 	unsigned long daqio;		/* PCI230's DAQ I/O space */
-	unsigned long state;		/* State flags */
 	unsigned int ai_scan_count;	/* Number of AI scans remaining */
 	unsigned int ai_scan_pos;	/* Current position within AI scan */
 	unsigned int ao_scan_count;	/* Number of AO scans remaining.  */
@@ -511,6 +506,8 @@ struct pci230_private {
 	bool intr_running:1;		/* Flag set in interrupt routine */
 	bool ai_bipolar:1;		/* Flag AI range is bipolar */
 	bool ao_bipolar:1;		/* Flag AO range is bipolar */
+	bool ai_cmd_started:1;		/* Flag AI command started */
+	bool ao_cmd_started:1;		/* Flag AO command started */
 };
 
 /* PCI230 clock source periods in ns */
@@ -1060,11 +1057,12 @@ static void pci230_ao_stop(struct comedi_device *dev,
 	struct pci230_private *devpriv = dev->private;
 	unsigned long irqflags;
 	unsigned char intsrc;
-	int started;
+	bool started;
 	struct comedi_cmd *cmd;
 
 	spin_lock_irqsave(&devpriv->ao_stop_spinlock, irqflags);
-	started = test_and_clear_bit(AO_CMD_STARTED, &devpriv->state);
+	started = devpriv->ao_cmd_started;
+	devpriv->ao_cmd_started = false;
 	spin_unlock_irqrestore(&devpriv->ao_stop_spinlock, irqflags);
 	if (!started)
 		return;
@@ -1263,7 +1261,7 @@ static int pci230_ao_inttrig_scan_begin(struct comedi_device *dev,
 		return -EINVAL;
 
 	spin_lock_irqsave(&devpriv->ao_stop_spinlock, irqflags);
-	if (test_bit(AO_CMD_STARTED, &devpriv->state)) {
+	if (devpriv->ao_cmd_started) {
 		/* Perform scan. */
 		if (devpriv->hwver < 2) {
 			/* Not using DAC FIFO. */
@@ -1296,7 +1294,7 @@ static void pci230_ao_start(struct comedi_device *dev,
 	struct comedi_cmd *cmd = &async->cmd;
 	unsigned long irqflags;
 
-	set_bit(AO_CMD_STARTED, &devpriv->state);
+	devpriv->ao_cmd_started = true;
 	if (cmd->stop_src == TRIG_COUNT && devpriv->ao_scan_count == 0) {
 		/* An empty acquisition! */
 		async->events |= COMEDI_CB_EOA;
@@ -1852,7 +1850,7 @@ static int pci230_ai_inttrig_convert(struct comedi_device *dev,
 		return -EINVAL;
 
 	spin_lock_irqsave(&devpriv->ai_stop_spinlock, irqflags);
-	if (test_bit(AI_CMD_STARTED, &devpriv->state)) {
+	if (devpriv->ai_cmd_started) {
 		unsigned int delayus;
 
 		/*
@@ -1899,7 +1897,7 @@ static int pci230_ai_inttrig_scan_begin(struct comedi_device *dev,
 		return -EINVAL;
 
 	spin_lock_irqsave(&devpriv->ai_stop_spinlock, irqflags);
-	if (test_bit(AI_CMD_STARTED, &devpriv->state)) {
+	if (devpriv->ai_cmd_started) {
 		/* Trigger scan by waggling CT0 gate source. */
 		zgat = GAT_CONFIG(0, GAT_GND);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
@@ -1917,10 +1915,11 @@ static void pci230_ai_stop(struct comedi_device *dev,
 	struct pci230_private *devpriv = dev->private;
 	unsigned long irqflags;
 	struct comedi_cmd *cmd;
-	int started;
+	bool started;
 
 	spin_lock_irqsave(&devpriv->ai_stop_spinlock, irqflags);
-	started = test_and_clear_bit(AI_CMD_STARTED, &devpriv->state);
+	started = devpriv->ai_cmd_started;
+	devpriv->ai_cmd_started = false;
 	spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);
 	if (!started)
 		return;
@@ -1970,7 +1969,7 @@ static void pci230_ai_start(struct comedi_device *dev,
 	struct comedi_async *async = s->async;
 	struct comedi_cmd *cmd = &async->cmd;
 
-	set_bit(AI_CMD_STARTED, &devpriv->state);
+	devpriv->ai_cmd_started = true;
 	if (cmd->stop_src == TRIG_COUNT && devpriv->ai_scan_count == 0) {
 		/* An empty acquisition! */
 		async->events |= COMEDI_CB_EOA;
-- 
2.0.4

[PATCH 22/28] staging: comedi: amplc_pci230: rewrite shared resource handling

[Ian Abbott]

Some counter channels may be required for AI commands and AO commands.
Depending on how the commands are set up, it may not be possible to run
both at the same time, so we keep some state and code to find out if the
required resources are busy or not.

The existing code is a bit unwieldy - the code for claiming resources
involves two `for` loops for example.  Rewrite it to make it simpler.

The new code just has a bit-mask value for each shared resource (counter
channels), and an array indexed by resource "owners" (AI and AO
commands), so the code for claiming resources now just has a single loop
that checks that none of the other owners have claimed the wanted
resources.

Rename the functions involved, because the old names involving 'put' and
'get' suggested some sort of usage counting.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 7a8291a..727de3d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -421,16 +421,15 @@
  * (Potentially) shared resources and their owners
  */
 enum {
-	RES_Z2CT0,		/* Z2-CT0 */
-	RES_Z2CT1,		/* Z2-CT1 */
-	RES_Z2CT2,		/* Z2-CT2 */
-	NUM_RESOURCES		/* Number of (potentially) shared resources. */
+	RES_Z2CT0 = (1U << 0),	/* Z2-CT0 */
+	RES_Z2CT1 = (1U << 1),	/* Z2-CT1 */
+	RES_Z2CT2 = (1U << 2)	/* Z2-CT2 */
 };
 
 enum {
-	OWNER_NONE,		/* Not owned */
 	OWNER_AICMD,		/* Owned by AI command */
-	OWNER_AOCMD		/* Owned by AO command */
+	OWNER_AOCMD,		/* Owned by AO command */
+	NUM_OWNERS		/* Number of owners */
 };
 
 /*
@@ -502,7 +501,7 @@ struct pci230_private {
 	unsigned short adcg;		/* ADCG register value */
 	unsigned char int_en;		/* Interrupt enable bits */
 	unsigned char ier;		/* Copy of interrupt enable register */
-	unsigned char res_owner[NUM_RESOURCES]; /* Shared resource owners */
+	unsigned char res_owned[NUM_OWNERS]; /* Owned resources */
 	bool intr_running:1;		/* Flag set in interrupt routine */
 	bool ai_bipolar:1;		/* Flag AI range is bipolar */
 	bool ao_bipolar:1;		/* Flag AO range is bipolar */
@@ -604,77 +603,43 @@ static void pci230_ao_write_fifo(struct comedi_device *dev,
 	     devpriv->daqio + PCI230P2_DACDATA);
 }
 
-static int pci230_get_resources(struct comedi_device *dev,
-				unsigned int res_mask, unsigned char owner)
+static bool pci230_claim_shared(struct comedi_device *dev,
+				unsigned char res_mask, unsigned int owner)
 {
 	struct pci230_private *devpriv = dev->private;
-	int ok;
-	unsigned int i;
-	unsigned int b;
-	unsigned int claimed;
+	unsigned int o;
 	unsigned long irqflags;
 
-	ok = 1;
-	claimed = 0;
 	spin_lock_irqsave(&devpriv->res_spinlock, irqflags);
-	for (b = 1, i = 0; i < NUM_RESOURCES && res_mask; b <<= 1, i++) {
-		if (res_mask & b) {
-			res_mask &= ~b;
-			if (devpriv->res_owner[i] == OWNER_NONE) {
-				devpriv->res_owner[i] = owner;
-				claimed |= b;
-			} else if (devpriv->res_owner[i] != owner) {
-				for (b = 1, i = 0; claimed; b <<= 1, i++) {
-					if (claimed & b) {
-						devpriv->res_owner[i] =
-						    OWNER_NONE;
-						claimed &= ~b;
-					}
-				}
-				ok = 0;
-				break;
-			}
+	for (o = 0; o < NUM_OWNERS; o++) {
+		if (o == owner)
+			continue;
+		if (devpriv->res_owned[o] & res_mask) {
+			spin_unlock_irqrestore(&devpriv->res_spinlock,
+					       irqflags);
+			return false;
 		}
 	}
+	devpriv->res_owned[owner] |= res_mask;
 	spin_unlock_irqrestore(&devpriv->res_spinlock, irqflags);
-	return ok;
-}
-
-static int pci230_get_one_resource(struct comedi_device *dev,
-				   unsigned int resource, unsigned char owner)
-{
-	return pci230_get_resources(dev, 1U << resource, owner);
+	return true;
 }
 
-static void pci230_put_resources(struct comedi_device *dev,
-				 unsigned int res_mask, unsigned char owner)
+static void pci230_release_shared(struct comedi_device *dev,
+				  unsigned char res_mask, unsigned int owner)
 {
 	struct pci230_private *devpriv = dev->private;
-	unsigned int i;
-	unsigned int b;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&devpriv->res_spinlock, irqflags);
-	for (b = 1, i = 0; i < NUM_RESOURCES && res_mask; b <<= 1, i++) {
-		if (res_mask & b) {
-			res_mask &= ~b;
-			if (devpriv->res_owner[i] == owner)
-				devpriv->res_owner[i] = OWNER_NONE;
-		}
-	}
+	devpriv->res_owned[owner] &= ~res_mask;
 	spin_unlock_irqrestore(&devpriv->res_spinlock, irqflags);
 }
 
-static void pci230_put_one_resource(struct comedi_device *dev,
-				    unsigned int resource, unsigned char owner)
-{
-	pci230_put_resources(dev, 1U << resource, owner);
-}
-
-static void pci230_put_all_resources(struct comedi_device *dev,
-				     unsigned char owner)
+static void pci230_release_all_resources(struct comedi_device *dev,
+					 unsigned int owner)
 {
-	pci230_put_resources(dev, (1U << NUM_RESOURCES) - 1, owner);
+	pci230_release_shared(dev, (unsigned char)~0, owner);
 }
 
 static unsigned int pci230_divide_ns(uint64_t ns, unsigned int timebase,
@@ -1105,7 +1070,7 @@ static void pci230_ao_stop(struct comedi_device *dev,
 		     devpriv->daqio + PCI230_DACCON);
 	}
 	/* Release resources. */
-	pci230_put_all_resources(dev, OWNER_AOCMD);
+	pci230_release_all_resources(dev, OWNER_AOCMD);
 }
 
 static void pci230_handle_ao_nofifo(struct comedi_device *dev,
@@ -1401,7 +1366,7 @@ static int pci230_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 
 	if (cmd->scan_begin_src == TRIG_TIMER) {
 		/* Claim Z2-CT1. */
-		if (!pci230_get_one_resource(dev, RES_Z2CT1, OWNER_AOCMD))
+		if (!pci230_claim_shared(dev, RES_Z2CT1, OWNER_AOCMD))
 			return -EBUSY;
 	}
 
@@ -1957,7 +1922,7 @@ static void pci230_ai_stop(struct comedi_device *dev,
 	outw(devpriv->adccon | PCI230_ADC_FIFO_RESET,
 	     devpriv->daqio + PCI230_ADCCON);
 	/* Release resources. */
-	pci230_put_all_resources(dev, OWNER_AICMD);
+	pci230_release_all_resources(dev, OWNER_AICMD);
 }
 
 static void pci230_ai_start(struct comedi_device *dev,
@@ -2106,7 +2071,7 @@ static void pci230_ai_start(struct comedi_device *dev,
 			}
 		} else if (cmd->convert_src != TRIG_INT) {
 			/* No longer need Z2-CT2. */
-			pci230_put_one_resource(dev, RES_Z2CT2, OWNER_AICMD);
+			pci230_release_shared(dev, RES_Z2CT2, OWNER_AICMD);
 		}
 	}
 }
@@ -2237,17 +2202,17 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	 * Need Z2-CT2 to supply a conversion trigger source at a high
 	 * logic level, even if not doing timed conversions.
 	 */
-	res_mask |= 1U << RES_Z2CT2;
+	res_mask |= RES_Z2CT2;
 	if (cmd->scan_begin_src != TRIG_FOLLOW) {
 		/* Using Z2-CT0 monostable to gate Z2-CT2 conversion timer */
-		res_mask |= 1U << RES_Z2CT0;
+		res_mask |= RES_Z2CT0;
 		if (cmd->scan_begin_src == TRIG_TIMER) {
 			/* Using Z2-CT1 for scan frequency */
-			res_mask |= 1U << RES_Z2CT1;
+			res_mask |= RES_Z2CT1;
 		}
 	}
 	/* Claim resources. */
-	if (!pci230_get_resources(dev, res_mask, OWNER_AICMD))
+	if (!pci230_claim_shared(dev, res_mask, OWNER_AICMD))
 		return -EBUSY;
 
 	/* Get number of scans required. */
-- 
2.0.4

[PATCH 23/28] staging: comedi: amplc_pci230: reduce indentation in pci230_ao_inttrig_scan_begin()

[Ian Abbott]

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 727de3d..a44b056 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1226,28 +1226,25 @@ static int pci230_ao_inttrig_scan_begin(struct comedi_device *dev,
 		return -EINVAL;
 
 	spin_lock_irqsave(&devpriv->ao_stop_spinlock, irqflags);
-	if (devpriv->ao_cmd_started) {
-		/* Perform scan. */
-		if (devpriv->hwver < 2) {
-			/* Not using DAC FIFO. */
-			spin_unlock_irqrestore(&devpriv->ao_stop_spinlock,
-					       irqflags);
-			pci230_handle_ao_nofifo(dev, s);
-			comedi_event(dev, s);
-		} else {
-			/* Using DAC FIFO. */
-			/* Read DACSWTRIG register to trigger conversion. */
-			inw(devpriv->daqio + PCI230P2_DACSWTRIG);
-			spin_unlock_irqrestore(&devpriv->ao_stop_spinlock,
-					       irqflags);
-		}
-		/* Delay.  Should driver be responsible for this? */
-		/* XXX TODO: See if DAC busy bit can be used. */
-		udelay(8);
+	if (!devpriv->ao_cmd_started) {
+		spin_unlock_irqrestore(&devpriv->ao_stop_spinlock, irqflags);
+		return 1;
+	}
+	/* Perform scan. */
+	if (devpriv->hwver < 2) {
+		/* Not using DAC FIFO. */
+		spin_unlock_irqrestore(&devpriv->ao_stop_spinlock, irqflags);
+		pci230_handle_ao_nofifo(dev, s);
+		comedi_event(dev, s);
 	} else {
+		/* Using DAC FIFO. */
+		/* Read DACSWTRIG register to trigger conversion. */
+		inw(devpriv->daqio + PCI230P2_DACSWTRIG);
 		spin_unlock_irqrestore(&devpriv->ao_stop_spinlock, irqflags);
 	}
-
+	/* Delay.  Should driver be responsible for this? */
+	/* XXX TODO: See if DAC busy bit can be used. */
+	udelay(8);
 	return 1;
 }
 
-- 
2.0.4

[PATCH 24/28] staging: comedi: amplc_pci230: reduce indentation in pci230_ao_start()

[Ian Abbott]

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index a44b056..e0b40b3 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1262,78 +1262,73 @@ static void pci230_ao_start(struct comedi_device *dev,
 		async->events |= COMEDI_CB_EOA;
 		pci230_ao_stop(dev, s);
 		comedi_event(dev, s);
-	} else {
-		if (devpriv->hwver >= 2) {
-			/* Using DAC FIFO. */
-			unsigned short scantrig;
-			int run;
-
-			/* Preload FIFO data. */
-			run = pci230_handle_ao_fifo(dev, s);
-			comedi_event(dev, s);
-			if (!run) {
-				/* Stopped. */
-				return;
-			}
-			/* Set scan trigger source. */
-			switch (cmd->scan_begin_src) {
-			case TRIG_TIMER:
-				scantrig = PCI230P2_DAC_TRIG_Z2CT1;
-				break;
-			case TRIG_EXT:
-				/* Trigger on EXTTRIG/EXTCONVCLK pin. */
-				if ((cmd->scan_begin_arg & CR_INVERT) == 0) {
-					/* +ve edge */
-					scantrig = PCI230P2_DAC_TRIG_EXTP;
-				} else {
-					/* -ve edge */
-					scantrig = PCI230P2_DAC_TRIG_EXTN;
-				}
-				break;
-			case TRIG_INT:
-				scantrig = PCI230P2_DAC_TRIG_SW;
-				break;
-			default:
-				/* Shouldn't get here. */
-				scantrig = PCI230P2_DAC_TRIG_NONE;
-				break;
-			}
-			devpriv->daccon =
-			    (devpriv->daccon & ~PCI230P2_DAC_TRIG_MASK) |
-			    scantrig;
-			outw(devpriv->daccon, devpriv->daqio + PCI230_DACCON);
+		return;
+	}
+	if (devpriv->hwver >= 2) {
+		/* Using DAC FIFO. */
+		unsigned short scantrig;
+		int run;
+
+		/* Preload FIFO data. */
+		run = pci230_handle_ao_fifo(dev, s);
+		comedi_event(dev, s);
+		if (!run) {
+			/* Stopped. */
+			return;
 		}
+		/* Set scan trigger source. */
 		switch (cmd->scan_begin_src) {
 		case TRIG_TIMER:
-			if (devpriv->hwver < 2) {
-				/* Not using DAC FIFO. */
-				/* Enable CT1 timer interrupt. */
-				spin_lock_irqsave(&devpriv->isr_spinlock,
-						  irqflags);
-				devpriv->int_en |= PCI230_INT_ZCLK_CT1;
-				devpriv->ier |= PCI230_INT_ZCLK_CT1;
-				outb(devpriv->ier,
-				     dev->iobase + PCI230_INT_SCE);
-				spin_unlock_irqrestore(&devpriv->isr_spinlock,
-						       irqflags);
+			scantrig = PCI230P2_DAC_TRIG_Z2CT1;
+			break;
+		case TRIG_EXT:
+			/* Trigger on EXTTRIG/EXTCONVCLK pin. */
+			if ((cmd->scan_begin_arg & CR_INVERT) == 0) {
+				/* +ve edge */
+				scantrig = PCI230P2_DAC_TRIG_EXTP;
+			} else {
+				/* -ve edge */
+				scantrig = PCI230P2_DAC_TRIG_EXTN;
 			}
-			/* Set CT1 gate high to start counting. */
-			outb(GAT_CONFIG(1, GAT_VCC),
-			     dev->iobase + PCI230_ZGAT_SCE);
 			break;
 		case TRIG_INT:
-			async->inttrig = pci230_ao_inttrig_scan_begin;
+			scantrig = PCI230P2_DAC_TRIG_SW;
+			break;
+		default:
+			/* Shouldn't get here. */
+			scantrig = PCI230P2_DAC_TRIG_NONE;
 			break;
 		}
-		if (devpriv->hwver >= 2) {
-			/* Using DAC FIFO.  Enable DAC FIFO interrupt. */
+		devpriv->daccon =
+		    (devpriv->daccon & ~PCI230P2_DAC_TRIG_MASK) | scantrig;
+		outw(devpriv->daccon, devpriv->daqio + PCI230_DACCON);
+	}
+	switch (cmd->scan_begin_src) {
+	case TRIG_TIMER:
+		if (devpriv->hwver < 2) {
+			/* Not using DAC FIFO. */
+			/* Enable CT1 timer interrupt. */
 			spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-			devpriv->int_en |= PCI230P2_INT_DAC;
-			devpriv->ier |= PCI230P2_INT_DAC;
+			devpriv->int_en |= PCI230_INT_ZCLK_CT1;
+			devpriv->ier |= PCI230_INT_ZCLK_CT1;
 			outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 			spin_unlock_irqrestore(&devpriv->isr_spinlock,
 					       irqflags);
 		}
+		/* Set CT1 gate high to start counting. */
+		outb(GAT_CONFIG(1, GAT_VCC), dev->iobase + PCI230_ZGAT_SCE);
+		break;
+	case TRIG_INT:
+		async->inttrig = pci230_ao_inttrig_scan_begin;
+		break;
+	}
+	if (devpriv->hwver >= 2) {
+		/* Using DAC FIFO.  Enable DAC FIFO interrupt. */
+		spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
+		devpriv->int_en |= PCI230P2_INT_DAC;
+		devpriv->ier |= PCI230P2_INT_DAC;
+		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
+		spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 	}
 }
 
-- 
2.0.4

[PATCH 25/28] staging: comedi: amplc_pci230: reduce indentation in pci230_ai_inttrig_convert()

[Ian Abbott]

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index e0b40b3..b0313c8 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1802,43 +1802,39 @@ static int pci230_ai_inttrig_convert(struct comedi_device *dev,
 {
 	struct pci230_private *devpriv = dev->private;
 	unsigned long irqflags;
+	unsigned int delayus;
 
 	if (trig_num)
 		return -EINVAL;
 
 	spin_lock_irqsave(&devpriv->ai_stop_spinlock, irqflags);
-	if (devpriv->ai_cmd_started) {
-		unsigned int delayus;
-
-		/*
-		 * Trigger conversion by toggling Z2-CT2 output.
-		 * Finish with output high.
-		 */
-		i8254_set_mode(dev->iobase + PCI230_Z2_CT_BASE, 0,
-			       2, I8254_MODE0);
-		i8254_set_mode(dev->iobase + PCI230_Z2_CT_BASE, 0,
-			       2, I8254_MODE1);
-		/*
-		 * Delay.  Should driver be responsible for this?  An
-		 * alternative would be to wait until conversion is complete,
-		 * but we can't tell when it's complete because the ADC busy
-		 * bit has a different meaning when FIFO enabled (and when
-		 * FIFO not enabled, it only works for software triggers).
-		 */
-		if ((devpriv->adccon & PCI230_ADC_IM_MASK) ==
-		    PCI230_ADC_IM_DIF && devpriv->hwver == 0) {
-			/* PCI230/260 in differential mode */
-			delayus = 8;
-		} else {
-			/* single-ended or PCI230+/260+ */
-			delayus = 4;
-		}
+	if (!devpriv->ai_cmd_started) {
 		spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);
-		udelay(delayus);
+		return 1;
+	}
+	/*
+	 * Trigger conversion by toggling Z2-CT2 output.
+	 * Finish with output high.
+	 */
+	i8254_set_mode(dev->iobase + PCI230_Z2_CT_BASE, 0, 2, I8254_MODE0);
+	i8254_set_mode(dev->iobase + PCI230_Z2_CT_BASE, 0, 2, I8254_MODE1);
+	/*
+	 * Delay.  Should driver be responsible for this?  An
+	 * alternative would be to wait until conversion is complete,
+	 * but we can't tell when it's complete because the ADC busy
+	 * bit has a different meaning when FIFO enabled (and when
+	 * FIFO not enabled, it only works for software triggers).
+	 */
+	if ((devpriv->adccon & PCI230_ADC_IM_MASK) == PCI230_ADC_IM_DIF &&
+	    devpriv->hwver == 0) {
+		/* PCI230/260 in differential mode */
+		delayus = 8;
 	} else {
-		spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);
+		/* single-ended or PCI230+/260+ */
+		delayus = 4;
 	}
-
+	spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);
+	udelay(delayus);
 	return 1;
 }
 
-- 
2.0.4

[PATCH 26/28] staging: comedi: amplc_pci230: reduce indentation in pci230_ai_start()

[Ian Abbott]

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index b0313c8..27ff5be 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1928,139 +1928,134 @@ static void pci230_ai_start(struct comedi_device *dev,
 		async->events |= COMEDI_CB_EOA;
 		pci230_ai_stop(dev, s);
 		comedi_event(dev, s);
-	} else {
-		/* Enable ADC FIFO trigger level interrupt. */
-		spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-		devpriv->int_en |= PCI230_INT_ADC;
-		devpriv->ier |= PCI230_INT_ADC;
-		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
-		spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
+		return;
+	}
 
-		/*
-		 * Update conversion trigger source which is currently set
-		 * to CT2 output, which is currently stuck high.
-		 */
-		switch (cmd->convert_src) {
-		default:
-			conv = PCI230_ADC_TRIG_NONE;
-			break;
-		case TRIG_TIMER:
-			/* Using CT2 output. */
-			conv = PCI230_ADC_TRIG_Z2CT2;
-			break;
-		case TRIG_EXT:
-			if (cmd->convert_arg & CR_EDGE) {
-				if ((cmd->convert_arg & CR_INVERT) == 0) {
-					/* Trigger on +ve edge. */
-					conv = PCI230_ADC_TRIG_EXTP;
-				} else {
-					/* Trigger on -ve edge. */
-					conv = PCI230_ADC_TRIG_EXTN;
-				}
+	/* Enable ADC FIFO trigger level interrupt. */
+	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
+	devpriv->int_en |= PCI230_INT_ADC;
+	devpriv->ier |= PCI230_INT_ADC;
+	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
+	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
+
+	/*
+	 * Update conversion trigger source which is currently set
+	 * to CT2 output, which is currently stuck high.
+	 */
+	switch (cmd->convert_src) {
+	default:
+		conv = PCI230_ADC_TRIG_NONE;
+		break;
+	case TRIG_TIMER:
+		/* Using CT2 output. */
+		conv = PCI230_ADC_TRIG_Z2CT2;
+		break;
+	case TRIG_EXT:
+		if (cmd->convert_arg & CR_EDGE) {
+			if ((cmd->convert_arg & CR_INVERT) == 0) {
+				/* Trigger on +ve edge. */
+				conv = PCI230_ADC_TRIG_EXTP;
 			} else {
-				/* Backwards compatibility. */
-				if (cmd->convert_arg) {
-					/* Trigger on +ve edge. */
-					conv = PCI230_ADC_TRIG_EXTP;
-				} else {
-					/* Trigger on -ve edge. */
-					conv = PCI230_ADC_TRIG_EXTN;
-				}
+				/* Trigger on -ve edge. */
+				conv = PCI230_ADC_TRIG_EXTN;
+			}
+		} else {
+			/* Backwards compatibility. */
+			if (cmd->convert_arg) {
+				/* Trigger on +ve edge. */
+				conv = PCI230_ADC_TRIG_EXTP;
+			} else {
+				/* Trigger on -ve edge. */
+				conv = PCI230_ADC_TRIG_EXTN;
 			}
-			break;
-		case TRIG_INT:
-			/*
-			 * Use CT2 output for software trigger due to problems
-			 * in differential mode on PCI230/260.
-			 */
-			conv = PCI230_ADC_TRIG_Z2CT2;
-			break;
 		}
-		devpriv->adccon =
-		    (devpriv->adccon & ~PCI230_ADC_TRIG_MASK) | conv;
-		outw(devpriv->adccon, devpriv->daqio + PCI230_ADCCON);
-		if (cmd->convert_src == TRIG_INT)
-			async->inttrig = pci230_ai_inttrig_convert;
-
+		break;
+	case TRIG_INT:
 		/*
-		 * Update FIFO interrupt trigger level, which is currently
-		 * set to "full".
+		 * Use CT2 output for software trigger due to problems
+		 * in differential mode on PCI230/260.
 		 */
-		pci230_ai_update_fifo_trigger_level(dev, s);
-		if (cmd->convert_src == TRIG_TIMER) {
-			/* Update timer gates. */
-			unsigned char zgat;
+		conv = PCI230_ADC_TRIG_Z2CT2;
+		break;
+	}
+	devpriv->adccon = (devpriv->adccon & ~PCI230_ADC_TRIG_MASK) | conv;
+	outw(devpriv->adccon, devpriv->daqio + PCI230_ADCCON);
+	if (cmd->convert_src == TRIG_INT)
+		async->inttrig = pci230_ai_inttrig_convert;
+
+	/*
+	 * Update FIFO interrupt trigger level, which is currently
+	 * set to "full".
+	 */
+	pci230_ai_update_fifo_trigger_level(dev, s);
+	if (cmd->convert_src == TRIG_TIMER) {
+		/* Update timer gates. */
+		unsigned char zgat;
 
-			if (cmd->scan_begin_src != TRIG_FOLLOW) {
+		if (cmd->scan_begin_src != TRIG_FOLLOW) {
+			/*
+			 * Conversion timer CT2 needs to be gated by
+			 * inverted output of monostable CT2.
+			 */
+			zgat = GAT_CONFIG(2, GAT_NOUTNM2);
+		} else {
+			/*
+			 * Conversion timer CT2 needs to be gated on
+			 * continuously.
+			 */
+			zgat = GAT_CONFIG(2, GAT_VCC);
+		}
+		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
+		if (cmd->scan_begin_src != TRIG_FOLLOW) {
+			/* Set monostable CT0 trigger source. */
+			switch (cmd->scan_begin_src) {
+			default:
+				zgat = GAT_CONFIG(0, GAT_VCC);
+				break;
+			case TRIG_EXT:
 				/*
-				 * Conversion timer CT2 needs to be gated by
-				 * inverted output of monostable CT2.
+				 * For CT0 on PCI230, the external trigger
+				 * (gate) signal comes from PPC0, which is
+				 * channel 16 of the DIO subdevice.  The
+				 * application needs to configure this as an
+				 * input in order to use it as an external scan
+				 * trigger.
 				 */
-				zgat = GAT_CONFIG(2, GAT_NOUTNM2);
-			} else {
+				zgat = GAT_CONFIG(0, GAT_EXT);
+				break;
+			case TRIG_TIMER:
 				/*
-				 * Conversion timer CT2 needs to be gated on
-				 * continuously.
+				 * Monostable CT0 triggered by rising edge on
+				 * inverted output of CT1 (falling edge on CT1).
 				 */
-				zgat = GAT_CONFIG(2, GAT_VCC);
+				zgat = GAT_CONFIG(0, GAT_NOUTNM2);
+				break;
+			case TRIG_INT:
+				/*
+				 * Monostable CT0 is triggered by inttrig
+				 * function waggling the CT0 gate source.
+				 */
+				zgat = GAT_CONFIG(0, GAT_VCC);
+				break;
 			}
 			outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
-			if (cmd->scan_begin_src != TRIG_FOLLOW) {
-				/* Set monostable CT0 trigger source. */
-				switch (cmd->scan_begin_src) {
-				default:
-					zgat = GAT_CONFIG(0, GAT_VCC);
-					break;
-				case TRIG_EXT:
-					/*
-					 * For CT0 on PCI230, the external
-					 * trigger (gate) signal comes from
-					 * PPC0, which is channel 16 of the DIO
-					 * subdevice.  The application needs to
-					 * configure this as an input in order
-					 * to use it as an external scan
-					 * trigger.
-					 */
-					zgat = GAT_CONFIG(0, GAT_EXT);
-					break;
-				case TRIG_TIMER:
-					/*
-					 * Monostable CT0 triggered by rising
-					 * edge on inverted output of CT1
-					 * (falling edge on CT1).
-					 */
-					zgat = GAT_CONFIG(0, GAT_NOUTNM2);
-					break;
-				case TRIG_INT:
-					/*
-					 * Monostable CT0 is triggered by
-					 * inttrig function waggling the CT0
-					 * gate source.
-					 */
-					zgat = GAT_CONFIG(0, GAT_VCC);
-					break;
-				}
+			switch (cmd->scan_begin_src) {
+			case TRIG_TIMER:
+				/*
+				 * Scan period timer CT1 needs to be
+				 * gated on to start counting.
+				 */
+				zgat = GAT_CONFIG(1, GAT_VCC);
 				outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
-				switch (cmd->scan_begin_src) {
-				case TRIG_TIMER:
-					/*
-					 * Scan period timer CT1 needs to be
-					 * gated on to start counting.
-					 */
-					zgat = GAT_CONFIG(1, GAT_VCC);
-					outb(zgat,
-					     dev->iobase + PCI230_ZGAT_SCE);
-					break;
-				case TRIG_INT:
-					async->inttrig =
-					    pci230_ai_inttrig_scan_begin;
-					break;
-				}
+				break;
+			case TRIG_INT:
+				async->inttrig = pci230_ai_inttrig_scan_begin;
+				break;
 			}
-		} else if (cmd->convert_src != TRIG_INT) {
-			/* No longer need Z2-CT2. */
-			pci230_release_shared(dev, RES_Z2CT2, OWNER_AICMD);
 		}
+	} else if (cmd->convert_src != TRIG_INT) {
+		/* No longer need Z2-CT2. */
+		pci230_release_shared(dev, RES_Z2CT2, OWNER_AICMD);
 	}
 }
 
-- 
2.0.4

[PATCH 27/28] staging: comedi: amplc_pci230: change pci230_handle_ao_fifo() return type

[Ian Abbott]

Change the return type of `pci230_handle_ao_fifo()` from `int` to
`bool`.  A return value of `true` indicates the AO command is still
running.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 27ff5be..7748e17 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -1109,10 +1109,12 @@ static void pci230_handle_ao_nofifo(struct comedi_device *dev,
 	}
 }
 
-/* Loads DAC FIFO (if using it) from buffer. */
-/* Returns 0 if AO finished due to completion or error, 1 if still going. */
-static int pci230_handle_ao_fifo(struct comedi_device *dev,
-				 struct comedi_subdevice *s)
+/*
+ * Loads DAC FIFO (if using it) from buffer.
+ * Returns false if AO finished due to completion or error, true if still going.
+ */
+static bool pci230_handle_ao_fifo(struct comedi_device *dev,
+				  struct comedi_subdevice *s)
 {
 	struct pci230_private *devpriv = dev->private;
 	struct comedi_async *async = s->async;
@@ -1122,7 +1124,7 @@ static int pci230_handle_ao_fifo(struct comedi_device *dev,
 	unsigned short dacstat;
 	unsigned int i, n;
 	unsigned int events = 0;
-	int running;
+	bool running;
 
 	/* Get DAC FIFO status. */
 	dacstat = inw(devpriv->daqio + PCI230_DACCON);
@@ -1207,9 +1209,9 @@ static int pci230_handle_ao_fifo(struct comedi_device *dev,
 	if (events & (COMEDI_CB_EOA | COMEDI_CB_ERROR | COMEDI_CB_OVERFLOW)) {
 		/* Stopping AO due to completion or error. */
 		pci230_ao_stop(dev, s);
-		running = 0;
+		running = false;
 	} else {
-		running = 1;
+		running = true;
 	}
 	async->events |= events;
 	return running;
@@ -1267,7 +1269,7 @@ static void pci230_ao_start(struct comedi_device *dev,
 	if (devpriv->hwver >= 2) {
 		/* Using DAC FIFO. */
 		unsigned short scantrig;
-		int run;
+		bool run;
 
 		/* Preload FIFO data. */
 		run = pci230_handle_ao_fifo(dev, s);
-- 
2.0.4

[PATCH 28/28] staging: comedi: amplc_pci230: simplify interrupt enable handling

[Ian Abbott]

`struct pci230_private` has two members to manage the enabled interrupt
sources.  `int_en` is the interrupt sources we want to be enabled and
`ier` is a shadow of the write-only interrupt enable register.  They
have the same value most of the time.  They differ in the interrupt
handler (`pci230_interrupt()`) itself when it temporarily clears bits in
the interrupt enable register and the `ier` member in order to unlatch
them in hardware, but leaves the `int_en` member alone.  They also
differ in `pci230_ai_stop()` and `pci230_ao_stop()` which clear bits in
the `int_en` member and wait for the interrupt handler to finish before
copying the value to the `ier` member and the interrupt enable register.

Simplify the handling a bit, by making the `ier` member take on the role
of the `int_en` member, and allowing the value to differ from the
interrupt enable register while the interrupt handler is running.

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

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 7748e17..66e7a47 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -499,8 +499,7 @@ struct pci230_private {
 	unsigned short daccon;		/* DACCON register value */
 	unsigned short adcfifothresh;	/* ADC FIFO threshold (PCI230+/260+) */
 	unsigned short adcg;		/* ADCG register value */
-	unsigned char int_en;		/* Interrupt enable bits */
-	unsigned char ier;		/* Copy of interrupt enable register */
+	unsigned char ier;		/* Interrupt enable bits */
 	unsigned char res_owned[NUM_OWNERS]; /* Owned resources */
 	bool intr_running:1;		/* Flag set in interrupt routine */
 	bool ai_bipolar:1;		/* Flag AI range is bipolar */
@@ -1049,15 +1048,12 @@ static void pci230_ao_stop(struct comedi_device *dev,
 	 * unless we are called from the interrupt routine.
 	 */
 	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-	devpriv->int_en &= ~intsrc;
+	devpriv->ier &= ~intsrc;
 	while (devpriv->intr_running && devpriv->intr_cpuid != THISCPU) {
 		spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 		spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
 	}
-	if (devpriv->ier != devpriv->int_en) {
-		devpriv->ier = devpriv->int_en;
-		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
-	}
+	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 	if (devpriv->hwver >= 2) {
 		/*
@@ -1311,7 +1307,6 @@ static void pci230_ao_start(struct comedi_device *dev,
 			/* Not using DAC FIFO. */
 			/* Enable CT1 timer interrupt. */
 			spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-			devpriv->int_en |= PCI230_INT_ZCLK_CT1;
 			devpriv->ier |= PCI230_INT_ZCLK_CT1;
 			outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 			spin_unlock_irqrestore(&devpriv->isr_spinlock,
@@ -1327,7 +1322,6 @@ static void pci230_ao_start(struct comedi_device *dev,
 	if (devpriv->hwver >= 2) {
 		/* Using DAC FIFO.  Enable DAC FIFO interrupt. */
 		spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-		devpriv->int_en |= PCI230P2_INT_DAC;
 		devpriv->ier |= PCI230P2_INT_DAC;
 		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 		spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
@@ -1892,15 +1886,12 @@ static void pci230_ai_stop(struct comedi_device *dev,
 	 * Disable ADC interrupt and wait for interrupt routine to finish
 	 * running unless we are called from the interrupt routine.
 	 */
-	devpriv->int_en &= ~PCI230_INT_ADC;
+	devpriv->ier &= ~PCI230_INT_ADC;
 	while (devpriv->intr_running && devpriv->intr_cpuid != THISCPU) {
 		spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 		spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
 	}
-	if (devpriv->ier != devpriv->int_en) {
-		devpriv->ier = devpriv->int_en;
-		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
-	}
+	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 	/*
 	 * Reset FIFO, disable FIFO and set start conversion source to none.
@@ -1935,7 +1926,6 @@ static void pci230_ai_start(struct comedi_device *dev,
 
 	/* Enable ADC FIFO trigger level interrupt. */
 	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-	devpriv->int_en |= PCI230_INT_ADC;
 	devpriv->ier |= PCI230_INT_ADC;
 	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
@@ -2379,7 +2369,7 @@ static int pci230_ai_cancel(struct comedi_device *dev,
 /* Interrupt handler */
 static irqreturn_t pci230_interrupt(int irq, void *d)
 {
-	unsigned char status_int, valid_status_int;
+	unsigned char status_int, valid_status_int, temp_ier;
 	struct comedi_device *dev = (struct comedi_device *)d;
 	struct pci230_private *devpriv = dev->private;
 	struct comedi_subdevice *s;
@@ -2392,14 +2382,14 @@ static irqreturn_t pci230_interrupt(int irq, void *d)
 		return IRQ_NONE;
 
 	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-	valid_status_int = devpriv->int_en & status_int;
+	valid_status_int = devpriv->ier & status_int;
 	/*
 	 * Disable triggered interrupts.
 	 * (Only those interrupts that need re-enabling, are, later in the
 	 * handler).
 	 */
-	devpriv->ier = devpriv->int_en & ~status_int;
-	outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
+	temp_ier = devpriv->ier & ~status_int;
+	outb(temp_ier, dev->iobase + PCI230_INT_SCE);
 	devpriv->intr_running = true;
 	devpriv->intr_cpuid = THISCPU;
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
@@ -2432,10 +2422,8 @@ static irqreturn_t pci230_interrupt(int irq, void *d)
 
 	/* Reenable interrupts. */
 	spin_lock_irqsave(&devpriv->isr_spinlock, irqflags);
-	if (devpriv->ier != devpriv->int_en) {
-		devpriv->ier = devpriv->int_en;
+	if (devpriv->ier != temp_ier)
 		outb(devpriv->ier, dev->iobase + PCI230_INT_SCE);
-	}
 	devpriv->intr_running = false;
 	spin_unlock_irqrestore(&devpriv->isr_spinlock, irqflags);
 
-- 
2.0.4

RE: [PATCH 00/28] staging: comedi: more clean-up and remove legacy attach

[Hartley Sweeten]

On Monday, September 01, 2014 4:04 AM, Ian Abbott wrote:
> Continue to clean up the amplc_pci230 driver code and remove the legacy
> attach mechanism, since it isn't very useful for this driver (see PATCH
> 05/28).

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