diff mbox

[1/1,net-next] net: fec: enable pause frame to improve rx prefomance for 1G network

Message ID 1357634609-4112-1-git-send-email-Frank.Li@freescale.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Frank Li Jan. 8, 2013, 8:43 a.m. UTC
The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
There will be many packages lost because FIFO over run.

This patch enable pause frame flow control.

Before this patch
iperf -s -i 1
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
[  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49773
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0- 1.0 sec  6.35 MBytes  53.3 Mbits/sec
[  4]  1.0- 2.0 sec  3.39 MBytes  28.5 Mbits/sec
[  4]  2.0- 3.0 sec  2.63 MBytes  22.1 Mbits/sec
[  4]  3.0- 4.0 sec  1.10 MBytes  9.23 Mbits/sec

ifconfig
   RX packets:46195 errors:1859 dropped:1 overruns:1859 frame:1859

After this patch
iperf -s -i 1

[  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49757
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0- 1.0 sec  49.8 MBytes   418 Mbits/sec
[  4]  1.0- 2.0 sec  50.1 MBytes   420 Mbits/sec
[  4]  2.0- 3.0 sec  47.5 MBytes   399 Mbits/sec
[  4]  3.0- 4.0 sec  45.9 MBytes   385 Mbits/sec
[  4]  4.0- 5.0 sec  44.8 MBytes   376 Mbits/sec

ifconfig
   RX packets:2348454 errors:0 dropped:16 overruns:0 frame:0

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Fugang Duan  <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.c |   26 ++++++++++++++++++++++++++
 drivers/net/ethernet/freescale/fec.h |    4 ++++
 2 files changed, 30 insertions(+), 0 deletions(-)

Comments

Behme Dirk (CM/ESO2) Jan. 8, 2013, 9:55 a.m. UTC | #1
On 08.01.2013 09:43, Frank Li wrote:
> The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
> There will be many packages lost because FIFO over run.
> 
> This patch enable pause frame flow control.

Many thanks for this patch!

Do we want/need anything similar for U-Boot, too?

Some people want to boot the system from U-Boot attached to 1G network 
and have issues there, too.

Best regards

Dirk
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings Jan. 8, 2013, 12:52 p.m. UTC | #2
On Tue, 2013-01-08 at 16:43 +0800, Frank Li wrote:
> The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
> There will be many packages lost because FIFO over run.

How sad...

> This patch enable pause frame flow control.
[...]

You should not enable pause frames unconditionally. It will cause
compatibility problems with some switches.  It should be possible to
control their use manually or through autonegotiation (if the medium
supports that).

You should:
1. Implement the ethtool get_pauseparam and set_pauseparam operations.
If the medium supports autoneog, use mii_advertise_flowctrl() in
set_pauseparam to update pause frame advertising and then restart
autoneg.
2. Allow pause frame advertising to be changed through set_settings (I
think phylib covers this for you).
3. Program the MAC according to the the parameters set with
set_pauseparam or (if pause autoneg was enabled) the result of autoneg.

Ben.
Ben Hutchings Jan. 8, 2013, 1:57 p.m. UTC | #3
On Tue, 2013-01-08 at 13:32 +0000, David Laight wrote:
> > If the medium supports autoneog, use mii_advertise_flowctrl() in
> > set_pauseparam to update pause frame advertising and then restart
> > autoneg.
> 
> Isn't autoneg continuous?
> In which case you just need to monitor the response register.
> Monitoring MII registers is usually a complete PITA though
> (due to the requirements for delays during the bit-bashing).

AIUI you have to break the link before renegotiating.  So you should
only need to read autoneg results after a PHY interrupt or polling for a
link change.

Ben.
Eric Nelson Jan. 8, 2013, 4:13 p.m. UTC | #4
On 01/08/2013 02:55 AM, Dirk Behme wrote:
> On 08.01.2013 09:43, Frank Li wrote:
>> The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
>> There will be many packages lost because FIFO over run.
>>
>> This patch enable pause frame flow control.
>
> Many thanks for this patch!
>
> Do we want/need anything similar for U-Boot, too?
>
> Some people want to boot the system from U-Boot attached to 1G network
> and have issues there, too.
>

It's hard to see how the typical use (dhcp+tftp) would be affected
because of the small packet sizes and half-duplex operation.

That said, the statistics registers make it pretty easy to see when
there are overflows as described here:
	http://boundarydevices.com/i-mx6-ethernet/

We should be able to just poke the ENET_MIBC register and watch
to see if the IEEE_R_MACERR register shows that overruns are
occurring using the U-Boot md and mw commands.

In a quick test, it appears that this doesn't work though.
This line of code in fec_halt from fec_mxc.c seems to clear the
statistics.

	writel(readl(&fec->eth->ecntrl) & ~FEC_ECNTRL_ETHER_EN,
			&fec->eth->ecntrl);

If you comment that out, you can see all of the statistics registers
as shown below:

U-Boot > mw.l 0x02188064 0    #ENET_MIBC
U-Boot > dhcp 10800000 192.168.0.55:uImage-bluemeany
BOOTP broadcast 1
DHCP client bound to address 192.168.0.91
Using FEC device
TFTP from server 192.168.0.55; our IP address is 192.168.0.91
Filename 'uImage-bluemeany'.
Load address: 0x10800000
Loading: ...
done
Bytes transferred = 3760504 (396178 hex)
U-Boot > md.l 0x02188200 0x39
02188200: 00000000 00001cb6 00000003 00000000    ................
02188210: 00000000 00000000 00000000 00000000    ................
02188220: 00000000 00000000 00001cb2 00000002    ................
02188230: 00000000 00000002 00000000 00000000    ................
02188240: 00000000 00072fd4 00000000 00001cb6    ...../..........
02188250: 00000000 00000000 00000000 00000000    ................
02188260: 00000000 00000000 00000000 00000000    ................
02188270: 00000000 00072fd4 00000000 00000000    ...../..........
02188280: 00000000 00001dca 0000002a 000000e6    ........*.......
02188290: 00000000 00000000 00000000 00000000    ................
021882a0: 00000000 00000000 00000037 00000057    ........7...W...
021882b0: 0000004e 00000023 00001ccb 00000000    N...#...........
021882c0: 00000000 003fd011 00000000 00001dcc    ......?.........
021882d0: 00000000 00000000 00000000 00000000    ................
021882e0: 003fd011    ..?.
U-Boot >

Register 0x021882d8 is the MACERR (fifo overlow) counter.

Regards,


Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Jan. 9, 2013, 1:58 a.m. UTC | #5
As others have mentioned, this is definitely something which must be
automatically detected via PHY autonegotiation results or the user
forcing the setting via ethtool.  Not like this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 6dc2094..4928260 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -68,6 +68,14 @@ 
 
 #define DRIVER_NAME	"fec"
 
+/* Pause frame feild and FIFO threshold */
+#define FEC_ENET_FCE	(1 << 5)
+#define FEC_ENET_RSEM_V	0x84
+#define FEC_ENET_RSFL_V	16
+#define FEC_ENET_RAEM_V	0x8
+#define FEC_ENET_RAFL_V	0x8
+#define FEC_ENET_OPD_V	0xFFF0
+
 /* Controller is ENET-MAC */
 #define FEC_QUIRK_ENET_MAC		(1 << 0)
 /* Controller needs driver to swap frame */
@@ -470,6 +478,21 @@  fec_restart(struct net_device *ndev, int duplex)
 		}
 #endif
 	}
