On Monday, June 09, 2014 10:48 PM, Dan Carpenter wrote:
> On Mon, Jun 09, 2014 at 09:24:35PM -0400, Marcos A. Di Pietro wrote:
>> Fixes warning static analysis warning raised by sparse in drivers/staging/comedi/drivers/ni_stc.h
>>
>> warning: shift too big (4294967295) for type int
>
> This warning seems wrong. I don't even understand how it thinks the
> shift is too big. The patch doesn't make sense and it introduces a bug.
The warning is a bit annoying and I also haven't figured out why sparse
thinks the shift is too big.
Following is an alternate patch to "fix" the warning.
I'm not sure if this patch will apply to the current linux-next tree due to
a number of patches I am holding until after the merge and Greg opens
the staging tree again.
Please just consider this as a RFC. If it looks like a clean solution I will rebase
it and repost after Greg opens the staging tree.
---
>From 2a0130631a70cf27195cb1b6bffd0af455dbe5dd Mon Sep 17 00:00:00 2001
From: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Date: Mon, 5 May 2014 10:23:32 -0700
Subject: [PATCH] staging: comedi: ni_stc.h: cleanup dma channel select
There are a number of inline functions in ni_stc.h that are used to
work out the bits needed to select a dma channel for the ai, ao, gpct,
and cdo subdevices.
Unfortunately, the ni_stc_dma_channel_select_bitfield() helper causes
a sparse warning:
drivers/staging/comedi/drivers/ni_stc.h:724:26: warning: shift too big (4294967295) for type int
As Dan Carpenter pointed out, this warning seems wrong, but it is
annoying.
The helper functions also have some BUG and BUG_ON statements that will
never happen.
For aesthetics, convert the helper functions to macros and remove the
BUG and BUG_ON code.
Simplify the users of the new macros in ni_mio_common.c.
Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Cc: Marcos A. Di Pietro <marcosadp@xxxxxxxxx>
---
drivers/staging/comedi/drivers/ni_mio_common.c | 72 ++++++++------------------
drivers/staging/comedi/drivers/ni_stc.h | 61 +++++++++-------------
2 files changed, 47 insertions(+), 86 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
index e934fd4..7daac1d 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -239,15 +239,17 @@ static inline void ni_set_bitfield(struct comedi_device *dev, int reg,
devpriv->stc_writew(dev, devpriv->io_bidirection_pin_reg,
IO_Bidirection_Pin_Register);
break;
- case AI_AO_Select:
+ case NI_STC_AI_AO_SELECT_REG:
devpriv->ai_ao_select_reg &= ~bit_mask;
devpriv->ai_ao_select_reg |= bit_values & bit_mask;
- devpriv->writeb(dev, devpriv->ai_ao_select_reg, AI_AO_Select);
+ devpriv->writeb(dev, devpriv->ai_ao_select_reg,
+ NI_STC_AI_AO_SELECT_REG);
break;
- case G0_G1_Select:
+ case NI_STC_G0_G1_SELECT_REG:
devpriv->g0_g1_select_reg &= ~bit_mask;
devpriv->g0_g1_select_reg |= bit_values & bit_mask;
- devpriv->writeb(dev, devpriv->g0_g1_select_reg, G0_G1_Select);
+ devpriv->writeb(dev, devpriv->g0_g1_select_reg,
+ NI_STC_G0_G1_SELECT_REG);
break;
default:
printk("Warning %s() called with invalid register\n", __func__);
@@ -262,67 +264,37 @@ static inline void ni_set_bitfield(struct comedi_device *dev, int reg,
/* DMA channel setup */
/* 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, int chan)
{
- unsigned bitfield;
-
- if (channel >= 0) {
- bitfield =
- (ni_stc_dma_channel_select_bitfield(channel) <<
- AI_DMA_Select_Shift) & AI_DMA_Select_Mask;
- } else {
- bitfield = 0;
- }
- ni_set_bitfield(dev, AI_AO_Select, AI_DMA_Select_Mask, bitfield);
+ ni_set_bitfield(dev, NI_STC_AI_AO_SELECT_REG, NI_STC_AI_DMA_MASK,
+ (chan >= 0) ? NI_STC_AI_DMA_CHAN(chan) : 0);
}
/* negative channel means no channel */
-static inline void ni_set_ao_dma_channel(struct comedi_device *dev, int channel)
+static inline void ni_set_ao_dma_channel(struct comedi_device *dev, int chan)
{
- unsigned bitfield;
-
- if (channel >= 0) {
- bitfield =
- (ni_stc_dma_channel_select_bitfield(channel) <<
- AO_DMA_Select_Shift) & AO_DMA_Select_Mask;
- } else {
- bitfield = 0;
- }
- ni_set_bitfield(dev, AI_AO_Select, AO_DMA_Select_Mask, bitfield);
+ ni_set_bitfield(dev, NI_STC_AI_AO_SELECT_REG, NI_STC_AO_DMA_MASK,
+ (chan >= 0) ? NI_STC_AO_DMA_CHAN(chan) : 0);
}
/* negative mite_channel means no channel */
static inline void ni_set_gpct_dma_channel(struct comedi_device *dev,
- unsigned gpct_index,
- int mite_channel)
+ unsigned gpct, int chan)
{
- unsigned bitfield;
-
- if (mite_channel >= 0)
- bitfield = GPCT_DMA_Select_Bits(gpct_index, mite_channel);
- else
- bitfield = 0;
- ni_set_bitfield(dev, G0_G1_Select, GPCT_DMA_Select_Mask(gpct_index),
- bitfield);
+ ni_set_bitfield(dev, NI_STC_G0_G1_SELECT_REG, NI_STC_GPCT_DMA_MASK(gpct),
+ (chan >= 0) ? NI_STC_GPCT_DMA_CHAN(gpct, chan) : 0);
}
/* negative mite_channel means no channel */
-static inline void ni_set_cdo_dma_channel(struct comedi_device *dev,
- int mite_channel)
+static inline void ni_set_cdo_dma_channel(struct comedi_device *dev, int chan)
{
struct ni_private *devpriv = dev->private;
unsigned long flags;
spin_lock_irqsave(&devpriv->soft_reg_copy_lock, flags);
- devpriv->cdio_dma_select_reg &= ~CDO_DMA_Select_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. */
- devpriv->cdio_dma_select_reg |=
- (ni_stc_dma_channel_select_bitfield(mite_channel) <<
- CDO_DMA_Select_Shift) & CDO_DMA_Select_Mask;
- }
+ devpriv->cdio_dma_select_reg &= ~NI_CDO_DMA_MASK;
+ if (chan >= 0)
+ devpriv->cdio_dma_select_reg |= NI_CDO_DMA_CHAN(chan);
devpriv->writeb(dev, devpriv->cdio_dma_select_reg, M_Offset_CDIO_DMA_Select);
mmiowb();
spin_unlock_irqrestore(&devpriv->soft_reg_copy_lock, flags);
@@ -5552,8 +5524,10 @@ static int ni_E_init(struct comedi_device *dev,
}
/* DMA setup */
- devpriv->writeb(dev, devpriv->ai_ao_select_reg, AI_AO_Select);
- devpriv->writeb(dev, devpriv->g0_g1_select_reg, G0_G1_Select);
+ devpriv->writeb(dev, devpriv->ai_ao_select_reg,
+ NI_STC_AI_AO_SELECT_REG);
+ devpriv->writeb(dev, devpriv->g0_g1_select_reg,
+ NI_STC_G0_G1_SELECT_REG);
if (board->reg_type & ni_reg_6xxx_mask) {
devpriv->writeb(dev, 0, Magic_611x);
diff --git a/drivers/staging/comedi/drivers/ni_stc.h b/drivers/staging/comedi/drivers/ni_stc.h
index 2710c24..0183eb6 100644
--- a/drivers/staging/comedi/drivers/ni_stc.h
+++ b/drivers/staging/comedi/drivers/ni_stc.h
@@ -706,39 +706,23 @@ enum XXX_Status_Bits {
#define Channel_A_Mode 0x03
#define Channel_B_Mode 0x05
#define Channel_C_Mode 0x07
-#define AI_AO_Select 0x09
-enum AI_AO_Select_Bits {
- AI_DMA_Select_Shift = 0,
- AI_DMA_Select_Mask = 0xf,
- AO_DMA_Select_Shift = 4,
- AO_DMA_Select_Mask = 0xf << AO_DMA_Select_Shift
-};
-#define G0_G1_Select 0x0b
-static inline unsigned ni_stc_dma_channel_select_bitfield(unsigned channel)
-{
- if (channel < 4)
- return 1 << channel;
- if (channel == 4)
- return 0x3;
- if (channel == 5)
- return 0x5;
- BUG();
- return 0;
-}
-static inline unsigned GPCT_DMA_Select_Bits(unsigned gpct_index,
- unsigned mite_channel)
-{
- BUG_ON(gpct_index > 1);
- return ni_stc_dma_channel_select_bitfield(mite_channel) << (4 *
- gpct_index);
-}
+#define NI_STC_DMA_CHAN_SEL(_c) (((_c) == 0) ? 0x01 : \
+ ((_c) == 1) ? 0x02 : \
+ ((_c) == 2) ? 0x04 : \
+ ((_c) == 3) ? 0x08 : \
+ ((_c) == 4) ? 0x03 : \
+ ((_c) == 5) ? 0x05 : 0x00)
-static inline unsigned GPCT_DMA_Select_Mask(unsigned gpct_index)
-{
- BUG_ON(gpct_index > 1);
- return 0xf << (4 * gpct_index);
-}
+#define NI_STC_AI_AO_SELECT_REG 0x09
+#define NI_STC_AI_DMA_CHAN(_c) (NI_STC_DMA_CHAN_SEL(_c) << 0)
+#define NI_STC_AI_DMA_MASK (0xf << 0)
+#define NI_STC_AO_DMA_CHAN(_c) (NI_STC_DMA_CHAN_SEL(_c) << 4)
+#define NI_STC_AO_DMA_MASK (0xf << 4)
+
+#define NI_STC_G0_G1_SELECT_REG 0x0b
+#define NI_STC_GPCT_DMA_CHAN(_g,_c) (NI_STC_DMA_CHAN_SEL(_c) << (4 * (_g)))
+#define NI_STC_GPCT_DMA_MASK(_g) (0xf << (4 * (_g)))
/* 16 bit registers */
@@ -1305,12 +1289,15 @@ static inline unsigned MSeries_PFI_Filter_Select_Bits(unsigned channel,
2)) & MSeries_PFI_Filter_Select_Mask(channel);
}
-enum CDIO_DMA_Select_Bits {
- CDI_DMA_Select_Shift = 0,
- CDI_DMA_Select_Mask = 0xf,
- CDO_DMA_Select_Shift = 4,
- CDO_DMA_Select_Mask = 0xf << CDO_DMA_Select_Shift
-};
+/*
+ * XXX just guessing NI_STC_DMA_CHAN_SEL() 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.
+ */
+#define NI_CDI_DMA_CHAN(_c) (NI_STC_DMA_CHAN_SEL(_c) << 0)
+#define NI_CDI_DMA_MASK (0xf << 0)
+#define NI_CDO_DMA_CHAN(_c) (NI_STC_DMA_CHAN_SEL(_c) << 4)
+#define NI_CDO_DMA_MASK (0xf << 4)
enum CDIO_Status_Bits {
CDO_FIFO_Empty_Bit = 0x1,
--
1.9.3
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
