[v2] ARM: ep93xx: Use HW MAC filter in ethernet driver

Message ID	1345406296.20237.4.camel@localhost (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org> Subject: [PATCH v2] ARM: ep93xx: Use HW MAC filter in ethernet driver From: Yan Burman <burman.yan@gmail.com> To: linux-arm-kernel@lists.infradead.org, hsweeten@visionengravers.com, rmallon@gmail.com Date: Sun, 19 Aug 2012 22:58:16 +0300 Message-Id: <1345406296.20237.4.camel@localhost> Mime-Version: 1.0 summary: Content analysis details: (1.1 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (burman.yan[at]gmail.com) -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.212.171 listed in list.dnswl.org] 1.0 DATE_IN_PAST_12_24 Date: is 12 to 24 hours before SPF: sender matches SPF record 2.8 FUZZY_XPILL BODY: Attempt to obfuscate words in spam -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Message ID

1345406296.20237.4.camel@localhost (mailing list archive)

State

New, archived

Headers

Subject: [PATCH v2] ARM: ep93xx: Use HW MAC filter in ethernet driver
From: Yan Burman <burman.yan@gmail.com>
To: linux-arm-kernel@lists.infradead.org, hsweeten@visionengravers.com,
	rmallon@gmail.com
Date: Sun, 19 Aug 2012 22:58:16 +0300
Message-Id: <1345406296.20237.4.camel@localhost>
Mime-Version: 1.0
Precedence: list
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: linux-arm-kernel-bounces@lists.infradead.org
Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Commit Message

Yan Burman Aug. 19, 2012, 7:58 p.m. UTC

ARM: ep93xx: Use HW MAC filter in ethernet driver
Use HW MAC filtering for broadcasts and multicasts.
Do not always work in promiscuous mode.
Partially based on kernel 2.6.20.4 driver from cirrus logic.
Tested on 9302 and 9315 based boards.

Changes from v1:
* Some minor style fixes
* Use CRC32 code instead of duplicating code
* Fixed multicast reception (tested with omping)

Signed-off-by: Yan Burman <burman.yan@gmail.com>

Comments

Ryan Mallon Aug. 20, 2012, 11:01 p.m. UTC | #1

On 20/08/12 05:58, Yan Burman wrote:
> ARM: ep93xx: Use HW MAC filter in ethernet driver
> Use HW MAC filtering for broadcasts and multicasts.
> Do not always work in promiscuous mode.
> Partially based on kernel 2.6.20.4 driver from cirrus logic.
> Tested on 9302 and 9315 based boards.
> 
> Changes from v1:
> * Some minor style fixes
> * Use CRC32 code instead of duplicating code
> * Fixed multicast reception (tested with omping)
> 
> Signed-off-by: Yan Burman <burman.yan@gmail.com>

Hi Yan,

Some very minor comments below. Otherwise looks fine. Repost with the
changes and I'll get this into my tree.

Thanks,
~Ryan

> 
> diff --git a/drivers/net/ethernet/cirrus/Kconfig b/drivers/net/ethernet/cirrus/Kconfig
> index 8388e36..b7b154f 100644
> --- a/drivers/net/ethernet/cirrus/Kconfig
> +++ b/drivers/net/ethernet/cirrus/Kconfig
> @@ -46,6 +46,7 @@ config EP93XX_ETH
>  	depends on ARM && ARCH_EP93XX
>  	select NET_CORE
>  	select MII
> +	select CRC32
>  	help
>  	  This is a driver for the ethernet hardware included in EP93xx CPUs.
>  	  Say Y if you are building a kernel for EP93xx based devices.
> diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
> index 78c5521..27299ce 100644
> --- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
> +++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
> @@ -25,6 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
> +#include <linux/crc32.h>
>  
>  #include <mach/hardware.h>
>  
> @@ -37,8 +38,32 @@
>  #define MAX_PKT_SIZE		2044
>  #define PKT_BUF_SIZE		2048
>  
> +#define HASH_TBL_ENTRIES	8
> +
>  #define REG_RXCTL		0x0000
> -#define  REG_RXCTL_DEFAULT	0x00073800
> +#define  REG_RXCTL_PauseA  (1 << 20)

Please use capitals only for register #defines.

> +#define  REG_RXCTL_RxFCE1  (1 << 19)
> +#define  REG_RXCTL_RxFCE0  (1 << 18)
> +#define  REG_RXCTL_BCRC    (1 << 17)
> +#define  REG_RXCTL_SRxON   (1 << 16)
> +#define  REG_RXCTL_RCRCA   (1 << 13)
> +#define  REG_RXCTL_RA      (1 << 12)
> +#define  REG_RXCTL_PA      (1 << 11)
> +#define  REG_RXCTL_BA      (1 << 10)
> +#define  REG_RXCTL_MA      (1 << 9)
> +#define  REG_RXCTL_IAHA    (1 << 8)
> +#define  REG_RXCTL_IA3     (1 << 3)
> +#define  REG_RXCTL_IA2     (1 << 2)
> +#define  REG_RXCTL_IA1     (1 << 1)
> +#define  REG_RXCTL_IA0     (1 << 0)
> +#define  REG_RXCTL_DEFAULT	(REG_RXCTL_BA | \
> +				REG_RXCTL_RA | \
> +				REG_RXCTL_RCRCA | \
> +				REG_RXCTL_SRxON | \
> +				REG_RXCTL_BCRC | \
> +				REG_RXCTL_RxFCE0 | \
> +				REG_RXCTL_IA0)
> +
>  #define REG_TXCTL		0x0004
>  #define  REG_TXCTL_ENABLE	0x00000001
>  #define REG_MIICMD		0x0010
> @@ -57,6 +82,12 @@
>  #define  REG_INTSTS_RX		0x00000004
>  #define REG_INTSTSC		0x002c
>  #define REG_AFP			0x004c
> +#define  REG_AFP_IA0		0
> +#define  REG_AFP_IA1		1
> +#define  REG_AFP_IA2		2
> +#define  REG_AFP_IA3		3
> +#define  REG_AFP_DTxP		6
> +#define  REG_AFP_HASH		7
>  #define REG_INDAD0		0x0050
>  #define REG_INDAD1		0x0051
>  #define REG_INDAD2		0x0052
> @@ -386,6 +417,51 @@ static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
>  	return NETDEV_TX_OK;
>  }
>  
> +static void ep93xx_set_mcast_tbl(struct net_device *dev, u8 *buf)
> +{
> +	struct netdev_hw_addr *ha;
> +	u32 crc;
> +
> +	netdev_for_each_mc_addr(ha, dev) {
> +		crc = ether_crc_le(ETH_ALEN, ha->addr);
> +		buf[crc >> 29] |= (1 << ((crc >> 26) & 7));
> +	}
> +}
> +
> +static int ep93xx_ind_addr_write(struct net_device *dev, int afp, char *buf)
> +{
> +	u32 rxctl;
> +	unsigned int i, len;
> +	struct ep93xx_priv *ep = netdev_priv(dev);
> +
> +	switch (afp) {
> +	case REG_AFP_IA0:
> +	case REG_AFP_IA1:
> +	case REG_AFP_IA2:
> +	case REG_AFP_IA3:
> +	case REG_AFP_DTxP:
> +		len = ETH_ALEN;
> +		break;
> +
> +	case REG_AFP_HASH:
> +		len = HASH_TBL_ENTRIES;
> +		break;
> +
> +	default:
> +		pr_crit("invalid afp value: %d\n", afp);
> +		return -1;

errno code please. Also, the return value of this function is never
checked by its callers. Should it be?

> +	}
> +
> +	rxctl = rdl(ep, REG_RXCTL);
> +	wrl(ep, REG_RXCTL, ~REG_RXCTL_SRxON & rxctl);

Nitpick - I think maybe it is more readable the other way round?

	wrl(ep, REG_RXCTL, rxctl & ~REG_RXCTL_SRXON);

> +	wrl(ep, REG_AFP, afp);
> +	for (i = 0; i < len; i++)
> +		wrb(ep, REG_INDAD0 + i, buf[i]);
> +	wrl(ep, REG_RXCTL, rxctl);
> +
> +	return 0;
> +}
> +
>  static void ep93xx_tx_complete(struct net_device *dev)
>  {
>  	struct ep93xx_priv *ep = netdev_priv(dev);
> @@ -615,13 +691,7 @@ static int ep93xx_start_hw(struct net_device *dev)
>  	wrl(ep, REG_RXDENQ, RX_QUEUE_ENTRIES);
>  	wrl(ep, REG_RXSTSENQ, RX_QUEUE_ENTRIES);
>  
> -	wrb(ep, REG_INDAD0, dev->dev_addr[0]);
> -	wrb(ep, REG_INDAD1, dev->dev_addr[1]);
> -	wrb(ep, REG_INDAD2, dev->dev_addr[2]);
> -	wrb(ep, REG_INDAD3, dev->dev_addr[3]);
> -	wrb(ep, REG_INDAD4, dev->dev_addr[4]);
> -	wrb(ep, REG_INDAD5, dev->dev_addr[5]);
> -	wrl(ep, REG_AFP, 0);
> +	ep93xx_ind_addr_write(dev, REG_AFP_IA0, dev->dev_addr);
>  
>  	wrl(ep, REG_MAXFRMLEN, (MAX_PKT_SIZE << 16) | MAX_PKT_SIZE);
>  
> @@ -647,6 +717,32 @@ static void ep93xx_stop_hw(struct net_device *dev)
>  		pr_crit("hw failed to reset\n");
>  }
>  
> +static void ep93xx_set_multicast_list(struct net_device *dev)
> +{
> +	struct ep93xx_priv *ep = netdev_priv(dev);
> +
> +	if (dev->flags & IFF_PROMISC) {
> +		wrl(ep, REG_RXCTL, REG_RXCTL_PA | rdl(ep, REG_RXCTL));
> +	} else if (dev->flags & IFF_ALLMULTI) {
> +		wrl(ep, REG_RXCTL, REG_RXCTL_MA |
> +			(~REG_RXCTL_PA & rdl(ep, REG_RXCTL)));
> +		ep93xx_ind_addr_write(dev, REG_AFP_HASH,
> +			      "\xff\xff\xff\xff\xff\xff\xff\xff");
> +	} else if (!netdev_mc_empty(dev)) {
> +		u8 tblMulti[HASH_TBL_ENTRIES + 1];
> +
> +		memset(tblMulti, 0, sizeof(tblMulti));

Nitpick - this can be written as:

	u8 tbl_multi[HASH_TBL_ENTRIES + 1] = {0};

Use underscores in variable names, no camel-case please.

> +
> +		ep93xx_set_mcast_tbl(dev, tblMulti);
> +		wrl(ep, REG_RXCTL, REG_RXCTL_MA |
> +			(~REG_RXCTL_PA & rdl(ep, REG_RXCTL)));
> +		ep93xx_ind_addr_write(dev, REG_AFP_HASH, tblMulti);
> +	} else {
> +		wrl(ep, REG_RXCTL,
> +			~(REG_RXCTL_PA | REG_RXCTL_MA) & rdl(ep, REG_RXCTL));
> +	}
> +}
> +
>  static int ep93xx_open(struct net_device *dev)
>  {
>  	struct ep93xx_priv *ep = netdev_priv(dev);
> @@ -754,6 +850,7 @@ static const struct net_device_ops ep93xx_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  	.ndo_change_mtu		= eth_change_mtu,
>  	.ndo_set_mac_address	= eth_mac_addr,
> +	.ndo_set_rx_mode	= ep93xx_set_multicast_list,
>  };
>  
>  static struct net_device *ep93xx_dev_alloc(struct ep93xx_eth_data *data)
> 
>

