[PATCH 0/4] bgmac: fixes and more netdev ops

[PATCH 0/4] bgmac: fixes and more netdev ops

[Hauke Mehrtens]

The first patch implements a TODO in the driver.
The other patches are fixing or adding some netdev ops.

This is based on current master of davem/net-next.git.

Hauke Mehrtens (4):
  bgmac: implement missing code for BCM53572
  bgmac: write mac address to hardware in ndo_set_mac_address
  bgmac: add generic ndo_validate_addr netdev ops
  bgmac: add ndo_set_rx_mode netdev ops

 drivers/net/ethernet/broadcom/bgmac.c |   56 ++++++++++++++++++++++++---------
 drivers/net/ethernet/broadcom/bgmac.h |    2 +-
 2 files changed, 43 insertions(+), 15 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] bgmac: implement missing code for BCM53572

[Hauke Mehrtens]

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |    6 ++++--
 drivers/net/ethernet/broadcom/bgmac.h |    2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 9bd33db..58e0c70 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -889,8 +889,10 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
 			sw_type = et_swtype;
 		} else if (ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == 9) {
 			sw_type = BGMAC_CHIPCTL_1_SW_TYPE_EPHYRMII;
-		} else if (0) {
-			/* TODO */
+		} else if ((ci->id != BCMA_CHIP_ID_BCM53572 && ci->pkg == 10) ||
+			   (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == 9)) {
+			sw_type = BGMAC_CHIPCTL_1_IF_TYPE_RGMII |
+				  BGMAC_CHIPCTL_1_SW_TYPE_RGMII;
 		}
 		bcma_chipco_chipctl_maskset(cc, 1,
 					    ~(BGMAC_CHIPCTL_1_IF_TYPE_MASK |
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 1299470..3dd6f6f 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -339,7 +339,7 @@
 #define BGMAC_CHIPCTL_1_SW_TYPE_EPHY		0x00000000
 #define BGMAC_CHIPCTL_1_SW_TYPE_EPHYMII		0x00000040
 #define BGMAC_CHIPCTL_1_SW_TYPE_EPHYRMII	0x00000080
-#define BGMAC_CHIPCTL_1_SW_TYPE_RGMI		0x000000C0
+#define BGMAC_CHIPCTL_1_SW_TYPE_RGMII		0x000000C0
 #define BGMAC_CHIPCTL_1_RXC_DLL_BYPASS		0x00010000
 
 #define BGMAC_SPEED_10				0x0001
-- 
1.7.10.4

[PATCH 2/4] bgmac: write mac address to hardware in ndo_set_mac_address

[Hauke Mehrtens]

The generic implementation just changes the netdev struct and does not
write the new mac address to the hardware or issues some command to do
so.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 58e0c70..c0152396 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -761,6 +761,16 @@ static void bgmac_cmdcfg_maskset(struct bgmac *bgmac, u32 mask, u32 set,
 	udelay(2);
 }
 
+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);
+	tmp = (addr[4] << 8) | addr[5];
+	bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp);
+}
+
 #if 0 /* We don't use that regs yet */
 static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
@@ -1006,8 +1016,6 @@ static void bgmac_enable(struct bgmac *bgmac)
 static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
 {
 	struct bgmac_dma_ring *ring;
-	u8 *mac = bgmac->net_dev->dev_addr;
-	u32 tmp;
 	int i;
 
 	/* 1 interrupt per received frame */
@@ -1021,11 +1029,7 @@ static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
 	else
 		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
 
-	/* Set MAC addr */
-	tmp = (mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3];
-	bgmac_write(bgmac, BGMAC_MACADDR_HIGH, tmp);
-	tmp = (mac[4] << 8) | mac[5];
-	bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp);
+	bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr);
 
 	if (bgmac->loopback)
 		bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_ML, true);
@@ -1162,6 +1166,19 @@ static netdev_tx_t bgmac_start_xmit(struct sk_buff *skb,
 	return bgmac_dma_tx_add(bgmac, ring, skb);
 }
 
+static int bgmac_set_mac_address(struct net_device *net_dev, void *addr)
+{
+	struct bgmac *bgmac = netdev_priv(net_dev);
+	int ret;
+
+	ret = eth_prepare_mac_addr_change(net_dev, addr);
+	if (ret < 0)
+		return ret;
+	bgmac_write_mac_address(bgmac, (u8 *)addr);
+	eth_commit_mac_addr_change(net_dev, addr);
+	return 0;
+}
+
 static int bgmac_ioctl(struct net_device *net_dev, struct ifreq *ifr, int cmd)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
@@ -1192,7 +1209,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
 	.ndo_open		= bgmac_open,
 	.ndo_stop		= bgmac_stop,
 	.ndo_start_xmit		= bgmac_start_xmit,
