From 62a02c98ce03bb214009509a4802b7b63f59621c Mon Sep 17 00:00:00 2001 From: Fugang Duan Date: Wed, 18 Jun 2014 08:33:52 +0800 Subject: net: fec: Don't clear IPV6 header checksum field when IP accelerator enable 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. Cc: Russell King Reported-and-tested-by: Russell King Signed-off-by: Fugang Duan Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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; -- cgit v1.2.3 From b44592ffb8e0da32f1d17f1586a9d2c59dd152a1 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:34 +0100 Subject: 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(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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 -- cgit v1.2.3 From 9671a42e4508db8a0196110b098fd8550304ab14 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:39 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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; } -- cgit v1.2.3 From 7a16807ce149d4e9cd53facca0cff6a4a647b6be Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:44 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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); -- cgit v1.2.3 From b49cd504c46dd729011049c91cf11ea72df01aef Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:49 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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) { -- cgit v1.2.3 From 635cf17ce220aa4fef93528775646e0a181b2dc2 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:54 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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); -- cgit v1.2.3 From 0b146ca8d467c1dc6ad50628089d183608a47e40 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:22:59 +0100 Subject: 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(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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); -- cgit v1.2.3 From d76cfae9674f325de35ff23ab07ff9c7d809de4a Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:04 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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; -- cgit v1.2.3 From 5d165c5543fbcbd26e443ee501063decb4ef73b4 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:09 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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); -- cgit v1.2.3 From 730ee3602f300b5133717048859358a02251322f Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:14 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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) { -- cgit v1.2.3 From d6bf31431b533629ad65feb954c29bbf5696e490 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:19 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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; -- cgit v1.2.3 From 8b7c9efa018f96e58fa379ddb8c03f002c4f6a84 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:25 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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) -- cgit v1.2.3 From ffdce2cc6a18e3460fd31a399934004bf4cf6539 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 00:23:30 +0100 Subject: 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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 -- cgit v1.2.3 From da1774e5f1974294dcfc2c08363bb103e769d302 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:39:57 +0100 Subject: net: fec: improve safety of suspend/resume/transmit timeout paths 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. 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, 7 insertions(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index f43c388e2eb9..1cd71a8d9996 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) { @@ -2680,11 +2682,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); @@ -2712,11 +2717,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; -- cgit v1.2.3 From 8bbbd3c19c469a1c6b8e97e9f5a083d029657be5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:02 +0100 Subject: net: fec: ensure fec_enet_close() copes with resume failure 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" 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 1cd71a8d9996..03785cd14b7c 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2197,10 +2197,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; -- cgit v1.2.3 From 8ce5624f5bbb991ae3aea2fcdae86d808351e173 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:07 +0100 Subject: net: fec: only restart or stop the device if it is present and running 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 03785cd14b7c..bfb2bb00c493 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; @@ -2184,6 +2194,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); @@ -2350,8 +2361,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); } } -- cgit v1.2.3 From dbc64a8ea231271c580b5a570bc48cf42203fe0e Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:12 +0100 Subject: net: fec: move calls to quiesce/resume packet processing out of fec_restart() 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 64 ++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 19 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index bfb2bb00c493..b71b7491f38f 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; } @@ -2359,8 +2371,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); } } @@ -2728,7 +2747,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); } -- cgit v1.2.3 From 6af42d420bcfa0c837911bd5b518fd4a3cfa4fe4 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:18 +0100 Subject: net: fec: remove inappropriate calls around fec_restart() 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b71b7491f38f..25a9f7fb30da 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; @@ -2371,15 +2362,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); } } @@ -2747,15 +2735,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(); -- cgit v1.2.3 From 31a6de34f3daa9b1f412931befd9f82fd4a1b968 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:23 +0100 Subject: net: fec: quiesce packet processing before stopping device in fec_suspend() 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.) Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 25a9f7fb30da..fc9f6f465e7a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2211,10 +2211,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; @@ -2701,8 +2702,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(); @@ -2735,12 +2739,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); } -- cgit v1.2.3 From 9a7ba4381af4defa7834c73c8a015532c06f4e8e Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:28 +0100 Subject: net: fec: quiesce packet processing before stopping device in fec_set_features() 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). 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(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index fc9f6f465e7a..2945cf6e65cf 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2362,9 +2362,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); -- cgit v1.2.3 From 8506fa1d8eb36fbf96ab5e2a7e4b87826853f1be Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:33 +0100 Subject: net: fec: quiesce packet processing before changing features 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 2945cf6e65cf..124d3c5f8046 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2346,12 +2346,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 */ @@ -2360,16 +2369,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; -- cgit v1.2.3 From f208ce10046888052d3a5e9fc88b7c1b819877d2 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:38 +0100 Subject: net: fec: quiesce packet processing when taking link down in fec_enet_adjust_link() 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. 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(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 124d3c5f8046..0186fec1f7f9 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; } -- cgit v1.2.3 From ef83337d138354e3d1e32d9f929e0afefe5552aa Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:43 +0100 Subject: net: fec: clean up duplex mode handling 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 0186fec1f7f9..9d82d915b06d 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); @@ -2201,7 +2201,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); @@ -2377,7 +2377,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); @@ -2481,7 +2481,7 @@ static int fec_enet_init(struct net_device *ndev) ndev->hw_features = ndev->features; - fec_restart(ndev, 0); + fec_restart(ndev); return 0; } @@ -2750,7 +2750,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); -- cgit v1.2.3 From ccea2968398c959493cdce503ae94206d2026fbe Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:01:38 +0100 Subject: net: fec: better implementation of iMX6 ERR006358 quirk 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. 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 | 30 ++++-------------------------- 2 files changed, 4 insertions(+), 27 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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 9d82d915b06d..8cfc3a3de7fb 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. -- cgit v1.2.3 From 36cdc743a320e78a5d12ca9765ec0f7d9f07b1f5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:01:44 +0100 Subject: net: fec: replace delayed work with standard work 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec.h | 8 ++------ drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------------ 2 files changed, 16 insertions(+), 26 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') 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 8cfc3a3de7fb..b2ae7e706d5e 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 @@ -2642,7 +2636,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: @@ -2667,7 +2661,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); -- cgit v1.2.3 From db3421c114cfa6326861bc95e604785f4c64293b Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:01:49 +0100 Subject: net: fec: clear receive interrupts before processing a packet 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. 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, 2 insertions(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b2ae7e706d5e..79d578d6db8a 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)) { -- cgit v1.2.3 From c1d7c48ff769021fab0591cfe331c0061186cb31 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:01:54 +0100 Subject: net: fec: reorder ethtool ops to match order in struct declaration This allows us to merge two separate preprocessor conditionals together. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 79d578d6db8a..c6b1bf797a39 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) -- cgit v1.2.3 From 344756f6e36b056ed361eedbd68244b108bdb1c6 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:01:59 +0100 Subject: net: fec: add support for dumping transmit ring on timeout 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c6b1bf797a39..09fcdc768931 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); -- cgit v1.2.3 From 96018f52c532b69424d354a848ac7812136650f6 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:02:04 +0100 Subject: net: fec: remove useless status check in tx reap path 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. Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 09fcdc768931..8679c919419c 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. */ -- cgit v1.2.3 From bfd4ecdd87d350e19457fe0d02fa1e046774c44e Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 13:02:09 +0100 Subject: net: fec: consolidate hwtstamp implementation 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 Acked-by: Fugang Duan Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 37 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 8679c919419c..e0efb212223f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1064,6 +1064,21 @@ static void fec_enet_timeout_work(struct work_struct *work) rtnl_unlock(); } +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) { @@ -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)) { -- cgit v1.2.3 From dd66d386552ad3687530c1bd0e001e96a060cc0f Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Thu, 24 Jul 2014 18:24:01 -0300 Subject: fec: Simplify the PM related hooks Get rid of the CONFIG_PM_SLEEP ifdef by annotating the suspend/resume functions with '__maybe_unused' in order to keep the code simpler and shorter. While at it, declare the suspend/resume functions in a single line. Signed-off-by: Fabio Estevam Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/fec_main.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index e0efb212223f..66fe1f672499 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2696,9 +2696,7 @@ fec_drv_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP -static int -fec_suspend(struct device *dev) +static int __maybe_unused fec_suspend(struct device *dev) { struct net_device *ndev = dev_get_drvdata(dev); struct fec_enet_private *fep = netdev_priv(ndev); @@ -2723,8 +2721,7 @@ fec_suspend(struct device *dev) return 0; } -static int -fec_resume(struct device *dev) +static int __maybe_unused fec_resume(struct device *dev) { struct net_device *ndev = dev_get_drvdata(dev); struct fec_enet_private *fep = netdev_priv(ndev); @@ -2759,7 +2756,6 @@ failed_clk: regulator_disable(fep->reg_phy); return ret; } -#endif /* CONFIG_PM_SLEEP */ static SIMPLE_DEV_PM_OPS(fec_pm_ops, fec_suspend, fec_resume); -- cgit v1.2.3 From 407066f8f3716ca2880a08c48dbab507ae2ec8d9 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Mon, 11 Aug 2014 17:35:33 +0200 Subject: net: fec: Support phys probed from devicetree and fixed-link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds support for specifying the phy to be used with the fec in the devicetree using the standard phy-handle property and also supports fixed-link. Signed-off-by: Sascha Hauer Signed-off-by: Uwe Kleine-König Signed-off-by: David S. Miller --- Documentation/devicetree/bindings/net/fsl-fec.txt | 29 ++++++++- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 76 ++++++++++++++++------- 3 files changed, 84 insertions(+), 22 deletions(-) (limited to 'drivers/net/ethernet/freescale/fec_main.c') diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 6bc84adb10c0..8a2c7b55ec16 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -12,7 +12,14 @@ Optional properties: only if property "phy-reset-gpios" is available. Missing the property will have the duration be 1 millisecond. Numbers greater than 1000 are invalid and 1 millisecond will be used instead. -- phy-supply: regulator that powers the Ethernet PHY. +- phy-supply : regulator that powers the Ethernet PHY. +- phy-handle : phandle to the PHY device connected to this device. +- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory. + Use instead of phy-handle. + +Optional subnodes: +- mdio : specifies the mdio bus in the FEC, used as a container for phy nodes + according to phy.txt in the same directory Example: @@ -25,3 +32,23 @@ ethernet@83fec000 { local-mac-address = [00 04 9F 01 1B B9]; phy-supply = <®_fec_supply>; }; + +Example with phy specified: + +ethernet@83fec000 { + compatible = "fsl,imx51-fec", "fsl,imx27-fec"; + reg = <0x83fec000 0x4000>; + interrupts = <87>; + phy-mode = "mii"; + phy-reset-gpios = <&gpio2 14 0>; /* GPIO2_14 */ + local-mac-address = [00 04 9F 01 1B B9]; + phy-supply = <®_fec_supply>; + phy-handle = <ðphy>; + mdio { + ethphy: ethernet-phy@6 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <6>; + max-speed = <100>; + }; + }; +}; diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index bd53caf1c1eb..9f7fa644a397 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -310,6 +310,7 @@ struct fec_enet_private { int mii_timeout; uint phy_speed; phy_interface_t phy_interface; + struct device_node *phy_node; int link; int full_duplex; int speed; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 66fe1f672499..4f87dffcb9b2 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include @@ -1648,29 +1649,37 @@ static int fec_enet_mii_probe(struct net_device *ndev) fep->phy_dev = NULL; - /* check for attached phy */ - for (phy_id = 0; (phy_id < PHY_MAX_ADDR); phy_id++) { - if ((fep->mii_bus->phy_mask & (1 << phy_id))) - continue; - if (fep->mii_bus->phy_map[phy_id] == NULL) - continue; - if (fep->mii_bus->phy_map[phy_id]->phy_id == 0) - continue; - if (dev_id--) - continue; - strncpy(mdio_bus_id, fep->mii_bus->id, MII_BUS_ID_SIZE); - break; - } + if (fep->phy_node) { + phy_dev = of_phy_connect(ndev, fep->phy_node, + &fec_enet_adjust_link, 0, + fep->phy_interface); + } else { + /* check for attached phy */ + for (phy_id = 0; (phy_id < PHY_MAX_ADDR); phy_id++) { + if ((fep->mii_bus->phy_mask & (1 << phy_id))) + continue; + if (fep->mii_bus->phy_map[phy_id] == NULL) + continue; + if (fep->mii_bus->phy_map[phy_id]->phy_id == 0) + continue; + if (dev_id--) + continue; + strncpy(mdio_bus_id, fep->mii_bus->id, MII_BUS_ID_SIZE); + break; + } - if (phy_id >= PHY_MAX_ADDR) { - netdev_info(ndev, "no PHY, assuming direct connection to switch\n"); - strncpy(mdio_bus_id, "fixed-0", MII_BUS_ID_SIZE); - phy_id = 0; + if (phy_id >= PHY_MAX_ADDR) { + netdev_info(ndev, "no PHY, assuming direct connection to switch\n"); + strncpy(mdio_bus_id, "fixed-0", MII_BUS_ID_SIZE); + phy_id = 0; + } + + snprintf(phy_name, sizeof(phy_name), + PHY_ID_FMT, mdio_bus_id, phy_id); + phy_dev = phy_connect(ndev, phy_name, &fec_enet_adjust_link, + fep->phy_interface); } - snprintf(phy_name, sizeof(phy_name), PHY_ID_FMT, mdio_bus_id, phy_id); - phy_dev = phy_connect(ndev, phy_name, &fec_enet_adjust_link, - fep->phy_interface); if (IS_ERR(phy_dev)) { netdev_err(ndev, "could not attach to PHY\n"); return PTR_ERR(phy_dev); @@ -1707,6 +1716,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); + struct device_node *node; int err = -ENXIO, i; /* @@ -1774,7 +1784,15 @@ static int fec_enet_mii_init(struct platform_device *pdev) for (i = 0; i < PHY_MAX_ADDR; i++) fep->mii_bus->irq[i] = PHY_POLL; - if (mdiobus_register(fep->mii_bus)) + node = of_get_child_by_name(pdev->dev.of_node, "mdio"); + if (node) { + err = of_mdiobus_register(fep->mii_bus, node); + of_node_put(node); + } else { + err = mdiobus_register(fep->mii_bus); + } + + if (err) goto err_out_free_mdio_irq; mii_cnt++; @@ -2527,6 +2545,7 @@ fec_probe(struct platform_device *pdev) struct resource *r; const struct of_device_id *of_id; static int dev_id; + struct device_node *np = pdev->dev.of_node, *phy_node; of_id = of_match_device(fec_dt_ids, &pdev->dev); if (of_id) @@ -2566,6 +2585,18 @@ fec_probe(struct platform_device *pdev) platform_set_drvdata(pdev, ndev); + phy_node = of_parse_phandle(np, "phy-handle", 0); + if (!phy_node && of_phy_is_fixed_link(np)) { + ret = of_phy_register_fixed_link(np); + if (ret < 0) { + dev_err(&pdev->dev, + "broken fixed-link specification\n"); + goto failed_phy; + } + phy_node = of_node_get(np); + } + fep->phy_node = phy_node; + ret = of_get_phy_mode(pdev->dev.of_node); if (ret < 0) { pdata = dev_get_platdata(&pdev->dev); @@ -2670,6 +2701,8 @@ failed_init: failed_regulator: fec_enet_clk_enable(ndev, false); failed_clk: +failed_phy: + of_node_put(phy_node); failed_ioremap: free_netdev(ndev); @@ -2691,6 +2724,7 @@ fec_drv_remove(struct platform_device *pdev) if (fep->ptp_clock) ptp_clock_unregister(fep->ptp_clock); fec_enet_clk_enable(ndev, false); + of_node_put(fep->phy_node); free_netdev(ndev); return 0; -- cgit v1.2.3