[PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[Russell King - ARM Linux]

This is v2 of my initial round of patches (roughly half of my total
patch set) for the Freescale FEC driver.

I'm sending this set out for comments and testing.  So far, I have
had only one ack for one patch in this series, this is pretty poor,
so I'm now sending it with a CFT tag instead.

I haven't changed too much - I've fixed the bug which Andy spotted in
the transmit handling, but otherwise the patches are much the same.

The remainder of the text is as per my previous message, but with
updated diffstat:

One of my motivations for only sending half is to get this half into
a state where DaveM is happy to merge it before I sort out the
remainder.  There's quite a lot here, so bear with me on this.

I've tried to sort the fixes before the cleanups as best I can, but
reordering the series is a full-time job due to it's size - it's taken
from Monday until now to get this far with it, so I'm hoping that there
won't be any "you should rearrange the patches as X" comments.

While the original series was well tested during it's original
development, including with performance tests on each patch, that
testing and validation has been lost due to the changes during the
last merge window, and subsequent rebasing and updating of the patches.

The series is based on v3.16-rc1, and as such, I have added Andy's
checksum fix to the start of the series.  This patch is not strictly
part of the RFC, but is included because my complete patch series needs
to account for that change.

Some of these patches are to fix theoretical problems in the driver
(ones which have been found via a review of the code) others address
real observable problems (such as poor half-duplex performance.)

Towards the end of this series, I have included a patch which was
initially at the beginning of the series for dumping out the state of
the transmit ring, which is very useful to debug transmit problems.
This was acceptable when the transmit ring was between 16 and 128
descriptors, but during the recent merge window, this was increased to
512 descriptors, so it will now print around 512 lines to the kernel
message log on transmit timeout.  I'm not entirely convinced this is a
good idea - maybe it should become optional, or maybe the timed-out ring
status should be available via debugfs, but that's a problem to retrieve
if you're running NFS rootfs and the timeout doesn't recover properly.

This series of patches is also available from the following *unstable*
git branch - unstable as it's provided for convenience so you don't
have to apply all these patches individually, unstable as the patches
need to have attributations added, unstable because DaveM will probably
want to apply them as patches to his tree:

  git://ftp.arm.linux.org.uk/~rmk/linux-arm.git fec-testing

 drivers/net/ethernet/freescale/fec.h      |  10 +-
 drivers/net/ethernet/freescale/fec_main.c | 382 +++++++++++++++++-------------
 2 files changed, 224 insertions(+), 168 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH CFT 01/30] net: fec: Don't clear IPV6 header checksum field when IP accelerator enable

[Russell King]

From: Fugang Duan <b38611@freescale.com>
To: linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org

The commit 96c50caa5148 (net: fec: Enable IP header hardware checksum)
enable HW IP header checksum for IPV4 and IPV6, which causes IPV6 TCP/UDP
cannot work. (The issue is reported by Russell King)

For FEC IP header checksum function: Insert IP header checksum. This "IINS"
bit is written by the user. If set, IP accelerator calculates the IP header
checksum and overwrites the IINS corresponding header field with the calculated
value. The checksum field must be cleared by user, otherwise the checksum
always is 0xFFFF.

So the previous patch clear IP header checksum field regardless of IP frame
type.

In fact, IP HW detect the packet as IPV6 type, even if the "IINS" bit is set,
the IP accelerator is not triggered to calculates IPV6 header checksum because
IPV6 frame format don't have checksum.

So this results in the IPV6 frame being corrupted.

The patch just add software detect the current packet type, if it is IPV6
frame, it don't clear IP header checksum field.

Reported-and-tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 38d9d276ab8b..77037fd377b8 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -320,6 +320,11 @@ static void *swap_buffer(void *bufaddr, int len)
 	return bufaddr;
 }
 
+static inline bool is_ipv4_pkt(struct sk_buff *skb)
+{
+	return skb->protocol == htons(ETH_P_IP) && ip_hdr(skb)->version == 4;
+}
+
 static int
 fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 {
@@ -330,7 +335,8 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 	if (unlikely(skb_cow_head(skb, 0)))
 		return -1;
 
-	ip_hdr(skb)->check = 0;
+	if (is_ipv4_pkt(skb))
+		ip_hdr(skb)->check = 0;
 	*(__sum16 *)(skb->head + skb->csum_start + skb->csum_offset) = 0;
 
 	return 0;
-- 
1.8.3.1

[PATCH CFT 02/30] net: fec: iMX6 FEC does not support half-duplex gigabit

[Russell King]

The iMX6 gigabit FEC does not support half-duplex gigabit operation.
Phys attacked to the FEC may support this, and we currently do nothing
to disable this feature.  This may result in an invalid configuration.
Mask out phy support for gigabit half-duplex operation.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 77037fd377b8..a91fe68030e6 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1667,6 +1667,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 	/* mask with MAC supported features */
 	if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
 		phy_dev->supported &= PHY_GBIT_FEATURES;
+		phy_dev->supported &= ~SUPPORTED_1000baseT_Half;
 #if !defined(CONFIG_M5272)
 		phy_dev->supported |= SUPPORTED_Pause;
 #endif
-- 
1.8.3.1

Re: [PATCH CFT 02/30] net: fec: iMX6 FEC does not support half-duplex gigabit

[Uwe Kleine-König]

Hi Russell,

On Fri, Jun 27, 2014 at 04:19:03PM +0100, Russell King wrote:
> The iMX6 gigabit FEC does not support half-duplex gigabit operation.
> Phys attacked to the FEC may support this, and we currently do nothing
Even if it might be interpreted as an attack to connect such a phy to
the FEC, I'd still call it "attach" here :-)

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH CFT 02/30] net: fec: iMX6 FEC does not support half-duplex gigabit

[Russell King - ARM Linux]

On Tue, Jul 08, 2014 at 08:46:11AM +0200, Uwe Kleine-König wrote:
> Hi Russell,
> 
> On Fri, Jun 27, 2014 at 04:19:03PM +0100, Russell King wrote:
> > The iMX6 gigabit FEC does not support half-duplex gigabit operation.
> > Phys attacked to the FEC may support this, and we currently do nothing
> Even if it might be interpreted as an attack to connect such a phy to
> the FEC, I'd still call it "attach" here :-)

You are right, but unfortunately your comment comes too late; this is
the actual submission for David to integrate the patches into net-next
rather than for review, and David has already integrated the change into
his tree.  Hence, the comment is unable to be fixed now.

Thanks anyway.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH CFT 03/30] net: fec: fix ethtool set_pauseparam duplex bug

[Russell King]

Setting the pause parameters causes a running network interface to be
restarted.  However, the restart forces the FEC into half-duplex mode,
whether or not the remote end is in half-duplex mode.  Misconfigured
duplex mode is a known source of problems on a link.

Fix this by always preserving the duplex mode on configuration changes.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index a91fe68030e6..045ea71f2b59 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1897,7 +1897,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 		phy_start_aneg(fep->phy_dev);
 	}
 	if (netif_running(ndev))
-		fec_restart(ndev, 0);
+		fec_restart(ndev, fep->full_duplex);
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH CFT 04/30] net: fec: fix interrupt handling races

[Russell King]

While running: while :; do iperf -c <HOST> -P 4; done, transmit timeouts
are regularly reported.  With the tx ring dumping in place, we can see
that all entries are in use, and the hardware has finished transmitting
these packets.  However, the driver has not reclaimed these ring
entries.

This can occur if the interrupt handler is invoked at the wrong moment -
eg:

	CPU0				CPU1
	fec_enet_tx()
					interrupt, IEVENT = FEC_ENET_TXF
					FEC_ENET_TXF cleared
					napi_schedule_prep()
	napi_complete()

The result is that we clear the transmit interrupt, but we don't trigger
any cleaning of the transmit ring.  Instead, use a different strategy:

- When receiving a transmit or receive interrupt, disable both tx and rx
  interrupts, but do not acknowledge them.  Schedule a napi poll.  Don't
  loop.

- When we are polled, read IEVENT, acknowledging the pending transmit
  and receive interrupts, before then going on to process the
  appropriate rings.

This allows us to avoid the race, and has a number of other advantages:
- we cut down on the number of transmit interrupts we have to process.
- we only look at the rings which have pending events.
- we gain additional throughput: the iperf total bandwidth increases
  from about 180Mbps to 240Mbps:

[  3]  0.0-10.0 sec  68.1 MBytes  57.0 Mbits/sec
[  5]  0.0-10.0 sec  72.4 MBytes  60.5 Mbits/sec
[  4]  0.0-10.1 sec  76.1 MBytes  63.5 Mbits/sec
[  6]  0.0-10.1 sec  71.9 MBytes  59.9 Mbits/sec
[SUM]  0.0-10.1 sec   288 MBytes   241 Mbits/sec

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 40 +++++++++++++++++--------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 045ea71f2b59..4e695b742030 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1369,29 +1369,25 @@ fec_enet_interrupt(int irq, void *dev_id)
 {
 	struct net_device *ndev = dev_id;
 	struct fec_enet_private *fep = netdev_priv(ndev);
+	const unsigned napi_mask = FEC_ENET_RXF | FEC_ENET_TXF;
 	uint int_events;
 	irqreturn_t ret = IRQ_NONE;
 
-	do {
-		int_events = readl(fep->hwp + FEC_IEVENT);
-		writel(int_events, fep->hwp + FEC_IEVENT);
+	int_events = readl(fep->hwp + FEC_IEVENT);
+	writel(int_events & ~napi_mask, fep->hwp + FEC_IEVENT);
 
-		if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
-			ret = IRQ_HANDLED;
+	if (int_events & napi_mask) {
+		ret = IRQ_HANDLED;
 
-			/* Disable the RX interrupt */
-			if (napi_schedule_prep(&fep->napi)) {
-				writel(FEC_RX_DISABLED_IMASK,
-					fep->hwp + FEC_IMASK);
-				__napi_schedule(&fep->napi);
-			}
-		}
+		/* Disable the NAPI interrupts */
+		writel(FEC_ENET_MII, fep->hwp + FEC_IMASK);
+		napi_schedule(&fep->napi);
+	}
 
-		if (int_events & FEC_ENET_MII) {
-			ret = IRQ_HANDLED;
-			complete(&fep->mdio_done);
-		}
-	} while (int_events);
+	if (int_events & FEC_ENET_MII) {
+		ret = IRQ_HANDLED;
+		complete(&fep->mdio_done);
+	}
 
 	return ret;
 }
@@ -1399,8 +1395,16 @@ fec_enet_interrupt(int irq, void *dev_id)
 static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
-	int pkts = fec_enet_rx(ndev, budget);
 	struct fec_enet_private *fep = netdev_priv(ndev);
+	int pkts;
+
+	/*
+	 * Clear any pending transmit or receive interrupts before
+	 * processing the rings to avoid racing with the hardware.
+	 */
+	writel(FEC_ENET_RXF | FEC_ENET_TXF, fep->hwp + FEC_IEVENT);
+
+	pkts = fec_enet_rx(ndev, budget);
 
 	fec_enet_tx(ndev);
 
-- 
1.8.3.1

[PATCH CFT 05/30] net: fec: use netif_tx_disable() rather than netif_stop_queue()

[Russell King]

We use netif_stop_queue() in several places where we want to ensure that
the start_xmit function is not running.  netif_stop_queue() is not
sufficient to achieve that - it merely sets a flag to indicate that the
transmit queue(s) should not be run.

netif_tx_disable() gives this guarantee, since it takes the transmit
queue lock while marking the queue stopped.  This will wait for the
transmit function to complete before returning.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4e695b742030..cb9ced738607 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -834,7 +834,7 @@ fec_restart(struct net_device *ndev, int duplex)
 	if (netif_running(ndev)) {
 		netif_device_detach(ndev);
 		napi_disable(&fep->napi);
-		netif_stop_queue(ndev);
+		netif_tx_disable(ndev);
 		netif_tx_lock_bh(ndev);
 	}
 
@@ -2181,7 +2181,7 @@ fec_enet_close(struct net_device *ndev)
 	/* Don't know what to do yet. */
 	napi_disable(&fep->napi);
 	fep->opened = 0;
-	netif_stop_queue(ndev);
+	netif_tx_disable(ndev);
 	fec_stop(ndev);
 
 	if (fep->phy_dev) {
-- 
1.8.3.1

[PATCH CFT 06/30] net: fec: remove checking for NULL phy_dev in fec_enet_close()

[Russell King]

fep->phy_dev can not be NULL here for two reasons:
- fec_enet_open() will have successfully connected the phy, or will have
  failed.
- fec_enet_open() will have called phy_start(fep->phy_dev), which
  unconditionally dereferences this pointer.

If it were to be NULL here, then fec_enet_open() will have already
oopsed.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cb9ced738607..6a03d7eced4d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2184,10 +2184,8 @@ fec_enet_close(struct net_device *ndev)
 	netif_tx_disable(ndev);
 	fec_stop(ndev);
 
-	if (fep->phy_dev) {
-		phy_stop(fep->phy_dev);
-		phy_disconnect(fep->phy_dev);
-	}
+	phy_stop(fep->phy_dev);
+	phy_disconnect(fep->phy_dev);
 
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&fep->pdev->dev);
-- 
1.8.3.1

[PATCH CFT 07/30] net: fec: ensure that a disconnected phy isn't configured

[Russell King]

When we disconnect from a phy, we should forget our pointer to it so we
don't accidentally try to configure it.  We handle a NULL phy pointer
correctly in most places, except fec_enet_set_pauseparam().  Fix this
too.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 6a03d7eced4d..cf805468eecc 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1875,6 +1875,9 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
+	if (!fep->phy_dev)
+		return -ENODEV;
+
 	if (pause->tx_pause != pause->rx_pause) {
 		netdev_info(ndev,
 			"hardware only support enable/disable both tx and rx");
@@ -2186,6 +2189,7 @@ fec_enet_close(struct net_device *ndev)
 
 	phy_stop(fep->phy_dev);
 	phy_disconnect(fep->phy_dev);
+	fep->phy_dev = NULL;
 
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&fep->pdev->dev);
-- 
1.8.3.1

[PATCH CFT 08/30] net: fec: stop the phy before shutting down the MAC

[Russell King]

When the network interface goes down, stop the phy to prevent further
link up status changes before taking the MAC or netif sections down.
This prevents further reception of link up events which could
potentially call fec_restart().

Since phy_stop() takes the mutex which adjust_link() runs under, we
also ensure that adjust_link() will not already be processing a link
up event.

We also need to do this when suspending as well - we don't want a
mis-timed phy state change to restart the MAC after we have stopped
it for suspend, and thus need to restart the phy when resuming.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cf805468eecc..e0a1ac1826b7 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2181,13 +2181,14 @@ fec_enet_close(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
+	phy_stop(fep->phy_dev);
+
 	/* Don't know what to do yet. */
 	napi_disable(&fep->napi);
 	fep->opened = 0;
 	netif_tx_disable(ndev);
 	fec_stop(ndev);
 
-	phy_stop(fep->phy_dev);
 	phy_disconnect(fep->phy_dev);
 	fep->phy_dev = NULL;
 
@@ -2669,6 +2670,7 @@ fec_suspend(struct device *dev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
 	if (netif_running(ndev)) {
+		phy_stop(fep->phy_dev);
 		fec_stop(ndev);
 		netif_device_detach(ndev);
 	}
@@ -2702,6 +2704,7 @@ fec_resume(struct device *dev)
 	if (netif_running(ndev)) {
 		fec_restart(ndev, fep->full_duplex);
 		netif_device_attach(ndev);
+		phy_start(fep->phy_dev);
 	}
 
 	return 0;
-- 
1.8.3.1

[PATCH CFT 09/30] net: fec: remove useless fep->opened

[Russell King]

napi_disable() waits until the NAPI processing has completed, and then
prevents any further polls.  At this point, the driver then clears
fep->opened.  The NAPI poll function uses this to stop processing in
the receive path.  Hence, it will never see this variable cleared,
because the NAPI poll has to complete before it will be cleared.

Therefore, this variable serves no purpose, so let's remove it.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec.h      | 1 -
 drivers/net/ethernet/freescale/fec_main.c | 5 -----
 2 files changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 671d080105a7..96d2a18f1b99 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -308,7 +308,6 @@ struct fec_enet_private {
 
 	struct	platform_device *pdev;
 
-	int	opened;
 	int	dev_id;
 
 	/* Phylib and MDIO interface */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e0a1ac1826b7..309aa2ff8cc9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1215,9 +1215,6 @@ fec_enet_rx(struct net_device *ndev, int budget)
 		if ((status & BD_ENET_RX_LAST) == 0)
 			netdev_err(ndev, "rcv is not +last\n");
 
-		if (!fep->opened)
-			goto rx_processing_done;
-
 		/* Check for errors. */
 		if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO |
 			   BD_ENET_RX_CR | BD_ENET_RX_OV)) {
@@ -2172,7 +2169,6 @@ fec_enet_open(struct net_device *ndev)
 	napi_enable(&fep->napi);
 	phy_start(fep->phy_dev);
 	netif_start_queue(ndev);
-	fep->opened = 1;
 	return 0;
 }
 
@@ -2185,7 +2181,6 @@ fec_enet_close(struct net_device *ndev)
 
 	/* Don't know what to do yet. */
 	napi_disable(&fep->napi);
-	fep->opened = 0;
 	netif_tx_disable(ndev);
 	fec_stop(ndev);
 
-- 
1.8.3.1

[PATCH CFT 10/30] net: fec: make rx skb handling more robust

[Russell King]

Allocate, and then map the receive skb before writing any data to the
ring descriptor or storing the skb.  When freeing the receive ring
entries, unmap and free the skb, and then clear the stored skb pointer.

This means we have ring data and skb pointer in one of two states:
either both fully setup, or nothing setup.

This simplifies the cleanup, as we can use just the skb pointer to
indicate whether the descriptor is setup, and thus avoids potentially
calling dma_unmap_single() on a DMA error value.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 309aa2ff8cc9..70853a59627a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2066,12 +2066,12 @@ static void fec_enet_free_buffers(struct net_device *ndev)
 	bdp = fep->rx_bd_base;
 	for (i = 0; i < fep->rx_ring_size; i++) {
 		skb = fep->rx_skbuff[i];
-
-		if (bdp->cbd_bufaddr)
+		fep->rx_skbuff[i] = NULL;
+		if (skb) {
 			dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
 					FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
-		if (skb)
 			dev_kfree_skb(skb);
+		}
 		bdp = fec_enet_get_nextdesc(bdp, fep);
 	}
 
@@ -2089,21 +2089,26 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 
 	bdp = fep->rx_bd_base;
 	for (i = 0; i < fep->rx_ring_size; i++) {
+		dma_addr_t addr;
+
 		skb = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
 		if (!skb) {
 			fec_enet_free_buffers(ndev);
 			return -ENOMEM;
 		}
-		fep->rx_skbuff[i] = skb;
 
-		bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, skb->data,
+		addr = dma_map_single(&fep->pdev->dev, skb->data,
 				FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
-		if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+		if (dma_mapping_error(&fep->pdev->dev, addr)) {
+			dev_kfree_skb(skb);
 			fec_enet_free_buffers(ndev);
 			if (net_ratelimit())
 				netdev_err(ndev, "Rx DMA memory map failed\n");
 			return -ENOMEM;
 		}
+
+		fep->rx_skbuff[i] = skb;
+		bdp->cbd_bufaddr = addr;
 		bdp->cbd_sc = BD_ENET_RX_EMPTY;
 
 		if (fep->bufdesc_ex) {
-- 
1.8.3.1

[PATCH CFT 11/30] net: fec: clean up transmit descriptor setup

[Russell King]

Avoid writing any state until we're certain we can proceed with the
transmission: this avoids writing mapping error address values to the
descriptors, or setting the skbuff pointer until we have successfully
mapped the skb.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 33 +++++++++++++++++--------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 70853a59627a..9c5570a3e32e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -373,6 +373,7 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev)
 	skb_frag_t *this_frag;
 	unsigned int index;
 	void *bufaddr;
+	dma_addr_t addr;
 	int i;
 
 	for (frag = 0; frag < nr_frags; frag++) {
@@ -415,15 +416,16 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev)
 				swap_buffer(bufaddr, frag_len);
 		}
 
-		bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
-						frag_len, DMA_TO_DEVICE);
-		if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+		addr = dma_map_single(&fep->pdev->dev, bufaddr, frag_len,
+				      DMA_TO_DEVICE);
+		if (dma_mapping_error(&fep->pdev->dev, addr)) {
 			dev_kfree_skb_any(skb);
 			if (net_ratelimit())
 				netdev_err(ndev, "Tx DMA memory map failed\n");
 			goto dma_mapping_error;
 		}
 
+		bdp->cbd_bufaddr = addr;
 		bdp->cbd_datlen = frag_len;
 		bdp->cbd_sc = status;
 	}
@@ -450,6 +452,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	int nr_frags = skb_shinfo(skb)->nr_frags;
 	struct bufdesc *bdp, *last_bdp;
 	void *bufaddr;
+	dma_addr_t addr;
 	unsigned short status;
 	unsigned short buflen;
 	unsigned int estatus = 0;
@@ -490,12 +493,9 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 			swap_buffer(bufaddr, buflen);
 	}
 
-	/* Push the data cache so the CPM does not get stale memory
-	 * data.
-	 */
-	bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
-					buflen, DMA_TO_DEVICE);
-	if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+	/* Push the data cache so the CPM does not get stale memory data. */
+	addr = dma_map_single(&fep->pdev->dev, bufaddr, buflen, DMA_TO_DEVICE);
+	if (dma_mapping_error(&fep->pdev->dev, addr)) {
 		dev_kfree_skb_any(skb);
 		if (net_ratelimit())
 			netdev_err(ndev, "Tx DMA memory map failed\n");
@@ -537,6 +537,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	fep->tx_skbuff[index] = skb;
 
 	bdp->cbd_datlen = buflen;
+	bdp->cbd_bufaddr = addr;
 
 	/* Send it on its way.  Tell FEC it's ready, interrupt when done,
 	 * it's the last BD of the frame, and to put the CRC on the end.
@@ -570,12 +571,12 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev,
 	struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
 	unsigned short status;
 	unsigned int estatus = 0;
+	dma_addr_t addr;
 
 	status = bdp->cbd_sc;
 	status &= ~BD_ENET_TX_STATS;
 
 	status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
-	bdp->cbd_datlen = size;
 
 	if (((unsigned long) data) & FEC_ALIGNMENT ||
 		id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
@@ -586,15 +587,17 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev,
 			swap_buffer(data, size);
 	}
 
-	bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
-					size, DMA_TO_DEVICE);
-	if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+	addr = dma_map_single(&fep->pdev->dev, data, size, DMA_TO_DEVICE);
+	if (dma_mapping_error(&fep->pdev->dev, addr)) {
 		dev_kfree_skb_any(skb);
 		if (net_ratelimit())
 			netdev_err(ndev, "Tx DMA memory map failed\n");
 		return NETDEV_TX_BUSY;
 	}
 
+	bdp->cbd_datlen = size;
+	bdp->cbd_bufaddr = addr;
+
 	if (fep->bufdesc_ex) {
 		if (skb->ip_summed == CHECKSUM_PARTIAL)
 			estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -801,7 +804,7 @@ static void fec_enet_bd_init(struct net_device *dev)
 
 		/* Initialize the BD for every fragment in the page. */
 		bdp->cbd_sc = 0;
-		if (bdp->cbd_bufaddr && fep->tx_skbuff[i]) {
+		if (fep->tx_skbuff[i]) {
 			dev_kfree_skb_any(fep->tx_skbuff[i]);
 			fep->tx_skbuff[i] = NULL;
 		}
@@ -1100,6 +1103,7 @@ fec_enet_tx(struct net_device *ndev)
 		index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep);
 
 		skb = fep->tx_skbuff[index];
+		fep->tx_skbuff[index] = NULL;
 		if (!IS_TSO_HEADER(fep, bdp->cbd_bufaddr))
 			dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
 					bdp->cbd_datlen, DMA_TO_DEVICE);
@@ -1154,7 +1158,6 @@ fec_enet_tx(struct net_device *ndev)
 
 		/* Free the sk buffer associated with this last transmit */
 		dev_kfree_skb_any(skb);
-		fep->tx_skbuff[index] = NULL;
 
 		fep->dirty_tx = bdp;
 
-- 
1.8.3.1

[PATCH CFT 12/30] net: fec: ensure fec_enet_free_buffers() properly cleans the rings

[Russell King]

Ensure that we do not double-free any allocations, and that any transmit
skbuffs are properly freed when we clean up the rings.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9c5570a3e32e..8024b7a8e7f4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2079,8 +2079,14 @@ static void fec_enet_free_buffers(struct net_device *ndev)
 	}
 
 	bdp = fep->tx_bd_base;
-	for (i = 0; i < fep->tx_ring_size; i++)
+	for (i = 0; i < fep->tx_ring_size; i++) {
 		kfree(fep->tx_bounce[i]);
+		fep->tx_bounce[i] = NULL;
+		skb = fep->tx_skbuff[i];
+		fep->tx_skbuff[i] = NULL;
+		if (skb)
+			dev_kfree_skb(skb);
+	}
 }
 
 static int fec_enet_alloc_buffers(struct net_device *ndev)
-- 
1.8.3.1

Re: [PATCH CFT 12/30] net: fec: ensure fec_enet_free_buffers() properly cleans the rings

[Sergei Shtylyov]

Hello.

On 06/27/2014 07:19 PM, Russell King wrote:

> Ensure that we do not double-free any allocations, and that any transmit
> skbuffs are properly freed when we clean up the rings.

> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/net/ethernet/freescale/fec_main.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 9c5570a3e32e..8024b7a8e7f4 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2079,8 +2079,14 @@ static void fec_enet_free_buffers(struct net_device *ndev)
>   	}
>
>   	bdp = fep->tx_bd_base;
> -	for (i = 0; i < fep->tx_ring_size; i++)
> +	for (i = 0; i < fep->tx_ring_size; i++) {
>   		kfree(fep->tx_bounce[i]);
> +		fep->tx_bounce[i] = NULL;
> +		skb = fep->tx_skbuff[i];
> +		fep->tx_skbuff[i] = NULL;
> +		if (skb)
> +			dev_kfree_skb(skb);

    dev_kfree_skb() (actually, consume_skb() that it calls) already checks 
'skb' for NULL.

WBR, Sergei

Re: [PATCH CFT 12/30] net: fec: ensure fec_enet_free_buffers() properly cleans the rings

[Russell King - ARM Linux]

On Fri, Jun 27, 2014 at 10:48:42PM +0400, Sergei Shtylyov wrote:
>    dev_kfree_skb() (actually, consume_skb() that it calls) already checks 
> 'skb' for NULL.

This patch has now been updated for that, thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH CFT 13/30] net: fec: fix missing kmalloc() failure check in fec_enet_alloc_buffers()

[Russell King]

fec_enet_alloc_buffers() assumes that kmalloc() will never fail, which
is an invalid assumption.  Fix this by implementing a common error
cleanup path, and use it to also clean up after failed bounce buffer
allocation.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 8024b7a8e7f4..5cc9eea6d8b3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2101,19 +2101,16 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 		dma_addr_t addr;
 
 		skb = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
-		if (!skb) {
-			fec_enet_free_buffers(ndev);
-			return -ENOMEM;
-		}
+		if (!skb)
+			goto err_alloc;
 
 		addr = dma_map_single(&fep->pdev->dev, skb->data,
 				FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
 		if (dma_mapping_error(&fep->pdev->dev, addr)) {
 			dev_kfree_skb(skb);
-			fec_enet_free_buffers(ndev);
 			if (net_ratelimit())
 				netdev_err(ndev, "Rx DMA memory map failed\n");
-			return -ENOMEM;
+			goto err_alloc;
 		}
 
 		fep->rx_skbuff[i] = skb;
@@ -2135,6 +2132,8 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 	bdp = fep->tx_bd_base;
 	for (i = 0; i < fep->tx_ring_size; i++) {
 		fep->tx_bounce[i] = kmalloc(FEC_ENET_TX_FRSIZE, GFP_KERNEL);
+		if (!fep->tx_bounce[i])
+			goto err_alloc;
 
 		bdp->cbd_sc = 0;
 		bdp->cbd_bufaddr = 0;
@@ -2152,6 +2151,10 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 	bdp->cbd_sc |= BD_SC_WRAP;
 
 	return 0;
+
+ err_alloc:
+	fec_enet_free_buffers(ndev);
+	return -ENOMEM;
 }
 
 static int
-- 
1.8.3.1

[PATCH CFT 14/30] net: fec: improve safety of suspend/resume/transmit timeout paths

[Russell King]

We should hold the rtnl lock while suspending, resuming or processing
the transmit timeout to ensure that nothing will interfere while we
bring up, take down or restart the hardware.  The transmit timeout
could run if we're preempted during suspend.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5cc9eea6d8b3..caf0854b7e4f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1068,8 +1068,10 @@ static void fec_enet_work(struct work_struct *work)
 
 	if (fep->delay_work.timeout) {
 		fep->delay_work.timeout = false;
+		rtnl_lock();
 		fec_restart(fep->netdev, fep->full_duplex);
 		netif_wake_queue(fep->netdev);
+		rtnl_unlock();
 	}
 
 	if (fep->delay_work.trig_tx) {
@@ -2681,11 +2683,14 @@ fec_suspend(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
+	rtnl_lock();
 	if (netif_running(ndev)) {
 		phy_stop(fep->phy_dev);
 		fec_stop(ndev);
 		netif_device_detach(ndev);
 	}
+	rtnl_unlock();
+
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&fep->pdev->dev);
 
@@ -2713,11 +2718,13 @@ fec_resume(struct device *dev)
 	if (ret)
 		goto failed_clk;
 
+	rtnl_lock();
 	if (netif_running(ndev)) {
 		fec_restart(ndev, fep->full_duplex);
 		netif_device_attach(ndev);
 		phy_start(fep->phy_dev);
 	}
+	rtnl_unlock();
 
 	return 0;
 
-- 
1.8.3.1

[PATCH CFT 15/30] net: fec: ensure fec_enet_close() copes with resume failure

[Russell King]

When the FEC is suspended, the device is detached.  Upon resume failure,
the device is left in detached mode, possibly with some of the required
clocks not running.  We don't want to be poking the device in that state
because as it may cause bus errors.

If the device is marked detached, avoid calling fec_stop().

This depends upon: "net:fec: improve safety of suspend/resume paths"

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index caf0854b7e4f..79951af81db5 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2198,10 +2198,10 @@ fec_enet_close(struct net_device *ndev)
 
 	phy_stop(fep->phy_dev);
 
-	/* Don't know what to do yet. */
 	napi_disable(&fep->napi);
 	netif_tx_disable(ndev);
-	fec_stop(ndev);
+	if (netif_device_present(ndev))
+		fec_stop(ndev);
 
 	phy_disconnect(fep->phy_dev);
 	fep->phy_dev = NULL;
-- 
1.8.3.1

[PATCH CFT 16/30] net: fec: only restart or stop the device if it is present and running

[Russell King]

Avoid calling fec_restart() or fec_stop() while the device is down
or not present (iow suspended.)

Although the ndo_timeout method will only be called if the device is
present and running, we defer this to a work queue.  The work queue
can run independently, and so needs to repeat these checks to ensure
that a restart doesn't occur after the device has been taken down or
detached for suspend.  In this case, we call fec_restart() in the
resume path, so nothing is lost.

For fec_set_features, we add a call to fec_restart() in fec_enet_open()
to ensure that the hardware is appropriate programmed when the interface
is opened.  fec_set_features() call should not occur while we're
suspended, so we don't have to worry about that case.

The adjust_link needs similar treatment - this also is called from a
work queue, which may be run independently after we have taken the
device down and detached it.  In this case, we just mark the link
down and take no further action.  We will reset things appropriately
once the device is up and running again, at which point we will receive
another adjust_link callback.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 79951af81db5..fc155ba85146 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1065,12 +1065,15 @@ static void fec_enet_work(struct work_struct *work)
 		container_of(work,
 			     struct fec_enet_private,
 			     delay_work.delay_work.work);
+	struct net_device *ndev = fep->netdev;
 
 	if (fep->delay_work.timeout) {
 		fep->delay_work.timeout = false;
 		rtnl_lock();
-		fec_restart(fep->netdev, fep->full_duplex);
-		netif_wake_queue(fep->netdev);
+		if (netif_device_present(ndev) || netif_running(ndev)) {
+			fec_restart(ndev, fep->full_duplex);
+			netif_wake_queue(ndev);
+		}
 		rtnl_unlock();
 	}
 
@@ -1504,7 +1507,14 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 		return;
 	}
 
-	if (phy_dev->link) {
+	/*
+	 * If the netdev is down, or is going down, we're not interested
+	 * in link state events, so just mark our idea of the link as down
+	 * and ignore the event.
+	 */
+	if (!netif_running(ndev) || !netif_device_present(ndev)) {
+		fep->link = 0;
+	} else if (phy_dev->link) {
 		if (!fep->link) {
 			fep->link = phy_dev->link;
 			status_change = 1;
@@ -2185,6 +2195,7 @@ fec_enet_open(struct net_device *ndev)
 		return ret;
 	}
 
+	fec_restart(ndev, fep->full_duplex);
 	napi_enable(&fep->napi);
 	phy_start(fep->phy_dev);
 	netif_start_queue(ndev);
@@ -2351,8 +2362,6 @@ static int fec_set_features(struct net_device *netdev,
 			fec_stop(netdev);
 			fec_restart(netdev, fep->phy_dev->duplex);
 			netif_wake_queue(netdev);
-		} else {
-			fec_restart(netdev, fep->phy_dev->duplex);
 		}
 	}
 
-- 
1.8.3.1

[PATCH CFT 17/30] net: fec: move calls to quiesce/resume packet processing out of fec_restart()

[Russell King]

Move the calls to quiesce and resume packet processing out of
fec_restart() to its call sites.  This is the first step in a two stage
clean up of this code, where we just move the calls out of fec_restart()
without changing them.  Not everywhere needs to issue these calls, and
not everywhere needs all of these calls to be issued.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 64 ++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index fc155ba85146..4a295b4bfb94 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -818,9 +818,10 @@ static void fec_enet_bd_init(struct net_device *dev)
 	fep->dirty_tx = bdp;
 }
 
-/* This function is called to start or restart the FEC during a link
- * change.  This only happens when switching between half and full
- * duplex.
+/*
+ * This function is called to start or restart the FEC during a link
+ * change, transmit timeout, or to reconfigure the FEC.  The network
+ * packet processing for this device must be stopped before this call.
  */
 static void
 fec_restart(struct net_device *ndev, int duplex)
@@ -834,13 +835,6 @@ fec_restart(struct net_device *ndev, int duplex)
 	u32 rcntl = OPT_FRAME_SIZE | 0x04;
 	u32 ecntl = 0x2; /* ETHEREN */
 
-	if (netif_running(ndev)) {
-		netif_device_detach(ndev);
-		napi_disable(&fep->napi);
-		netif_tx_disable(ndev);
-		netif_tx_lock_bh(ndev);
-	}
-
 	/* Whack a reset.  We should wait for this. */
 	writel(1, fep->hwp + FEC_ECNTRL);
 	udelay(10);
@@ -1009,13 +1003,6 @@ fec_restart(struct net_device *ndev, int duplex)
 
 	/* Enable interrupts we wish to service */
 	writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
-
-	if (netif_running(ndev)) {
-		netif_tx_unlock_bh(ndev);
-		netif_wake_queue(ndev);
-		napi_enable(&fep->napi);
-		netif_device_attach(ndev);
-	}
 }
 
 static void
@@ -1071,8 +1058,15 @@ static void fec_enet_work(struct work_struct *work)
 		fep->delay_work.timeout = false;
 		rtnl_lock();
 		if (netif_device_present(ndev) || netif_running(ndev)) {
+			netif_device_detach(ndev);
+			napi_disable(&fep->napi);
+			netif_tx_disable(ndev);
+			netif_tx_lock_bh(ndev);
 			fec_restart(ndev, fep->full_duplex);
+			netif_tx_unlock_bh(ndev);
 			netif_wake_queue(ndev);
+			napi_enable(&fep->napi);
+			netif_device_attach(ndev);
 		}
 		rtnl_unlock();
 	}
@@ -1529,8 +1523,17 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 		}
 
 		/* if any of the above changed restart the FEC */
-		if (status_change)
+		if (status_change) {
+			netif_device_detach(ndev);
+			napi_disable(&fep->napi);
+			netif_tx_disable(ndev);
+			netif_tx_lock_bh(ndev);
 			fec_restart(ndev, phy_dev->duplex);
+			netif_tx_unlock_bh(ndev);
+			netif_wake_queue(ndev);
+			napi_enable(&fep->napi);
+			netif_device_attach(ndev);
+		}
 	} else {
 		if (fep->link) {
 			fec_stop(ndev);
@@ -1915,8 +1918,17 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 			fec_stop(ndev);
 		phy_start_aneg(fep->phy_dev);
 	}
-	if (netif_running(ndev))
+	if (netif_running(ndev)) {
+		netif_device_detach(ndev);
+		napi_disable(&fep->napi);
+		netif_tx_disable(ndev);
+		netif_tx_lock_bh(ndev);
 		fec_restart(ndev, fep->full_duplex);
+		netif_tx_unlock_bh(ndev);
+		netif_wake_queue(ndev);
+		napi_enable(&fep->napi);
+		netif_device_attach(ndev);
+	}
 
 	return 0;
 }
@@ -2360,8 +2372,15 @@ static int fec_set_features(struct net_device *netdev,
 
 		if (netif_running(netdev)) {
 			fec_stop(netdev);
+			netif_device_detach(netdev);
+			napi_disable(&fep->napi);
+			netif_tx_disable(netdev);
+			netif_tx_lock_bh(netdev);
 			fec_restart(netdev, fep->phy_dev->duplex);
+			netif_tx_unlock_bh(netdev);
 			netif_wake_queue(netdev);
+			napi_enable(&fep->napi);
+			netif_device_attach(netdev);
 		}
 	}
 
@@ -2729,7 +2748,14 @@ fec_resume(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
+		netif_device_detach(ndev);
+		napi_disable(&fep->napi);
+		netif_tx_disable(ndev);
+		netif_tx_lock_bh(ndev);
 		fec_restart(ndev, fep->full_duplex);
+		netif_tx_unlock_bh(ndev);
+		netif_wake_queue(ndev);
+		napi_enable(&fep->napi);
 		netif_device_attach(ndev);
 		phy_start(fep->phy_dev);
 	}
-- 
1.8.3.1

[PATCH CFT 18/30] net: fec: remove inappropriate calls around fec_restart()

[Russell King]

This is the second stage to "move calls to quiesce/resume packet
processing out of fec_restart()", where we remove calls which are not
appropriate to the call site.

In the majority of cases, there is no need to detach and reattach the
interface as we are holding the queue xmit lock across the reset.  The
exception to that is in fec_resume(), where we are already detached by
the suspend function.  Here, we can remove the call to detach the
interface.

We also do not need to stop the transmit queue.  Holding the xmit lock
is enough to ensure that the transmit packet processing is not running
while we perform our task.  However, since fec_restart() always cleans
the rings, we call netif_wake_queue() (or netif_device_attach() in the
case of resume) just before dropping the xmit lock.  This prevents the
watchdog firing.

Lastly, always call napi_enable() after the device has been reattached
in the resume path so that we know that the transmit packet processing
is already in an enabled state, so we don't call netif_wake_queue()
while detached.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4a295b4bfb94..49c154af6da2 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1058,15 +1058,12 @@ static void fec_enet_work(struct work_struct *work)
 		fep->delay_work.timeout = false;
 		rtnl_lock();
 		if (netif_device_present(ndev) || netif_running(ndev)) {
-			netif_device_detach(ndev);
 			napi_disable(&fep->napi);
-			netif_tx_disable(ndev);
 			netif_tx_lock_bh(ndev);
 			fec_restart(ndev, fep->full_duplex);
-			netif_tx_unlock_bh(ndev);
 			netif_wake_queue(ndev);
+			netif_tx_unlock_bh(ndev);
 			napi_enable(&fep->napi);
-			netif_device_attach(ndev);
 		}
 		rtnl_unlock();
 	}
@@ -1524,15 +1521,12 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 
 		/* if any of the above changed restart the FEC */
 		if (status_change) {
-			netif_device_detach(ndev);
 			napi_disable(&fep->napi);
-			netif_tx_disable(ndev);
 			netif_tx_lock_bh(ndev);
 			fec_restart(ndev, phy_dev->duplex);
-			netif_tx_unlock_bh(ndev);
 			netif_wake_queue(ndev);
+			netif_tx_unlock_bh(ndev);
 			napi_enable(&fep->napi);
-			netif_device_attach(ndev);
 		}
 	} else {
 		if (fep->link) {
@@ -1919,15 +1913,12 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 		phy_start_aneg(fep->phy_dev);
 	}
 	if (netif_running(ndev)) {
-		netif_device_detach(ndev);
 		napi_disable(&fep->napi);
-		netif_tx_disable(ndev);
 		netif_tx_lock_bh(ndev);
 		fec_restart(ndev, fep->full_duplex);
-		netif_tx_unlock_bh(ndev);
 		netif_wake_queue(ndev);
+		netif_tx_unlock_bh(ndev);
 		napi_enable(&fep->napi);
-		netif_device_attach(ndev);
 	}
 
 	return 0;
@@ -2372,15 +2363,12 @@ static int fec_set_features(struct net_device *netdev,
 
 		if (netif_running(netdev)) {
 			fec_stop(netdev);
-			netif_device_detach(netdev);
 			napi_disable(&fep->napi);
-			netif_tx_disable(netdev);
 			netif_tx_lock_bh(netdev);
 			fec_restart(netdev, fep->phy_dev->duplex);
-			netif_tx_unlock_bh(netdev);
 			netif_wake_queue(netdev);
+			netif_tx_unlock_bh(netdev);
 			napi_enable(&fep->napi);
-			netif_device_attach(netdev);
 		}
 	}
 
@@ -2748,15 +2736,13 @@ fec_resume(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
-		netif_device_detach(ndev);
 		napi_disable(&fep->napi);
-		netif_tx_disable(ndev);
 		netif_tx_lock_bh(ndev);
 		fec_restart(ndev, fep->full_duplex);
+		netif_device_attach(ndev);
 		netif_tx_unlock_bh(ndev);
-		netif_wake_queue(ndev);
-		napi_enable(&fep->napi);
 		netif_device_attach(ndev);
+		napi_enable(&fep->napi);
 		phy_start(fep->phy_dev);
 	}
 	rtnl_unlock();
-- 
1.8.3.1

[PATCH CFT 19/30] net: fec: quiesce packet processing before stopping device in fec_suspend()

[Russell King]

fec_suspend() calls fec_stop() to stop the transmit ring while the
transmit packet processing is still active.  This can lead to the
transmit queue being restarted by an intervening packet queued for
transmission, or by the tx quirk timer expiring.

Fix this by disabling NAPI first, which will ensure that the NAPI
handlers are not running.  Then, take the transmit lock before
detaching the netif device.  This ensures that there are no races
with the transmit path - and also ensures that the watchdog won't
fire.

We can then safely stop the ethernet device itself, knowing that the
rest of the driver is safely shut down.

On resume, we bring the device back up in reverse order - we restart
the device, reattach the device (under the tx lock), and then enable
the NAPI handlers.

We also need to adjust the close function to cope with this new
sequence, so that it's possible to cleanly close down the driver
after the hardware fails to resume (eg, due to the regulator_enable()
or pinctrl calls in the resume path returning an error.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 49c154af6da2..7a3cfb19ae90 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2212,10 +2212,11 @@ fec_enet_close(struct net_device *ndev)
 
 	phy_stop(fep->phy_dev);
 
-	napi_disable(&fep->napi);
-	netif_tx_disable(ndev);
-	if (netif_device_present(ndev))
+	if (netif_device_present(ndev)) {
+		napi_disable(&fep->napi);
+		netif_tx_disable(ndev);
 		fec_stop(ndev);
+	}
 
 	phy_disconnect(fep->phy_dev);
 	fep->phy_dev = NULL;
@@ -2702,8 +2703,11 @@ fec_suspend(struct device *dev)
 	rtnl_lock();
 	if (netif_running(ndev)) {
 		phy_stop(fep->phy_dev);
-		fec_stop(ndev);
+		napi_disable(&fep->napi);
+		netif_tx_lock_bh(ndev);
 		netif_device_detach(ndev);
+		netif_tx_unlock_bh(ndev);
+		fec_stop(ndev);
 	}
 	rtnl_unlock();
 
@@ -2736,12 +2740,10 @@ fec_resume(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
-		napi_disable(&fep->napi);
-		netif_tx_lock_bh(ndev);
 		fec_restart(ndev, fep->full_duplex);
+		netif_tx_lock_bh(ndev);
 		netif_device_attach(ndev);
 		netif_tx_unlock_bh(ndev);
-		netif_device_attach(ndev);
 		napi_enable(&fep->napi);
 		phy_start(fep->phy_dev);
 	}
-- 
1.8.3.1

[PATCH CFT 20/30] net: fec: quiesce packet processing before stopping device in fec_set_features()

[Russell King]

fec_set_features() calls fec_stop() to stop the transmit ring while the
transmit queue is still active.  This can lead to the transmit ring
being restarted by an intervening packet queued for transmission, or
by the tx quirk timer expiring.

Fix this by disabling NAPI (which ensures that the NAPI handlers are
not running), and then take the transmit lock while we stop and
restart the adapter (which prevents new packets being queued).

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 7a3cfb19ae90..cc2fab7333fe 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2363,9 +2363,9 @@ static int fec_set_features(struct net_device *netdev,
 			fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED;
 
 		if (netif_running(netdev)) {
-			fec_stop(netdev);
 			napi_disable(&fep->napi);
 			netif_tx_lock_bh(netdev);
+			fec_stop(netdev);
 			fec_restart(netdev, fep->phy_dev->duplex);
 			netif_wake_queue(netdev);
 			netif_tx_unlock_bh(netdev);
-- 
1.8.3.1

[PATCH CFT 21/30] net: fec: quiesce packet processing before changing features

[Russell King]

Changing the features (receive checksumming) requires the hardware to be
reprogrammed, and also changes the checks in the receive packet
processing.

The current implementation has a race - fec_set_features() changes the
flags which alter the receive packet processing while the adapter is
active, and potentially receiving frames.  Only after we've modified
the software flag do we shutdown and reconfigure the hardware.

This can lead to packets being received and marked with a valid checksum
(via CHECKSUM_UNNECESSARY) when the hardware checksum validation has not
yet been enabled.

We must quiesce the device, then change the software configuration for
this feature, and then resume the device if it was previously running.

The resulting code structure also allows us to add other configuration
features in this path without having to quiesce and resume the network
interface and device.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cc2fab7333fe..466268fffadd 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2347,12 +2347,21 @@ static void fec_poll_controller(struct net_device *dev)
 }
 #endif
 
+#define FEATURES_NEED_QUIESCE NETIF_F_RXCSUM
+
 static int fec_set_features(struct net_device *netdev,
 	netdev_features_t features)
 {
 	struct fec_enet_private *fep = netdev_priv(netdev);
 	netdev_features_t changed = features ^ netdev->features;
 
+	/* Quiesce the device if necessary */
+	if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
+		napi_disable(&fep->napi);
+		netif_tx_lock_bh(netdev);
+		fec_stop(netdev);
+	}
+
 	netdev->features = features;
 
 	/* Receive checksum has been changed */
@@ -2361,16 +2370,14 @@ static int fec_set_features(struct net_device *netdev,
 			fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
 		else
 			fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED;
+	}
 
-		if (netif_running(netdev)) {
-			napi_disable(&fep->napi);
-			netif_tx_lock_bh(netdev);
-			fec_stop(netdev);
-			fec_restart(netdev, fep->phy_dev->duplex);
-			netif_wake_queue(netdev);
-			netif_tx_unlock_bh(netdev);
-			napi_enable(&fep->napi);
-		}
+	/* Resume the device after updates */
+	if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
+		fec_restart(netdev, fep->phy_dev->duplex);
+		netif_wake_queue(netdev);
+		netif_tx_unlock_bh(netdev);
+		napi_enable(&fep->napi);
 	}
 
 	return 0;
-- 
1.8.3.1

[PATCH CFT 22/30] net: fec: quiesce packet processing when taking link down in fec_enet_adjust_link()

[Russell King]

When the link goes down, the adjust_link method will be called, but
there is no synchronisation to ensure that we won't be processing some
last remaining packets via the NAPI handlers while performing a reset of
the device.

Add the necessary synchronisation to ensure that packet processing
is complete before we stop and reset the FEC.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 466268fffadd..b2b8aff3f73c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1530,7 +1530,11 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 		}
 	} else {
 		if (fep->link) {
+			napi_disable(&fep->napi);
+			netif_tx_lock_bh(ndev);
 			fec_stop(ndev);
+			netif_tx_unlock_bh(ndev);
+			napi_enable(&fep->napi);
 			fep->link = phy_dev->link;
 			status_change = 1;
 		}
-- 
1.8.3.1

[PATCH CFT 23/30] net: fec: clean up duplex mode handling

[Russell King]

Many places call fec_restart() with the second parameter being some kind
of previously saved duplex value, but only two places call it with some
other setting.  This is at odds with how the other link settings are
handled, and used to be racy before the rtnl locks were added to
fec_restart()'s various call paths.

Clean this up so all link capabilities are handled in the same way -
saved into the fec_enet_private structure, and then fec_restart() acts
on those settings.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index b2b8aff3f73c..62120e47ef5a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -824,7 +824,7 @@ static void fec_enet_bd_init(struct net_device *dev)
  * packet processing for this device must be stopped before this call.
  */
 static void
-fec_restart(struct net_device *ndev, int duplex)
+fec_restart(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	const struct platform_device_id *id_entry =
@@ -875,7 +875,7 @@ fec_restart(struct net_device *ndev, int duplex)
 	}
 
 	/* Enable MII mode */
-	if (duplex) {
+	if (fep->full_duplex == DUPLEX_FULL) {
 		/* FD enable */
 		writel(0x04, fep->hwp + FEC_X_CNTRL);
 	} else {
@@ -884,8 +884,6 @@ fec_restart(struct net_device *ndev, int duplex)
 		writel(0x0, fep->hwp + FEC_X_CNTRL);
 	}
 
-	fep->full_duplex = duplex;
-
 	/* Set MII speed */
 	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
 
@@ -1060,7 +1058,7 @@ static void fec_enet_work(struct work_struct *work)
 		if (netif_device_present(ndev) || netif_running(ndev)) {
 			napi_disable(&fep->napi);
 			netif_tx_lock_bh(ndev);
-			fec_restart(ndev, fep->full_duplex);
+			fec_restart(ndev);
 			netif_wake_queue(ndev);
 			netif_tx_unlock_bh(ndev);
 			napi_enable(&fep->napi);
@@ -1511,8 +1509,10 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 			status_change = 1;
 		}
 
-		if (fep->full_duplex != phy_dev->duplex)
+		if (fep->full_duplex != phy_dev->duplex) {
+			fep->full_duplex = phy_dev->duplex;
 			status_change = 1;
+		}
 
 		if (phy_dev->speed != fep->speed) {
 			fep->speed = phy_dev->speed;
@@ -1523,7 +1523,7 @@ static void fec_enet_adjust_link(struct net_device *ndev)
 		if (status_change) {
 			napi_disable(&fep->napi);
 			netif_tx_lock_bh(ndev);
-			fec_restart(ndev, phy_dev->duplex);
+			fec_restart(ndev);
 			netif_wake_queue(ndev);
 			netif_tx_unlock_bh(ndev);
 			napi_enable(&fep->napi);
@@ -1919,7 +1919,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 	if (netif_running(ndev)) {
 		napi_disable(&fep->napi);
 		netif_tx_lock_bh(ndev);
-		fec_restart(ndev, fep->full_duplex);
+		fec_restart(ndev);
 		netif_wake_queue(ndev);
 		netif_tx_unlock_bh(ndev);
 		napi_enable(&fep->napi);
@@ -2202,7 +2202,7 @@ fec_enet_open(struct net_device *ndev)
 		return ret;
 	}
 
-	fec_restart(ndev, fep->full_duplex);
+	fec_restart(ndev);
 	napi_enable(&fep->napi);
 	phy_start(fep->phy_dev);
 	netif_start_queue(ndev);
@@ -2378,7 +2378,7 @@ static int fec_set_features(struct net_device *netdev,
 
 	/* Resume the device after updates */
 	if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
-		fec_restart(netdev, fep->phy_dev->duplex);
+		fec_restart(netdev);
 		netif_wake_queue(netdev);
 		netif_tx_unlock_bh(netdev);
 		napi_enable(&fep->napi);
@@ -2482,7 +2482,7 @@ static int fec_enet_init(struct net_device *ndev)
 
 	ndev->hw_features = ndev->features;
 
-	fec_restart(ndev, 0);
+	fec_restart(ndev);
 
 	return 0;
 }
@@ -2751,7 +2751,7 @@ fec_resume(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
-		fec_restart(ndev, fep->full_duplex);
+		fec_restart(ndev);
 		netif_tx_lock_bh(ndev);
 		netif_device_attach(ndev);
 		netif_tx_unlock_bh(ndev);
-- 
1.8.3.1

[PATCH CFT 24/30] net: fec: better implementation of iMX6 ERR006358 quirk

[Russell King]

Using a (delayed) workqueue for ERR006358 is not correct - a work queue
is a single-trigger device.  Once the work queue has been scheduled, it
can't be re-scheduled until it has been run.  This can cause problems -
with an appropriate packet timing, we can end up with packets queued,
but not sent by the hardware, resulting in the transmit timeout firing.

Re-implement this as per the workaround detailed in the ERR006358
documentation - if there are packets waiting to be sent when we service
the transmit ring, and we see that the transmitter is not running,
kick the transmitter to run the pending entries in the ring.

Testing here with a 10Mbit half duplex link sees the resulting iperf
TCP bandwidth increase from between 1 to 2Mbps to between 8 to 9Mbps.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec.h      |  1 -
 drivers/net/ethernet/freescale/fec_main.c | 30 ++++--------------------------
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 96d2a18f1b99..17e294970207 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -259,7 +259,6 @@ struct bufdesc_ex {
 struct fec_enet_delayed_work {
 	struct delayed_work delay_work;
 	bool timeout;
-	bool trig_tx;
 };
 
 /* The FEC buffer descriptors track the ring buffers.  The rx_bd_base and
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 62120e47ef5a..d9c1ba58d6ea 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -342,22 +342,6 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 	return 0;
 }
 
-static void
-fec_enet_submit_work(struct bufdesc *bdp, struct fec_enet_private *fep)
-{
-	const struct platform_device_id *id_entry =
-				platform_get_device_id(fep->pdev);
-	struct bufdesc *bdp_pre;
-
-	bdp_pre = fec_enet_get_prevdesc(bdp, fep);
-	if ((id_entry->driver_data & FEC_QUIRK_ERR006358) &&
-	    !(bdp_pre->cbd_sc & BD_ENET_TX_READY)) {
-		fep->delay_work.trig_tx = true;
-		schedule_delayed_work(&(fep->delay_work.delay_work),
-					msecs_to_jiffies(1));
-	}
-}
-
 static int
 fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev)
 {
@@ -545,8 +529,6 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	status |= (BD_ENET_TX_READY | BD_ENET_TX_TC);
 	bdp->cbd_sc = status;
 
-	fec_enet_submit_work(bdp, fep);
-
 	/* If this was the last BD in the ring, start at the beginning again. */
 	bdp = fec_enet_get_nextdesc(last_bdp, fep);
 
@@ -735,8 +717,6 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev)
 	/* Save skb pointer */
 	fep->tx_skbuff[index] = skb;
 
-	fec_enet_submit_work(bdp, fep);
-
 	skb_tx_timestamp(skb);
 	fep->cur_tx = bdp;
 
@@ -1065,11 +1045,6 @@ static void fec_enet_work(struct work_struct *work)
 		}
 		rtnl_unlock();
 	}
-
-	if (fep->delay_work.trig_tx) {
-		fep->delay_work.trig_tx = false;
-		writel(0, fep->hwp + FEC_X_DES_ACTIVE);
-	}
 }
 
 static void
@@ -1166,7 +1141,10 @@ fec_enet_tx(struct net_device *ndev)
 				netif_wake_queue(ndev);
 		}
 	}
