[PATCH 0/2] staging: comedi: addi_apci_1564: provide interface to read diagnostic status

[PATCH 0/2] staging: comedi: addi_apci_1564: provide interface to read diagnostic status

[Chase Southwood]

This patchset creates a simple subdevice to allow for reading of the
board's diagnostic status, and then removes any code which is related to
diagnostic interrupts, as the driver will not support these at this time.

Chase Southwood (2):
  staging: comedi: addi_apci_1564: add subdevice to check diagnostic
    status
  staging: comedi: addi_apci_1564: remove diagnostic interrupt support
    code

 .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 14 --------
 drivers/staging/comedi/drivers/addi_apci_1564.c    | 41 ++++++++++++----------
 2 files changed, 22 insertions(+), 33 deletions(-)

-- 
2.0.3

[PATCH 1/2] staging: comedi: addi_apci_1564: add subdevice to check diagnostic status

[Chase Southwood]

This board provides VCC/CC diagnostic information, and it also supports
diagnostic interrupts.  However, as per Ian, these interrupts aren't very
useful and it is enough to simply provide an interface for accessing the
diagnostic status on-demand.  This patch adds a 2-channel digital input
subdevice with an insn_bits handler to access this information.

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_apci_1564.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 190b026..819255b 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -157,6 +157,18 @@ static int apci1564_do_insn_bits(struct comedi_device *dev,
 	return insn->n;
 }
 
+static int apci1564_diag_insn_bits(struct comedi_device *dev,
+				   struct comedi_subdevice *s,
+				   struct comedi_insn *insn,
+				   unsigned int *data)
+{
+	struct apci1564_private *devpriv = dev->private;
+
+	data[1] = inl(devpriv->amcc_iobase + APCI1564_DO_INT_STATUS_REG) & 3;
+
+	return insn->n;
+}
+
 /*
  * Change-Of-State (COS) interrupt configuration
  *
@@ -373,7 +385,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 			dev->irq = pcidev->irq;
 	}
 
-	ret = comedi_alloc_subdevices(dev, 5);
+	ret = comedi_alloc_subdevices(dev, 6);
 	if (ret)
 		return ret;
 
@@ -434,6 +446,15 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	if (ret)
 		return ret;
 
+	/* Initialize the diagnostic status subdevice */
+	s = &dev->subdevices[5];
+	s->type = COMEDI_SUBD_DI;
+	s->subdev_flags = SDF_READABLE;
+	s->n_chan = 2;
+	s->maxdata = 1;
+	s->range_table = &range_digital;
+	s->insn_bits = apci1564_diag_insn_bits;
+
 	return 0;
 }
 
-- 
2.0.3

[PATCH 2/2] staging: comedi: addi_apci_1564: remove diagnostic interrupt support code

[Chase Southwood]

As per Ian, at this point in time it is not worth implementing an async
command interface for diagnostic interrupts for this board.  As this is
the case, this patch removes the code which supports such interrupts as it
is now unused.

This includes removing apci1564_do_read(), which was the insn_read
operation for the digital output subdevice, since all it was doing was
reading the current diagnostic interrupt type and returning it in 'data'.
This doesn't follow the comedi API and this operation can be emulated by
the comedi core anyway since the insn_bits operation follows the comedi
API.  So it is safe to simply remove this function.

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

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index a1730e9..8a613ae 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -340,17 +340,3 @@ static int apci1564_timer_read(struct comedi_device *dev,
 	}
 	return insn->n;
 }
-
-/*
- * Reads the interrupt status register
- */
-static int apci1564_do_read(struct comedi_device *dev,
-			    struct comedi_subdevice *s,
-			    struct comedi_insn *insn,
-			    unsigned int *data)
-{
-	struct apci1564_private *devpriv = dev->private;
-
-	*data = devpriv->do_int_type;
-	return insn->n;
-}
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 819255b..543cb07 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -13,7 +13,6 @@ struct apci1564_private {
 	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;
 	struct task_struct *tsk_current;
@@ -25,8 +24,6 @@ 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);
@@ -83,20 +80,6 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
 		outl(status, devpriv->amcc_iobase + APCI1564_DI_IRQ_REG);
 	}
 
-	status = inl(devpriv->amcc_iobase + APCI1564_DO_IRQ_REG);
-	if (status & 0x01) {
-		/* 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);
-	}
-
 	status = inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG);
 	if (status & 0x01) {
 		/*  Disable Timer Interrupt */
@@ -407,7 +390,6 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 	s->range_table = &range_digital;
 	s->insn_config = apci1564_do_config;
 	s->insn_bits = apci1564_do_insn_bits;
-	s->insn_read = apci1564_do_read;
 
 	/* Change-Of-State (COS) interrupt subdevice */
 	s = &dev->subdevices[2];
-- 
2.0.3

Re: [PATCH 0/2] staging: comedi: addi_apci_1564: provide interface to read diagnostic status

[Ian Abbott]

On 01/08/14 02:55, Chase Southwood wrote:
> This patchset creates a simple subdevice to allow for reading of the
> board's diagnostic status, and then removes any code which is related to
> diagnostic interrupts, as the driver will not support these at this time.
>
> Chase Southwood (2):
>    staging: comedi: addi_apci_1564: add subdevice to check diagnostic
>      status
>    staging: comedi: addi_apci_1564: remove diagnostic interrupt support
>      code
>
>   .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 14 --------
>   drivers/staging/comedi/drivers/addi_apci_1564.c    | 41 ++++++++++++----------
>   2 files changed, 22 insertions(+), 33 deletions(-)
>

The Digital Output subdevice should behave more sanely now!

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         )=-