[PATCH 04/19] staging: comedi: ni_mio_common: tidy up DIO subdevice ifdef'ery
Ian Abbott
abbotti at mev.co.uk
Wed Apr 13 12:04:17 UTC 2016
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 at visionengravers.com>
> Cc: Ian Abbott <abbotti at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at 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: <abbotti at mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
More information about the devel
mailing list