-	return;
+
+	/* ERR006538: Keep the transmitter going */
+	if (bdp != fep->cur_tx && readl(fep->hwp + FEC_X_DES_ACTIVE) == 0)
+		writel(0, fep->hwp + FEC_X_DES_ACTIVE);
 }
 
 /* During a receive, the cur_rx points to the current incoming buffer.
-- 
1.8.3.1

[PATCH CFT 25/30] net: fec: replace delayed work with standard work

[Russell King]

As of "better implementation of iMX6 ERR006358 quirk", we no longer have
a requirement for a delayed work.  Moreover, the work is now only used
for timeout purposes, so the timeout flag is also pointless - we set it
each time we queue the work, and the work clears it.

Replace the fec_enet_delayed_work struct with a standard work_struct,
resulting in simplified timeout handling code.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec.h      |  8 ++------
 drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------------
 2 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 17e294970207..bd53caf1c1eb 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -256,11 +256,6 @@ struct bufdesc_ex {
 #define FLAG_RX_CSUM_ENABLED	(BD_ENET_RX_ICE | BD_ENET_RX_PCR)
 #define FLAG_RX_CSUM_ERROR	(BD_ENET_RX_ICE | BD_ENET_RX_PCR)
 
-struct fec_enet_delayed_work {
-	struct delayed_work delay_work;
-	bool timeout;
-};
-
 /* The FEC buffer descriptors track the ring buffers.  The rx_bd_base and
  * tx_bd_base always point to the base of the buffer descriptors.  The
  * cur_rx and cur_tx point to the currently available buffer.
@@ -326,6 +321,8 @@ struct fec_enet_private {
 	struct	napi_struct napi;
 	int	csum_flags;
 
+	struct work_struct tx_timeout_work;
+
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
 	unsigned long last_overflow_check;
@@ -338,7 +335,6 @@ struct fec_enet_private {
 	int hwts_rx_en;
 	int hwts_tx_en;
 	struct timer_list time_keep;
-	struct fec_enet_delayed_work delay_work;
 	struct regulator *reg_phy;
 };
 
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d9c1ba58d6ea..05982fb42000 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1020,31 +1020,25 @@ fec_timeout(struct net_device *ndev)
 
 	ndev->stats.tx_errors++;
 
-	fep->delay_work.timeout = true;
-	schedule_delayed_work(&(fep->delay_work.delay_work), 0);
+	schedule_work(&fep->tx_timeout_work);
 }
 
-static void fec_enet_work(struct work_struct *work)
+static void fec_enet_timeout_work(struct work_struct *work)
 {
 	struct fec_enet_private *fep =
-		container_of(work,
-			     struct fec_enet_private,
-			     delay_work.delay_work.work);
+		container_of(work, struct fec_enet_private, tx_timeout_work);
 	struct net_device *ndev = fep->netdev;
 
-	if (fep->delay_work.timeout) {
-		fep->delay_work.timeout = false;
-		rtnl_lock();
-		if (netif_device_present(ndev) || netif_running(ndev)) {
-			napi_disable(&fep->napi);
-			netif_tx_lock_bh(ndev);
-			fec_restart(ndev);
-			netif_wake_queue(ndev);
-			netif_tx_unlock_bh(ndev);
-			napi_enable(&fep->napi);
-		}
-		rtnl_unlock();
+	rtnl_lock();
+	if (netif_device_present(ndev) || netif_running(ndev)) {
+		napi_disable(&fep->napi);
+		netif_tx_lock_bh(ndev);
+		fec_restart(ndev);
+		netif_wake_queue(ndev);
+		netif_tx_unlock_bh(ndev);
+		napi_enable(&fep->napi);
 	}
+	rtnl_unlock();
 }
 
 static void
@@ -2643,7 +2637,7 @@ fec_probe(struct platform_device *pdev)
 	if (fep->bufdesc_ex && fep->ptp_clock)
 		netdev_info(ndev, "registered PHC device %d\n", fep->dev_id);
 
-	INIT_DELAYED_WORK(&(fep->delay_work.delay_work), fec_enet_work);
+	INIT_WORK(&fep->tx_timeout_work, fec_enet_timeout_work);
 	return 0;
 
 failed_register:
@@ -2668,7 +2662,7 @@ fec_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
-	cancel_delayed_work_sync(&(fep->delay_work.delay_work));
+	cancel_work_sync(&fep->tx_timeout_work);
 	unregister_netdev(ndev);
 	fec_enet_mii_remove(fep);
 	del_timer_sync(&fep->time_keep);
-- 
1.8.3.1

[PATCH CFT 26/30] net: fec: clear receive interrupts before processing a packet

[Russell King]

Clear any pending receive interrupt before we process a pending packet.
This helps to avoid any spurious interrupts being raised after we have
fully cleaned the receive ring, while still allowing an interrupt to be
raised if we receive another packet.

The position of this is critical: we must do this prior to reading the
next packet status to avoid potentially dropping an interrupt when a
packet is still pending.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 05982fb42000..e15cb9bc2eae 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1184,6 +1184,8 @@ fec_enet_rx(struct net_device *ndev, int budget)
 		if ((status & BD_ENET_RX_LAST) == 0)
 			netdev_err(ndev, "rcv is not +last\n");
 
+		writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT);
+
 		/* Check for errors. */
 		if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO |
 			   BD_ENET_RX_CR | BD_ENET_RX_OV)) {
-- 
1.8.3.1

Re: [PATCH CFT 26/30] net: fec: clear receive interrupts before processing a packet

[Russell King - ARM Linux]

On Fri, Jun 27, 2014 at 04:21:06PM +0100, Russell King wrote:
> Clear any pending receive interrupt before we process a pending packet.
> This helps to avoid any spurious interrupts being raised after we have
> fully cleaned the receive ring, while still allowing an interrupt to be
> raised if we receive another packet.
> 
> The position of this is critical: we must do this prior to reading the
> next packet status to avoid potentially dropping an interrupt when a
> packet is still pending.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

While there is a better algorithm for this, in order to implement that,
I also need to have feedback from the transmit reaping whether any work
was done there.

I have elected to implement that after a patch which adds byte queue
limits to the transmit side (which is in my second half of patches)
otherwise I need to do quite a bit of time consuming patch shuffling
and fixing to get it into the first half (at the moment the two halves
are separated by a complete revert of Andy's work from the last window.
The point of this half is to reduce the number of patches I'm currently
carrying so I can sensibly sort out the second half.)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH CFT 27/30] net: fec: reorder ethtool ops to match order in struct declaration

