Home » Mailing list archives » linux-netdev » 2010 » June » 30

Re: [PATCH v2 2/3] gianfar: Implement workaround for eTSEC76 erratum

Previous thread: [PATCH v2] bonding: check if clients MAC addr has changed by Flavio Leitner on Tuesday, June 29, 2010 - 11:24 am. (2 messages)

Next thread: [PATCH 2/3] gianfar: Implement workaround for eTSEC76 erratum by Anton Vorontsov on Tuesday, June 29, 2010 - 2:00 pm. (1 message)
[view original message]
From: Anton Vorontsov
Subject: [PATCH 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Tuesday, June 29, 2010 - 1:59 pm

MPC8313ECE says:

"If MACCFG2[Huge Frame]=0 and the Ethernet controller receives frames
 which are larger than MAXFRM, the controller truncates the frames to
 length MAXFRM and marks RxBD[TR]=1 to indicate the error. The controller
 also erroneously marks RxBD[TR]=1 if the received frame length is MAXFRM
 or MAXFRM-1, even though those frames are not truncated.
 No truncation or truncation error occurs if MACCFG2[Huge Frame]=1."

There are two options to workaround the issue:

"1. Set MACCFG2[Huge Frame]=1, so no truncation occurs for invalid large
 frames. Software can determine if a frame is larger than MAXFRM by
 reading RxBD[LG] or RxBD[Data Length].

 2. Set MAXFRM to 1538 (0x602) instead of the default 1536 (0x600), so
 normal-length frames are not marked as truncated. Software can examine
 RxBD[Data Length] to determine if the frame was larger than MAXFRM-2."

This patch implements the first workaround option by setting HUGEFRAME
bit, and gfar_clean_rx_ring() already checks the RxBD[Data Length].

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
 drivers/net/Kconfig   |   10 ++++++++++
 drivers/net/gianfar.c |   36 ++++++++++++++++++++++++++++++++++--
 drivers/net/gianfar.h |   11 +++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ce2fcdd..d3fcaba 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2400,6 +2400,16 @@ config GIANFAR
 	  This driver supports the Gigabit TSEC on the MPC83xx, MPC85xx,
 	  and MPC86xx family of chips, and the FEC on the 8540.
 
+config GIANFAR_ERRATA
+	bool "Gianfar Ethernet errata handling"
+	depends on GIANFAR && (PPC_MPC837x || PPC_MPC831x)
+	default y
+	help
+	  With this option enabled, gianfar driver will able to cope with
+	  some TSEC errata.
+
+	  If unsure, say Y.
+
 config UCC_GETH
 	tristate "Freescale QE Gigabit Ethernet"
 	depends on QUICC_ENGINE
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Tuesday, June 29, 2010 - 3:16 pm

I really don't see any value at all to this config option,
the errata fixup code should be there all the time.

It really does no harm to be there in the cases where it isn't
used, and forcing users to turn this on is just another
obscure way to end up with an incorrect configuration.
--


[ message continues ]
[view original message]
From: Anton Vorontsov
Subject: Re: [PATCH 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Wednesday, June 30, 2010 - 9:38 am

Well, at least for eTSEC76 erratum (patch 2/3) we have to touch
fast path (i.e. start_xmit), so I just wanted to make zero
overhead for controllers that don't need any fixups.

Not that there's much of the overhead in a single additional

OK, resending the new patches, without Kconfig stuff...

If we'll have too many or too big errata so that it would cause
major performance or code size penalty for non-affected SOCs, we
can always do:

enum gfar_errata {
#ifdef CONFIG_PPC_FOO
	GFAR_ERRATA_FOO = 0x01,
#else
	GFAR_ERRATA_FOO = 0,
#endif
}

And then, priv->errata & GFAR_ERRATA_FOO will be optimized
away by the compiler.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
--


[ message continues ]
[view original message]
From: Anton Vorontsov
Subject: [PATCH v2 2/3] gianfar: Implement workaround for eTSEC76 erratum
Date: Wednesday, June 30, 2010 - 9:39 am

MPC8313ECE says:

"For TOE=1 huge or jumbo frames, the data required to generate the
 checksum may exceed the 2500-byte threshold beyond which the controller
 constrains itself to one memory fetch every 256 eTSEC system clocks.

 This throttling threshold is supposed to trigger only when the
 controller has sufficient data to keep transmit active for the duration
 of the memory fetches. The state machine handling this threshold,
 however, fails to take large TOE frames into account. As a result,
 TOE=1 frames larger than 2500 bytes often see excess delays before start
 of transmission."

This patch implements the workaround as suggested by the errata
document, i.e.:

"Limit TOE=1 frames to less than 2500 bytes to avoid excess delays due to
 memory throttling.
 When using packets larger than 2700 bytes, it is recommended to turn TOE
 off."

To be sure, we limit the TOE frames to 2500 bytes, and do software
checksumming instead.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
 drivers/net/gianfar.c |   19 +++++++++++++++++++
 drivers/net/gianfar.h |    1 +
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 9abcb39..8ba2973 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -942,6 +942,11 @@ static void gfar_detect_errata(struct gfar_private *priv)
 			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
 		priv->errata |= GFAR_ERRATA_74;
 
+	/* MPC8313 and MPC837x all rev */
+	if ((pvr == 0x80850010 && mod == 0x80b0) ||
+			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
+		priv->errata |= GFAR_ERRATA_76;
+
 	if (priv->errata)
 		dev_info(dev, "enabled errata workarounds, flags: 0x%x\n",
 			 priv->errata);
@@ -2011,6 +2016,20 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned int nr_frags, nr_txbds, length;
 	union skb_shared_tx *shtx;
 
+	/*
+	 * TOE=1 frames larger than 2500 bytes may see excess delays
+	 * before start of ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH v2 2/3] gianfar: Implement workaround for eTSEC76 erratum
Date: Wednesday, June 30, 2010 - 11:37 am

From: Anton Vorontsov <avorontsov@mvista.com>

Applied.
--


[ message continues ]
[view original message]
From: Anton Vorontsov
Subject: [PATCH v2 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Wednesday, June 30, 2010 - 9:39 am

MPC8313ECE says:

"If MACCFG2[Huge Frame]=0 and the Ethernet controller receives frames
 which are larger than MAXFRM, the controller truncates the frames to
 length MAXFRM and marks RxBD[TR]=1 to indicate the error. The controller
 also erroneously marks RxBD[TR]=1 if the received frame length is MAXFRM
 or MAXFRM-1, even though those frames are not truncated.
 No truncation or truncation error occurs if MACCFG2[Huge Frame]=1."

There are two options to workaround the issue:

"1. Set MACCFG2[Huge Frame]=1, so no truncation occurs for invalid large
 frames. Software can determine if a frame is larger than MAXFRM by
 reading RxBD[LG] or RxBD[Data Length].

 2. Set MAXFRM to 1538 (0x602) instead of the default 1536 (0x600), so
 normal-length frames are not marked as truncated. Software can examine
 RxBD[Data Length] to determine if the frame was larger than MAXFRM-2."

This patch implements the first workaround option by setting HUGEFRAME
bit, and gfar_clean_rx_ring() already checks the RxBD[Data Length].

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
 drivers/net/gianfar.c |   29 +++++++++++++++++++++++++++--
 drivers/net/gianfar.h |   11 +++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 28b53d1..9abcb39 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -85,6 +85,7 @@
 #include <linux/net_tstamp.h>
 
 #include <asm/io.h>
+#include <asm/reg.h>
 #include <asm/irq.h>
 #include <asm/uaccess.h>
 #include <linux/module.h>
@@ -928,6 +929,24 @@ static void gfar_init_filer_table(struct gfar_private *priv)
 	}
 }
 
+static void gfar_detect_errata(struct gfar_private *priv)
+{
+	struct device *dev = &priv->ofdev->dev;
+	unsigned int pvr = mfspr(SPRN_PVR);
+	unsigned int svr = mfspr(SPRN_SVR);
+	unsigned int mod = (svr >> 16) & 0xfff6; /* w/o E suffix */
+	unsigned int rev = svr & 0xffff;
+
+	/* MPC8313 Rev 2.0 and higher; All MPC837x */
+	if ((pvr == 0x80850010 ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH v2 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Wednesday, June 30, 2010 - 11:37 am

From: Anton Vorontsov <avorontsov@mvista.com>

Applied.
--


[ message continues ]
[view original message]
From: Anton Vorontsov
Subject: [PATCH v2 3/3] gianfar: Implement workaround for eTSEC-A002 erratum
Date: Wednesday, June 30, 2010 - 9:39 am

MPC8313ECE says:

"If the controller receives a 1- or 2-byte frame (such as an illegal
 runt packet or a packet with RX_ER asserted) before GRS is asserted
 and does not receive any other frames, the controller may fail to set
 GRSC even when the receive logic is completely idle. Any subsequent
 receive frame that is larger than two bytes will reset the state so
 the graceful stop can complete. A MAC receiver (Rx) reset will also
 reset the state."

This patch implements the proposed workaround:

"If IEVENT[GRSC] is still not set after the timeout, read the eTSEC
 register at offset 0xD1C. If bits 7-14 are the same as bits 23-30,
 the eTSEC Rx is assumed to be idle and the Rx can be safely reset.
 If the register fields are not equal, wait for another timeout
 period and check again."

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
 drivers/net/gianfar.c |   40 +++++++++++++++++++++++++++++++++++++---
 drivers/net/gianfar.h |    1 +
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 8ba2973..35626eb 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -947,6 +947,11 @@ static void gfar_detect_errata(struct gfar_private *priv)
 			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
 		priv->errata |= GFAR_ERRATA_76;
 
+	/* MPC8313 and MPC837x all rev */
+	if ((pvr == 0x80850010 && mod == 0x80b0) ||
+			(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
+		priv->errata |= GFAR_ERRATA_A002;
+
 	if (priv->errata)
 		dev_info(dev, "enabled errata workarounds, flags: 0x%x\n",
 			 priv->errata);
@@ -1570,6 +1575,29 @@ static void init_registers(struct net_device *dev)
 	gfar_write(&regs->minflr, MINFLR_INIT_SETTINGS);
 }
 
+static int __gfar_is_rx_idle(struct gfar_private *priv)
+{
+	u32 res;
+
+	/*
+	 * Normaly TSEC should not hang on GRS commands, so we should
+	 * actually wait for IEVENT_GRSC flag.
+	 */
+	if (likely(!gfar_has_errata(priv, GFAR_ERRATA_A002)))
+		return ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH v2 3/3] gianfar: Implement workaround for eTSEC-A002 erratum
Date: Wednesday, June 30, 2010 - 11:37 am

From: Anton Vorontsov <avorontsov@mvista.com>

Applied.
--


[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH 1/3] gianfar: Implement workaround for eTSEC74 erratum
Date: Wednesday, June 30, 2010 - 11:37 am

From: Anton Vorontsov <avorontsov@mvista.com>

The register accesses will dominate the costs with this chip.

The only case where a if() test is going to potentially create
some practical performance impact is if the TX is performed
purely using changes to a shared memory data structure and
absolutely no MMIO register reads or writes.
--


[ message continues ]
Previous thread: [PATCH v2] bonding: check if clients MAC addr has changed by Flavio Leitner on Tuesday, June 29, 2010 - 11:24 am. (2 messages)

Next thread: [PATCH 2/3] gianfar: Implement workaround for eTSEC76 erratum by Anton Vorontsov on Tuesday, June 29, 2010 - 2:00 pm. (1 message)