Re: [PATCH 09/15] staging: comedi: mite: introduce mite_ack_linkc()

[Ian Abbott <abbotti@xxxxxxxxx>]

On 18/04/16 21:28, H Hartley Sweeten wrote:
> Introduce a helper function to handle the ack of a LINKC interrupt.
> Tidy up the drivers that use the new helper.
>
> The mite_get_status() function is not only used by the mite driver.
> Make it static and remove the export.
>
> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
> Cc: Ian Abbott <abbotti@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>   drivers/staging/comedi/drivers/mite.c          | 17 ++++++++++++++--
>   drivers/staging/comedi/drivers/mite.h          |  2 +-
>   drivers/staging/comedi/drivers/ni_mio_common.c | 28 +++++---------------------
>   drivers/staging/comedi/drivers/ni_pcidio.c     | 10 ++-------
>   drivers/staging/comedi/drivers/ni_tiocmd.c     | 11 ++--------
>   5 files changed, 25 insertions(+), 43 deletions(-)
[snip]
> diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c b/drivers/staging/comedi/drivers/ni_pcidio.c
> index c044c8b..b67358d 100644
> --- a/drivers/staging/comedi/drivers/ni_pcidio.c
> +++ b/drivers/staging/comedi/drivers/ni_pcidio.c
> @@ -381,12 +381,10 @@ static irqreturn_t nidio_interrupt(int irq, void *d)
>   	struct nidio96_private *devpriv = dev->private;
>   	struct comedi_subdevice *s = dev->read_subdev;
>   	struct comedi_async *async = s->async;
> -	struct mite_struct *mite = devpriv->mite;
>   	unsigned int auxdata;
>   	int flags;
>   	int status;
>   	int work = 0;
> -	unsigned int m_status = 0;
>
>   	/* interrupcions parasites */
>   	if (!dev->attached) {
> @@ -401,14 +399,10 @@ static irqreturn_t nidio_interrupt(int irq, void *d)
>   	flags = readb(dev->mmio + Group_1_Flags);
>
>   	spin_lock(&devpriv->mite_channel_lock);
> -	if (devpriv->di_mite_chan)
> -		m_status = mite_get_status(devpriv->di_mite_chan);
> +	if (devpriv->di_mite_chan) {
> +		unsigned int m_status = mite_ack_linkc(devpriv->di_mite_chan);
>
> -	if (m_status & CHSR_INT) {

Is the removal of that `m_status & CHSR_INT` test deliberate?  It looks 
a bit iffy.

>   		if (m_status & CHSR_LINKC) {
> -			writel(CHOR_CLRLC,
> -			       mite->mite_io_addr +
> -			       MITE_CHOR(devpriv->di_mite_chan->channel));
>   			mite_sync_input_dma(devpriv->di_mite_chan, s);
>   			/* XXX need to byteswap */
>   		}
> diff --git a/drivers/staging/comedi/drivers/ni_tiocmd.c b/drivers/staging/comedi/drivers/ni_tiocmd.c
> index 3c3f543..e9ee06b 100644
> --- a/drivers/staging/comedi/drivers/ni_tiocmd.c
> +++ b/drivers/staging/comedi/drivers/ni_tiocmd.c
> @@ -400,7 +400,6 @@ void ni_tio_handle_interrupt(struct ni_gpct *counter,
>   			     struct comedi_subdevice *s)
>   {
>   	unsigned int cidx = counter->counter_index;
> -	unsigned int gpct_mite_status;
>   	unsigned long flags;
>   	int gate_error;
>   	int tc_error;
> @@ -430,15 +429,9 @@ void ni_tio_handle_interrupt(struct ni_gpct *counter,
>   	}
>   	spin_lock_irqsave(&counter->lock, flags);
>   	if (!counter->mite_chan) {
> -		spin_unlock_irqrestore(&counter->lock, flags);
> -		return;
> +		mite_ack_linkc(counter->mite_chan);
> +		mite_sync_input_dma(counter->mite_chan, s);

That's wrong.  I think you forgot to invert the `!counter->mite_chan` test.

>   	}
> -	gpct_mite_status = mite_get_status(counter->mite_chan);
> -	if (gpct_mite_status & CHSR_LINKC)
> -		writel(CHOR_CLRLC,
> -		       counter->mite_chan->mite->mite_io_addr +
> -		       MITE_CHOR(counter->mite_chan->channel));
> -	mite_sync_input_dma(counter->mite_chan, s);
>   	spin_unlock_irqrestore(&counter->lock, flags);
>   }
>   EXPORT_SYMBOL_GPL(ni_tio_handle_interrupt);


--
-=( 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