[Russell King]

This allows us to merge two separate preprocessor conditionals together.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e15cb9bc2eae..9af9350565be 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2014,21 +2014,19 @@ static int fec_enet_nway_reset(struct net_device *dev)
 }
 
 static const struct ethtool_ops fec_enet_ethtool_ops = {
-#if !defined(CONFIG_M5272)
-	.get_pauseparam		= fec_enet_get_pauseparam,
-	.set_pauseparam		= fec_enet_set_pauseparam,
-#endif
 	.get_settings		= fec_enet_get_settings,
 	.set_settings		= fec_enet_set_settings,
 	.get_drvinfo		= fec_enet_get_drvinfo,
-	.get_link		= ethtool_op_get_link,
-	.get_ts_info		= fec_enet_get_ts_info,
 	.nway_reset		= fec_enet_nway_reset,
+	.get_link		= ethtool_op_get_link,
 #ifndef CONFIG_M5272
-	.get_ethtool_stats	= fec_enet_get_ethtool_stats,
+	.get_pauseparam		= fec_enet_get_pauseparam,
+	.set_pauseparam		= fec_enet_set_pauseparam,
 	.get_strings		= fec_enet_get_strings,
+	.get_ethtool_stats	= fec_enet_get_ethtool_stats,
 	.get_sset_count		= fec_enet_get_sset_count,
 #endif
+	.get_ts_info		= fec_enet_get_ts_info,
 };
 
 static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
-- 
1.8.3.1

[PATCH CFT 28/30] net: fec: add support for dumping transmit ring on timeout

[Russell King]

When we timeout on transmit, it would be useful to dump the transmit
ring, so we can see the ring state.  This can be helpful to diagnose
the cause of transmit timeouts.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9af9350565be..1e0eeb43ff6a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -320,6 +320,27 @@ static void *swap_buffer(void *bufaddr, int len)
 	return bufaddr;
 }
 
+static void fec_dump(struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct bufdesc *bdp = fep->tx_bd_base;
+	unsigned int index = 0;
+
+	netdev_info(ndev, "TX ring dump\n");
+	pr_info("Nr     SC     addr       len  SKB\n");
+
+	do {
+		pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p\n",
+			index,
+			bdp == fep->cur_tx ? 'S' : ' ',
+			bdp == fep->dirty_tx ? 'H' : ' ',
+			bdp->cbd_sc, bdp->cbd_bufaddr, bdp->cbd_datlen,
+			fep->tx_skbuff[index]);
+		bdp = fec_enet_get_nextdesc(bdp, fep);
+		index++;
+	} while (bdp != fep->tx_bd_base);
+}
+
 static inline bool is_ipv4_pkt(struct sk_buff *skb)
 {
 	return skb->protocol == htons(ETH_P_IP) && ip_hdr(skb)->version == 4;
@@ -1018,6 +1039,8 @@ fec_timeout(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
+	fec_dump(ndev);
+
 	ndev->stats.tx_errors++;
 
 	schedule_work(&fep->tx_timeout_work);
-- 
1.8.3.1

[PATCH CFT 29/30] net: fec: remove useless status check in tx reap path

[Russell King]

Remove a useless status check in the transmit reap path - we have
already checked that the BD_ENET_TX_READY bit is clear, and as the
hardware only ever clears this bit, there is no way this test can ever
be true.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 1e0eeb43ff6a..916b9ab97da4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1133,9 +1133,6 @@ fec_enet_tx(struct net_device *ndev)
 			skb_tstamp_tx(skb, &shhwtstamps);
 		}
 
-		if (status & BD_ENET_TX_READY)
-			netdev_err(ndev, "HEY! Enet xmit interrupt and TX_READY\n");
-
 		/* Deferred means some collisions occurred during transmit,
 		 * but we eventually sent the packet OK.
 		 */
-- 
1.8.3.1

[PATCH CFT 30/30] net: fec: consolidate hwtstamp implementation

[Russell King]

Both transmit and receive use the same infrastructure for calculating
the packet timestamp.  Rather than duplicating the code, provide a
function to do this common work.  Model this function in the Intel
e1000e version which avoids calling ns_to_ktime() within the spinlock;
the spinlock is critical for timecounter_cyc2time() but not
ns_to_ktime().

Acked-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/freescale/fec_main.c | 37 ++++++++++++++++---------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 916b9ab97da4..dc925136ff01 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1065,6 +1065,21 @@ static void fec_enet_timeout_work(struct work_struct *work)
 }
 
 static void
+fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts,
+	struct skb_shared_hwtstamps *hwtstamps)
+{
+	unsigned long flags;
+	u64 ns;
+
+	spin_lock_irqsave(&fep->tmreg_lock, flags);
+	ns = timecounter_cyc2time(&fep->tc, ts);
+	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
+
+	memset(hwtstamps, 0, sizeof(*hwtstamps));
+	hwtstamps->hwtstamp = ns_to_ktime(ns);
+}
+
+static void
 fec_enet_tx(struct net_device *ndev)
 {
 	struct	fec_enet_private *fep;
@@ -1122,14 +1137,9 @@ fec_enet_tx(struct net_device *ndev)
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
 			fep->bufdesc_ex) {
 			struct skb_shared_hwtstamps shhwtstamps;
-			unsigned long flags;
 			struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
 
-			memset(&shhwtstamps, 0, sizeof(shhwtstamps));
-			spin_lock_irqsave(&fep->tmreg_lock, flags);
-			shhwtstamps.hwtstamp = ns_to_ktime(
-				timecounter_cyc2time(&fep->tc, ebdp->ts));
-			spin_unlock_irqrestore(&fep->tmreg_lock, flags);
+			fec_enet_hwtstamp(fep, ebdp->ts, &shhwtstamps);
 			skb_tstamp_tx(skb, &shhwtstamps);
 		}
 