diff --git a/drivers/net/ethernet/cirrus/Kconfig b/drivers/net/ethernet/cirrus/Kconfig
index 8388e36..b7b154f 100644
--- a/drivers/net/ethernet/cirrus/Kconfig
+++ b/drivers/net/ethernet/cirrus/Kconfig
@@ -46,6 +46,7 @@  config EP93XX_ETH
 	depends on ARM && ARCH_EP93XX
 	select NET_CORE
 	select MII
+	select CRC32
 	help
 	  This is a driver for the ethernet hardware included in EP93xx CPUs.
 	  Say Y if you are building a kernel for EP93xx based devices.
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index 78c5521..27299ce 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -25,6 +25,7 @@ 
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/crc32.h>
 
 #include <mach/hardware.h>
 
@@ -37,8 +38,32 @@ 
 #define MAX_PKT_SIZE		2044
 #define PKT_BUF_SIZE		2048
 
+#define HASH_TBL_ENTRIES	8
+
 #define REG_RXCTL		0x0000
-#define  REG_RXCTL_DEFAULT	0x00073800
+#define  REG_RXCTL_PauseA  (1 << 20)
+#define  REG_RXCTL_RxFCE1  (1 << 19)
+#define  REG_RXCTL_RxFCE0  (1 << 18)
+#define  REG_RXCTL_BCRC    (1 << 17)
+#define  REG_RXCTL_SRxON   (1 << 16)
+#define  REG_RXCTL_RCRCA   (1 << 13)
+#define  REG_RXCTL_RA      (1 << 12)
+#define  REG_RXCTL_PA      (1 << 11)
+#define  REG_RXCTL_BA      (1 << 10)
+#define  REG_RXCTL_MA      (1 << 9)
+#define  REG_RXCTL_IAHA    (1 << 8)
+#define  REG_RXCTL_IA3     (1 << 3)
+#define  REG_RXCTL_IA2     (1 << 2)
+#define  REG_RXCTL_IA1     (1 << 1)
+#define  REG_RXCTL_IA0     (1 << 0)
+#define  REG_RXCTL_DEFAULT	(REG_RXCTL_BA | \
+				REG_RXCTL_RA | \
+				REG_RXCTL_RCRCA | \
+				REG_RXCTL_SRxON | \
+				REG_RXCTL_BCRC | \
+				REG_RXCTL_RxFCE0 | \
+				REG_RXCTL_IA0)
+
 #define REG_TXCTL		0x0004
 #define  REG_TXCTL_ENABLE	0x00000001
 #define REG_MIICMD		0x0010
