All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: arnd@arndb.de
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, mathieu@codeaurora.org,
	peppe.cavallaro@st.com
Subject: Re: [PATCH] stmmac: avoid ipq806x constant overflow warning
Date: Thu, 12 Nov 2015 12:25:28 -0500 (EST)	[thread overview]
Message-ID: <20151112.122528.1794206832971213176.davem@davemloft.net> (raw)
In-Reply-To: <4391788.dtYArQ4mVi@wuerfel>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 12 Nov 2015 15:12:48 +0100

> Building dwmac-ipq806x on a 64-bit architecture produces a harmless
> warning from gcc:
> 
> stmmac/dwmac-ipq806x.c: In function 'ipq806x_gmac_probe':
> include/linux/bitops.h:6:19: warning: overflow in implicit constant conversion [-Woverflow]
>   val = QSGMII_PHY_CDR_EN |
> stmmac/dwmac-ipq806x.c:333:8: note: in expansion of macro 'QSGMII_PHY_CDR_EN'
>  #define QSGMII_PHY_CDR_EN   BIT(0)
>  #define BIT(nr)   (1UL << (nr))
> 
> The compiler warns about the fact that a 64-bit literal is passed
> into a function that takes a 32-bit argument. I could not fully understand
> why it warns despite the fact that this number is always small enough
> to fit, but changing the use of BIT() macros into the equivalent hexadecimal
> representation avoids the warning
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: b1c17215d718 ("stmmac: add ipq806x glue layer")

I've seen this warning too on x86_64 and had been meaning to look
into it, thanks for taking the initiative. :)

Moving away from using BIT() is somewhat disappointing, because we
want to encourage people to use these macros.

I think it's also easier from a driver author and auditing
perspective, you can see that something is being defined as bit X and
then check the documentation for the chip to see if bit X is correct
or not.

With the hex values there is more mental work and room for... mistakes.

Also I don't even understand the compiler's behavior, it's warning
about QSGMII_PHY_CDR_EN but if you define only that to "0x1u" it still
warns about QSGMII_PHY_CDR_EN.

The warning goes away only if you change all 5 BIT() uses.

This makes me like the change even less, something foul is going on
here and I'd rather figure out what that is than install this patch.

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: davem@davemloft.net (David Miller)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] stmmac: avoid ipq806x constant overflow warning
Date: Thu, 12 Nov 2015 12:25:28 -0500 (EST)	[thread overview]
Message-ID: <20151112.122528.1794206832971213176.davem@davemloft.net> (raw)
In-Reply-To: <4391788.dtYArQ4mVi@wuerfel>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 12 Nov 2015 15:12:48 +0100

> Building dwmac-ipq806x on a 64-bit architecture produces a harmless
> warning from gcc:
> 
> stmmac/dwmac-ipq806x.c: In function 'ipq806x_gmac_probe':
> include/linux/bitops.h:6:19: warning: overflow in implicit constant conversion [-Woverflow]
>   val = QSGMII_PHY_CDR_EN |
> stmmac/dwmac-ipq806x.c:333:8: note: in expansion of macro 'QSGMII_PHY_CDR_EN'
>  #define QSGMII_PHY_CDR_EN   BIT(0)
>  #define BIT(nr)   (1UL << (nr))
> 
> The compiler warns about the fact that a 64-bit literal is passed
> into a function that takes a 32-bit argument. I could not fully understand
> why it warns despite the fact that this number is always small enough
> to fit, but changing the use of BIT() macros into the equivalent hexadecimal
> representation avoids the warning
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: b1c17215d718 ("stmmac: add ipq806x glue layer")

I've seen this warning too on x86_64 and had been meaning to look
into it, thanks for taking the initiative. :)

Moving away from using BIT() is somewhat disappointing, because we
want to encourage people to use these macros.

I think it's also easier from a driver author and auditing
perspective, you can see that something is being defined as bit X and
then check the documentation for the chip to see if bit X is correct
or not.

With the hex values there is more mental work and room for... mistakes.

Also I don't even understand the compiler's behavior, it's warning
about QSGMII_PHY_CDR_EN but if you define only that to "0x1u" it still
warns about QSGMII_PHY_CDR_EN.

The warning goes away only if you change all 5 BIT() uses.

This makes me like the change even less, something foul is going on
here and I'd rather figure out what that is than install this patch.

Thanks.

  reply	other threads:[~2015-11-12 17:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 14:12 [PATCH] stmmac: avoid ipq806x constant overflow warning Arnd Bergmann
2015-11-12 14:12 ` Arnd Bergmann
2015-11-12 17:25 ` David Miller [this message]
2015-11-12 17:25   ` David Miller
2015-11-12 20:52   ` Arnd Bergmann
2015-11-12 20:52     ` Arnd Bergmann
2015-11-12 21:03     ` [PATCH v2] " Arnd Bergmann
2015-11-12 21:03       ` Arnd Bergmann
2015-11-12 21:12       ` David Miller
2015-11-12 21:12         ` David Miller
2015-11-13  7:37       ` Geert Uytterhoeven
2015-11-13  7:37         ` Geert Uytterhoeven
2015-11-13  7:37         ` Geert Uytterhoeven
2015-11-13  7:52         ` Joe Perches
2015-11-13  7:52           ` Joe Perches
2015-11-13  7:52           ` Joe Perches
2015-11-13  8:00           ` Geert Uytterhoeven
2015-11-13  8:00             ` Geert Uytterhoeven
2015-11-13  8:00             ` Geert Uytterhoeven

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=20151112.122528.1794206832971213176.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu@codeaurora.org \
    --cc=netdev@vger.kernel.org \
    --cc=peppe.cavallaro@st.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.