@@ -1288,18 +1298,9 @@ fec_enet_rx(struct net_device *ndev, int budget)
 			skb->protocol = eth_type_trans(skb, ndev);
 
 			/* Get receive timestamp from the skb */
-			if (fep->hwts_rx_en && fep->bufdesc_ex) {
-				struct skb_shared_hwtstamps *shhwtstamps =
-							    skb_hwtstamps(skb);
-				unsigned long flags;
-
-				memset(shhwtstamps, 0, sizeof(*shhwtstamps));
-
-				spin_lock_irqsave(&fep->tmreg_lock, flags);
-				shhwtstamps->hwtstamp = ns_to_ktime(
-				    timecounter_cyc2time(&fep->tc, ebdp->ts));
-				spin_unlock_irqrestore(&fep->tmreg_lock, flags);
-			}
+			if (fep->hwts_rx_en && fep->bufdesc_ex)
+				fec_enet_hwtstamp(fep, ebdp->ts,
+						  skb_hwtstamps(skb));
 
 			if (fep->bufdesc_ex &&
 			    (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) {
-- 
1.8.3.1

RE: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[fugang.duan]

From: Russell King - ARM Linux <linux@arm.linux.org.uk> Data: Friday, June 27, 2014 11:16 PM
>To: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
>Cc: Duan Fugang-B38611
>Subject: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches
>
>This is v2 of my initial round of patches (roughly half of my total patch
>set) for the Freescale FEC driver.
>
>I'm sending this set out for comments and testing.  So far, I have had
>only one ack for one patch in this series, this is pretty poor, so I'm now
>sending it with a CFT tag instead.
>
>I haven't changed too much - I've fixed the bug which Andy spotted in the
>transmit handling, but otherwise the patches are much the same.
>
>The remainder of the text is as per my previous message, but with updated
>diffstat:
>
>One of my motivations for only sending half is to get this half into a
>state where DaveM is happy to merge it before I sort out the remainder.
>There's quite a lot here, so bear with me on this.
>
>I've tried to sort the fixes before the cleanups as best I can, but
>reordering the series is a full-time job due to it's size - it's taken
>from Monday until now to get this far with it, so I'm hoping that there
>won't be any "you should rearrange the patches as X" comments.
>
>While the original series was well tested during it's original development,
>including with performance tests on each patch, that testing and
>validation has been lost due to the changes during the last merge window,
>and subsequent rebasing and updating of the patches.
>
>The series is based on v3.16-rc1, and as such, I have added Andy's
>checksum fix to the start of the series.  This patch is not strictly part
>of the RFC, but is included because my complete patch series needs to
>account for that change.
>
>Some of these patches are to fix theoretical problems in the driver (ones
>which have been found via a review of the code) others address real
>observable problems (such as poor half-duplex performance.)
>
>Towards the end of this series, I have included a patch which was
>initially at the beginning of the series for dumping out the state of the
>transmit ring, which is very useful to debug transmit problems.
>This was acceptable when the transmit ring was between 16 and 128
>descriptors, but during the recent merge window, this was increased to
>512 descriptors, so it will now print around 512 lines to the kernel
>message log on transmit timeout.  I'm not entirely convinced this is a
>good idea - maybe it should become optional, or maybe the timed-out ring
>status should be available via debugfs, but that's a problem to retrieve
>if you're running NFS rootfs and the timeout doesn't recover properly.
>
>This series of patches is also available from the following *unstable* git
>branch - unstable as it's provided for convenience so you don't have to
>apply all these patches individually, unstable as the patches need to have
>attributations added, unstable because DaveM will probably want to apply
>them as patches to his tree:
>
>  git://ftp.arm.linux.org.uk/~rmk/linux-arm.git fec-testing
>
> drivers/net/ethernet/freescale/fec.h      |  10 +-
> drivers/net/ethernet/freescale/fec_main.c | 382 +++++++++++++++++--------
>-----
> 2 files changed, 224 insertions(+), 168 deletions(-)
>

Hi, Russell,

The V2 patch set are fine for me. Thanks for your clean for the driver.

Acked-by: Fugang Duan <B38611@freescale.com>

Thanks,
Andy

Re: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[Nathan Lynch]

[apologies if this is a duplicate, I got a weird SMTP error from my
organization's mail server when I tried to send this yesterday]

On 06/27/2014 10:15 AM, Russell King - ARM Linux wrote:
> This is v2 of my initial round of patches (roughly half of my total
> patch set) for the Freescale FEC driver.
> 
> I'm sending this set out for comments and testing.  So far, I have
> had only one ack for one patch in this series, this is pretty poor,
> so I'm now sending it with a CFT tag instead.

FWIW I've given your fec-testing branch some testing on BD-SL-i.MX6
(Sabre Lite) with 3.16-rc2 (-rc1 has some issue with detecting the mmc).
 Mainly running glibc 'make check' with SSH, NFSv4, IPv4 on a gigabit
switch.  This workload wasn't exhibiting problems before your patches
and it does not appear to be regressed by them.

I wanted to test it because I've noticed hard-to-characterize sluggish
interactive response in SSH sessions on this system.  Like key echo
takes 0.2 seconds too long... sometimes.  I guess it could be anything,
but I haven't encountered it yet with your patches.

Re: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[Russell King - ARM Linux]

On Tue, Jul 01, 2014 at 09:21:28AM -0500, Nathan Lynch wrote:
> [apologies if this is a duplicate, I got a weird SMTP error from my
> organization's mail server when I tried to send this yesterday]
> 
> On 06/27/2014 10:15 AM, Russell King - ARM Linux wrote:
> > This is v2 of my initial round of patches (roughly half of my total
> > patch set) for the Freescale FEC driver.
> > 
> > I'm sending this set out for comments and testing.  So far, I have
> > had only one ack for one patch in this series, this is pretty poor,
> > so I'm now sending it with a CFT tag instead.
> 
> FWIW I've given your fec-testing branch some testing on BD-SL-i.MX6
> (Sabre Lite) with 3.16-rc2 (-rc1 has some issue with detecting the mmc).
>  Mainly running glibc 'make check' with SSH, NFSv4, IPv4 on a gigabit
> switch.  This workload wasn't exhibiting problems before your patches
> and it does not appear to be regressed by them.
> 
> I wanted to test it because I've noticed hard-to-characterize sluggish
> interactive response in SSH sessions on this system.  Like key echo
> takes 0.2 seconds too long... sometimes.  I guess it could be anything,
> but I haven't encountered it yet with your patches.

Thanks for testing.

Can I add a tested-by tag for you for those patches?

It is still possible for that sluggishness to occur with these patches,
more so as the transmit ring is now soo large - with 512 entries in
the ring, it is theoretically possible to have up to 512 * 1514 = 757KB
of data queued in the ring irrespective of the wire speed.

If the transmit ring has a significant number of large packets queued,
and then your interactive ssh packet comes along, it will be tacked
on the end of the queue, and it will have to wait for all the packets
ahead of it to be transmitted first.

This is where the byte queue limits really help - it provides more
control over the amount of data in the transmit ring, and is designed
to help prevent this kind of issue.

I have such a patch, but it's based upon the work I did on the driver
prior to the merge window, which does not take account of the changes
which happened during the window - it's part of my "second half" of
this series which will be worked on now that the first half is mostly
out of the way.

Thanks again for testing.

David - how would you like to take the patches?  Shall I just re-send
them without the CFT tag To: you?  Do you have anything other than
the IPv6 fix queued up at the moment for this driver?  Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[Nathan Lynch]

On 07/01/2014 09:34 AM, Russell King - ARM Linux wrote:
> On Tue, Jul 01, 2014 at 09:21:28AM -0500, Nathan Lynch wrote:
>> [apologies if this is a duplicate, I got a weird SMTP error from my
>> organization's mail server when I tried to send this yesterday]
>>
>> On 06/27/2014 10:15 AM, Russell King - ARM Linux wrote:
>>> This is v2 of my initial round of patches (roughly half of my total
>>> patch set) for the Freescale FEC driver.
>>>
>>> I'm sending this set out for comments and testing.  So far, I have
>>> had only one ack for one patch in this series, this is pretty poor,
>>> so I'm now sending it with a CFT tag instead.
>>
>> FWIW I've given your fec-testing branch some testing on BD-SL-i.MX6
>> (Sabre Lite) with 3.16-rc2 (-rc1 has some issue with detecting the mmc).
>>  Mainly running glibc 'make check' with SSH, NFSv4, IPv4 on a gigabit
>> switch.  This workload wasn't exhibiting problems before your patches
>> and it does not appear to be regressed by them.
>>
>> I wanted to test it because I've noticed hard-to-characterize sluggish
>> interactive response in SSH sessions on this system.  Like key echo
>> takes 0.2 seconds too long... sometimes.  I guess it could be anything,
>> but I haven't encountered it yet with your patches.
> 
> Thanks for testing.
> 
> Can I add a tested-by tag for you for those patches?

Of course.

Tested-by: Nathan Lynch <nathan_lynch@mentor.com>

Re: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[Russell King - ARM Linux]

On Fri, Jun 27, 2014 at 04:15:42PM +0100, Russell King - ARM Linux wrote:
> This is v2 of my initial round of patches (roughly half of my total
> patch set) for the Freescale FEC driver.

As there have been no further comments on this series, and I have
Andy's ack for all the patches, I'm intending to send the set to
David in the next few days.

David, are you happy for me to send you 29 patches, or I shall supply
a pull request against an -rc kernel (which will be on
ftp.arm.linux.org.uk).  I don't have a kernel.org account.

Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH CFT 00/30] Initial round of Freescale FEC ethernet patches

[David Miller]

From: Russell King - ARM Linux <linux@arm.linux.org.uk>
Date: Mon, 7 Jul 2014 22:41:33 +0100

> David, are you happy for me to send you 29 patches, or I shall supply
> a pull request against an -rc kernel (which will be on
> ftp.arm.linux.org.uk).  I don't have a kernel.org account.

Please break it down into smaller groups of patches, say ~15.