2014-06-21 22:24:30

by Chase Southwood

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

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


2014-06-21 22:25:31

by Chase Southwood

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

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 <[email protected]>
Cc: Ian Abbott <[email protected]>
Cc: H Hartley Sweeeten <[email protected]>
---
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

2014-06-21 22:25:57

by Chase Southwood

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

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 <[email protected]>
Cc: Ian Abbott <[email protected]>
Cc: H Hartley Sweeten <[email protected]>
---
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

2014-06-21 22:26:16

by Chase Southwood

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

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 <[email protected]>
Cc: Ian Abbott <[email protected]>
Cc: H Hartley Sweeten <[email protected]>
---
.../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

2014-06-21 22:26:28

by Chase Southwood

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

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 <[email protected]>
Cc: Ian Abbott <[email protected]>
Cc: H Hartley Sweeten <[email protected]>
---
.../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

2014-06-21 22:26:41

by Chase Southwood

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

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 <[email protected]>
Cc: Ian Abbott <[email protected]>
Cc: H Hartley Sweeten <[email protected]>
---
.../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

2014-06-23 10:23:23

by Ian Abbott

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

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 <[email protected]>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-