-	.ndo_set_mac_address	= eth_mac_addr, /* generic, sets dev_addr */
+	.ndo_set_mac_address	= bgmac_set_mac_address,
 	.ndo_do_ioctl           = bgmac_ioctl,
 };
 
-- 
1.7.10.4

[PATCH 3/4] bgmac: add generic ndo_validate_addr netdev ops

[Hauke Mehrtens]

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index c0152396..e0775c4 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1210,6 +1210,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
 	.ndo_stop		= bgmac_stop,
 	.ndo_start_xmit		= bgmac_start_xmit,
 	.ndo_set_mac_address	= bgmac_set_mac_address,
+	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl           = bgmac_ioctl,
 };
 
-- 
1.7.10.4

[PATCH 4/4] bgmac: add ndo_set_rx_mode netdev ops

[Hauke Mehrtens]

Currently it is only possible to activate or deactivate promisc mode
while the device is up.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index e0775c4..23c8d4a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -771,6 +771,16 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr)
 	bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp);
 }
 
+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, false);
+	else
+		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
+}
+
 #if 0 /* We don't use that regs yet */
 static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
@@ -1024,10 +1034,7 @@ static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
 	/* Enable 802.3x tx flow control (honor received PAUSE frames) */
 	bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true);
 
-	if (bgmac->net_dev->flags & IFF_PROMISC)
-		bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, false);
-	else
-		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
+	bgmac_set_rx_mode(bgmac->net_dev);
 
 	bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr);
 
@@ -1209,6 +1216,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
 	.ndo_open		= bgmac_open,
 	.ndo_stop		= bgmac_stop,
 	.ndo_start_xmit		= bgmac_start_xmit,
+	.ndo_set_rx_mode	= bgmac_set_rx_mode,
 	.ndo_set_mac_address	= bgmac_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl           = bgmac_ioctl,
-- 
1.7.10.4

Re: [PATCH 1/4] bgmac: implement missing code for BCM53572

[Rafał Miłecki]

2013/2/6 Hauke Mehrtens <hauke@hauke-m.de>:
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/net/ethernet/broadcom/bgmac.c |    6 ++++--
>  drivers/net/ethernet/broadcom/bgmac.h |    2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 9bd33db..58e0c70 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -889,8 +889,10 @@ static void bgmac_chip_reset(struct bgmac *bgmac)
>                         sw_type = et_swtype;
>                 } else if (ci->id == BCMA_CHIP_ID_BCM5357 && ci->pkg == 9) {
>                         sw_type = BGMAC_CHIPCTL_1_SW_TYPE_EPHYRMII;
> -               } else if (0) {
> -                       /* TODO */
> +               } else if ((ci->id != BCMA_CHIP_ID_BCM53572 && ci->pkg == 10) ||
> +                          (ci->id == BCMA_CHIP_ID_BCM53572 && ci->pkg == 9)) {
> +                       sw_type = BGMAC_CHIPCTL_1_IF_TYPE_RGMII |
> +                                 BGMAC_CHIPCTL_1_SW_TYPE_RGMII;

Check "chippkg is 10" looked too generous for me, that's why I didn't
implement this part. "ci->id != BCMA_CHIP_ID_BCM53572" doesn't make it
much better, but OK, if that's the way it is... ;)

Could you update specs?
http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipreset

-- 
Rafał

Re: [PATCH 2/4] bgmac: write mac address to hardware in ndo_set_mac_address

[Rafał Miłecki]

2013/2/6 Hauke Mehrtens <hauke@hauke-m.de>:
> The generic implementation just changes the netdev struct and does not
> write the new mac address to the hardware or issues some command to do
> so.

That's because generic "eth_mac_addr" allows changing MAC only when
interface is down. And we "upload" MAC address during bringing
interface up. So there wasn't any bug, it was just working that way :)

One question: how you tested if changing MAC with keeping interface up
works correctly? I just don't know if simple writes to
BGMAC_MACADDR_HIGH and BGMAC_MACADDR_LOW are enough.

If it works for you (changing MAC without stopping interface), I'm OK with that.

-- 
Rafał

Re: [PATCH 4/4] bgmac: add ndo_set_rx_mode netdev ops

[Florian Fainelli]

Hello Hauke,

On 02/06/2013 03:45 PM, Hauke Mehrtens wrote:
> Currently it is only possible to activate or deactivate promisc mode
> while the device is up.

This is required for bridging to work, so it is definitively a good fix 
to have.

>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>   drivers/net/ethernet/broadcom/bgmac.c |   16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index e0775c4..23c8d4a 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -771,6 +771,16 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr)
>   	bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp);
>   }
>
> +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, false);
> +	else
> +		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
> +}
> +
>   #if 0 /* We don't use that regs yet */
>   static void bgmac_chip_stats_update(struct bgmac *bgmac)
>   {
> @@ -1024,10 +1034,7 @@ static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
>   	/* Enable 802.3x tx flow control (honor received PAUSE frames) */
>   	bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true);
>
> -	if (bgmac->net_dev->flags & IFF_PROMISC)
> -		bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, false);
> -	else
> -		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
> +	bgmac_set_rx_mode(bgmac->net_dev);
>
>   	bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr);
>
> @@ -1209,6 +1216,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
>   	.ndo_open		= bgmac_open,
>   	.ndo_stop		= bgmac_stop,
>   	.ndo_start_xmit		= bgmac_start_xmit,
> +	.ndo_set_rx_mode	= bgmac_set_rx_mode,
>   	.ndo_set_mac_address	= bgmac_set_mac_address,
>   	.ndo_validate_addr	= eth_validate_addr,
>   	.ndo_do_ioctl           = bgmac_ioctl,
>