+
+	/* enable pause frame*/
+	if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
+		rcntl |= FEC_ENET_FCE;
+
+		/* set FIFO thresh hold parameter to reduce overrun */
+		writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
+		writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL);
+		writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM);
+		writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL);
+
+		/* OPD */
+		writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
+	}
+
 	writel(rcntl, fep->hwp + FEC_R_CNTRL);
 
 	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
@@ -1021,6 +1044,9 @@  static int fec_enet_mii_probe(struct net_device *ndev)
 	else
 		phy_dev->supported &= PHY_BASIC_FEATURES;
 
+	/* enable phy pause frame for any platform */
+	phy_dev->supported |= ADVERTISED_Pause;
+
 	phy_dev->advertising = phy_dev->supported;
 
 	fep->phy_dev = phy_dev;
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 4862394..7dac581 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -48,6 +48,10 @@ 
 #define FEC_R_DES_START		0x180 /* Receive descriptor ring */
 #define FEC_X_DES_START		0x184 /* Transmit descriptor ring */
 #define FEC_R_BUFF_SIZE		0x188 /* Maximum receive buff size */
+#define FEC_R_FIFO_RSFL		0x190 /* Receive FIFO section full threshold */
+#define FEC_R_FIFO_RSEM		0x194 /* Receive FIFO section empty threshold */
+#define FEC_R_FIFO_RAEM		0x198 /* Receive FIFO almost empty threshold */
+#define FEC_R_FIFO_RAFL		0x19c /* Receive FIFO almost full threshold */
 #define FEC_MIIGSK_CFGR		0x300 /* MIIGSK Configuration reg */
 #define FEC_MIIGSK_ENR		0x308 /* MIIGSK Enable reg */