staging: comedi: dt2814: Fix asynchronous command interrupt handling

The support for asynchronous commands in this driver is currently
broken.  If interrupts are enabled, the interrupt handler is called at
the end of every A/D conversion.  A/D conversions could be due to
software-triggered conversions resulting from Comedi `INSN_READ`
instruction handling, or due to timer-trigger conversions enabled when
a Comedi asynchronous command is set up.  We only want the interrupt
handler to read a sample from the A/D Data register for timer-triggered
conversions, but currently it always reads the A/D Data register.  Since
the A/D Data register is read twice (to read a 12-bit value from an
8-bit register), that probably interferes with the reading for
software-triggered conversions.

The interrupt handler does not currently do anything with the data, it
just ignores it.  It should be written to the Comedi buffer if handling
an asynchronous command.

Other problems are that the driver has no Comedi `cancel` handler to
call when the asynchronous command is being stopped manually, and it
does not handle "infinite" acquisitions (when the command's `stop_src ==
TRIG_NONE`) properly.

Change the interrupt handler to check the timer enable (ENB) bit to
check the asynchronous command is active and return early if not
enabled.  Also check the error (ERR) and "conversion complete" (FINISH)
bits, and return early if neither is set.  Then the sample can be read
from the A/D Data register to clear the ERR and FINISH bits.  If the ERR
bit was set, terminate the acquisition with an error, otherwise write
the data to the Comedi buffer and check for end of acquisition.  Replace
the current check for end of acquisition, using the count of completed
scans in `scans_done` (updated by calls to `comedi_buf_write_samples()`)
when `stop_src == TRIG_COUNT`) and allowing "infinite" acquisitions when
`stop_src == TRIG_NONE`.

Add a `cancel` handler function `dt2814_ai_cancel()` that will be called
when the end of acquisition event is processed and when the acquisition
is stopped manually.  It turns off the timer enable (ENB) bit in the
Control register, leaving the current channel selected.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Link: https://lore.kernel.org/r/20210301165757.243065-5-abbotti@mev.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/comedi/drivers/dt2814.c b/drivers/staging/comedi/drivers/dt2814.c
index da4dc4df3a9508eae23d9e4bde3069b874b20d2f..6f6d0b2bb44b5eddefaae16332f12b7b88da595e 100644 (file)
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -223,30 +223,87 @@ static int dt2814_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
        return 0;
 }
 
+static int dt2814_ai_cancel(struct comedi_device *dev,
+                           struct comedi_subdevice *s)
+{
+       unsigned int status;
+       unsigned long flags;
+
+       spin_lock_irqsave(&dev->spinlock, flags);
+       status = inb(dev->iobase + DT2814_CSR);
+       if (status & DT2814_ENB) {
+               /*
+                * Clear the timed trigger enable bit.
+                *
+                * Note: turning off timed mode triggers another
+                * sample.  This will be mopped up by the calls to
+                * dt2814_ai_clear().
+                */
+               outb(status & DT2814_CHANMASK, dev->iobase + DT2814_CSR);
+       }
+       spin_unlock_irqrestore(&dev->spinlock, flags);
+       return 0;
+}
+
 static irqreturn_t dt2814_interrupt(int irq, void *d)
 {
        struct comedi_device *dev = d;
-       struct dt2814_private *devpriv = dev->private;
        struct comedi_subdevice *s = dev->read_subdev;
+       struct comedi_async *async;
+       unsigned int lo, hi;
+       unsigned short data;
+       unsigned int status;
 
        if (!dev->attached) {
                dev_err(dev->class_dev, "spurious interrupt\n");
                return IRQ_HANDLED;
        }
 
-       inb(dev->iobase + DT2814_DATA);
-       inb(dev->iobase + DT2814_DATA);
+       async = s->async;
 
-       if (!(--devpriv->ntrig)) {
-               outb(0, dev->iobase + DT2814_CSR);
+       spin_lock(&dev->spinlock);
+
+       status = inb(dev->iobase + DT2814_CSR);
+       if (!(status & DT2814_ENB)) {
+               /* Timed acquisition not enabled.  Nothing to do. */
+               spin_unlock(&dev->spinlock);
+               return IRQ_HANDLED;
+       }
+
+       if (!(status & (DT2814_FINISH | DT2814_ERR))) {
+               /* Spurious interrupt? */
+               spin_unlock(&dev->spinlock);
+               return IRQ_HANDLED;
+       }
+
+       /* Read data or clear error. */
+       hi = inb(dev->iobase + DT2814_DATA);
+       lo = inb(dev->iobase + DT2814_DATA);
+
+       data = (hi << 4) | (lo >> 4);
+
+       if (status & DT2814_ERR) {
+               async->events |= COMEDI_CB_ERROR;
+       } else {
+               comedi_buf_write_samples(s, &data, 1);
+               if (async->cmd.stop_src == TRIG_COUNT &&
+                   async->scans_done >=  async->cmd.stop_arg) {
+                       async->events |= COMEDI_CB_EOA;
+               }
+       }
+       if (async->events & COMEDI_CB_CANCEL_MASK) {
                /*
+                * Disable timed mode.
+                *
                 * Note: turning off timed mode triggers another
                 * sample.  This will be mopped up by the calls to
                 * dt2814_ai_clear().
                 */
-
-               s->async->events |= COMEDI_CB_EOA;
+               outb(status & DT2814_CHANMASK, dev->iobase + DT2814_CSR);
        }
+
+       spin_unlock(&dev->spinlock);
+
        comedi_handle_events(dev, s);
        return IRQ_HANDLED;
 }
@@ -295,6 +352,7 @@ static int dt2814_attach(struct comedi_device *dev, struct comedi_devconfig *it)
                s->len_chanlist = 1;
                s->do_cmd = dt2814_ai_cmd;
                s->do_cmdtest = dt2814_ai_cmdtest;
+               s->cancel = dt2814_ai_cancel;
        }
 
        return 0;