@@ -57,6 +82,12 @@ 
 #define  REG_INTSTS_RX		0x00000004
 #define REG_INTSTSC		0x002c
 #define REG_AFP			0x004c
+#define  REG_AFP_IA0		0
+#define  REG_AFP_IA1		1
+#define  REG_AFP_IA2		2
+#define  REG_AFP_IA3		3
+#define  REG_AFP_DTxP		6
+#define  REG_AFP_HASH		7
 #define REG_INDAD0		0x0050
 #define REG_INDAD1		0x0051
 #define REG_INDAD2		0x0052
@@ -386,6 +417,51 @@  static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static void ep93xx_set_mcast_tbl(struct net_device *dev, u8 *buf)
+{
+	struct netdev_hw_addr *ha;
+	u32 crc;
+
+	netdev_for_each_mc_addr(ha, dev) {
+		crc = ether_crc_le(ETH_ALEN, ha->addr);
+		buf[crc >> 29] |= (1 << ((crc >> 26) & 7));
+	}
+}
+
+static int ep93xx_ind_addr_write(struct net_device *dev, int afp, char *buf)
+{
+	u32 rxctl;
+	unsigned int i, len;
+	struct ep93xx_priv *ep = netdev_priv(dev);
+
+	switch (afp) {
+	case REG_AFP_IA0:
+	case REG_AFP_IA1:
+	case REG_AFP_IA2:
+	case REG_AFP_IA3:
+	case REG_AFP_DTxP:
+		len = ETH_ALEN;
+		break;
+
+	case REG_AFP_HASH:
+		len = HASH_TBL_ENTRIES;
+		break;
+
+	default:
+		pr_crit("invalid afp value: %d\n", afp);
+		return -1;
+	}
+
+	rxctl = rdl(ep, REG_RXCTL);
+	wrl(ep, REG_RXCTL, ~REG_RXCTL_SRxON & rxctl);
+	wrl(ep, REG_AFP, afp);
+	for (i = 0; i < len; i++)
+		wrb(ep, REG_INDAD0 + i, buf[i]);
+	wrl(ep, REG_RXCTL, rxctl);
+
+	return 0;
+}
+
 static void ep93xx_tx_complete(struct net_device *dev)
 {
 	struct ep93xx_priv *ep = netdev_priv(dev);
@@ -615,13 +691,7 @@  static int ep93xx_start_hw(struct net_device *dev)
 	wrl(ep, REG_RXDENQ, RX_QUEUE_ENTRIES);
 	wrl(ep, REG_RXSTSENQ, RX_QUEUE_ENTRIES);
 
-	wrb(ep, REG_INDAD0, dev->dev_addr[0]);
-	wrb(ep, REG_INDAD1, dev->dev_addr[1]);
-	wrb(ep, REG_INDAD2, dev->dev_addr[2]);
-	wrb(ep, REG_INDAD3, dev->dev_addr[3]);
-	wrb(ep, REG_INDAD4, dev->dev_addr[4]);
-	wrb(ep, REG_INDAD5, dev->dev_addr[5]);
-	wrl(ep, REG_AFP, 0);
+	ep93xx_ind_addr_write(dev, REG_AFP_IA0, dev->dev_addr);
 
 	wrl(ep, REG_MAXFRMLEN, (MAX_PKT_SIZE << 16) | MAX_PKT_SIZE);
 
@@ -647,6 +717,32 @@  static void ep93xx_stop_hw(struct net_device *dev)
 		pr_crit("hw failed to reset\n");
 }
 
