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

[Alexander Duyck <alexander.duyck@gmail.com>]

On Wed, Apr 13, 2016 at 2:31 PM, Jacob Keller <jacob.e.keller@intel.com> 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