[PATCH] gianfar: rx parser

[PATCH] gianfar: rx parser

[Sebastian Pöhn]

Only let the rx parser be enabled if it is necessary (if VLAN extracrtion, IP or TCP checksumming or the rx queue filer are enabled). Otherwise disable it.

The new routine gfar_check_rx_parser_mode should be runt after every change on this features and will enable/disable the parser as necessary.

I will send a patch for the rx queue filer the next days.



Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
---

 drivers/net/gianfar.c |   25 ++++++++++++++++++++-----
 drivers/net/gianfar.h |    3 ++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 3321d71..6da3712 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2287,6 +2287,23 @@ static int gfar_set_mac_address(struct net_device *dev)
 	return 0;
 }
 
+/* Check if rx parser should be activated */
+void gfar_check_rx_parser_mode(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs = NULL;
+	u32 tempval = 0;
+	
+	regs = priv->gfargrp[0].regs;
+	
+	tempval = gfar_read(&regs->rctrl);
+	/* If parse is no longer required, then disable parser */
+	if (tempval & RCTRL_REQ_PARSER)
+		tempval |= RCTRL_PRSDEP_INIT;
+	else
+		tempval &= ~RCTRL_PRSDEP_INIT;
+	gfar_write(&regs->rctrl, tempval);
+}
+
 
 /* Enables and disables VLAN insertion/extraction */
 static void gfar_vlan_rx_register(struct net_device *dev,
@@ -2323,12 +2340,10 @@ static void gfar_vlan_rx_register(struct net_device *dev,
 		/* Disable VLAN tag extraction */
 		tempval = gfar_read(&regs->rctrl);
 		tempval &= ~RCTRL_VLEX;
-		/* If parse is no longer required, then disable parser */
-		if (tempval & RCTRL_REQ_PARSER)
-			tempval |= RCTRL_PRSDEP_INIT;
-		else
-			tempval &= ~RCTRL_PRSDEP_INIT;
 		gfar_write(&regs->rctrl, tempval);
+
+		gfar_check_rx_parser_mode(priv);
+		
 	}
 
 	gfar_change_mtu(dev, dev->mtu);
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index 27499c6..87c1d86 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -286,7 +286,7 @@ extern const char gfar_driver_version[];
 #define RCTRL_PROM		0x00000008
 #define RCTRL_EMEN		0x00000002
 #define RCTRL_REQ_PARSER	(RCTRL_VLEX | RCTRL_IPCSEN | \
-				 RCTRL_TUCSEN)
+				 RCTRL_TUCSEN | RCTRL_FILREN)
 #define RCTRL_CHECKSUMMING	(RCTRL_IPCSEN | RCTRL_TUCSEN | \
 				RCTRL_PRSDEP_INIT)
 #define RCTRL_EXTHASH		(RCTRL_GHTX)
@@ -1182,6 +1182,7 @@ extern void gfar_configure_coalescing(struct gfar_private *priv,
 		unsigned long tx_mask, unsigned long rx_mask);
 void gfar_init_sysfs(struct net_device *dev);
 int gfar_set_features(struct net_device *dev, u32 features);
+extern void gfar_check_rx_parser_mode(struct gfar_private *priv);
 
 extern const struct ethtool_ops gfar_ethtool_ops;
 

 

Re: [PATCH] gianfar: rx parser

[Jiri Pirko]

Fri, Jul 15, 2011 at 01:09:01PM CEST, sebastian.belden@googlemail.com wrote:
>Only let the rx parser be enabled if it is necessary (if VLAN extracrtion, IP or TCP checksumming or the rx queue filer are enabled). Otherwise disable it.
>
>The new routine gfar_check_rx_parser_mode should be runt after every change on this features and will enable/disable the parser as necessary.
>
>I will send a patch for the rx queue filer the next days.

Looks good to me.

Reviewed-by: Jiri Pirko <jpirko@redhat.com>


