* [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers @ 2021-01-06 7:32 Rafał Miłecki 2021-01-06 7:32 ` [PATCH net-next 2/2] net: broadcom: share header defining " Rafał Miłecki 2021-01-06 19:28 ` [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing " Florian Fainelli 0 siblings, 2 replies; 8+ messages in thread From: Rafał Miłecki @ 2021-01-06 7:32 UTC (permalink / raw) To: David S . Miller, Jakub Kicinski Cc: Florian Fainelli, Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> UniMAC is a hardware block commonly used in Broadcom Ethernet controllers that should get its own header file. Not every controller has it mapped at the 0x800 offset so add bgmac access helpers. They will allow using shared register defines. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- drivers/net/ethernet/broadcom/bgmac.c | 80 +++++++++++++-------------- drivers/net/ethernet/broadcom/bgmac.h | 15 +++++ 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index 98ec1b8a7d8e..b8b2538303ed 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -746,10 +746,10 @@ static int bgmac_dma_init(struct bgmac *bgmac) /* TODO: can we just drop @force? Can we don't reset MAC at all if there is * nothing to change? Try if after stabilizng driver. */ -static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 mask, u32 set, - bool force) +static void bgmac_umac_cmd_maskset(struct bgmac *bgmac, u32 mask, u32 set, + bool force) { - u32 cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG); + u32 cmdcfg = bgmac_umac_read(bgmac, BGMAC_CMDCFG); u32 new_val = (cmdcfg & mask) | set; u32 cmdcfg_sr; @@ -758,13 +758,13 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 mask, u32 set, else cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; - bgmac_set(bgmac, BGMAC_CMDCFG, cmdcfg_sr); + bgmac_umac_maskset(bgmac, BGMAC_CMDCFG, ~0, cmdcfg_sr); udelay(2); if (new_val != cmdcfg || force) - bgmac_write(bgmac, BGMAC_CMDCFG, new_val); + bgmac_umac_write(bgmac, BGMAC_CMDCFG, new_val); - bgmac_mask(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr); + bgmac_umac_maskset(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr, 0); udelay(2); } @@ -773,9 +773,9 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr) u32 tmp; tmp = (addr[0] << 24) | (addr[1] << 16) | (addr[2] << 8) | addr[3]; - bgmac_write(bgmac, BGMAC_MACADDR_HIGH, tmp); + bgmac_umac_write(bgmac, BGMAC_MACADDR_HIGH, tmp); tmp = (addr[4] << 8) | addr[5]; - bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp); + bgmac_umac_write(bgmac, BGMAC_MACADDR_LOW, tmp); } static void bgmac_set_rx_mode(struct net_device *net_dev) @@ -783,9 +783,9 @@ static void bgmac_set_rx_mode(struct net_device *net_dev) struct bgmac *bgmac = netdev_priv(net_dev); if (net_dev->flags & IFF_PROMISC) - bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, true); + bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, true); else - bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, true); + bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, true); } #if 0 /* We don't use that regs yet */ @@ -849,7 +849,7 @@ static void bgmac_mac_speed(struct bgmac *bgmac) if (bgmac->mac_duplex == DUPLEX_HALF) set |= BGMAC_CMDCFG_HD; - bgmac_cmdcfg_maskset(bgmac, mask, set, true); + bgmac_umac_cmd_maskset(bgmac, mask, set, true); } static void bgmac_miiconfig(struct bgmac *bgmac) @@ -917,7 +917,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac) for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) bgmac_dma_tx_reset(bgmac, &bgmac->tx_ring[i]); - bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); + bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); udelay(1); for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) @@ -995,25 +995,25 @@ static void bgmac_chip_reset(struct bgmac *bgmac) else cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; - bgmac_cmdcfg_maskset(bgmac, - ~(BGMAC_CMDCFG_TE | - BGMAC_CMDCFG_RE | - BGMAC_CMDCFG_RPI | - BGMAC_CMDCFG_TAI | - BGMAC_CMDCFG_HD | - BGMAC_CMDCFG_ML | + bgmac_umac_cmd_maskset(bgmac, + ~(BGMAC_CMDCFG_TE | + BGMAC_CMDCFG_RE | + BGMAC_CMDCFG_RPI | + BGMAC_CMDCFG_TAI | + BGMAC_CMDCFG_HD | + BGMAC_CMDCFG_ML | + BGMAC_CMDCFG_CFE | + BGMAC_CMDCFG_RL | + BGMAC_CMDCFG_RED | + BGMAC_CMDCFG_PE | + BGMAC_CMDCFG_TPI | + BGMAC_CMDCFG_PAD_EN | + BGMAC_CMDCFG_PF), + BGMAC_CMDCFG_PROM | + BGMAC_CMDCFG_NLC | BGMAC_CMDCFG_CFE | - BGMAC_CMDCFG_RL | - BGMAC_CMDCFG_RED | - BGMAC_CMDCFG_PE | - BGMAC_CMDCFG_TPI | - BGMAC_CMDCFG_PAD_EN | - BGMAC_CMDCFG_PF), - BGMAC_CMDCFG_PROM | - BGMAC_CMDCFG_NLC | - BGMAC_CMDCFG_CFE | - cmdcfg_sr, - false); + cmdcfg_sr, + false); bgmac->mac_speed = SPEED_UNKNOWN; bgmac->mac_duplex = DUPLEX_UNKNOWN; @@ -1053,12 +1053,12 @@ static void bgmac_enable(struct bgmac *bgmac) else cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; - cmdcfg = bgmac_read(bgmac, BGMAC_CMDCFG); - bgmac_cmdcfg_maskset(bgmac, ~(BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE), - cmdcfg_sr, true); + cmdcfg = bgmac_umac_read(bgmac, BGMAC_CMDCFG); + bgmac_umac_cmd_maskset(bgmac, ~(BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE), + cmdcfg_sr, true); udelay(2); cmdcfg |= BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE; - bgmac_write(bgmac, BGMAC_CMDCFG, cmdcfg); + bgmac_umac_write(bgmac, BGMAC_CMDCFG, cmdcfg); mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT; @@ -1078,7 +1078,7 @@ static void bgmac_enable(struct bgmac *bgmac) fl_ctl = 0x03cb04cb; bgmac_write(bgmac, BGMAC_FLOW_CTL_THRESH, fl_ctl); - bgmac_write(bgmac, BGMAC_PAUSE_CTL, 0x27fff); + bgmac_umac_write(bgmac, BGMAC_PAUSE_CTL, 0x27fff); } if (bgmac->feature_flags & BGMAC_FEAT_SET_RXQ_CLK) { @@ -1105,18 +1105,18 @@ static void bgmac_chip_init(struct bgmac *bgmac) bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT); /* Enable 802.3x tx flow control (honor received PAUSE frames) */ - bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true); + bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true); bgmac_set_rx_mode(bgmac->net_dev); bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr); if (bgmac->loopback) - bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); + bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); else - bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_ML, 0, false); + bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_ML, 0, false); - bgmac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + ETHER_MAX_LEN); + bgmac_umac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + ETHER_MAX_LEN); bgmac_chip_intrs_on(bgmac); @@ -1252,7 +1252,7 @@ static int bgmac_change_mtu(struct net_device *net_dev, int mtu) { struct bgmac *bgmac = netdev_priv(net_dev); - bgmac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + mtu); + bgmac_umac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + mtu); return 0; } diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h index 351c598a3ec6..c069107d0d95 100644 --- a/drivers/net/ethernet/broadcom/bgmac.h +++ b/drivers/net/ethernet/broadcom/bgmac.h @@ -556,6 +556,16 @@ static inline void bgmac_write(struct bgmac *bgmac, u16 offset, u32 value) bgmac->write(bgmac, offset, value); } +static inline u32 bgmac_umac_read(struct bgmac *bgmac, u16 offset) +{ + return bgmac_read(bgmac, offset); +} + +static inline void bgmac_umac_write(struct bgmac *bgmac, u16 offset, u32 value) +{ + bgmac_write(bgmac, offset, value); +} + static inline u32 bgmac_idm_read(struct bgmac *bgmac, u16 offset) { return bgmac->idm_read(bgmac, offset); @@ -609,6 +619,11 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set) bgmac_maskset(bgmac, offset, ~0, set); } +static inline void bgmac_umac_maskset(struct bgmac *bgmac, u16 offset, u32 mask, u32 set) +{ + bgmac_maskset(bgmac, offset, mask, set); +} + static inline int bgmac_phy_connect(struct bgmac *bgmac) { return bgmac->phy_connect(bgmac); -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-06 7:32 [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers Rafał Miłecki @ 2021-01-06 7:32 ` Rafał Miłecki 2021-01-06 19:26 ` Florian Fainelli 2021-01-06 19:28 ` [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing " Florian Fainelli 1 sibling, 1 reply; 8+ messages in thread From: Rafał Miłecki @ 2021-01-06 7:32 UTC (permalink / raw) To: David S . Miller, Jakub Kicinski Cc: Florian Fainelli, Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@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 <rafal@milecki.pl> --- MAINTAINERS | 2 + drivers/net/ethernet/broadcom/bcmsysport.h | 35 +------ drivers/net/ethernet/broadcom/bgmac.c | 98 +++++++++---------- drivers/net/ethernet/broadcom/bgmac.h | 50 ++-------- .../net/ethernet/broadcom/genet/bcmgenet.h | 59 +---------- drivers/net/ethernet/broadcom/genet/bcmmii.c | 6 +- drivers/net/ethernet/broadcom/unimac.h | 68 +++++++++++++ 7 files changed, 132 insertions(+), 186 deletions(-) create mode 100644 drivers/net/ethernet/broadcom/unimac.h diff --git a/MAINTAINERS b/MAINTAINERS index 7c1e45c416b1..3de86229b17c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3625,6 +3625,7 @@ S: Supported F: Documentation/devicetree/bindings/net/brcm,bcmgenet.txt F: Documentation/devicetree/bindings/net/brcm,unimac-mdio.txt F: drivers/net/ethernet/broadcom/genet/ +F: drivers/net/ethernet/broadcom/unimac.h F: drivers/net/mdio/mdio-bcm-unimac.c F: include/linux/platform_data/bcmgenet.h F: include/linux/platform_data/mdio-bcm-unimac.h @@ -3737,6 +3738,7 @@ L: bcm-kernel-feedback-list@broadcom.com L: netdev@vger.kernel.org S: Supported F: drivers/net/ethernet/broadcom/bcmsysport.* +F: drivers/net/ethernet/broadcom/unimac.h BROADCOM TG3 GIGABIT ETHERNET DRIVER M: Siva Reddy Kallam <siva.kallam@broadcom.com> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h index 3a5cb6f128f5..a76c24c1af6d 100644 --- a/drivers/net/ethernet/broadcom/bcmsysport.h +++ b/drivers/net/ethernet/broadcom/bcmsysport.h @@ -13,6 +13,8 @@ #include <linux/if_vlan.h> #include <linux/dim.h> +#include "unimac.h" + /* Receive/transmit descriptor format */ #define DESC_ADDR_HI_STATUS_LEN 0x00 #define DESC_ADDR_HI_SHIFT 0 @@ -213,39 +215,6 @@ struct bcm_rsb { /* UniMAC offset and defines */ #define SYS_PORT_UMAC_OFFSET 0x800 -#define UMAC_CMD 0x008 -#define CMD_TX_EN (1 << 0) -#define CMD_RX_EN (1 << 1) -#define CMD_SPEED_SHIFT 2 -#define CMD_SPEED_10 0 -#define CMD_SPEED_100 1 -#define CMD_SPEED_1000 2 -#define CMD_SPEED_2500 3 -#define CMD_SPEED_MASK 3 -#define CMD_PROMISC (1 << 4) -#define CMD_PAD_EN (1 << 5) -#define CMD_CRC_FWD (1 << 6) -#define CMD_PAUSE_FWD (1 << 7) -#define CMD_RX_PAUSE_IGNORE (1 << 8) -#define CMD_TX_ADDR_INS (1 << 9) -#define CMD_HD_EN (1 << 10) -#define CMD_SW_RESET (1 << 13) -#define CMD_LCL_LOOP_EN (1 << 15) -#define CMD_AUTO_CONFIG (1 << 22) -#define CMD_CNTL_FRM_EN (1 << 23) -#define CMD_NO_LEN_CHK (1 << 24) -#define CMD_RMT_LOOP_EN (1 << 25) -#define CMD_PRBL_EN (1 << 27) -#define CMD_TX_PAUSE_IGNORE (1 << 28) -#define CMD_TX_RX_EN (1 << 29) -#define CMD_RUNT_FILTER_DIS (1 << 30) - -#define UMAC_MAC0 0x00c -#define UMAC_MAC1 0x010 -#define UMAC_MAX_FRAME_LEN 0x014 - -#define UMAC_TX_FLUSH 0x334 - #define UMAC_MIB_START 0x400 /* There is a 0xC gap between the end of RX and beginning of TX stats and then diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index b8b2538303ed..075f6e146b29 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -749,22 +749,22 @@ static int bgmac_dma_init(struct bgmac *bgmac) static void bgmac_umac_cmd_maskset(struct bgmac *bgmac, u32 mask, u32 set, bool force) { - u32 cmdcfg = bgmac_umac_read(bgmac, BGMAC_CMDCFG); + u32 cmdcfg = bgmac_umac_read(bgmac, UMAC_CMD); u32 new_val = (cmdcfg & mask) | set; u32 cmdcfg_sr; if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4) - cmdcfg_sr = BGMAC_CMDCFG_SR_REV4; + cmdcfg_sr = CMD_SW_RESET; else - cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; + cmdcfg_sr = CMD_SW_RESET_OLD; - bgmac_umac_maskset(bgmac, BGMAC_CMDCFG, ~0, cmdcfg_sr); + bgmac_umac_maskset(bgmac, UMAC_CMD, ~0, cmdcfg_sr); udelay(2); if (new_val != cmdcfg || force) - bgmac_umac_write(bgmac, BGMAC_CMDCFG, new_val); + bgmac_umac_write(bgmac, UMAC_CMD, new_val); - bgmac_umac_maskset(bgmac, BGMAC_CMDCFG, ~cmdcfg_sr, 0); + bgmac_umac_maskset(bgmac, UMAC_CMD, ~cmdcfg_sr, 0); udelay(2); } @@ -773,9 +773,9 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr) u32 tmp; tmp = (addr[0] << 24) | (addr[1] << 16) | (addr[2] << 8) | addr[3]; - bgmac_umac_write(bgmac, BGMAC_MACADDR_HIGH, tmp); + bgmac_umac_write(bgmac, UMAC_MAC0, tmp); tmp = (addr[4] << 8) | addr[5]; - bgmac_umac_write(bgmac, BGMAC_MACADDR_LOW, tmp); + bgmac_umac_write(bgmac, UMAC_MAC1, tmp); } static void bgmac_set_rx_mode(struct net_device *net_dev) @@ -783,9 +783,9 @@ static void bgmac_set_rx_mode(struct net_device *net_dev) struct bgmac *bgmac = netdev_priv(net_dev); if (net_dev->flags & IFF_PROMISC) - bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, true); + bgmac_umac_cmd_maskset(bgmac, ~0, CMD_PROMISC, true); else - bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, true); + bgmac_umac_cmd_maskset(bgmac, ~CMD_PROMISC, 0, true); } #if 0 /* We don't use that regs yet */ @@ -825,21 +825,21 @@ static void bgmac_clear_mib(struct bgmac *bgmac) /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/gmac_speed */ static void bgmac_mac_speed(struct bgmac *bgmac) { - u32 mask = ~(BGMAC_CMDCFG_ES_MASK | BGMAC_CMDCFG_HD); + u32 mask = ~(CMD_SPEED_MASK << CMD_SPEED_SHIFT | CMD_HD_EN); u32 set = 0; switch (bgmac->mac_speed) { case SPEED_10: - set |= BGMAC_CMDCFG_ES_10; + set |= CMD_SPEED_10 << CMD_SPEED_SHIFT; break; case SPEED_100: - set |= BGMAC_CMDCFG_ES_100; + set |= CMD_SPEED_100 << CMD_SPEED_SHIFT; break; case SPEED_1000: - set |= BGMAC_CMDCFG_ES_1000; + set |= CMD_SPEED_1000 << CMD_SPEED_SHIFT; break; case SPEED_2500: - set |= BGMAC_CMDCFG_ES_2500; + set |= CMD_SPEED_2500 << CMD_SPEED_SHIFT; break; default: dev_err(bgmac->dev, "Unsupported speed: %d\n", @@ -847,7 +847,7 @@ static void bgmac_mac_speed(struct bgmac *bgmac) } if (bgmac->mac_duplex == DUPLEX_HALF) - set |= BGMAC_CMDCFG_HD; + set |= CMD_HD_EN; bgmac_umac_cmd_maskset(bgmac, mask, set, true); } @@ -917,7 +917,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac) for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) bgmac_dma_tx_reset(bgmac, &bgmac->tx_ring[i]); - bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); + bgmac_umac_cmd_maskset(bgmac, ~0, CMD_LCL_LOOP_EN, false); udelay(1); for (i = 0; i < BGMAC_MAX_RX_RINGS; i++) @@ -986,32 +986,32 @@ static void bgmac_chip_reset(struct bgmac *bgmac) } /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/gmac_reset - * Specs don't say about using BGMAC_CMDCFG_SR, but in this routine - * BGMAC_CMDCFG is read _after_ putting chip in a reset. So it has to + * Specs don't say about using UMAC_CMD_SR, but in this routine + * UMAC_CMD is read _after_ putting chip in a reset. So it has to * be keps until taking MAC out of the reset. */ if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4) - cmdcfg_sr = BGMAC_CMDCFG_SR_REV4; + cmdcfg_sr = CMD_SW_RESET; else - cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; + cmdcfg_sr = CMD_SW_RESET_OLD; bgmac_umac_cmd_maskset(bgmac, - ~(BGMAC_CMDCFG_TE | - BGMAC_CMDCFG_RE | - BGMAC_CMDCFG_RPI | - BGMAC_CMDCFG_TAI | - BGMAC_CMDCFG_HD | - BGMAC_CMDCFG_ML | - BGMAC_CMDCFG_CFE | - BGMAC_CMDCFG_RL | - BGMAC_CMDCFG_RED | - BGMAC_CMDCFG_PE | - BGMAC_CMDCFG_TPI | - BGMAC_CMDCFG_PAD_EN | - BGMAC_CMDCFG_PF), - BGMAC_CMDCFG_PROM | - BGMAC_CMDCFG_NLC | - BGMAC_CMDCFG_CFE | + ~(CMD_TX_EN | + CMD_RX_EN | + CMD_RX_PAUSE_IGNORE | + CMD_TX_ADDR_INS | + CMD_HD_EN | + CMD_LCL_LOOP_EN | + CMD_CNTL_FRM_EN | + CMD_RMT_LOOP_EN | + CMD_RX_ERR_DISC | + CMD_PRBL_EN | + CMD_TX_PAUSE_IGNORE | + CMD_PAD_EN | + CMD_PAUSE_FWD), + CMD_PROMISC | + CMD_NO_LEN_CHK | + CMD_CNTL_FRM_EN | cmdcfg_sr, false); bgmac->mac_speed = SPEED_UNKNOWN; @@ -1049,16 +1049,16 @@ static void bgmac_enable(struct bgmac *bgmac) u32 mode; if (bgmac->feature_flags & BGMAC_FEAT_CMDCFG_SR_REV4) - cmdcfg_sr = BGMAC_CMDCFG_SR_REV4; + cmdcfg_sr = CMD_SW_RESET; else - cmdcfg_sr = BGMAC_CMDCFG_SR_REV0; + cmdcfg_sr = CMD_SW_RESET_OLD; - cmdcfg = bgmac_umac_read(bgmac, BGMAC_CMDCFG); - bgmac_umac_cmd_maskset(bgmac, ~(BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE), + cmdcfg = bgmac_umac_read(bgmac, UMAC_CMD); + bgmac_umac_cmd_maskset(bgmac, ~(CMD_TX_EN | CMD_RX_EN), cmdcfg_sr, true); udelay(2); - cmdcfg |= BGMAC_CMDCFG_TE | BGMAC_CMDCFG_RE; - bgmac_umac_write(bgmac, BGMAC_CMDCFG, cmdcfg); + cmdcfg |= CMD_TX_EN | CMD_RX_EN; + bgmac_umac_write(bgmac, UMAC_CMD, cmdcfg); mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >> BGMAC_DS_MM_SHIFT; @@ -1078,7 +1078,7 @@ static void bgmac_enable(struct bgmac *bgmac) fl_ctl = 0x03cb04cb; bgmac_write(bgmac, BGMAC_FLOW_CTL_THRESH, fl_ctl); - bgmac_umac_write(bgmac, BGMAC_PAUSE_CTL, 0x27fff); + bgmac_umac_write(bgmac, UMAC_PAUSE_CTRL, 0x27fff); } if (bgmac->feature_flags & BGMAC_FEAT_SET_RXQ_CLK) { @@ -1105,18 +1105,18 @@ static void bgmac_chip_init(struct bgmac *bgmac) bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT); /* Enable 802.3x tx flow control (honor received PAUSE frames) */ - bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true); + bgmac_umac_cmd_maskset(bgmac, ~CMD_RX_PAUSE_IGNORE, 0, true); bgmac_set_rx_mode(bgmac->net_dev); bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr); if (bgmac->loopback) - bgmac_umac_cmd_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, false); + bgmac_umac_cmd_maskset(bgmac, ~0, CMD_LCL_LOOP_EN, false); else - bgmac_umac_cmd_maskset(bgmac, ~BGMAC_CMDCFG_ML, 0, false); + bgmac_umac_cmd_maskset(bgmac, ~CMD_LCL_LOOP_EN, 0, false); - bgmac_umac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + ETHER_MAX_LEN); + bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN, 32 + ETHER_MAX_LEN); bgmac_chip_intrs_on(bgmac); @@ -1252,7 +1252,7 @@ static int bgmac_change_mtu(struct net_device *net_dev, int mtu) { struct bgmac *bgmac = netdev_priv(net_dev); - bgmac_umac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + mtu); + bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN, 32 + mtu); return 0; } diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h index c069107d0d95..110088e662ea 100644 --- a/drivers/net/ethernet/broadcom/bgmac.h +++ b/drivers/net/ethernet/broadcom/bgmac.h @@ -4,6 +4,8 @@ #include <linux/netdevice.h> +#include "unimac.h" + #define BGMAC_DEV_CTL 0x000 #define BGMAC_DC_TSM 0x00000002 #define BGMAC_DC_CFCO 0x00000004 @@ -169,47 +171,7 @@ #define BGMAC_RX_NONPAUSE_PKTS 0x420 #define BGMAC_RX_SACHANGES 0x424 #define BGMAC_RX_UNI_PKTS 0x428 -#define BGMAC_UNIMAC_VERSION 0x800 -#define BGMAC_HDBKP_CTL 0x804 -#define BGMAC_CMDCFG 0x808 /* Configuration */ -#define BGMAC_CMDCFG_TE 0x00000001 /* Set to activate TX */ -#define BGMAC_CMDCFG_RE 0x00000002 /* Set to activate RX */ -#define BGMAC_CMDCFG_ES_MASK 0x0000000c /* Ethernet speed see gmac_speed */ -#define BGMAC_CMDCFG_ES_10 0x00000000 -#define BGMAC_CMDCFG_ES_100 0x00000004 -#define BGMAC_CMDCFG_ES_1000 0x00000008 -#define BGMAC_CMDCFG_ES_2500 0x0000000C -#define BGMAC_CMDCFG_PROM 0x00000010 /* Set to activate promiscuous mode */ -#define BGMAC_CMDCFG_PAD_EN 0x00000020 -#define BGMAC_CMDCFG_CF 0x00000040 -#define BGMAC_CMDCFG_PF 0x00000080 -#define BGMAC_CMDCFG_RPI 0x00000100 /* Unset to enable 802.3x tx flow control */ -#define BGMAC_CMDCFG_TAI 0x00000200 -#define BGMAC_CMDCFG_HD 0x00000400 /* Set if in half duplex mode */ -#define BGMAC_CMDCFG_HD_SHIFT 10 -#define BGMAC_CMDCFG_SR_REV0 0x00000800 /* Set to reset mode, for core rev 0-3 */ -#define BGMAC_CMDCFG_SR_REV4 0x00002000 /* Set to reset mode, for core rev >= 4 */ -#define BGMAC_CMDCFG_ML 0x00008000 /* Set to activate mac loopback mode */ -#define BGMAC_CMDCFG_AE 0x00400000 -#define BGMAC_CMDCFG_CFE 0x00800000 -#define BGMAC_CMDCFG_NLC 0x01000000 -#define BGMAC_CMDCFG_RL 0x02000000 -#define BGMAC_CMDCFG_RED 0x04000000 -#define BGMAC_CMDCFG_PE 0x08000000 -#define BGMAC_CMDCFG_TPI 0x10000000 -#define BGMAC_CMDCFG_AT 0x20000000 -#define BGMAC_MACADDR_HIGH 0x80c /* High 4 octets of own mac address */ -#define BGMAC_MACADDR_LOW 0x810 /* Low 2 octets of own mac address */ -#define BGMAC_RXMAX_LENGTH 0x814 /* Max receive frame length with vlan tag */ -#define BGMAC_PAUSEQUANTA 0x818 -#define BGMAC_MAC_MODE 0x844 -#define BGMAC_OUTERTAG 0x848 -#define BGMAC_INNERTAG 0x84c -#define BGMAC_TXIPG 0x85c -#define BGMAC_PAUSE_CTL 0xb30 -#define BGMAC_TX_FLUSH 0xb34 -#define BGMAC_RX_STATUS 0xb38 -#define BGMAC_TX_STATUS 0xb3c +#define BGMAC_UNIMAC 0x800 /* BCMA GMAC core specific IO Control (BCMA_IOCTL) flags */ #define BGMAC_BCMA_IOCTL_SW_CLKEN 0x00000004 /* PHY Clock Enable */ @@ -558,12 +520,12 @@ static inline void bgmac_write(struct bgmac *bgmac, u16 offset, u32 value) static inline u32 bgmac_umac_read(struct bgmac *bgmac, u16 offset) { - return bgmac_read(bgmac, offset); + return bgmac_read(bgmac, BGMAC_UNIMAC + offset); } static inline void bgmac_umac_write(struct bgmac *bgmac, u16 offset, u32 value) { - bgmac_write(bgmac, offset, value); + bgmac_write(bgmac, BGMAC_UNIMAC + offset, value); } static inline u32 bgmac_idm_read(struct bgmac *bgmac, u16 offset) @@ -621,7 +583,7 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set) static inline void bgmac_umac_maskset(struct bgmac *bgmac, u16 offset, u32 mask, u32 set) { - bgmac_maskset(bgmac, offset, mask, set); + bgmac_maskset(bgmac, BGMAC_UNIMAC + offset, mask, set); } static inline int bgmac_phy_connect(struct bgmac *bgmac) diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h index f6ca01da141d..0a6d91b0f0aa 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h @@ -16,6 +16,8 @@ #include <linux/dim.h> #include <linux/ethtool.h> +#include "../unimac.h" + /* total number of Buffer Descriptors, same for Rx/Tx */ #define TOTAL_DESC 256 @@ -150,63 +152,6 @@ struct bcmgenet_mib_counters { u32 tx_realloc_tsb_failed; }; -#define UMAC_HD_BKP_CTRL 0x004 -#define HD_FC_EN (1 << 0) -#define HD_FC_BKOFF_OK (1 << 1) -#define IPG_CONFIG_RX_SHIFT 2 -#define IPG_CONFIG_RX_MASK 0x1F - -#define UMAC_CMD 0x008 -#define CMD_TX_EN (1 << 0) -#define CMD_RX_EN (1 << 1) -#define UMAC_SPEED_10 0 -#define UMAC_SPEED_100 1 -#define UMAC_SPEED_1000 2 -#define UMAC_SPEED_2500 3 -#define CMD_SPEED_SHIFT 2 -#define CMD_SPEED_MASK 3 -#define CMD_PROMISC (1 << 4) -#define CMD_PAD_EN (1 << 5) -#define CMD_CRC_FWD (1 << 6) -#define CMD_PAUSE_FWD (1 << 7) -#define CMD_RX_PAUSE_IGNORE (1 << 8) -#define CMD_TX_ADDR_INS (1 << 9) -#define CMD_HD_EN (1 << 10) -#define CMD_SW_RESET (1 << 13) -#define CMD_LCL_LOOP_EN (1 << 15) -#define CMD_AUTO_CONFIG (1 << 22) -#define CMD_CNTL_FRM_EN (1 << 23) -#define CMD_NO_LEN_CHK (1 << 24) -#define CMD_RMT_LOOP_EN (1 << 25) -#define CMD_PRBL_EN (1 << 27) -#define CMD_TX_PAUSE_IGNORE (1 << 28) -#define CMD_TX_RX_EN (1 << 29) -#define CMD_RUNT_FILTER_DIS (1 << 30) - -#define UMAC_MAC0 0x00C -#define UMAC_MAC1 0x010 -#define UMAC_MAX_FRAME_LEN 0x014 - -#define UMAC_MODE 0x44 -#define MODE_LINK_STATUS (1 << 5) - -#define UMAC_EEE_CTRL 0x064 -#define EN_LPI_RX_PAUSE (1 << 0) -#define EN_LPI_TX_PFC (1 << 1) -#define EN_LPI_TX_PAUSE (1 << 2) -#define EEE_EN (1 << 3) -#define RX_FIFO_CHECK (1 << 4) -#define EEE_TX_CLK_DIS (1 << 5) -#define DIS_EEE_10M (1 << 6) -#define LP_IDLE_PREDICTION_MODE (1 << 7) - -#define UMAC_EEE_LPI_TIMER 0x068 -#define UMAC_EEE_WAKE_TIMER 0x06C -#define UMAC_EEE_REF_COUNT 0x070 -#define EEE_REFERENCE_COUNT_MASK 0xffff - -#define UMAC_TX_FLUSH 0x334 - #define UMAC_MIB_START 0x400 #define UMAC_MDIO_CMD 0x614 diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index 6fb6c3556285..17f997ef950f 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -63,11 +63,11 @@ void bcmgenet_mii_setup(struct net_device *dev) /* speed */ if (phydev->speed == SPEED_1000) - cmd_bits = UMAC_SPEED_1000; + cmd_bits = CMD_SPEED_1000; else if (phydev->speed == SPEED_100) - cmd_bits = UMAC_SPEED_100; + cmd_bits = CMD_SPEED_100; else - cmd_bits = UMAC_SPEED_10; + cmd_bits = CMD_SPEED_10; cmd_bits <<= CMD_SPEED_SHIFT; /* duplex */ diff --git a/drivers/net/ethernet/broadcom/unimac.h b/drivers/net/ethernet/broadcom/unimac.h new file mode 100644 index 000000000000..4223a56b84d8 --- /dev/null +++ b/drivers/net/ethernet/broadcom/unimac.h @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __UNIMAC_H +#define __UNIMAC_H + +#define UMAC_HD_BKP_CTRL 0x004 +#define HD_FC_EN BIT(0) +#define HD_FC_BKOFF_OK BIT(1) +#define IPG_CONFIG_RX_SHIFT 2 +#define IPG_CONFIG_RX_MASK 0x1F +#define UMAC_CMD 0x008 +#define CMD_TX_EN BIT(0) +#define CMD_RX_EN BIT(1) +#define CMD_SPEED_10 0 +#define CMD_SPEED_100 1 +#define CMD_SPEED_1000 2 +#define CMD_SPEED_2500 3 +#define CMD_SPEED_SHIFT 2 +#define CMD_SPEED_MASK 3 +#define CMD_PROMISC BIT(4) +#define CMD_PAD_EN BIT(5) +#define CMD_CRC_FWD BIT(6) +#define CMD_PAUSE_FWD BIT(7) +#define CMD_RX_PAUSE_IGNORE BIT(8) +#define CMD_TX_ADDR_INS BIT(9) +#define CMD_HD_EN BIT(10) +#define CMD_SW_RESET_OLD BIT(11) +#define CMD_SW_RESET BIT(13) +#define CMD_LCL_LOOP_EN BIT(15) +#define CMD_AUTO_CONFIG BIT(22) +#define CMD_CNTL_FRM_EN BIT(23) +#define CMD_NO_LEN_CHK BIT(24) +#define CMD_RMT_LOOP_EN BIT(25) +#define CMD_RX_ERR_DISC BIT(26) +#define CMD_PRBL_EN BIT(27) +#define CMD_TX_PAUSE_IGNORE BIT(28) +#define CMD_TX_RX_EN BIT(29) +#define CMD_RUNT_FILTER_DIS BIT(30) +#define UMAC_MAC0 0x00c +#define UMAC_MAC1 0x010 +#define UMAC_MAX_FRAME_LEN 0x014 +#define UMAC_PAUSE_QUANTA 0x018 +#define UMAC_MODE 0x044 +#define MODE_LINK_STATUS BIT(5) +#define UMAC_FRM_TAG0 0x048 /* outer tag */ +#define UMAC_FRM_TAG1 0x04c /* inner tag */ +#define UMAC_TX_IPG_LEN 0x05c +#define UMAC_EEE_CTRL 0x064 +#define EN_LPI_RX_PAUSE BIT(0) +#define EN_LPI_TX_PFC BIT(1) +#define EN_LPI_TX_PAUSE BIT(2) +#define EEE_EN BIT(3) +#define RX_FIFO_CHECK BIT(4) +#define EEE_TX_CLK_DIS BIT(5) +#define DIS_EEE_10M BIT(6) +#define LP_IDLE_PREDICTION_MODE BIT(7) +#define UMAC_EEE_LPI_TIMER 0x068 +#define UMAC_EEE_WAKE_TIMER 0x06C +#define UMAC_EEE_REF_COUNT 0x070 +#define EEE_REFERENCE_COUNT_MASK 0xffff +#define UMAC_RX_IPG_INV 0x078 +#define UMAC_MACSEC_PROG_TX_CRC 0x310 +#define UMAC_MACSEC_CTRL 0x314 +#define UMAC_PAUSE_CTRL 0x330 +#define UMAC_TX_FLUSH 0x334 +#define UMAC_RX_FIFO_STATUS 0x338 +#define UMAC_TX_FIFO_STATUS 0x33c + +#endif -- 2.26.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-06 7:32 ` [PATCH net-next 2/2] net: broadcom: share header defining " Rafał Miłecki @ 2021-01-06 19:26 ` Florian Fainelli 2021-01-06 19:45 ` Florian Fainelli 2021-01-06 20:37 ` Rafał Miłecki 0 siblings, 2 replies; 8+ messages in thread From: Florian Fainelli @ 2021-01-06 19:26 UTC (permalink / raw) To: Rafał Miłecki, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki On 1/5/21 11:32 PM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@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 <rafal@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 did verify that the md5sum of the objects does not change before and after changes (except bgmac.o, which is expected due to the warning above0, so that gives me good confidence that the changes are correct :) Acked-by: Florian Fainelli <f.fainelli@gmail.com> Thanks for doing this. -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-06 19:26 ` Florian Fainelli @ 2021-01-06 19:45 ` Florian Fainelli 2021-01-06 20:37 ` Rafał Miłecki 1 sibling, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2021-01-06 19:45 UTC (permalink / raw) To: Florian Fainelli, Rafał Miłecki, David S . Miller, Jakub Kicinski Cc: Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki On 1/6/21 11:26 AM, Florian Fainelli wrote: > On 1/5/21 11:32 PM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@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 <rafal@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 did verify that the md5sum of the objects does not change before and > after changes (except bgmac.o, which is expected due to the warning > above0, so that gives me good confidence that the changes are correct :) > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > > Thanks for doing this. For GENET and SYSTEMPORT you should be able to share the MIB counters as well, and in premise we could even get a step further and share the ethtool stats array between drivers since they are the exact same. You don't have to include that as part of your series though, we can address it later. We have a third driver coming up which is also using an UniMAC and could benefit from not replicating these headers. -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-06 19:26 ` Florian Fainelli 2021-01-06 19:45 ` Florian Fainelli @ 2021-01-06 20:37 ` Rafał Miłecki 2021-01-07 17:14 ` Florian Fainelli 1 sibling, 1 reply; 8+ messages in thread From: Rafał Miłecki @ 2021-01-06 20:37 UTC (permalink / raw) To: Florian Fainelli, David S . Miller, Jakub Kicinski Cc: Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki On 06.01.2021 20:26, Florian Fainelli wrote: > On 1/5/21 11:32 PM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@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 <rafal@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? diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index 075f6e146b29..1cb0ec3d9b3a 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -785,7 +785,7 @@ static void bgmac_set_rx_mode(struct net_device *net_dev) if (net_dev->flags & IFF_PROMISC) bgmac_umac_cmd_maskset(bgmac, ~0, CMD_PROMISC, true); else - bgmac_umac_cmd_maskset(bgmac, ~CMD_PROMISC, 0, true); + bgmac_umac_cmd_maskset(bgmac, (u32)~CMD_PROMISC, 0, true); } #if 0 /* We don't use that regs yet */ @@ -825,7 +825,7 @@ static void bgmac_clear_mib(struct bgmac *bgmac) /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/gmac_speed */ static void bgmac_mac_speed(struct bgmac *bgmac) { - u32 mask = ~(CMD_SPEED_MASK << CMD_SPEED_SHIFT | CMD_HD_EN); + u32 mask = (u32)~(CMD_SPEED_MASK << CMD_SPEED_SHIFT | CMD_HD_EN); u32 set = 0; switch (bgmac->mac_speed) { @@ -996,7 +996,7 @@ static void bgmac_chip_reset(struct bgmac *bgmac) cmdcfg_sr = CMD_SW_RESET_OLD; bgmac_umac_cmd_maskset(bgmac, - ~(CMD_TX_EN | + (u32)~(CMD_TX_EN | CMD_RX_EN | CMD_RX_PAUSE_IGNORE | CMD_TX_ADDR_INS | @@ -1054,7 +1054,7 @@ static void bgmac_enable(struct bgmac *bgmac) cmdcfg_sr = CMD_SW_RESET_OLD; cmdcfg = bgmac_umac_read(bgmac, UMAC_CMD); - bgmac_umac_cmd_maskset(bgmac, ~(CMD_TX_EN | CMD_RX_EN), + bgmac_umac_cmd_maskset(bgmac, (u32)~(CMD_TX_EN | CMD_RX_EN), cmdcfg_sr, true); udelay(2); cmdcfg |= CMD_TX_EN | CMD_RX_EN; @@ -1105,7 +1105,7 @@ static void bgmac_chip_init(struct bgmac *bgmac) bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT); /* Enable 802.3x tx flow control (honor received PAUSE frames) */ - bgmac_umac_cmd_maskset(bgmac, ~CMD_RX_PAUSE_IGNORE, 0, true); + bgmac_umac_cmd_maskset(bgmac, (u32)~CMD_RX_PAUSE_IGNORE, 0, true); bgmac_set_rx_mode(bgmac->net_dev); @@ -1114,7 +1114,7 @@ static void bgmac_chip_init(struct bgmac *bgmac) if (bgmac->loopback) bgmac_umac_cmd_maskset(bgmac, ~0, CMD_LCL_LOOP_EN, false); else - bgmac_umac_cmd_maskset(bgmac, ~CMD_LCL_LOOP_EN, 0, false); + bgmac_umac_cmd_maskset(bgmac, (u32)~CMD_LCL_LOOP_EN, 0, false); bgmac_umac_write(bgmac, UMAC_MAX_FRAME_LEN, 32 + ETHER_MAX_LEN); > I did verify that the md5sum of the objects does not change before and > after changes (except bgmac.o, which is expected due to the warning > above0, so that gives me good confidence that the changes are correct :) > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> Thanks! ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-06 20:37 ` Rafał Miłecki @ 2021-01-07 17:14 ` Florian Fainelli 2021-01-07 17:21 ` Jakub Kicinski 0 siblings, 1 reply; 8+ messages in thread From: Florian Fainelli @ 2021-01-07 17:14 UTC (permalink / raw) To: Rafał Miłecki, Florian Fainelli, David S . Miller, Jakub Kicinski Cc: Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki 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 <rafal@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 <rafal@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: broadcom: share header defining UniMAC registers 2021-01-07 17:14 ` Florian Fainelli @ 2021-01-07 17:21 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2021-01-07 17:21 UTC (permalink / raw) To: Florian Fainelli Cc: Rafał Miłecki, David S . Miller, Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki On Thu, 7 Jan 2021 09:14:17 -0800 Florian Fainelli wrote: > > 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. No objections from my side, I think we already have a number of drivers open coding the shifts for that very reason already. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers 2021-01-06 7:32 [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers Rafał Miłecki 2021-01-06 7:32 ` [PATCH net-next 2/2] net: broadcom: share header defining " Rafał Miłecki @ 2021-01-06 19:28 ` Florian Fainelli 1 sibling, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2021-01-06 19:28 UTC (permalink / raw) To: Rafał Miłecki, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, Doug Berger, Ray Jui, Arun Parameswaran, Murali Krishna Policharla, Timur Tabi, Heiner Kallweit, Vladimir Oltean, netdev, bcm-kernel-feedback-list, Rafał Miłecki On 1/5/21 11:32 PM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > UniMAC is a hardware block commonly used in Broadcom Ethernet controllers > that should get its own header file. Not every controller has it mapped at > the 0x800 offset so add bgmac access helpers. They will allow using > shared register defines. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Acked-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-07 17:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-06 7:32 [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers Rafał Miłecki 2021-01-06 7:32 ` [PATCH net-next 2/2] net: broadcom: share header defining " Rafał Miłecki 2021-01-06 19:26 ` Florian Fainelli 2021-01-06 19:45 ` Florian Fainelli 2021-01-06 20:37 ` Rafał Miłecki 2021-01-07 17:14 ` Florian Fainelli 2021-01-07 17:21 ` Jakub Kicinski 2021-01-06 19:28 ` [PATCH net-next 1/2] bgmac: add bgmac_umac_*() helpers for accessing " Florian Fainelli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).