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 <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
  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: <abbo...@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to