[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