[PATCH] staging: comedi: ni_mio_common: add "no_channel" versions of some functions

Ian Abbott abbotti at mev.co.uk
Wed Nov 18 11:02:35 UTC 2015


On 14/11/15 18:19, Andrzej Pietrasiewicz wrote:
> ni_release_ai_mite_channel(), ni_release_ao_mite_channel(),
> ni_release_gpct_mite_channel() and ni_release_cdo_mite_channel()
> call functions which interpret -1 as a special value meaning "no channel".
> This patch adds explicit "no_channel" versions instead.
>
> On the other hand, after "no_channel" versions are used,
> ni_set_ai_dma_channel(), ni_set_ao_dma_channel(),
> ni_set_gpct_dma_channel(), ni_set_cdo_dma_channel() are called with actual
> "channel" parameter being always unsigned, so their signatures are changed
> accordingly.
>
> A side benefit of the changes is suppressesing 4 sparse warnings:
> "warning: shift too big (4294967295) for type int".
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzejtp2010 at gmail.com>
> ---
>   drivers/staging/comedi/drivers/ni_mio_common.c | 82 +++++++++++++++-----------
>   1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
> index 6cc304a..3caadd1 100644
> --- a/drivers/staging/comedi/drivers/ni_mio_common.c
> +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
> @@ -579,48 +579,54 @@ static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
>   	return 0;
>   }
>
> -/* negative channel means no channel */
> -static inline void ni_set_ai_dma_channel(struct comedi_device *dev, int channel)
> +static inline void ni_set_ai_dma_channel(struct comedi_device *dev,
> +					 unsigned channel)
>   {
> -	unsigned bits = 0;
> -
> -	if (channel >= 0)
> -		bits = ni_stc_dma_channel_select_bitfield(channel);
> +	unsigned bits = ni_stc_dma_channel_select_bitfield(channel);
>
>   	ni_set_bitfield(dev, NI_E_DMA_AI_AO_SEL_REG,
>   			NI_E_DMA_AI_SEL_MASK, NI_E_DMA_AI_SEL(bits));
>   }
>
> -/* negative channel means no channel */
> -static inline void ni_set_ao_dma_channel(struct comedi_device *dev, int channel)
> +static inline void ni_set_ai_dma_no_channel(struct comedi_device *dev)
>   {
> -	unsigned bits = 0;
> +	ni_set_bitfield(dev, NI_E_DMA_AI_AO_SEL_REG, NI_E_DMA_AI_SEL_MASK, 0);
> +}
>
> -	if (channel >= 0)
> -		bits = ni_stc_dma_channel_select_bitfield(channel);
> +static inline void ni_set_ao_dma_channel(struct comedi_device *dev,
> +					 unsigned channel)
> +{
> +	unsigned bits = ni_stc_dma_channel_select_bitfield(channel);
>
>   	ni_set_bitfield(dev, NI_E_DMA_AI_AO_SEL_REG,
>   			NI_E_DMA_AO_SEL_MASK, NI_E_DMA_AO_SEL(bits));
>   }
>
> -/* negative channel means no channel */
> +static inline void ni_set_ao_dma_no_channel(struct comedi_device *dev)
> +{
> +	ni_set_bitfield(dev, NI_E_DMA_AI_AO_SEL_REG, NI_E_DMA_AO_SEL_MASK, 0);
> +}
> +
>   static inline void ni_set_gpct_dma_channel(struct comedi_device *dev,
>   					   unsigned gpct_index,
> -					   int channel)
> +					   unsigned channel)
>   {
> -	unsigned bits = 0;
> -
> -	if (channel >= 0)
> -		bits = ni_stc_dma_channel_select_bitfield(channel);
> +	unsigned bits = ni_stc_dma_channel_select_bitfield(channel);
>
>   	ni_set_bitfield(dev, NI_E_DMA_G0_G1_SEL_REG,
>   			NI_E_DMA_G0_G1_SEL_MASK(gpct_index),
>   			NI_E_DMA_G0_G1_SEL(gpct_index, bits));
>   }
>
> -/* negative mite_channel means no channel */
> +static inline void ni_set_gpct_dma_no_channel(struct comedi_device *dev,
> +					      unsigned gpct_index)
> +{
> +	ni_set_bitfield(dev, NI_E_DMA_G0_G1_SEL_REG,
> +			NI_E_DMA_G0_G1_SEL_MASK(gpct_index), 0);
> +}
> +
>   static inline void ni_set_cdo_dma_channel(struct comedi_device *dev,
> -					  int mite_channel)
> +					  unsigned mite_channel)
>   {
>   	struct ni_private *devpriv = dev->private;
>   	unsigned long flags;
> @@ -628,16 +634,26 @@ static inline void ni_set_cdo_dma_channel(struct comedi_device *dev,
>
>   	spin_lock_irqsave(&devpriv->soft_reg_copy_lock, flags);
>   	devpriv->cdio_dma_select_reg &= ~NI_M_CDIO_DMA_SEL_CDO_MASK;
> -	if (mite_channel >= 0) {
> -		/*
> -		 * XXX just guessing ni_stc_dma_channel_select_bitfield()
> -		 * returns the right bits, under the assumption the cdio dma
> -		 * selection works just like ai/ao/gpct.
> -		 * Definitely works for dma channels 0 and 1.
> -		 */
> -		bits = ni_stc_dma_channel_select_bitfield(mite_channel);
> -		devpriv->cdio_dma_select_reg |= NI_M_CDIO_DMA_SEL_CDO(bits);
> -	}
> +	/*
> +	 * XXX just guessing ni_stc_dma_channel_select_bitfield()
> +	 * returns the right bits, under the assumption the cdio dma
> +	 * selection works just like ai/ao/gpct.
> +	 * Definitely works for dma channels 0 and 1.
> +	 */
> +	bits = ni_stc_dma_channel_select_bitfield(mite_channel);
> +	devpriv->cdio_dma_select_reg |= NI_M_CDIO_DMA_SEL_CDO(bits);
> +	ni_writeb(dev, devpriv->cdio_dma_select_reg, NI_M_CDIO_DMA_SEL_REG);
> +	mmiowb();
> +	spin_unlock_irqrestore(&devpriv->soft_reg_copy_lock, flags);
> +}
> +
> +static inline void ni_set_cdo_dma_no_channel(struct comedi_device *dev)
> +{
> +	struct ni_private *devpriv = dev->private;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&devpriv->soft_reg_copy_lock, flags);
> +	devpriv->cdio_dma_select_reg &= ~NI_M_CDIO_DMA_SEL_CDO_MASK;
>   	ni_writeb(dev, devpriv->cdio_dma_select_reg, NI_M_CDIO_DMA_SEL_REG);
>   	mmiowb();
>   	spin_unlock_irqrestore(&devpriv->soft_reg_copy_lock, flags);
> @@ -745,7 +761,7 @@ static void ni_release_ai_mite_channel(struct comedi_device *dev)
>
>   	spin_lock_irqsave(&devpriv->mite_channel_lock, flags);
>   	if (devpriv->ai_mite_chan) {
> -		ni_set_ai_dma_channel(dev, -1);
> +		ni_set_ai_dma_no_channel(dev);
>   		mite_release_channel(devpriv->ai_mite_chan);
>   		devpriv->ai_mite_chan = NULL;
>   	}
> @@ -761,7 +777,7 @@ static void ni_release_ao_mite_channel(struct comedi_device *dev)
>
>   	spin_lock_irqsave(&devpriv->mite_channel_lock, flags);
>   	if (devpriv->ao_mite_chan) {
> -		ni_set_ao_dma_channel(dev, -1);
> +		ni_set_ao_dma_no_channel(dev);
>   		mite_release_channel(devpriv->ao_mite_chan);
>   		devpriv->ao_mite_chan = NULL;
>   	}
> @@ -781,7 +797,7 @@ static void ni_release_gpct_mite_channel(struct comedi_device *dev,
>   		struct mite_channel *mite_chan =
>   		    devpriv->counter_dev->counters[gpct_index].mite_chan;
>
> -		ni_set_gpct_dma_channel(dev, gpct_index, -1);
> +		ni_set_gpct_dma_no_channel(dev, gpct_index);
>   		ni_tio_set_mite_channel(&devpriv->
>   					counter_dev->counters[gpct_index],
>   					NULL);
> @@ -799,7 +815,7 @@ static void ni_release_cdo_mite_channel(struct comedi_device *dev)
>
>   	spin_lock_irqsave(&devpriv->mite_channel_lock, flags);
>   	if (devpriv->cdo_mite_chan) {
> -		ni_set_cdo_dma_channel(dev, -1);
> +		ni_set_cdo_dma_no_channel(dev);
>   		mite_release_channel(devpriv->cdo_mite_chan);
>   		devpriv->cdo_mite_chan = NULL;
>   	}
>

Thanks!

Reviewed-by: Ian Abbott <abbotti at mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-


More information about the devel mailing list