From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Duyck Date: Wed, 13 Apr 2016 14:57:52 -0700 Subject: [Intel-wired-lan] [PATCH v1 00/13] Fix several GCC6 warnings In-Reply-To: <1460583084-16900-1-git-send-email-jacob.e.keller@intel.com> References: <1460583084-16900-1-git-send-email-jacob.e.keller@intel.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On Wed, Apr 13, 2016 at 2:31 PM, Jacob Keller 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