Commit 3d7b3101 authored by Ian Abbott's avatar Ian Abbott Committed by Greg Kroah-Hartman

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: default avatarIan Abbott <abbotti@mev.co.uk>
Link: https://lore.kernel.org/r/20210301165757.243065-5-abbotti@mev.co.ukSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 5fc336c6
...@@ -223,30 +223,87 @@ static int dt2814_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) ...@@ -223,30 +223,87 @@ static int dt2814_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
return 0; 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) static irqreturn_t dt2814_interrupt(int irq, void *d)
{ {
struct comedi_device *dev = d; struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev; struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async;
unsigned int lo, hi;
unsigned short data;
unsigned int status;
if (!dev->attached) { if (!dev->attached) {
dev_err(dev->class_dev, "spurious interrupt\n"); dev_err(dev->class_dev, "spurious interrupt\n");
return IRQ_HANDLED; return IRQ_HANDLED;
} }
inb(dev->iobase + DT2814_DATA); async = s->async;
inb(dev->iobase + DT2814_DATA);
if (!(--devpriv->ntrig)) { spin_lock(&dev->spinlock);
outb(0, dev->iobase + DT2814_CSR);
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 * Note: turning off timed mode triggers another
* sample. This will be mopped up by the calls to * sample. This will be mopped up by the calls to
* dt2814_ai_clear(). * dt2814_ai_clear().
*/ */
outb(status & DT2814_CHANMASK, dev->iobase + DT2814_CSR);
s->async->events |= COMEDI_CB_EOA;
} }
spin_unlock(&dev->spinlock);
comedi_handle_events(dev, s); comedi_handle_events(dev, s);
return IRQ_HANDLED; return IRQ_HANDLED;
} }
...@@ -295,6 +352,7 @@ static int dt2814_attach(struct comedi_device *dev, struct comedi_devconfig *it) ...@@ -295,6 +352,7 @@ static int dt2814_attach(struct comedi_device *dev, struct comedi_devconfig *it)
s->len_chanlist = 1; s->len_chanlist = 1;
s->do_cmd = dt2814_ai_cmd; s->do_cmd = dt2814_ai_cmd;
s->do_cmdtest = dt2814_ai_cmdtest; s->do_cmdtest = dt2814_ai_cmdtest;
s->cancel = dt2814_ai_cancel;
} }
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment