Re: [PATCH 04/19] staging: comedi: ni_mio_common: tidy up DIO subdevice ifdef'ery

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

 



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



[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