On 12/04/16 22:12, H Hartley Sweeten wrote:
This file is is bit of a mess. It's included by the ni_atmio, ni_mio_cs, and ni_pcimio drivers. The ni_pcimio driver is the only one that uses DMA. It defines PCIDMA so that the dma code is compiled it. This causes a bunch of ifdef'ery in the file. The DIO subdevice for the ni_pcidio "is_m_series" boards is quite different from the standard e-series DIO. Mainly it supports async commands that use DMA. Tidy up some of the ifdef'ery by adding ifdef to the subdevice init. Also and ifdef to the interrupt handler and remove the unnecessary if (!devpriv->is_m_series) check in handle_cdio_interrupt(). Consolidate the other ifdef's to block out the affected code. Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> Cc: Ian Abbott <abbotti@xxxxxxxxx> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/staging/comedi/drivers/ni_mio_common.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 11a2453..8544de1 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
[snip]
@@ -3716,13 +3707,8 @@ static void handle_cdio_interrupt(struct comedi_device *dev) struct ni_private *devpriv = dev->private; unsigned cdio_status; struct comedi_subdevice *s = &dev->subdevices[NI_DIO_SUBDEV]; -#ifdef PCIDMA unsigned long flags; -#endif - if (!devpriv->is_m_series) - return;
Removal of that test causes the function to screw up for E-series cards...
-#ifdef PCIDMA spin_lock_irqsave(&devpriv->mite_channel_lock, flags); if (devpriv->cdo_mite_chan) { unsigned cdo_mite_status = @@ -3735,7 +3721,6 @@ static void handle_cdio_interrupt(struct comedi_device *dev) mite_sync_output_dma(devpriv->cdo_mite_chan, s); } spin_unlock_irqrestore(&devpriv->mite_channel_lock, flags); -#endif cdio_status = ni_readl(dev, NI_M_CDIO_STATUS_REG); if (cdio_status & NI_M_CDIO_STATUS_CDO_ERROR) { @@ -3751,6 +3736,7 @@ static void handle_cdio_interrupt(struct comedi_device *dev) } comedi_handle_events(dev, s); }
...by reading a bogus (on E-series) register, possibly setting s->async->events and always calling comedi_handle_events() (which also uses s->async), even though s->async will be NULL for this subdevice on E-series cards.
So it needs some sort of test, and as its using M-series registers, testing devpriv->is_m_series seems reasonable enough. Alternatively, it could test that s->async is non-NULL.
+#endif /* PCIDMA */ static int ni_serial_hw_readwrite8(struct comedi_device *dev, struct comedi_subdevice *s, @@ -5266,7 +5252,9 @@ static irqreturn_t ni_E_interrupt(int irq, void *d) handle_b_interrupt(dev, b_status, ao_mite_status); handle_gpct_interrupt(dev, 0); handle_gpct_interrupt(dev, 1); +#ifdef PCIDMA handle_cdio_interrupt(dev); +#endif
Alternatively, handle_cdio_interrupt() could be called conditionally here. -- -=( 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