[PATCH v3 0/5] staging: comedi: addi_apci_1564: fix Change-of-State interrupt functionality

[PATCH v3 0/5] staging: comedi: addi_apci_1564: fix Change-of-State interrupt functionality

[Chase Southwood]

This patchset introduces a new private data struct for this driver, adds
all of the code required to support Change-of-State interrupts for the
digital input subdevice, and finally fixes
apci1564_interrupt() to service this type of interrupt correctly.

CHANGES FROM v1:
*leave the send_sig() calls in for now, until the proper replacement can
be implemented.
*removed unused static globals, removed the remaining one into the private
data struct.
*private data struct moved into addi_apci_1564.c.
*patches 4/6 and 5/6 from v1 have been merged into patch 4/5 here.
*(*cancel) for the interrupt subdevice has been reduced to just disable DI
interrupts rather than using the board's reset function.
*support for all interrupts issued by the board has been kept in
apci1564_interrupt(), rather than stripping everything out except for DI
interrupt support.  However, DI interrupt support has still been fixed.

CHANGES FROM v2:
*Just refreshed set against staging-next.  Otherwise everything is
exactly the same, and should apply now.

Chase Southwood (5):
  staging: comedi: addi_apci_1564: remove use of
    devpriv->b_OutputMemoryStatus
  staging: comedi: addi_apci_1564: remove unused static variables
  staging: comedi: addi_apci_1564: introduce apci1564_private struct
  staging: comedi: addi_apci_1564: add Change-of-State interrupt
    subdevice and required functions
  staging: comedi: addi_apci_1564: move apci1564_interrupt() into
    addi_apci_1564.c

 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 304 +++------------
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 433 +++++++++++++++++++--
 2 files changed, 454 insertions(+), 283 deletions(-)

-- 
1.9.3

[PATCH v3 1/5] staging: comedi: addi_apci_1564: remove use of devpriv->b_OutputMemoryStatus

[Chase Southwood]

This member of the private data struct is only set at one location in the
entire driver, and then never even used for anything.  Let's just remove
its use.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 0ba5385..19de400 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -149,11 +149,6 @@ static int apci1564_do_config(struct comedi_device *dev,
 		return -EINVAL;
 	}
 
-	if (data[0])
-		devpriv->b_OutputMemoryStatus = 1;
-	else
-		devpriv->b_OutputMemoryStatus = 0;
-
 	if (data[1] == 1)
 		ul_Command = ul_Command | 0x1;
 	else
-- 
1.9.3

[PATCH v3 2/5] staging: comedi: addi_apci_1564: remove unused static variables

[Chase Southwood]

The global variables ui_InterruptStatus_1564 and ui_InterruptData are both
set but never used.  Just remove them from the driver.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 19de400..95038f2 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -90,8 +90,7 @@
 #define APCI1564_TCW_WARN_TIMEBASE_REG(x)		(0x1c + ((x) * 0x20))
 
 /* Global variables */
-static unsigned int ui_InterruptStatus_1564;
-static unsigned int ui_InterruptData, ui_Type;
+static unsigned int ui_Type;
 
 /*
  * Configures the digital input Subdevice
@@ -160,7 +159,6 @@ static int apci1564_do_config(struct comedi_device *dev,
 		ul_Command = ul_Command & 0xFFFFFFFD;
 
 	outl(ul_Command, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
-	ui_InterruptData = inl(devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
 	devpriv->tsk_Current = current;
 	return insn->n;
 }
@@ -428,9 +426,6 @@ static void apci1564_interrupt(int irq, void *d)
 	if (ui_DI == 1) {
 		ui_DI = inl(devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
 		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
-		ui_InterruptStatus_1564 =
-			inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
-		ui_InterruptStatus_1564 = ui_InterruptStatus_1564 & 0X000FFFF0;
 		/* send signal to the sample */
 		send_sig(SIGIO, devpriv->tsk_Current, 0);
 		/* enable the interrupt */
-- 
1.9.3

[PATCH v3 3/5] staging: comedi: addi_apci_1564: introduce apci1564_private struct

[Chase Southwood]

The addi_private struct defined in addi-data/addi_common.h is very bloated
and contains many fields which addi_apci_1564 does not require.  In the
interest of eventually removing this driver's dependency on
addi_common.h, we can create a private data struct specifically for
addi_apci_1564 containing only the fields it will actually use.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 182 ++++++++++-----------
 drivers/staging/comedi/drivers/addi_apci_1564.c    |  46 +++---
 2 files changed, 118 insertions(+), 110 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 95038f2..732f43c 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -49,7 +49,7 @@
 #define APCI1564_COUNTER4				3
 
 /*
- * devpriv->i_IobaseAmcc Register Map
+ * devpriv->amcc_iobase Register Map
  */
 #define APCI1564_DI_REG					0x04
 #define APCI1564_DI_INT_MODE1_REG			0x08
@@ -89,9 +89,6 @@
 #define APCI1564_TCW_WARN_TIMEVAL_REG(x)		(0x18 + ((x) * 0x20))
 #define APCI1564_TCW_WARN_TIMEBASE_REG(x)		(0x1c + ((x) * 0x20))
 
-/* Global variables */
-static unsigned int ui_Type;
-
 /*
  * Configures the digital input Subdevice
  *
@@ -105,24 +102,24 @@ static int apci1564_di_config(struct comedi_device *dev,
 			      struct comedi_insn *insn,
 			      unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 
-	devpriv->tsk_Current = current;
+	devpriv->tsk_current = current;
 
 	/* Set the digital input logic */
 	if (data[0] == 1) {
 		data[2] = data[2] << 4;
 		data[3] = data[3] << 4;
-		outl(data[2], devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
-		outl(data[3], devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
+		outl(data[2], devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+		outl(data[3], devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
 		if (data[1] == ADDIDATA_OR)
-			outl(0x4, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+			outl(0x4, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 		else
-			outl(0x6, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+			outl(0x6, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 	} else {
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 	}
 
 	return insn->n;
@@ -139,7 +136,7 @@ static int apci1564_do_config(struct comedi_device *dev,
 			      struct comedi_insn *insn,
 			      unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 	unsigned int ul_Command = 0;
 
 	if ((data[0] != 0) && (data[0] != 1)) {
@@ -158,8 +155,8 @@ static int apci1564_do_config(struct comedi_device *dev,
 	else
 		ul_Command = ul_Command & 0xFFFFFFFD;
 
-	outl(ul_Command, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
-	devpriv->tsk_Current = current;
+	outl(ul_Command, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
+	devpriv->tsk_current = current;
 	return insn->n;
 }
 
@@ -179,31 +176,31 @@ static int apci1564_timer_config(struct comedi_device *dev,
 				 struct comedi_insn *insn,
 				 unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 	unsigned int ul_Command1 = 0;
 
-	devpriv->tsk_Current = current;
+	devpriv->tsk_current = current;
 	if (data[0] == ADDIDATA_WATCHDOG) {
-		devpriv->b_TimerSelectMode = ADDIDATA_WATCHDOG;
+		devpriv->timer_select_mode = ADDIDATA_WATCHDOG;
 
 		/* Disable the watchdog */
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_WDOG_CTRL_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_WDOG_CTRL_REG);
 		/* Loading the Reload value */
-		outl(data[3], devpriv->i_IobaseAmcc + APCI1564_WDOG_RELOAD_REG);
+		outl(data[3], devpriv->amcc_iobase + APCI1564_WDOG_RELOAD_REG);
 	} else if (data[0] == ADDIDATA_TIMER) {
 		/* First Stop The Timer */
-		ul_Command1 = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+		ul_Command1 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
 		/* Stop The Timer */
-		outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+		outl(ul_Command1, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 
-		devpriv->b_TimerSelectMode = ADDIDATA_TIMER;
+		devpriv->timer_select_mode = ADDIDATA_TIMER;
 		if (data[1] == 1) {
 			/* Enable TIMER int & DISABLE ALL THE OTHER int SOURCES */
-			outl(0x02, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_IRQ_REG);
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_WDOG_IRQ_REG);
+			outl(0x02, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_DO_IRQ_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_WDOG_IRQ_REG);
 			outl(0x0,
 			     dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER1));
 			outl(0x0,
@@ -214,22 +211,22 @@ static int apci1564_timer_config(struct comedi_device *dev,
 			     dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER4));
 		} else {
 			/* disable Timer interrupt */
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		}
 
 		/*  Loading Timebase */
-		outl(data[2], devpriv->i_IobaseAmcc + APCI1564_TIMER_TIMEBASE_REG);
+		outl(data[2], devpriv->amcc_iobase + APCI1564_TIMER_TIMEBASE_REG);
 
 		/* Loading the Reload value */
-		outl(data[3], devpriv->i_IobaseAmcc + APCI1564_TIMER_RELOAD_REG);
+		outl(data[3], devpriv->amcc_iobase + APCI1564_TIMER_RELOAD_REG);
 
-		ul_Command1 = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+		ul_Command1 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		ul_Command1 = (ul_Command1 & 0xFFF719E2UL) | 2UL << 13UL | 0x10UL;
 		/* mode 2 */
-		outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+		outl(ul_Command1, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 	} else if (data[0] == ADDIDATA_COUNTER) {
-		devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
-		devpriv->b_ModeSelectRegister = data[5];
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		devpriv->mode_select_register = data[5];
 
 		/* First Stop The Counter */
 		ul_Command1 = inl(dev->iobase + APCI1564_TCW_CTRL_REG(data[5] - 1));
@@ -278,45 +275,45 @@ static int apci1564_timer_write(struct comedi_device *dev,
 				struct comedi_insn *insn,
 				unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 	unsigned int ul_Command1 = 0;
 
-	if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
+	if (devpriv->timer_select_mode == ADDIDATA_WATCHDOG) {
 		switch (data[1]) {
 		case 0:	/* stop the watchdog */
 			/* disable the watchdog */
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_WDOG_CTRL_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_WDOG_CTRL_REG);
 			break;
 		case 1:	/* start the watchdog */
-			outl(0x0001, devpriv->i_IobaseAmcc + APCI1564_WDOG_CTRL_REG);
+			outl(0x0001, devpriv->amcc_iobase + APCI1564_WDOG_CTRL_REG);
 			break;
 		case 2:	/* Software trigger */
-			outl(0x0201, devpriv->i_IobaseAmcc + APCI1564_WDOG_CTRL_REG);
+			outl(0x0201, devpriv->amcc_iobase + APCI1564_WDOG_CTRL_REG);
 			break;
 		default:
 			dev_err(dev->class_dev, "Specified functionality does not exist.\n");
 			return -EINVAL;
 		}
 	}
-	if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER) {
+	if (devpriv->timer_select_mode == ADDIDATA_TIMER) {
 		if (data[1] == 1) {
-			ul_Command1 = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			ul_Command1 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 			ul_Command1 = (ul_Command1 & 0xFFFFF9FFUL) | 0x1UL;
 
 			/* Enable the Timer */
-			outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			outl(ul_Command1, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		} else if (data[1] == 0) {
 			/* Stop The Timer */
 
-			ul_Command1 = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			ul_Command1 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 			ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
-			outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			outl(ul_Command1, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		}
 	}
-	if (devpriv->b_TimerSelectMode == ADDIDATA_COUNTER) {
+	if (devpriv->timer_select_mode == ADDIDATA_COUNTER) {
 		ul_Command1 =
 			inl(dev->iobase +
-			    APCI1564_TCW_CTRL_REG(devpriv->b_ModeSelectRegister - 1));
+			    APCI1564_TCW_CTRL_REG(devpriv->mode_select_register - 1));
 		if (data[1] == 1) {
 			/* Start the Counter subdevice */
 			ul_Command1 = (ul_Command1 & 0xFFFFF9FFUL) | 0x1UL;
@@ -329,7 +326,7 @@ static int apci1564_timer_write(struct comedi_device *dev,
 			ul_Command1 = (ul_Command1 & 0xFFFFF9FFUL) | 0x400;
 		}
 		outl(ul_Command1, dev->iobase +
-		     APCI1564_TCW_CTRL_REG(devpriv->b_ModeSelectRegister - 1));
+		     APCI1564_TCW_CTRL_REG(devpriv->mode_select_register - 1));
 	}
 	return insn->n;
 }
@@ -342,27 +339,27 @@ static int apci1564_timer_read(struct comedi_device *dev,
 			       struct comedi_insn *insn,
 			       unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 	unsigned int ul_Command1 = 0;
 
-	if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
+	if (devpriv->timer_select_mode == ADDIDATA_WATCHDOG) {
 		/*  Stores the status of the Watchdog */
-		data[0] = inl(devpriv->i_IobaseAmcc + APCI1564_WDOG_STATUS_REG) & 0x1;
-		data[1] = inl(devpriv->i_IobaseAmcc + APCI1564_WDOG_REG);
-	} else if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER) {
+		data[0] = inl(devpriv->amcc_iobase + APCI1564_WDOG_STATUS_REG) & 0x1;
+		data[1] = inl(devpriv->amcc_iobase + APCI1564_WDOG_REG);
+	} else if (devpriv->timer_select_mode == ADDIDATA_TIMER) {
 		/*  Stores the status of the Timer */
-		data[0] = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_STATUS_REG) & 0x1;
+		data[0] = inl(devpriv->amcc_iobase + APCI1564_TIMER_STATUS_REG) & 0x1;
 
 		/*  Stores the Actual value of the Timer */
-		data[1] = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_REG);
-	} else if (devpriv->b_TimerSelectMode == ADDIDATA_COUNTER) {
+		data[1] = inl(devpriv->amcc_iobase + APCI1564_TIMER_REG);
+	} else if (devpriv->timer_select_mode == ADDIDATA_COUNTER) {
 		/*  Read the Counter Actual Value. */
 		data[0] =
 			inl(dev->iobase +
-			    APCI1564_TCW_REG(devpriv->b_ModeSelectRegister - 1));
+			    APCI1564_TCW_REG(devpriv->mode_select_register - 1));
 		ul_Command1 =
 			inl(dev->iobase +
-			    APCI1564_TCW_STATUS_REG(devpriv->b_ModeSelectRegister - 1));
+			    APCI1564_TCW_STATUS_REG(devpriv->mode_select_register - 1));
 
 		/* Get the software trigger status */
 		data[1] = (unsigned char) ((ul_Command1 >> 1) & 1);
@@ -375,9 +372,9 @@ static int apci1564_timer_read(struct comedi_device *dev,
 
 		/* Get the overflow status */
 		data[4] = (unsigned char) ((ul_Command1 >> 0) & 1);
-	} else if ((devpriv->b_TimerSelectMode != ADDIDATA_TIMER)
-		&& (devpriv->b_TimerSelectMode != ADDIDATA_WATCHDOG)
-		&& (devpriv->b_TimerSelectMode != ADDIDATA_COUNTER)) {
+	} else if ((devpriv->timer_select_mode != ADDIDATA_TIMER)
+		&& (devpriv->timer_select_mode != ADDIDATA_WATCHDOG)
+		&& (devpriv->timer_select_mode != ADDIDATA_COUNTER)) {
 		dev_err(dev->class_dev, "Invalid Subdevice!\n");
 	}
 	return insn->n;
@@ -391,7 +388,9 @@ static int apci1564_do_read(struct comedi_device *dev,
 			    struct comedi_insn *insn,
 			    unsigned int *data)
 {
-	*data = ui_Type;
+	struct apci1564_private *devpriv = dev->private;
+
+	*data = devpriv->do_int_type;
 	return insn->n;
 }
 
@@ -401,15 +400,15 @@ static int apci1564_do_read(struct comedi_device *dev,
 static void apci1564_interrupt(int irq, void *d)
 {
 	struct comedi_device *dev = d;
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 	unsigned int ui_DO, ui_DI;
 	unsigned int ui_Timer;
 	unsigned int ui_C1, ui_C2, ui_C3, ui_C4;
 	unsigned int ul_Command2 = 0;
 
-	ui_DI = inl(devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG) & 0x01;
-	ui_DO = inl(devpriv->i_IobaseAmcc + APCI1564_DO_IRQ_REG) & 0x01;
-	ui_Timer = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_IRQ_REG) & 0x01;
+	ui_DI = inl(devpriv->amcc_iobase + APCI1564_DI_IRQ_REG) & 0x01;
+	ui_DO = inl(devpriv->amcc_iobase + APCI1564_DO_IRQ_REG) & 0x01;
+	ui_Timer = inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG) & 0x01;
 	ui_C1 =
 		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER1)) & 0x1;
 	ui_C2 =
@@ -424,12 +423,12 @@ static void apci1564_interrupt(int irq, void *d)
 	}
 
 	if (ui_DI == 1) {
-		ui_DI = inl(devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+		ui_DI = inl(devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 		/* send signal to the sample */
-		send_sig(SIGIO, devpriv->tsk_Current, 0);
+		send_sig(SIGIO, devpriv->tsk_current, 0);
 		/* enable the interrupt */
-		outl(ui_DI, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
+		outl(ui_DI, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 		return;
 	}
 
@@ -437,34 +436,35 @@ static void apci1564_interrupt(int irq, void *d)
 		/* Check for Digital Output interrupt Type */
 		/* 1: VCC interrupt			   */
 		/* 2: CC interrupt			   */
-		ui_Type = inl(devpriv->i_IobaseAmcc + APCI1564_DO_INT_STATUS_REG) & 0x3;
+		devpriv->do_int_type = inl(devpriv->amcc_iobase +
+					  APCI1564_DO_INT_STATUS_REG) & 0x3;
 		/* Disable the  Interrupt */
-		outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
 
 		/* Sends signal to user space */
-		send_sig(SIGIO, devpriv->tsk_Current, 0);
+		send_sig(SIGIO, devpriv->tsk_current, 0);
 	}
 
 	if (ui_Timer == 1) {
-		devpriv->b_TimerSelectMode = ADDIDATA_TIMER;
-		if (devpriv->b_TimerSelectMode) {
+		devpriv->timer_select_mode = ADDIDATA_TIMER;
+		if (devpriv->timer_select_mode) {
 
 			/*  Disable Timer Interrupt */
-			ul_Command2 = inl(devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
-			outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			ul_Command2 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 
 			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
+			send_sig(SIGIO, devpriv->tsk_current, 0);
 
 			/*  Enable Timer Interrupt */
 
-			outl(ul_Command2, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
+			outl(ul_Command2, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
 		}
 	}
 
 	if (ui_C1 == 1) {
-		devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
-		if (devpriv->b_TimerSelectMode) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
 
 			/*  Disable Counter Interrupt */
 			ul_Command2 =
@@ -473,7 +473,7 @@ static void apci1564_interrupt(int irq, void *d)
 			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
 
 			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
+			send_sig(SIGIO, devpriv->tsk_current, 0);
 
 			/*  Enable Counter Interrupt */
 			outl(ul_Command2,
@@ -482,8 +482,8 @@ static void apci1564_interrupt(int irq, void *d)
 	}
 
 	if (ui_C2 == 1) {
-		devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
-		if (devpriv->b_TimerSelectMode) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
 
 			/*  Disable Counter Interrupt */
 			ul_Command2 =
@@ -492,7 +492,7 @@ static void apci1564_interrupt(int irq, void *d)
 			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
 
 			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
+			send_sig(SIGIO, devpriv->tsk_current, 0);
 
 			/*  Enable Counter Interrupt */
 			outl(ul_Command2,
@@ -501,8 +501,8 @@ static void apci1564_interrupt(int irq, void *d)
 	}
 
 	if (ui_C3 == 1) {
-		devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
-		if (devpriv->b_TimerSelectMode) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
 
 			/*  Disable Counter Interrupt */
 			ul_Command2 =
@@ -511,7 +511,7 @@ static void apci1564_interrupt(int irq, void *d)
 			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
 
 			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
+			send_sig(SIGIO, devpriv->tsk_current, 0);
 
 			/*  Enable Counter Interrupt */
 			outl(ul_Command2,
@@ -520,8 +520,8 @@ static void apci1564_interrupt(int irq, void *d)
 	}
 
 	if (ui_C4 == 1) {
-		devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
-		if (devpriv->b_TimerSelectMode) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
 
 			/*  Disable Counter Interrupt */
 			ul_Command2 =
@@ -530,7 +530,7 @@ static void apci1564_interrupt(int irq, void *d)
 			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
 
 			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_Current, 0);
+			send_sig(SIGIO, devpriv->tsk_current, 0);
 
 			/*  Enable Counter Interrupt */
 			outl(ul_Command2,
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 13d9962..1971da3 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -6,6 +6,14 @@
 
 #include "addi-data/addi_common.h"
 
+struct apci1564_private {
+	unsigned int amcc_iobase;	/* base of AMCC I/O registers */
+	unsigned int do_int_type;
+	unsigned char timer_select_mode;
+	unsigned char mode_select_register;
+	struct task_struct *tsk_current;
+};
+
 #include "addi-data/hwdrv_apci1564.c"
 
 static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
@@ -19,9 +27,9 @@ static int apci1564_di_insn_bits(struct comedi_device *dev,
 				 struct comedi_insn *insn,
 				 unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 
-	data[1] = inl(devpriv->i_IobaseAmcc + APCI1564_DI_REG);
+	data[1] = inl(devpriv->amcc_iobase + APCI1564_DI_REG);
 
 	return insn->n;
 }
@@ -31,12 +39,12 @@ static int apci1564_do_insn_bits(struct comedi_device *dev,
 				 struct comedi_insn *insn,
 				 unsigned int *data)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 
-	s->state = inl(devpriv->i_IobaseAmcc + APCI1564_DO_REG);
+	s->state = inl(devpriv->amcc_iobase + APCI1564_DO_REG);
 
 	if (comedi_dio_update_state(s, data))
-		outl(s->state, devpriv->i_IobaseAmcc + APCI1564_DO_REG);
+		outl(s->state, devpriv->amcc_iobase + APCI1564_DO_REG);
 
 	data[1] = s->state;
 
@@ -45,26 +53,26 @@ static int apci1564_do_insn_bits(struct comedi_device *dev,
 
 static int apci1564_reset(struct comedi_device *dev)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 
-	ui_Type = 0;
+	devpriv->do_int_type = 0;
 
 	/* Disable the input interrupts and reset status register */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
-	inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+	inl(devpriv->amcc_iobase + APCI1564_DI_INT_STATUS_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
 
 	/* Reset the output channels and disable interrupts */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
 
 	/* Reset the watchdog registers */
-	addi_watchdog_reset(devpriv->i_IobaseAmcc + APCI1564_WDOG_REG);
+	addi_watchdog_reset(devpriv->amcc_iobase + APCI1564_WDOG_REG);
 
 	/* Reset the timer registers */
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
-	outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_RELOAD_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_RELOAD_REG);
 
 	/* Reset the counter registers */
 	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
@@ -79,7 +87,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 				      unsigned long context_unused)
 {
 	struct pci_dev *pcidev = comedi_to_pci_dev(dev);
-	struct addi_private *devpriv;
+	struct apci1564_private *devpriv;
 	struct comedi_subdevice *s;
 	int ret;
 
@@ -94,7 +102,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 		return ret;
 
 	dev->iobase = pci_resource_start(pcidev, 1);
-	devpriv->i_IobaseAmcc = pci_resource_start(pcidev, 0);
+	devpriv->amcc_iobase = pci_resource_start(pcidev, 0);
 
 	apci1564_reset(dev);
 
@@ -149,7 +157,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
 static void apci1564_detach(struct comedi_device *dev)
 {
-	struct addi_private *devpriv = dev->private;
+	struct apci1564_private *devpriv = dev->private;
 
 	if (devpriv) {
 		if (dev->iobase)
-- 
1.9.3

[PATCH v3 4/5] staging: comedi: addi_apci_1564: add Change-of-State interrupt subdevice and required functions

[Chase Southwood]

This board supports an interrupt that can be generated by an AND/OR
combination of 16 of the input channels.

Create a separate subdevice to handle this interrupt.

The apci1564_di_config() function is used to configure which
inputs are used to generate the interrupt. Currently this function
is broken since it does not follow the comedi API for insn_config
functions. Fix this function by implementing the config instruction
INSN_CONFIG_DIGITAL_TRIG.

Add the remaining subdevice operations necessary for the interrupt
subdevice to support async commands.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c      |  44 +---
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 250 +++++++++++++++++++--
 2 files changed, 232 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 732f43c..697ee76 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -26,12 +26,12 @@
 #define APCI1564_ADDRESS_RANGE				128
 
 /* Digital Input IRQ Function Selection */
-#define ADDIDATA_OR					0
-#define ADDIDATA_AND					1
+#define APCI1564_DI_INT_OR				(0 << 1)
+#define APCI1564_DI_INT_AND				(1 << 1)
 
 /* Digital Input Interrupt Enable Disable. */
-#define APCI1564_DIGITAL_IP_INTERRUPT_ENABLE		0x4
-#define APCI1564_DIGITAL_IP_INTERRUPT_DISABLE		0xfffffffb
+#define APCI1564_DI_INT_ENABLE				0x4
+#define APCI1564_DI_INT_DISABLE				0xfffffffb
 
 /* Digital Output Interrupt Enable Disable. */
 #define APCI1564_DIGITAL_OP_VCC_INTERRUPT_ENABLE	0x1
@@ -90,42 +90,6 @@
 #define APCI1564_TCW_WARN_TIMEBASE_REG(x)		(0x1c + ((x) * 0x20))
 
 /*
- * Configures the digital input Subdevice
- *
- * data[0] 1 = Enable interrupt, 0 = Disable interrupt
- * data[1] 0 = ADDIDATA Interrupt OR LOGIC, 1 = ADDIDATA Interrupt AND LOGIC
- * data[2] Interrupt mask for the mode 1
- * data[3] Interrupt mask for the mode 2
- */
-static int apci1564_di_config(struct comedi_device *dev,
-			      struct comedi_subdevice *s,
-			      struct comedi_insn *insn,
-			      unsigned int *data)
-{
-	struct apci1564_private *devpriv = dev->private;
-
-	devpriv->tsk_current = current;
-
-	/* Set the digital input logic */
-	if (data[0] == 1) {
-		data[2] = data[2] << 4;
-		data[3] = data[3] << 4;
-		outl(data[2], devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
-		outl(data[3], devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
-		if (data[1] == ADDIDATA_OR)
-			outl(0x4, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-		else
-			outl(0x6, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-	} else {
-		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
-		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
-		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-	}
-
-	return insn->n;
-}
-
-/*
  * Configures The Digital Output Subdevice.
  *
  * data[1] 0 = Disable VCC Interrupt, 1 = Enable VCC Interrupt
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 1971da3..fec478c 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -8,6 +8,9 @@
 
 struct apci1564_private {
 	unsigned int amcc_iobase;	/* base of AMCC I/O registers */
+	unsigned int mode1;		/* riding-edge/high level channels */
+	unsigned int mode2;		/* falling-edge/low level channels */
+	unsigned int ctrl;		/* interrupt mode OR (edge) . AND (level) */
 	unsigned int do_int_type;
 	unsigned char timer_select_mode;
 	unsigned char mode_select_register;
@@ -16,6 +19,38 @@ struct apci1564_private {
 
 #include "addi-data/hwdrv_apci1564.c"
 
+static int apci1564_reset(struct comedi_device *dev)
+{
+	struct apci1564_private *devpriv = dev->private;
+
+	devpriv->do_int_type = 0;
+
+	/* Disable the input interrupts and reset status register */
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+	inl(devpriv->amcc_iobase + APCI1564_DI_INT_STATUS_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
+
+	/* Reset the output channels and disable interrupts */
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
+
+	/* Reset the watchdog registers */
+	addi_watchdog_reset(devpriv->amcc_iobase + APCI1564_WDOG_REG);
+
+	/* Reset the timer registers */
+	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_RELOAD_REG);
+
+	/* Reset the counter registers */
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
+	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
+
+	return 0;
+}
+
 static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
 {
 	apci1564_interrupt(irq, d);
@@ -51,34 +86,187 @@ static int apci1564_do_insn_bits(struct comedi_device *dev,
 	return insn->n;
 }
 
-static int apci1564_reset(struct comedi_device *dev)
+/*
+ * Change-Of-State (COS) interrupt configuration
+ *
+ * Channels 0 to 15 are interruptible. These channels can be configured
+ * to generate interrupts based on AND/OR logic for the desired channels.
+ *
+ *	OR logic
+ *		- reacts to rising or falling edges
+ *		- interrupt is generated when any enabled channel
+ *		  meet the desired interrupt condition
+ *
+ *	AND logic
+ *		- reacts to changes in level of the selected inputs
+ *		- interrupt is generated when all enabled channels
+ *		  meet the desired interrupt condition
+ *		- after an interrupt, a change in level must occur on
+ *		  the selected inputs to release the IRQ logic
+ *
+ * The COS interrupt must be configured before it can be enabled.
+ *
+ *	data[0] : INSN_CONFIG_DIGITAL_TRIG
+ *	data[1] : trigger number (= 0)
+ *	data[2] : configuration operation:
+ *	          COMEDI_DIGITAL_TRIG_DISABLE = no interrupts
+ *	          COMEDI_DIGITAL_TRIG_ENABLE_EDGES = OR (edge) interrupts
+ *	          COMEDI_DIGITAL_TRIG_ENABLE_LEVELS = AND (level) interrupts
+ *	data[3] : left-shift for data[4] and data[5]
+ *	data[4] : rising-edge/high level channels
+ *	data[5] : falling-edge/low level channels
+ */
+static int apci1564_cos_insn_config(struct comedi_device *dev,
+				    struct comedi_subdevice *s,
+				    struct comedi_insn *insn,
+				    unsigned int *data)
 {
 	struct apci1564_private *devpriv = dev->private;
+	unsigned int shift, oldmask;
+
+	switch (data[0]) {
+	case INSN_CONFIG_DIGITAL_TRIG:
+		if (data[1] != 0)
+			return -EINVAL;
+		shift = data[3];
+		oldmask = (1U << shift) - 1;
+		switch (data[2]) {
+		case COMEDI_DIGITAL_TRIG_DISABLE:
+			devpriv->ctrl = 0;
+			devpriv->mode1 = 0;
+			devpriv->mode2 = 0;
+			apci1564_reset(dev);
+			break;
+		case COMEDI_DIGITAL_TRIG_ENABLE_EDGES:
+			if (devpriv->ctrl != (APCI1564_DI_INT_ENABLE |
+					      APCI1564_DI_INT_OR)) {
+				/* switching to 'OR' mode */
+				devpriv->ctrl = APCI1564_DI_INT_ENABLE |
+						APCI1564_DI_INT_OR;
+				/* wipe old channels */
+				devpriv->mode1 = 0;
+				devpriv->mode2 = 0;
+			} else {
+				/* preserve unspecified channels */
+				devpriv->mode1 &= oldmask;
+				devpriv->mode2 &= oldmask;
+			}
+			/* configure specified channels */
+			devpriv->mode1 |= data[4] << shift;
+			devpriv->mode2 |= data[5] << shift;
+			break;
+		case COMEDI_DIGITAL_TRIG_ENABLE_LEVELS:
+			if (devpriv->ctrl != (APCI1564_DI_INT_ENABLE |
+					      APCI1564_DI_INT_AND)) {
+				/* switching to 'AND' mode */
+				devpriv->ctrl = APCI1564_DI_INT_ENABLE |
+						APCI1564_DI_INT_AND;
+				/* wipe old channels */
+				devpriv->mode1 = 0;
+				devpriv->mode2 = 0;
+			} else {
+				/* preserve unspecified channels */
+				devpriv->mode1 &= oldmask;
+				devpriv->mode2 &= oldmask;
+			}
+			/* configure specified channels */
+			devpriv->mode1 |= data[4] << shift;
+			devpriv->mode2 |= data[5] << shift;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+	return insn->n;
+}
 
-	devpriv->do_int_type = 0;
+static int apci1564_cos_insn_bits(struct comedi_device *dev,
+				  struct comedi_subdevice *s,
+				  struct comedi_insn *insn,
+				  unsigned int *data)
+{
+	data[1] = s->state;
 
-	/* Disable the input interrupts and reset status register */
-	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-	inl(devpriv->amcc_iobase + APCI1564_DI_INT_STATUS_REG);
-	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
-	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
+	return 0;
+}
 
-	/* Reset the output channels and disable interrupts */
-	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_REG);
-	outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
+static int apci1564_cos_cmdtest(struct comedi_device *dev,
+				struct comedi_subdevice *s,
+				struct comedi_cmd *cmd)
+{
+	int err = 0;
 
-	/* Reset the watchdog registers */
-	addi_watchdog_reset(devpriv->amcc_iobase + APCI1564_WDOG_REG);
+	/* Step 1 : check if triggers are trivially valid */
 
-	/* Reset the timer registers */
-	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
-	outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_RELOAD_REG);
+	err |= cfc_check_trigger_src(&cmd->start_src, TRIG_NOW);
+	err |= cfc_check_trigger_src(&cmd->scan_begin_src, TRIG_EXT);
+	err |= cfc_check_trigger_src(&cmd->convert_src, TRIG_FOLLOW);
+	err |= cfc_check_trigger_src(&cmd->scan_end_src, TRIG_COUNT);
+	err |= cfc_check_trigger_src(&cmd->stop_src, TRIG_NONE);
 
-	/* Reset the counter registers */
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
-	outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
+	if (err)
+		return 1;
+
+	/* Step 2a : make sure trigger sources are unique */
+	/* Step 2b : and mutually compatible */
+
+	if (err)
+		return 2;
+
+	/* Step 3: check if arguments are trivially valid */
+
+	err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0);
+	err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
+	err |= cfc_check_trigger_arg_is(&cmd->convert_arg, 0);
+	err |= cfc_check_trigger_arg_is(&cmd->scan_end_arg, cmd->chanlist_len);
+	err |= cfc_check_trigger_arg_is(&cmd->stop_arg, 0);
+
+	if (err)
+		return 3;
+
+	/* step 4: ignored */
+
+	if (err)
+		return 4;
+
+	return 0;
+}
+
+/*
+ * Change-Of-State (COS) 'do_cmd' operation
+ *
+ * Enable the COS interrupt as configured by apci1564_cos_insn_config().
+ */
+static int apci1564_cos_cmd(struct comedi_device *dev,
+			    struct comedi_subdevice *s)
+{
+	struct apci1564_private *devpriv = dev->private;
+
+	if (!devpriv->ctrl) {
+		dev_warn(dev->class_dev,
+			"Interrupts disabled due to mode configuration!\n");
+		return -EINVAL;
+	}
+
+	outl(devpriv->mode1, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+	outl(devpriv->mode2, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
+	outl(devpriv->ctrl, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+
+	return 0;
+}
+
+static int apci1564_cos_cancel(struct comedi_device *dev,
+			       struct comedi_subdevice *s)
+{
+	struct apci1564_private *devpriv = dev->private;
+
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+	inl(devpriv->amcc_iobase + APCI1564_DI_INT_STATUS_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE1_REG);
+	outl(0x0, devpriv->amcc_iobase + APCI1564_DI_INT_MODE2_REG);
 
 	return 0;
 }
@@ -113,7 +301,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	ret = comedi_alloc_subdevices(dev, 3);
+	ret = comedi_alloc_subdevices(dev, 4);
 	if (ret)
 		return ret;
 
@@ -125,7 +313,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	s->maxdata = 1;
 	s->len_chanlist = 32;
 	s->range_table = &range_digital;
-	s->insn_config = apci1564_di_config;
 	s->insn_bits = apci1564_di_insn_bits;
 
 	/*  Allocate and Initialise DO Subdevice Structures */
@@ -152,6 +339,25 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	s->insn_read = apci1564_timer_read;
 	s->insn_config = apci1564_timer_config;
 
+	/* Change-Of-State (COS) interrupt subdevice */
+	s = &dev->subdevices[3];
+	if (dev->irq) {
+		dev->read_subdev = s;
+		s->type = COMEDI_SUBD_DI;
+		s->subdev_flags = SDF_READABLE | SDF_CMD_READ;
+		s->n_chan = 1;
+		s->maxdata = 1;
+		s->range_table = &range_digital;
+		s->len_chanlist = 1;
+		s->insn_config = apci1564_cos_insn_config;
+		s->insn_bits = apci1564_cos_insn_bits;
+		s->do_cmdtest = apci1564_cos_cmdtest;
+		s->do_cmd = apci1564_cos_cmd;
+		s->cancel = apci1564_cos_cancel;
+	} else {
+		s->type = COMEDI_SUBD_UNUSED;
+	}
+
 	return 0;
 }
 
-- 
1.9.3

[PATCH v3 5/5] staging: comedi: addi_apci_1564: move apci1564_interrupt() into addi_apci_1564.c

[Chase Southwood]

On moving the function into the driver proper, also check the device is
asserting the shared interrupt line.

This patch also fixes the interrupt handling for the digital input
change-of-state interrupts.

Signed-off-by: Chase Southwood <chase.southwood@gmail.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 146 -------------------
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 157 ++++++++++++++++++++-
 2 files changed, 153 insertions(+), 150 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 697ee76..ef419b4 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -357,149 +357,3 @@ static int apci1564_do_read(struct comedi_device *dev,
 	*data = devpriv->do_int_type;
 	return insn->n;
 }
-
-/*
- * Interrupt handler for the interruptible digital inputs
- */
-static void apci1564_interrupt(int irq, void *d)
-{
-	struct comedi_device *dev = d;
-	struct apci1564_private *devpriv = dev->private;
-	unsigned int ui_DO, ui_DI;
-	unsigned int ui_Timer;
-	unsigned int ui_C1, ui_C2, ui_C3, ui_C4;
-	unsigned int ul_Command2 = 0;
-
-	ui_DI = inl(devpriv->amcc_iobase + APCI1564_DI_IRQ_REG) & 0x01;
-	ui_DO = inl(devpriv->amcc_iobase + APCI1564_DO_IRQ_REG) & 0x01;
-	ui_Timer = inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG) & 0x01;
-	ui_C1 =
-		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER1)) & 0x1;
-	ui_C2 =
-		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER2)) & 0x1;
-	ui_C3 =
-		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER3)) & 0x1;
-	ui_C4 =
-		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER4)) & 0x1;
-	if (ui_DI == 0 && ui_DO == 0 && ui_Timer == 0 && ui_C1 == 0
-		&& ui_C2 == 0 && ui_C3 == 0 && ui_C4 == 0) {
-		dev_err(dev->class_dev, "Interrupt from unknown source.\n");
-	}
-
-	if (ui_DI == 1) {
-		ui_DI = inl(devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-		outl(0x0, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-		/* send signal to the sample */
-		send_sig(SIGIO, devpriv->tsk_current, 0);
-		/* enable the interrupt */
-		outl(ui_DI, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
-		return;
-	}
-
-	if (ui_DO == 1) {
-		/* Check for Digital Output interrupt Type */
-		/* 1: VCC interrupt			   */
-		/* 2: CC interrupt			   */
-		devpriv->do_int_type = inl(devpriv->amcc_iobase +
-					  APCI1564_DO_INT_STATUS_REG) & 0x3;
-		/* Disable the  Interrupt */
-		outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
-
-		/* Sends signal to user space */
-		send_sig(SIGIO, devpriv->tsk_current, 0);
-	}
-
-	if (ui_Timer == 1) {
-		devpriv->timer_select_mode = ADDIDATA_TIMER;
-		if (devpriv->timer_select_mode) {
-
-			/*  Disable Timer Interrupt */
-			ul_Command2 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
-			outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_current, 0);
-
-			/*  Enable Timer Interrupt */
-
-			outl(ul_Command2, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
-		}
-	}
-
-	if (ui_C1 == 1) {
-		devpriv->timer_select_mode = ADDIDATA_COUNTER;
-		if (devpriv->timer_select_mode) {
-
-			/*  Disable Counter Interrupt */
-			ul_Command2 =
-				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
-			outl(0x0,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_current, 0);
-
-			/*  Enable Counter Interrupt */
-			outl(ul_Command2,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
-		}
-	}
-
-	if (ui_C2 == 1) {
-		devpriv->timer_select_mode = ADDIDATA_COUNTER;
-		if (devpriv->timer_select_mode) {
-
-			/*  Disable Counter Interrupt */
-			ul_Command2 =
-				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
-			outl(0x0,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_current, 0);
-
-			/*  Enable Counter Interrupt */
-			outl(ul_Command2,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
-		}
-	}
-
-	if (ui_C3 == 1) {
-		devpriv->timer_select_mode = ADDIDATA_COUNTER;
-		if (devpriv->timer_select_mode) {
-
-			/*  Disable Counter Interrupt */
-			ul_Command2 =
-				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
-			outl(0x0,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_current, 0);
-
-			/*  Enable Counter Interrupt */
-			outl(ul_Command2,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
-		}
-	}
-
-	if (ui_C4 == 1) {
-		devpriv->timer_select_mode = ADDIDATA_COUNTER;
-		if (devpriv->timer_select_mode) {
-
-			/*  Disable Counter Interrupt */
-			ul_Command2 =
-				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
-			outl(0x0,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
-
-			/* Send a signal to from kernel to user space */
-			send_sig(SIGIO, devpriv->tsk_current, 0);
-
-			/*  Enable Counter Interrupt */
-			outl(ul_Command2,
-			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
-		}
-	}
-	return;
-}
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index fec478c..f71ee02 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -3,6 +3,7 @@
 
 #include "../comedidev.h"
 #include "comedi_fc.h"
+#include "amcc_s5933.h"
 
 #include "addi-data/addi_common.h"
 
@@ -51,10 +52,158 @@ static int apci1564_reset(struct comedi_device *dev)
 	return 0;
 }
 
-static irqreturn_t v_ADDI_Interrupt(int irq, void *d)
+static irqreturn_t apci1564_interrupt(int irq, void *d)
 {
-	apci1564_interrupt(irq, d);
-	return IRQ_RETVAL(1);
+	struct comedi_device *dev = d;
+	struct apci1564_private *devpriv = dev->private;
+	struct comedi_subdevice *s = dev->read_subdev;
+	unsigned int ui_DO, ui_DI;
+	unsigned int ui_Timer;
+	unsigned int ui_C1, ui_C2, ui_C3, ui_C4;
+	unsigned int ul_Command2 = 0;
+
+	/* check interrupt is from this device */
+	if ((inl(devpriv->amcc_iobase + AMCC_OP_REG_INTCSR) &
+	     INTCSR_INTR_ASSERTED) == 0)
+		return IRQ_NONE;
+
+	/* check which interrupt was triggered */
+	ui_DI = inl(devpriv->amcc_iobase + APCI1564_DI_IRQ_REG) &
+		   APCI1564_DI_INT_ENABLE;
+	ui_DO = inl(devpriv->amcc_iobase + APCI1564_DO_IRQ_REG) & 0x01;
+	ui_Timer = inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG) & 0x01;
+	ui_C1 =
+		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER1)) & 0x1;
+	ui_C2 =
+		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER2)) & 0x1;
+	ui_C3 =
+		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER3)) & 0x1;
+	ui_C4 =
+		inl(dev->iobase + APCI1564_TCW_IRQ_REG(APCI1564_COUNTER4)) & 0x1;
+	if (ui_DI == 0 && ui_DO == 0 && ui_Timer == 0 && ui_C1 == 0
+		&& ui_C2 == 0 && ui_C3 == 0 && ui_C4 == 0) {
+		return IRQ_HANDLED;
+	}
+
+	if (ui_DI) {
+		/* disable the interrupt */
+		outl(ui_DI & APCI1564_DI_INT_DISABLE, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+
+		s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG) & 0xffff;
+		comedi_buf_put(s, s->state);
+		s->async->events |= COMEDI_CB_BLOCK | COMEDI_CB_EOS;
+		comedi_event(dev, s);
+
+		/* enable the interrupt */
+		outl(ui_DI, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
+	}
+
+	if (ui_DO == 1) {
+		/* Check for Digital Output interrupt Type */
+		/* 1: VCC interrupt			   */
+		/* 2: CC interrupt			   */
+		devpriv->do_int_type = inl(devpriv->amcc_iobase +
+					  APCI1564_DO_INT_STATUS_REG) & 0x3;
+		/* Disable the  Interrupt */
+		outl(0x0, devpriv->amcc_iobase + APCI1564_DO_INT_CTRL_REG);
+
+		/* Sends signal to user space */
+		send_sig(SIGIO, devpriv->tsk_current, 0);
+	}
+
+	if (ui_Timer == 1) {
+		devpriv->timer_select_mode = ADDIDATA_TIMER;
+		if (devpriv->timer_select_mode) {
+
+			/*  Disable Timer Interrupt */
+			ul_Command2 = inl(devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+			outl(0x0, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+
+			/* Send a signal to from kernel to user space */
+			send_sig(SIGIO, devpriv->tsk_current, 0);
+
+			/*  Enable Timer Interrupt */
+
+			outl(ul_Command2, devpriv->amcc_iobase + APCI1564_TIMER_CTRL_REG);
+		}
+	}
+
+	if (ui_C1 == 1) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
+
+			/*  Disable Counter Interrupt */
+			ul_Command2 =
+				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
+			outl(0x0,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
+
+			/* Send a signal to from kernel to user space */
+			send_sig(SIGIO, devpriv->tsk_current, 0);
+
+			/*  Enable Counter Interrupt */
+			outl(ul_Command2,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
+		}
+	}
+
+	if (ui_C2 == 1) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
+
+			/*  Disable Counter Interrupt */
+			ul_Command2 =
+				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
+			outl(0x0,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
+
+			/* Send a signal to from kernel to user space */
+			send_sig(SIGIO, devpriv->tsk_current, 0);
+
+			/*  Enable Counter Interrupt */
+			outl(ul_Command2,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
+		}
+	}
+
+	if (ui_C3 == 1) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
+
+			/*  Disable Counter Interrupt */
+			ul_Command2 =
+				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
+			outl(0x0,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
+
+			/* Send a signal to from kernel to user space */
+			send_sig(SIGIO, devpriv->tsk_current, 0);
+
+			/*  Enable Counter Interrupt */
+			outl(ul_Command2,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
+		}
+	}
+
+	if (ui_C4 == 1) {
+		devpriv->timer_select_mode = ADDIDATA_COUNTER;
+		if (devpriv->timer_select_mode) {
+
+			/*  Disable Counter Interrupt */
+			ul_Command2 =
+				inl(dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
+			outl(0x0,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
+
+			/* Send a signal to from kernel to user space */
+			send_sig(SIGIO, devpriv->tsk_current, 0);
+
+			/*  Enable Counter Interrupt */
+			outl(ul_Command2,
+			     dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
+		}
+	}
+	return IRQ_HANDLED;
 }
 
 static int apci1564_di_insn_bits(struct comedi_device *dev,
@@ -295,7 +444,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	apci1564_reset(dev);
 
 	if (pcidev->irq > 0) {
-		ret = request_irq(pcidev->irq, v_ADDI_Interrupt, IRQF_SHARED,
+		ret = request_irq(pcidev->irq, apci1564_interrupt, IRQF_SHARED,
 				  dev->board_name, dev);
 		if (ret == 0)
 			dev->irq = pcidev->irq;
-- 
1.9.3

Re: [PATCH v3 0/5] staging: comedi: addi_apci_1564: fix Change-of-State interrupt functionality

[Ian Abbott]

On 2014-06-21 23:23, Chase Southwood wrote:
> This patchset introduces a new private data struct for this driver, adds
> all of the code required to support Change-of-State interrupts for the
> digital input subdevice, and finally fixes
> apci1564_interrupt() to service this type of interrupt correctly.
>
> CHANGES FROM v1:
> *leave the send_sig() calls in for now, until the proper replacement can
> be implemented.
> *removed unused static globals, removed the remaining one into the private
> data struct.
> *private data struct moved into addi_apci_1564.c.
> *patches 4/6 and 5/6 from v1 have been merged into patch 4/5 here.
> *(*cancel) for the interrupt subdevice has been reduced to just disable DI
> interrupts rather than using the board's reset function.
> *support for all interrupts issued by the board has been kept in
> apci1564_interrupt(), rather than stripping everything out except for DI
> interrupt support.  However, DI interrupt support has still been fixed.
>
> CHANGES FROM v2:
> *Just refreshed set against staging-next.  Otherwise everything is
> exactly the same, and should apply now.
>
> Chase Southwood (5):
>    staging: comedi: addi_apci_1564: remove use of
>      devpriv->b_OutputMemoryStatus
>    staging: comedi: addi_apci_1564: remove unused static variables
>    staging: comedi: addi_apci_1564: introduce apci1564_private struct
>    staging: comedi: addi_apci_1564: add Change-of-State interrupt
>      subdevice and required functions
>    staging: comedi: addi_apci_1564: move apci1564_interrupt() into
>      addi_apci_1564.c

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

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