>
>
>
>Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
>---
>
> drivers/net/gianfar.c |   25 ++++++++++++++++++++-----
> drivers/net/gianfar.h |    3 ++-
> 2 files changed, 22 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>index 3321d71..6da3712 100644
>--- a/drivers/net/gianfar.c
>+++ b/drivers/net/gianfar.c
>@@ -2287,6 +2287,23 @@ static int gfar_set_mac_address(struct net_device *dev)
> 	return 0;
> }
> 
>+/* Check if rx parser should be activated */
>+void gfar_check_rx_parser_mode(struct gfar_private *priv)
>+{
>+	struct gfar __iomem *regs = NULL;
>+	u32 tempval = 0;
>+	
>+	regs = priv->gfargrp[0].regs;
>+	
>+	tempval = gfar_read(&regs->rctrl);
>+	/* If parse is no longer required, then disable parser */
>+	if (tempval & RCTRL_REQ_PARSER)
>+		tempval |= RCTRL_PRSDEP_INIT;
>+	else
>+		tempval &= ~RCTRL_PRSDEP_INIT;
>+	gfar_write(&regs->rctrl, tempval);
>+}
>+
> 
> /* Enables and disables VLAN insertion/extraction */
> static void gfar_vlan_rx_register(struct net_device *dev,
>@@ -2323,12 +2340,10 @@ static void gfar_vlan_rx_register(struct net_device *dev,
> 		/* Disable VLAN tag extraction */
> 		tempval = gfar_read(&regs->rctrl);
> 		tempval &= ~RCTRL_VLEX;
>-		/* If parse is no longer required, then disable parser */
>-		if (tempval & RCTRL_REQ_PARSER)
>-			tempval |= RCTRL_PRSDEP_INIT;
>-		else
>-			tempval &= ~RCTRL_PRSDEP_INIT;
> 		gfar_write(&regs->rctrl, tempval);
>+
>+		gfar_check_rx_parser_mode(priv);
>+		
> 	}
> 
> 	gfar_change_mtu(dev, dev->mtu);
>diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
>index 27499c6..87c1d86 100644
>--- a/drivers/net/gianfar.h
>+++ b/drivers/net/gianfar.h
>@@ -286,7 +286,7 @@ extern const char gfar_driver_version[];
> #define RCTRL_PROM		0x00000008
> #define RCTRL_EMEN		0x00000002
> #define RCTRL_REQ_PARSER	(RCTRL_VLEX | RCTRL_IPCSEN | \
>-				 RCTRL_TUCSEN)
>+				 RCTRL_TUCSEN | RCTRL_FILREN)
> #define RCTRL_CHECKSUMMING	(RCTRL_IPCSEN | RCTRL_TUCSEN | \
> 				RCTRL_PRSDEP_INIT)
> #define RCTRL_EXTHASH		(RCTRL_GHTX)
>@@ -1182,6 +1182,7 @@ extern void gfar_configure_coalescing(struct gfar_private *priv,
> 		unsigned long tx_mask, unsigned long rx_mask);
> void gfar_init_sysfs(struct net_device *dev);
> int gfar_set_features(struct net_device *dev, u32 features);
>+extern void gfar_check_rx_parser_mode(struct gfar_private *priv);
> 
> extern const struct ethtool_ops gfar_ethtool_ops;
> 
>
> 
>
>

Re: [PATCH] gianfar: rx parser

[David Miller]

From: "Sebastian Pöhn" <sebastian.belden@googlemail.com>
Date: Fri, 15 Jul 2011 13:09:01 +0200

> +	struct gfar __iomem *regs = NULL;
> +	u32 tempval = 0;
> +	
> +	regs = priv->gfargrp[0].regs;

There is no reason to initialize the variable 'regs' to NULL only to
initialize it to something else 2 lines later.

The initial 'tempval' initialization is also redundant, for the same
reason.

Re: [PATCH] gianfar: rx parser

[Jiri Pirko]

Fri, Jul 15, 2011 at 05:06:10PM CEST, davem@davemloft.net wrote:
>From: "Sebastian Pöhn" <sebastian.belden@googlemail.com>
>Date: Fri, 15 Jul 2011 13:09:01 +0200
>
>> +	struct gfar __iomem *regs = NULL;
>> +	u32 tempval = 0;
>> +	
>> +	regs = priv->gfargrp[0].regs;
>
>There is no reason to initialize the variable 'regs' to NULL only to
>initialize it to something else 2 lines later.
>
>The initial 'tempval' initialization is also redundant, for the same
>reason.


