On 1/6/21 12:37 PM, Rafał Miłecki wrote:
> On 06.01.2021 20:26, Florian Fainelli wrote:
>> On 1/5/21 11:32 PM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <ra...@milecki.pl>
>>>
>>> UniMAC is integrated into multiple Broadcom's Ethernet controllers so
>>> use a shared header file for it and avoid some code duplication.
>>>
>>> Signed-off-by: Rafał Miłecki <ra...@milecki.pl>
>>> ---
>>> MAINTAINERS | 2 +
>>
>> Don't you need to update the BGMAC section to also list unimac.h since
>> it is a shared header now? This looks good to me, the conversion does
>> produce the following warnings on x86-64 (and probably arm64, too):
>>
>> drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_set_rx_mode':
>> drivers/net/ethernet/broadcom/bgmac.c:788:33: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073709551599' to '4294967279' [-Woverflow]
>> 788 | bgmac_umac_cmd_maskset(bgmac, ~CMD_PROMISC, 0, true);
>> drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_mac_speed':
>> drivers/net/ethernet/broadcom/bgmac.c:828:13: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073709550579' to '4294966259' [-Woverflow]
>> 828 | u32 mask = ~(CMD_SPEED_MASK << CMD_SPEED_SHIFT | CMD_HD_EN);
>> | ^
>> drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_chip_reset':
>> drivers/net/ethernet/broadcom/bgmac.c:999:11: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073197811804' to '3783227484' [-Woverflow]
>> 999 | ~(CMD_TX_EN |
>> | ^~~~~~~~~~~~~
>> 1000 | CMD_RX_EN |
>> | ~~~~~~~~~~~
>> 1001 | CMD_RX_PAUSE_IGNORE |
>> | ~~~~~~~~~~~~~~~~~~~~~
>> 1002 | CMD_TX_ADDR_INS |
>> | ~~~~~~~~~~~~~~~~~
>> 1003 | CMD_HD_EN |
>> | ~~~~~~~~~~~
>> 1004 | CMD_LCL_LOOP_EN |
>> | ~~~~~~~~~~~~~~~~~
>> 1005 | CMD_CNTL_FRM_EN |
>> | ~~~~~~~~~~~~~~~~~
>> 1006 | CMD_RMT_LOOP_EN |
>> | ~~~~~~~~~~~~~~~~~
>> 1007 | CMD_RX_ERR_DISC |
>> | ~~~~~~~~~~~~~~~~~
>> 1008 | CMD_PRBL_EN |
>> | ~~~~~~~~~~~~~
>> 1009 | CMD_TX_PAUSE_IGNORE |
>> | ~~~~~~~~~~~~~~~~~~~~~
>> 1010 | CMD_PAD_EN |
>> | ~~~~~~~~~~~~
>> 1011 | CMD_PAUSE_FWD),
>> | ~~~~~~~~~~~~~~
>> drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_enable':
>> drivers/net/ethernet/broadcom/bgmac.c:1057:32: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073709551612' to '4294967292' [-Woverflow]
>> 1057 | bgmac_umac_cmd_maskset(bgmac, ~(CMD_TX_EN | CMD_RX_EN),
>> | ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/broadcom/bgmac.c: In function 'bgmac_chip_init':
>> drivers/net/ethernet/broadcom/bgmac.c:1108:32: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073709551359' to '4294967039' [-Woverflow]
>> 1108 | bgmac_umac_cmd_maskset(bgmac, ~CMD_RX_PAUSE_IGNORE, 0, true);
>> drivers/net/ethernet/broadcom/bgmac.c:1117:33: warning: conversion from
>> 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from
>> '18446744073709518847' to '4294934527' [-Woverflow]
>> 1117 | bgmac_umac_cmd_maskset(bgmac, ~CMD_LCL_LOOP_EN, 0, false);
>
> I can reproduce that after switching from mips to arm64. Before this
> change bgmac.h was not using BIT() macro. Now it does and that macro
> forces UL (unsigned long).
>
> Is there any cleaner solution than below one?
Don't use BIT(), if the constants are 32-bit unsigned integer, maybe
open coding them as (1 << x) is acceptable for that purpose.
--
Florian