+static void ep93xx_set_multicast_list(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+
+	if (dev->flags & IFF_PROMISC) {
+		wrl(ep, REG_RXCTL, REG_RXCTL_PA | rdl(ep, REG_RXCTL));
+	} else if (dev->flags & IFF_ALLMULTI) {
+		wrl(ep, REG_RXCTL, REG_RXCTL_MA |
+			(~REG_RXCTL_PA & rdl(ep, REG_RXCTL)));
+		ep93xx_ind_addr_write(dev, REG_AFP_HASH,
+			      "\xff\xff\xff\xff\xff\xff\xff\xff");
+	} else if (!netdev_mc_empty(dev)) {
+		u8 tblMulti[HASH_TBL_ENTRIES + 1];
+
+		memset(tblMulti, 0, sizeof(tblMulti));
+
+		ep93xx_set_mcast_tbl(dev, tblMulti);
+		wrl(ep, REG_RXCTL, REG_RXCTL_MA |
+			(~REG_RXCTL_PA & rdl(ep, REG_RXCTL)));
+		ep93xx_ind_addr_write(dev, REG_AFP_HASH, tblMulti);
+	} else {
+		wrl(ep, REG_RXCTL,
+			~(REG_RXCTL_PA | REG_RXCTL_MA) & rdl(ep, REG_RXCTL));
+	}
+}
+
 static int ep93xx_open(struct net_device *dev)
 {
 	struct ep93xx_priv *ep = netdev_priv(dev);
@@ -754,6 +850,7 @@  static const struct net_device_ops ep93xx_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_change_mtu		= eth_change_mtu,
 	.ndo_set_mac_address	= eth_mac_addr,
+	.ndo_set_rx_mode	= ep93xx_set_multicast_list,
 };
 
 static struct net_device *ep93xx_dev_alloc(struct ep93xx_eth_data *data)

[v2] ARM: ep93xx: Use HW MAC filter in ethernet driver

Commit Message

Comments

Patch