fixed:

From: Sebastian Pöhn <sebastian.belden@googlemail.com>
Subject: [patch net-next-2.6] gianfar: rx parser

Only let the rx parser be enabled if it is necessary (if VLAN extraction,
IP or TCP checksumming or the rx queue filer are enabled). Otherwise
disable it.

The new routine gfar_check_rx_parser_mode should be run after every
change on this features and will enable/disable the parser as necessary.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
---
 drivers/net/gianfar.c |   24 +++++++++++++++++++-----
 drivers/net/gianfar.h |    3 ++-
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 3321d71..d265c6e 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2287,6 +2287,23 @@ static int gfar_set_mac_address(struct net_device *dev)
 	return 0;
 }
 
+/* Check if rx parser should be activated */
+void gfar_check_rx_parser_mode(struct gfar_private *priv)
+{
+	struct gfar __iomem *regs;
+	u32 tempval;
+
+	regs = priv->gfargrp[0].regs;
+
+	tempval = gfar_read(&regs->rctrl);
+	/* If parse is no longer required, then disable parser */
+	if (tempval & RCTRL_REQ_PARSER)
+		tempval |= RCTRL_PRSDEP_INIT;
+	else
+		tempval &= ~RCTRL_PRSDEP_INIT;
+	gfar_write(&regs->rctrl, tempval);
+}
+
 
 /* Enables and disables VLAN insertion/extraction */
 static void gfar_vlan_rx_register(struct net_device *dev,
@@ -2323,12 +2340,9 @@ static void gfar_vlan_rx_register(struct net_device *dev,
 		/* Disable VLAN tag extraction */
 		tempval = gfar_read(&regs->rctrl);
 		tempval &= ~RCTRL_VLEX;
-		/* If parse is no longer required, then disable parser */
-		if (tempval & RCTRL_REQ_PARSER)
-			tempval |= RCTRL_PRSDEP_INIT;
-		else
-			tempval &= ~RCTRL_PRSDEP_INIT;
 		gfar_write(&regs->rctrl, tempval);
+
+		gfar_check_rx_parser_mode(priv);
 	}
 
 	gfar_change_mtu(dev, dev->mtu);
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index 27499c6..87c1d86 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -286,7 +286,7 @@ extern const char gfar_driver_version[];
 #define RCTRL_PROM		0x00000008
 #define RCTRL_EMEN		0x00000002
 #define RCTRL_REQ_PARSER	(RCTRL_VLEX | RCTRL_IPCSEN | \
-				 RCTRL_TUCSEN)
+				 RCTRL_TUCSEN | RCTRL_FILREN)
 #define RCTRL_CHECKSUMMING	(RCTRL_IPCSEN | RCTRL_TUCSEN | \
 				RCTRL_PRSDEP_INIT)
 #define RCTRL_EXTHASH		(RCTRL_GHTX)
@@ -1182,6 +1182,7 @@ extern void gfar_configure_coalescing(struct gfar_private *priv,
 		unsigned long tx_mask, unsigned long rx_mask);
 void gfar_init_sysfs(struct net_device *dev);
 int gfar_set_features(struct net_device *dev, u32 features);
+extern void gfar_check_rx_parser_mode(struct gfar_private *priv);
 
 extern const struct ethtool_ops gfar_ethtool_ops;
 
-- 
1.7.6

Re: [PATCH] gianfar: rx parser

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Fri, 15 Jul 2011 21:51:34 +0200

> From: Sebastian Pöhn <sebastian.belden@googlemail.com>
> Subject: [patch net-next-2.6] gianfar: rx parser
> 
> Only let the rx parser be enabled if it is necessary (if VLAN extraction,
> IP or TCP checksumming or the rx queue filer are enabled). Otherwise
> disable it.
> 
> The new routine gfar_check_rx_parser_mode should be run after every
> change on this features and will enable/disable the parser as necessary.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>

Applied, thanks.