From b44592ffb8e0da32f1d17f1586a9d2c59dd152a1 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:34 +0100 Subject: [PATCH 01/12] net: fec: iMX6 FEC does not support half-duplex gigabit 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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 From 9671a42e4508db8a0196110b098fd8550304ab14 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:39 +0100 Subject: [PATCH 02/12] net: fec: fix ethtool set_pauseparam duplex bug 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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; } From 7a16807ce149d4e9cd53facca0cff6a4a647b6be Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:44 +0100 Subject: [PATCH 03/12] net: fec: fix interrupt handling races While running: while :; do iperf -c -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 Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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); From b49cd504c46dd729011049c91cf11ea72df01aef Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:49 +0100 Subject: [PATCH 04/12] net: fec: use netif_tx_disable() rather than netif_stop_queue() 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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) { From 635cf17ce220aa4fef93528775646e0a181b2dc2 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:54 +0100 Subject: [PATCH 05/12] net: fec: remove checking for NULL phy_dev in fec_enet_close() 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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); From 0b146ca8d467c1dc6ad50628089d183608a47e40 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:59 +0100 Subject: [PATCH 06/12] net: fec: ensure that a disconnected phy isn't configured 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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); From d76cfae9674f325de35ff23ab07ff9c7d809de4a Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:04 +0100 Subject: [PATCH 07/12] net: fec: stop the phy before shutting down the MAC 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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; From 5d165c5543fbcbd26e443ee501063decb4ef73b4 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:09 +0100 Subject: [PATCH 08/12] net: fec: remove useless fep->opened 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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); From 730ee3602f300b5133717048859358a02251322f Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:14 +0100 Subject: [PATCH 09/12] net: fec: make rx skb handling more robust 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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) { From d6bf31431b533629ad65feb954c29bbf5696e490 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:19 +0100 Subject: [PATCH 10/12] net: fec: clean up transmit descriptor setup 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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; From 8b7c9efa018f96e58fa379ddb8c03f002c4f6a84 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:25 +0100 Subject: [PATCH 11/12] net: fec: ensure fec_enet_free_buffers() properly cleans the rings Ensure that we do not double-free any allocations, and that any transmit skbuffs are properly freed when we clean up the rings. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 9c5570a3e32e..5499bd8ad0a5 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2079,8 +2079,13 @@ 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; + dev_kfree_skb(skb); + } } static int fec_enet_alloc_buffers(struct net_device *ndev) From ffdce2cc6a18e3460fd31a399934004bf4cf6539 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:30 +0100 Subject: [PATCH 12/12] net: fec: fix missing kmalloc() failure check in fec_enet_alloc_buffers() 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- 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 5499bd8ad0a5..f43c388e2eb9 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2100,19 +2100,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; @@ -2134,6 +2131,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; @@ -2151,6 +2150,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