gianfar: Implement workaround for eTSEC74 erratum
Subsystems (2)
PATCH 1/3 gianfar: Implement workaround for eTSEC74 erratum
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].
--- drivers/net/Kconfig | 10 ++++++++++ drivers/net/gianfar.c | 36 ++++++++++++++++++++++++++++++++++-- drivers/net/gianfar.h | 11 +++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-)
drivers/net/Kconfig +10 -0
@@ -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
drivers/net/gianfar.c +34 -2
@@ -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,31 @@ 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 && mod ==0x80b0 && rev >=0x0020 ) || + (pvr ==0x80861010 && (mod &0xfff9 ) ==0x80c0 )) + priv->errata |=GFAR_ERRATA_74 ; + +if (priv->errata) { +#ifdef CONFIG_GIANFAR_ERRATA +dev_info (dev,"enabled errata workarounds, flags: 0x%x\n" , + priv->errata); +#else +dev_warn (dev,"consider enabling CONFIG_GIANFAR_ERRATA, " +"flags: 0x%x\n" , priv->errata); +WARN_ON (1 ); +#endif /* CONFIG_GIANFAR_ERRATA */ + } +} +/* Set up the ethernet device structure, private data, * and anything else we need before we start */ static int gfar_probe(struct of_device *ofdev,
@@ -960,6 +986,8 @@ static int gfar_probe(struct of_device *ofdev,dev_set_drvdata (&ofdev->dev, priv); regs = priv->gfargrp[0 ].regs; +gfar_detect_errata (priv); +/* Stop the DMA engine now, in case it was running before */ /* (The firmware could have used it, and left it running). */ gfar_halt (dev);
@@ -974,7 +1002,10 @@ static int gfar_probe(struct of_device *ofdev,gfar_write (®s->maccfg1, tempval);/* Initialize MACCFG2. */ -gfar_write (®s->maccfg2,MACCFG2_INIT_SETTINGS ); + tempval =MACCFG2_INIT_SETTINGS ; +if (gfar_has_errata (priv,74 )) + tempval |=MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK ; +gfar_write (®s->maccfg2, tempval);/* Initialize ECNTRL */ gfar_write (®s->ecntrl,ECNTRL_INIT_SETTINGS );
@@ -2300,7 +2331,8 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)* to allow huge frames, and to check the length */ tempval =gfar_read (®s->maccfg2); -if (priv->rx_buffer_size >DEFAULT_RX_BUFFER_SIZE ) +if (priv->rx_buffer_size >DEFAULT_RX_BUFFER_SIZE || +gfar_has_errata (priv,74 )) tempval |= (MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK );else tempval &= ~(MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK );
drivers/net/gianfar.h +11 -0
@@ -1025,6 +1025,16 @@ struct gfar_priv_grp {char int_name_er[GFAR_INT_NAME_MAX ]; }; +enum gfar_errata { +GFAR_ERRATA_74 =0x01 , +}; + +#ifdef CONFIG_GIANFAR_ERRATA +#define gfar_has_errata (priv, err) ((priv)->errata & GFAR_ERRATA_##err) +#else +#define gfar_has_errata (priv, err) 0 +#endif /* CONFIG_GIANFAR_ERRATA */ +/* Struct stolen almost completely (and shamelessly) from the FCC enet source * (Ok, that's not so true anymore, but there is a family resemblence) * The GFAR buffer descriptors track the ring buffers. The rx_bd_base
@@ -1049,6 +1059,7 @@ struct gfar_private {struct device_node *node;struct net_device *ndev;struct of_device *ofdev; +enum gfar_errata errata;struct gfar_priv_grp gfargrp[MAXGROUPS ];struct gfar_priv_tx_q *tx_queue[MAX_TX_QS ];
Signature · 1 line
1.7.0.5
David Miller
maintainerI 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.
Quoted 3 lines · David Miller
I really don't see any value at all to this config option, the errata fixup code should be there all the time.
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 'if' condition, no. ;-)
Quoted 3 lines
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.
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,
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
PATCH v2 1/3 gianfar: Implement workaround for eTSEC74 erratum
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].
--- drivers/net/gianfar.c | 29 +++++++++++++++++++++++++++-- drivers/net/gianfar.h | 11 +++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)
drivers/net/gianfar.c +27 -2
@@ -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 && mod ==0x80b0 && rev >=0x0020 ) || + (pvr ==0x80861010 && (mod &0xfff9 ) ==0x80c0 )) + priv->errata |=GFAR_ERRATA_74 ; + +if (priv->errata) +dev_info (dev,"enabled errata workarounds, flags: 0x%x\n" , + priv->errata); +} +/* Set up the ethernet device structure, private data, * and anything else we need before we start */ static int gfar_probe(struct of_device *ofdev,
@@ -960,6 +979,8 @@ static int gfar_probe(struct of_device *ofdev,dev_set_drvdata (&ofdev->dev, priv); regs = priv->gfargrp[0 ].regs; +gfar_detect_errata (priv); +/* Stop the DMA engine now, in case it was running before */ /* (The firmware could have used it, and left it running). */ gfar_halt (dev);
@@ -974,7 +995,10 @@ static int gfar_probe(struct of_device *ofdev,gfar_write (®s->maccfg1, tempval);/* Initialize MACCFG2. */ -gfar_write (®s->maccfg2,MACCFG2_INIT_SETTINGS ); + tempval =MACCFG2_INIT_SETTINGS ; +if (gfar_has_errata (priv,GFAR_ERRATA_74 )) + tempval |=MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK ; +gfar_write (®s->maccfg2, tempval);/* Initialize ECNTRL */ gfar_write (®s->ecntrl,ECNTRL_INIT_SETTINGS );
@@ -2300,7 +2324,8 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)* to allow huge frames, and to check the length */ tempval =gfar_read (®s->maccfg2); -if (priv->rx_buffer_size >DEFAULT_RX_BUFFER_SIZE ) +if (priv->rx_buffer_size >DEFAULT_RX_BUFFER_SIZE || +gfar_has_errata (priv,GFAR_ERRATA_74 )) tempval |= (MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK );else tempval &= ~(MACCFG2_HUGEFRAME |MACCFG2_LENGTHCHECK );
drivers/net/gianfar.h +11 -0
@@ -1025,6 +1025,10 @@ struct gfar_priv_grp {char int_name_er[GFAR_INT_NAME_MAX ]; }; +enum gfar_errata { +GFAR_ERRATA_74 =0x01 , +}; +/* Struct stolen almost completely (and shamelessly) from the FCC enet source * (Ok, that's not so true anymore, but there is a family resemblence) * The GFAR buffer descriptors track the ring buffers. The rx_bd_base
@@ -1049,6 +1053,7 @@ struct gfar_private {struct device_node *node;struct net_device *ndev;struct of_device *ofdev; +enum gfar_errata errata;struct gfar_priv_grp gfargrp[MAXGROUPS ];struct gfar_priv_tx_q *tx_queue[MAX_TX_QS ];
@@ -1111,6 +1116,12 @@ struct gfar_private {extern unsigned int ftp_rqfpr[MAX_FILER_IDX +1 ];extern unsigned int ftp_rqfcr[MAX_FILER_IDX +1 ]; +static inline int gfar_has_errata (struct gfar_private *priv, +enum gfar_errata err) +{ +return priv->errata & err; +} +static inline u32 gfar_read (volatile unsigned __iomem *addr) {u32 val;
Signature · 1 line
1.7.0.5
From: Anton Vorontsov <[email protected]>
Date: Wed, 30 Jun 2010 20:39:12 +0400
Quoted 15 lines
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].
Quoted 8 lines
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 <[email protected]>
Applied.
PATCH v2 2/3 gianfar: Implement workaround for eTSEC76 erratum
In reply to Anton VorontsovMPC8313ECE 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.
--- drivers/net/gianfar.c | 19 +++++++++++++++++++ drivers/net/gianfar.h | 1 + 2 files changed, 20 insertions(+), 0 deletions(-)
drivers/net/gianfar.c +19 -0
@@ -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 transmission. +*/ +if (unlikely (gfar_has_errata (priv,GFAR_ERRATA_76 ) && + skb->ip_summed ==CHECKSUM_PARTIAL && + skb->len >2500 )) { +int ret; + + ret =skb_checksum_help (skb); +if (ret) +return ret; + } + rq = skb->queue_mapping; tx_queue = priv->tx_queue[rq]; txq =netdev_get_tx_queue (dev, rq);
drivers/net/gianfar.h +1 -0
@@ -1027,6 +1027,7 @@ struct gfar_priv_grp {enum gfar_errata {GFAR_ERRATA_74 =0x01 , +GFAR_ERRATA_76 =0x02 , };/* Struct stolen almost completely (and shamelessly) from the FCC enet source
Signature · 1 line
1.7.0.5
Re: [PATCH v2 2/3] gianfar: Implement workaround for eTSEC76 erratum
From: Anton Vorontsov <[email protected]>
Date: Wed, 30 Jun 2010 20:39:13 +0400
Quoted 21 lines
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."
Quoted 4 lines
To be sure, we limit the TOE frames to 2500 bytes, and do software checksumming instead.
Signed-off-by Anton Vorontsov <[email protected]>
Applied.
PATCH v2 3/3 gianfar: Implement workaround for eTSEC-A002 erratum
In reply to Anton VorontsovMPC8313ECE 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."
--- drivers/net/gianfar.c | 40 +++++++++++++++++++++++++++++++++++++--- drivers/net/gianfar.h | 1 + 2 files changed, 38 insertions(+), 3 deletions(-)
drivers/net/gianfar.c +37 -3
@@ -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 (®s->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 0 ; + +/* +* 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. +*/ + res =gfar_read ((void __iomem *)priv->gfargrp[0 ].regs +0xd1c ); + res &=0x7f807f80 ; +if ((res &0xffff ) == (res >>16 )) +return 1 ; + +return 0 ; +}/* Halt the receive and transmit queues */ static void gfar_halt_nodisable (struct net_device *dev)
@@ -1593,12 +1621,18 @@ static void gfar_halt_nodisable(struct net_device *dev) tempval =gfar_read (®s->dmactrl);if ((tempval & (DMACTRL_GRS |DMACTRL_GTS )) != (DMACTRL_GRS |DMACTRL_GTS )) { +int ret; + tempval |= (DMACTRL_GRS |DMACTRL_GTS );gfar_write (®s->dmactrl, tempval); -spin_event_timeout (((gfar_read (®s->ievent) & - (IEVENT_GRSC |IEVENT_GTSC )) == - (IEVENT_GRSC |IEVENT_GTSC )),-1 ,0 ); +do { + ret =spin_event_timeout (((gfar_read (®s->ievent) & + (IEVENT_GRSC |IEVENT_GTSC )) == + (IEVENT_GRSC |IEVENT_GTSC )),1000000 ,0 ); +if (!ret && !(gfar_read (®s->ievent) &IEVENT_GRSC )) + ret =__gfar_is_rx_idle (priv); + }while (!ret); } }
drivers/net/gianfar.h +1 -0
@@ -1028,6 +1028,7 @@ struct gfar_priv_grp {enum gfar_errata {GFAR_ERRATA_74 =0x01 ,GFAR_ERRATA_76 =0x02 , +GFAR_ERRATA_A002 =0x04 , };/* Struct stolen almost completely (and shamelessly) from the FCC enet source
Signature · 1 line
1.7.0.5
Re: [PATCH v2 3/3] gianfar: Implement workaround for eTSEC-A002 erratum
From: Anton Vorontsov <[email protected]>
Date: Wed, 30 Jun 2010 20:39:15 +0400
Quoted 12 lines
MPC8313ECE says:
"If the controller receives a 1- or 2-byte frame (such as an illegal
runt packet or a packet withRX_ERasserted) 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:
Quoted 7 lines
"If IEVENT[GRSC] is still not set after the timeout, read the eTSEC
register at offset0xD1C. 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 <[email protected]>
Applied.
David Miller
maintainerFrom: Anton Vorontsov <[email protected]>
Date: Wed, 30 Jun 2010 20:38:04 +0400
Quoted 11 lines · David Miller
Quoted 3 lines
I really don't see any value at all to this config option, the errata fixup code should be there all the time.
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 'if' condition, no. ;-)
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.