From da1774e5f1974294dcfc2c08363bb103e769d302 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:39:57 +0100 Subject: [PATCH 01/10] 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(+) 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; From 8bbbd3c19c469a1c6b8e97e9f5a083d029657be5 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:02 +0100 Subject: [PATCH 02/10] 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(-) 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; From 8ce5624f5bbb991ae3aea2fcdae86d808351e173 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:07 +0100 Subject: [PATCH 03/10] 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(-) 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); } } From dbc64a8ea231271c580b5a570bc48cf42203fe0e Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:12 +0100 Subject: [PATCH 04/10] 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(-) 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); } From 6af42d420bcfa0c837911bd5b518fd4a3cfa4fe4 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:18 +0100 Subject: [PATCH 05/10] 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 | 28 ++++++----------------- 1 file changed, 7 insertions(+), 21 deletions(-) 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_tx_unlock_bh(ndev); - netif_wake_queue(ndev); - napi_enable(&fep->napi); netif_device_attach(ndev); + netif_tx_unlock_bh(ndev); + netif_device_attach(ndev); + napi_enable(&fep->napi); phy_start(fep->phy_dev); } rtnl_unlock(); From 31a6de34f3daa9b1f412931befd9f82fd4a1b968 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:23 +0100 Subject: [PATCH 06/10] 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(-) 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); } From 9a7ba4381af4defa7834c73c8a015532c06f4e8e Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:28 +0100 Subject: [PATCH 07/10] 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(-) 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); From 8506fa1d8eb36fbf96ab5e2a7e4b87826853f1be Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:33 +0100 Subject: [PATCH 08/10] 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(-) 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; From f208ce10046888052d3a5e9fc88b7c1b819877d2 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:38 +0100 Subject: [PATCH 09/10] 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(+) 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; } From ef83337d138354e3d1e32d9f929e0afefe5552aa Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 8 Jul 2014 12:40:43 +0100 Subject: [PATCH 10/10] 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(-) 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);