Re: [PATCH 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/06/16 22:58, H Hartley Sweeten wrote:
This board supports change-of-state interrupts on digital inputs 4 to 19
not 0 to 15.

The current code "works" but it could set inappropriate bits in the mode1
and mode2 registers that setup which channels are enabled. It also doesn't
return the status of the upper 4 channels (19 to 16).

Fix the comment and mask the mode1/mode2 values so that only the interrupt
capable channels can be enabled.

Add the SDF_LSAMPL flag to the subdevice so that 32-bit samples are used
instead of 16-bit ones. This allows returning the upper 4 channels. Use
the remaining bits in the sample to return "event" flags to the user.

The timer and counter subdevices can also generate interrupts and are a bit
hacked. They don't currently follow the comedi API and they use send_sig()
to let the task that know that the interrupt occured. The "event" flags will
be used instead when these subdevices are fixed.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  drivers/staging/comedi/drivers/addi_apci_1564.c | 37 +++++++++++++++++--------
  1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
index f1ccfbd..37e18b3 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -77,6 +77,7 @@
  #define APCI1564_DI_REG				0x00
  #define APCI1564_DI_INT_MODE1_REG		0x04
  #define APCI1564_DI_INT_MODE2_REG		0x08
+#define APCI1564_DI_INT_MODE_MASK		0x000ffff0 /* chans [19:4] */
  #define APCI1564_DI_INT_STATUS_REG		0x0c
  #define APCI1564_DI_IRQ_REG			0x10
  #define APCI1564_DI_IRQ_ENA			BIT(2)
@@ -111,6 +112,13 @@
   */
  #define APCI1564_COUNTER(x)			((x) * 0x20)

+/*
+ * The dev->read_subdev is used to return the interrupt events along with
+ * the state of the interrupt capable inputs.
+ */
+#define APCI1564_EVENT_COS			BIT(31)
+#define APCI1564_EVENT_MASK			0xfff0000f /* all but [19:4] */
+
  struct apci1564_private {
  	unsigned long eeprom;	/* base address of EEPROM register */
  	unsigned long timer;	/* base address of 12-bit timer */
@@ -165,18 +173,16 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
  	unsigned int ctrl;
  	unsigned int chan;

+	s->state &= ~APCI1564_EVENT_MASK;
+
  	status = inl(dev->iobase + APCI1564_DI_IRQ_REG);
  	if (status & APCI1564_DI_IRQ_ENA) {
-		/* disable the interrupt */
+		s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG);
+		s->state |= APCI1564_EVENT_COS;
+
+		/* clear the interrupt */
  		outl(status & ~APCI1564_DI_IRQ_ENA,
  		     dev->iobase + APCI1564_DI_IRQ_REG);
-
-		s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG) &
-			   0xffff;
-		comedi_buf_write_samples(s, &s->state, 1);
-		comedi_handle_events(dev, s);
-
-		/* enable the interrupt */
  		outl(status, dev->iobase + APCI1564_DI_IRQ_REG);
  	}

@@ -214,6 +220,11 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
  		}
  	}

+	if (s->state & APCI1564_EVENT_MASK) {
+		comedi_buf_write_samples(s, &s->state, 1);
+		comedi_handle_events(dev, s);
+	}
+
  	return IRQ_HANDLED;
  }

I'm struggling to see how that works. It looks like once APCI1564_EVENT_COS has been set in s->state, it never gets cleared (or at least it only gets cleared momentarily), and after that, the `if (s->state & APCI1564_EVENT_MASK)` test will always be true, and so it will write a sample to the buffer on every interrupt.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux