[Intel-wired-lan] [PATCH v1 00/13] Fix several GCC6 warnings

[Keller, Jacob E]

On Wed, 2016-04-13 at 14:57 -0700, Alexander Duyck wrote:
> On Wed, Apr 13, 2016 at 2:31 PM, Jacob Keller <jacob.e.keller at intel.c
> om> wrote:
> > 
> > This patch series fixes several warnings on Intel(R) drivers,
> > including
> > for igb, igbvf, e1000e, ixgbe, ixgbevf, i40e, i40evf and fm10k.
> > 
> > The primary change is to use BIT() macro where appropriate, and use
> > the
> > unsigned postfix for various other bitshifts. While much of this 
> > change
> > doesn't prevent any current warnings, it helps ensure that future
> > additions make use of BIT() macro or the unsigned postfix and
> > prevent
> > signed bitshift errors in the future. A few places (especially in
> > ixgbe)
> > don't use BIT even though it's technically equivalent for style and
> > understanding the real intent of the code.
> > 
> > The series also fixes some other warnings, and makes use of GENMASK
> > in a
> > few locations.
> So after looking over a few patches I would say you are bit to eager
> to apply BIT to things, and there are a few places where GENMASK
> should be used that you were using bit.
> 
> Specifically in the cases where we are shifting a value that just
> happens to be a 1 by some shift I would recommend not using the BIT
> macro.  Such cases are the context descriptor index value or the
> version number for ethtool.  In addition I am not sure it makes much
> sense to use BIT when we are calculating 2 to the power of a given
> value.  For example PAGE_SIZE is defined at (1 << PAGE_SHIFT), not
> BIT(PAGE_SHIFT) within the kernel.
> 
> You probably need to apply GENMASK in all the cases where we have
> BIT(x) - 1 being used.  You will want to set it up as GENMASK(x - 1,
> 0).
> 
> - Alex

Yep. I'll try to clean those up.

Thanks,
Jake