diff mbox series

[net-next,1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers

Message ID 20210106073245.32597-1-zajec5@gmail.com
State New
Headers show
Series [net-next,1/2] bgmac: add bgmac_umac_*() helpers for accessing UniMAC registers | expand

Commit Message

Rafał Miłecki Jan. 6, 2021, 7:32 a.m. UTC
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(-)

Comments

Florian Fainelli Jan. 6, 2021, 7:26 p.m. UTC | #1
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 Fainelli Jan. 6, 2021, 7:28 p.m. UTC | #2
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 Fainelli Jan. 6, 2021, 7:45 p.m. UTC | #3
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 Fainelli Jan. 7, 2021, 5:14 p.m. UTC | #4
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
Jakub Kicinski Jan. 7, 2021, 5:21 p.m. UTC | #5
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.
diff mbox series

Patch

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);