Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Keller, Jacob E <jacob.e.keller@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v1 00/13] Fix several GCC6 warnings
Date: Wed, 13 Apr 2016 22:32:59 +0000	[thread overview]
Message-ID: <1460586779.9106.39.camel@intel.com> (raw)
In-Reply-To: <CAKgT0UfDoSrRW5S_xSiireqMoXkB0B4tqjZ5gxLReMQ=4bQG4A@mail.gmail.com>

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@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

  reply	other threads:[~2016-04-13 22:32 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-13 21:31 [Intel-wired-lan] [PATCH v1 00/13] Fix several GCC6 warnings Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 01/13] ixgbe: use BIT() macro Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 02/13] ixgbe: resolve shift of negative value warning Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 03/13] ixgbevf: make use of BIT() macro to avoid shift of signed values Jacob Keller
2016-04-13 21:39   ` Alexander Duyck
2016-04-13 21:47     ` Keller, Jacob E
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 04/13] i40e/i40evf: fix I40E_MASK signed shift overflow warnings Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 05/13] i40e: make use of BIT() macro to prevent left shift of signed values Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 06/13] i40evf: make use of BIT() macro to avoid signed left shift Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 07/13] igb: use BIT() macro or unsigned prefix Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 08/13] igb: make igb_update_pf_vlvf static Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 09/13] igbvf: remove unused variable and dead code Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 10/13] igbvf: use BIT() macro instead of shifts Jacob Keller
2016-04-13 21:44   ` Alexander Duyck
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 11/13] e1000e: use BIT() macro for bit defines Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 12/13] e1000e: mark shifted values as unsigned Jacob Keller
2016-04-13 21:31 ` [Intel-wired-lan] [PATCH v1 13/13] e1000e: remove unused variable Jacob Keller
2016-04-13 21:39   ` Keller, Jacob E
2016-04-13 21:57 ` [Intel-wired-lan] [PATCH v1 00/13] Fix several GCC6 warnings Alexander Duyck
2016-04-13 22:32   ` Keller, Jacob E [this message]
  -- strict thread matches above, loose matches on Subject: below --
2016-04-13 23:08 Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1460586779.9106.39.camel@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox