| Message ID | 3825178.3luTQFAgny@wuerfel |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Arnd Bergmann <arnd@arndb.de> Date: Thu, 12 Nov 2015 22:03:40 +0100 > @@ -337,11 +337,11 @@ static int ipq806x_gmac_probe(struct platform_device *pdev) > QSGMII_PHY_RX_SIGNAL_DETECT_EN | > QSGMII_PHY_TX_DRIVER_EN | > QSGMII_PHY_QSGMII_EN | > - 0x4 << QSGMII_PHY_PHASE_LOOP_GAIN_OFFSET | > - 0x3 << QSGMII_PHY_RX_DC_BIAS_OFFSET | > - 0x1 << QSGMII_PHY_RX_INPUT_EQU_OFFSET | > - 0x2 << QSGMII_PHY_CDR_PI_SLEW_OFFSET | > - 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET); > + 0x4ul << QSGMII_PHY_PHASE_LOOP_GAIN_OFFSET | > + 0x3ul << QSGMII_PHY_RX_DC_BIAS_OFFSET | > + 0x1ul << QSGMII_PHY_RX_INPUT_EQU_OFFSET | > + 0x2ul << QSGMII_PHY_CDR_PI_SLEW_OFFSET | > + 0xCul << QSGMII_PHY_TX_DRV_AMP_OFFSET); > } > Indeed, this looks so much better. Applied. Thanks for looking more deeply into this! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 12, 2015 at 10:03 PM, Arnd Bergmann <arnd@arndb.de> wrote: > 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)) > > This is a result of the type conversion rules in C, when we take the > logical OR of multiple different types. In particular, we have > and unsigned long > > QSGMII_PHY_CDR_EN == BIT(0) == (1ul << 0) == 0x0000000000000001ul > > and a signed int > > 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET == 0xc0000000 > > which together gives a signed long value > > 0xffffffffc0000001l > > and when this is passed into a function that takes an unsigned int type, > gcc warns about the signed overflow and the loss of the upper 32-bits that > are all ones. > > This patch adds 'ul' type modifiers to the literal numbers passed in > here, so now the expression remains an 'unsigned long' with the upper > bits all zero, and that avoids the signed overflow and the warning. FWIW, the 64-bitness of BIT() on 64-bit platforms is also causing subtle warnings in other places, e.g. when inverting them to create bit mask, cfr. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a9efeca613a8fe5281d7c91f5c8c9ea46f2312f6 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-11-13 at 08:37 +0100, Geert Uytterhoeven wrote: > On Thu, Nov 12, 2015 at 10:03 PM, Arnd Bergmann <arnd@arndb.de> > wrote: > > 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)) > > > > This is a result of the type conversion rules in C, when we take > > the > > logical OR of multiple different types. In particular, we have > > and unsigned long > > > > QSGMII_PHY_CDR_EN == BIT(0) == (1ul << 0) == > > 0x0000000000000001ul > > > > and a signed int > > > > 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET == 0xc0000000 > > > > which together gives a signed long value > > > > 0xffffffffc0000001l > > > > and when this is passed into a function that takes an unsigned int > > type, > > gcc warns about the signed overflow and the loss of the upper 32 > > -bits that > > are all ones. > > > > This patch adds 'ul' type modifiers to the literal numbers passed > > in > > here, so now the expression remains an 'unsigned long' with the > > upper > > bits all zero, and that avoids the signed overflow and the warning. > > FWIW, the 64-bitness of BIT() on 64-bit platforms is also causing > subtle > warnings in other places, e.g. when inverting them to create bit > mask, cfr. > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commi > t/?id=a9efeca613a8fe5281d7c91f5c8c9ea46f2312f6 > > Gr{oetje,eeting}s, I still think specific length BIT macros can be useful. https://lkml.org/lkml/2015/10/16/852 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Joe, On Fri, Nov 13, 2015 at 8:52 AM, Joe Perches <joe@perches.com> wrote: > On Fri, 2015-11-13 at 08:37 +0100, Geert Uytterhoeven wrote: >> On Thu, Nov 12, 2015 at 10:03 PM, Arnd Bergmann <arnd@arndb.de> >> wrote: >> > 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)) >> > >> > This is a result of the type conversion rules in C, when we take >> > the >> > logical OR of multiple different types. In particular, we have >> > and unsigned long >> > >> > QSGMII_PHY_CDR_EN == BIT(0) == (1ul << 0) == >> > 0x0000000000000001ul >> > >> > and a signed int >> > >> > 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET == 0xc0000000 >> > >> > which together gives a signed long value >> > >> > 0xffffffffc0000001l >> > >> > and when this is passed into a function that takes an unsigned int >> > type, >> > gcc warns about the signed overflow and the loss of the upper 32 >> > -bits that >> > are all ones. >> > >> > This patch adds 'ul' type modifiers to the literal numbers passed >> > in >> > here, so now the expression remains an 'unsigned long' with the >> > upper >> > bits all zero, and that avoids the signed overflow and the warning. >> >> FWIW, the 64-bitness of BIT() on 64-bit platforms is also causing >> subtle >> warnings in other places, e.g. when inverting them to create bit >> mask, cfr. >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commi >> t/?id=a9efeca613a8fe5281d7c91f5c8c9ea46f2312f6 > > I still think specific length BIT macros > can be useful. > > https://lkml.org/lkml/2015/10/16/852 Yeah! I only recently started liking the BIT() macro (before I preferred hex, too). Perhaps because Renesas datasheets use bit numbers all over the place ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c index 9d89bdbf029f..82de68b1a452 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c @@ -337,11 +337,11 @@ static int ipq806x_gmac_probe(struct platform_device *pdev) QSGMII_PHY_RX_SIGNAL_DETECT_EN | QSGMII_PHY_TX_DRIVER_EN | QSGMII_PHY_QSGMII_EN | - 0x4 << QSGMII_PHY_PHASE_LOOP_GAIN_OFFSET | - 0x3 << QSGMII_PHY_RX_DC_BIAS_OFFSET | - 0x1 << QSGMII_PHY_RX_INPUT_EQU_OFFSET | - 0x2 << QSGMII_PHY_CDR_PI_SLEW_OFFSET | - 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET); + 0x4ul << QSGMII_PHY_PHASE_LOOP_GAIN_OFFSET | + 0x3ul << QSGMII_PHY_RX_DC_BIAS_OFFSET | + 0x1ul << QSGMII_PHY_RX_INPUT_EQU_OFFSET | + 0x2ul << QSGMII_PHY_CDR_PI_SLEW_OFFSET | + 0xCul << QSGMII_PHY_TX_DRV_AMP_OFFSET); } plat_dat->has_gmac = true;
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)) This is a result of the type conversion rules in C, when we take the logical OR of multiple different types. In particular, we have and unsigned long QSGMII_PHY_CDR_EN == BIT(0) == (1ul << 0) == 0x0000000000000001ul and a signed int 0xC << QSGMII_PHY_TX_DRV_AMP_OFFSET == 0xc0000000 which together gives a signed long value 0xffffffffc0000001l and when this is passed into a function that takes an unsigned int type, gcc warns about the signed overflow and the loss of the upper 32-bits that are all ones. This patch adds 'ul' type modifiers to the literal numbers passed in here, so now the expression remains an 'unsigned long' with the upper bits all zero, and that avoids the signed overflow and the warning. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: b1c17215d718 ("stmmac: add ipq806x glue layer") -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html