Re: [PATCH 2/4] bgmac: write mac address to hardware in ndo_set_mac_address

[Hauke Mehrtens]

On 02/06/2013 03:57 PM, Rafał Miłecki wrote:
> 2013/2/6 Hauke Mehrtens <hauke@hauke-m.de>:
>> The generic implementation just changes the netdev struct and does not
>> write the new mac address to the hardware or issues some command to do
>> so.
> 
> That's because generic "eth_mac_addr" allows changing MAC only when
> interface is down. And we "upload" MAC address during bringing
> interface up. So there wasn't any bug, it was just working that way :)
> 
> One question: how you tested if changing MAC with keeping interface up
> works correctly? I just don't know if simple writes to
> BGMAC_MACADDR_HIGH and BGMAC_MACADDR_LOW are enough.
> 
> If it works for you (changing MAC without stopping interface), I'm OK with that.
> 
Yes that works in promisc and non promisc mode, I did this while pinging
the device and I had no package lose.

Hauke

[PATCH v2 4/4] bgmac: add ndo_set_rx_mode netdev ops

[Hauke Mehrtens]

When changing the device from or to promisc mode this only affects the
device after the device is bought up the next time. For bridging it is
needed to change the device to promisc mode while it is up, which is
possible with this patch.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---

v2:
  * Improved comit message

 drivers/net/ethernet/broadcom/bgmac.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index e0775c4..23c8d4a 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -771,6 +771,16 @@ static void bgmac_write_mac_address(struct bgmac *bgmac, u8 *addr)
 	bgmac_write(bgmac, BGMAC_MACADDR_LOW, tmp);
 }
 
+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, false);
+	else
+		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
+}
+
 #if 0 /* We don't use that regs yet */
 static void bgmac_chip_stats_update(struct bgmac *bgmac)
 {
@@ -1024,10 +1034,7 @@ static void bgmac_chip_init(struct bgmac *bgmac, bool full_init)
 	/* Enable 802.3x tx flow control (honor received PAUSE frames) */
 	bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_RPI, 0, true);
 
-	if (bgmac->net_dev->flags & IFF_PROMISC)
-		bgmac_cmdcfg_maskset(bgmac, ~0, BGMAC_CMDCFG_PROM, false);
-	else
-		bgmac_cmdcfg_maskset(bgmac, ~BGMAC_CMDCFG_PROM, 0, false);
+	bgmac_set_rx_mode(bgmac->net_dev);
 
 	bgmac_write_mac_address(bgmac, bgmac->net_dev->dev_addr);
 
@@ -1209,6 +1216,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
 	.ndo_open		= bgmac_open,
 	.ndo_stop		= bgmac_stop,
 	.ndo_start_xmit		= bgmac_start_xmit,
+	.ndo_set_rx_mode	= bgmac_set_rx_mode,
 	.ndo_set_mac_address	= bgmac_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl           = bgmac_ioctl,
-- 
1.7.10.4

Re: [PATCH 0/4] bgmac: fixes and more netdev ops

[David Miller]

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Wed,  6 Feb 2013 15:44:56 +0100

> The first patch implements a TODO in the driver.
> The other patches are fixing or adding some netdev ops.
> 
> This is based on current master of davem/net-next.git.
> 
> Hauke Mehrtens (4):
>   bgmac: implement missing code for BCM53572
>   bgmac: write mac address to hardware in ndo_set_mac_address
>   bgmac: add generic ndo_validate_addr netdev ops
>   bgmac: add ndo_set_rx_mode netdev ops

Applied with updated version of patch #4.