From 8afb6c474b0dd8f9cf7e122a19c71924566fdb97 Mon Sep 17 00:00:00 2001 From: Rabin Vincent Date: Mon, 29 Feb 2016 16:22:30 +0100 Subject: [PATCH 1/5] dwc_eth_qos: fix race condition in dwceqos_start_xmit The xmit handler and the tx_reclaim tasklet had a race on the tx_free variable which could lead to a tx timeout if tx_free was updated after the tx complete interrupt. Signed-off-by: Rabin Vincent Signed-off-by: Lars Persson Signed-off-by: David S. Miller --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index fc8bbff2d7e3..926db2d58fb5 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -2178,12 +2178,10 @@ static int dwceqos_start_xmit(struct sk_buff *skb, struct net_device *ndev) ((trans.initial_descriptor + trans.nr_descriptors) % DWCEQOS_TX_DCNT)); - dwceqos_tx_finalize(skb, lp, &trans); - - netdev_sent_queue(ndev, skb->len); - spin_lock_bh(&lp->tx_lock); lp->tx_free -= trans.nr_descriptors; + dwceqos_tx_finalize(skb, lp, &trans); + netdev_sent_queue(ndev, skb->len); spin_unlock_bh(&lp->tx_lock); ndev->trans_start = jiffies; From d4dc35f26e1017e2a632339ecae43706cbc06986 Mon Sep 17 00:00:00 2001 From: Lars Persson Date: Mon, 29 Feb 2016 16:22:31 +0100 Subject: [PATCH 2/5] dwc_eth_qos: release descriptors outside netif_tx_lock To prepare for using the CMA, we can not be in atomic context when de-allocating DMA buffers. The tx lock was needed only to protect the hw reset against the xmit handler. Now we briefly grab the tx lock while stopping the queue to make sure no thread is inside or will enter the xmit handler. Signed-off-by: Lars Persson Signed-off-by: David S. Miller --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index 926db2d58fb5..53d48c0f41bc 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -1918,15 +1918,17 @@ static int dwceqos_stop(struct net_device *ndev) phy_stop(lp->phy_dev); tasklet_disable(&lp->tx_bdreclaim_tasklet); - netif_stop_queue(ndev); napi_disable(&lp->napi); - dwceqos_drain_dma(lp); + /* Stop all tx before we drain the tx dma. */ + netif_tx_lock_bh(lp->ndev); + netif_stop_queue(ndev); + netif_tx_unlock_bh(lp->ndev); - netif_tx_lock(lp->ndev); + dwceqos_drain_dma(lp); dwceqos_reset_hw(lp); + dwceqos_descriptor_free(lp); - netif_tx_unlock(lp->ndev); return 0; } From e8b0c32eae319e287d0329b4148c9a1a51525053 Mon Sep 17 00:00:00 2001 From: Rabin Vincent Date: Mon, 29 Feb 2016 16:22:32 +0100 Subject: [PATCH 3/5] dwc_eth_qos: use GFP_KERNEL in dma_alloc_coherent() Since we are in non-atomic context here we can pass GFP_KERNEL to dma_alloc_coherent(). This enables use of the CMA. Signed-off-by: Rabin Vincent Signed-off-by: Lars Persson Signed-off-by: David S. Miller --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index 53d48c0f41bc..3ca2d5c9707a 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -1113,7 +1113,7 @@ static int dwceqos_descriptor_init(struct net_local *lp) /* Allocate DMA descriptors */ size = DWCEQOS_RX_DCNT * sizeof(struct dwceqos_dma_desc); lp->rx_descs = dma_alloc_coherent(lp->ndev->dev.parent, size, - &lp->rx_descs_addr, 0); + &lp->rx_descs_addr, GFP_KERNEL); if (!lp->rx_descs) goto err_out; lp->rx_descs_tail_addr = lp->rx_descs_addr + @@ -1121,7 +1121,7 @@ static int dwceqos_descriptor_init(struct net_local *lp) size = DWCEQOS_TX_DCNT * sizeof(struct dwceqos_dma_desc); lp->tx_descs = dma_alloc_coherent(lp->ndev->dev.parent, size, - &lp->tx_descs_addr, 0); + &lp->tx_descs_addr, GFP_KERNEL); if (!lp->tx_descs) goto err_out; lp->tx_descs_tail_addr = lp->tx_descs_addr + From 016a91c64de6621e5917b2ddfe2afc0abb0a884c Mon Sep 17 00:00:00 2001 From: Rabin Vincent Date: Mon, 29 Feb 2016 16:22:33 +0100 Subject: [PATCH 4/5] dwc_eth_qos: use DWCEQOS_MSG_DEFAULT Since debug is hardcoded to 3, the defaults in the DWCEQOS_MSG_DEFAULT macro are never used, which does not seem to be the intended behaviour here. Set debug to -1 like other drivers so that DWCEQOS_MSG_DEFAULT is actually used by default. Signed-off-by: Rabin Vincent Signed-off-by: Lars Persson Signed-off-by: David S. Miller --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index 3ca2d5c9707a..6897c1d0f86e 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -426,7 +426,7 @@ #define DWC_MMC_RXOCTETCOUNT_GB 0x0784 #define DWC_MMC_RXPACKETCOUNT_GB 0x0780 -static int debug = 3; +static int debug = -1; module_param(debug, int, 0); MODULE_PARM_DESC(debug, "DWC_eth_qos debug level (0=none,...,16=all)"); From cd5e41234729966153595fd3c2a7912cc4f95bc2 Mon Sep 17 00:00:00 2001 From: Lars Persson Date: Mon, 29 Feb 2016 16:22:34 +0100 Subject: [PATCH 5/5] dwc_eth_qos: do phy_start before resetting hardware This reverts the changed init order from commit 3647bc35bd42 ("dwc_eth_qos: Reset hardware before PHY start") and makes another fix for the race. It turned out that the reset state machine of the dwceqos hardware requires PHY clocks to be present in order to complete the reset cycle. To plug the race with the phy state machine we defer link speed setting until the hardware init has finished. Signed-off-by: Lars Persson Signed-off-by: David S. Miller --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 23 ++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index 6897c1d0f86e..af11ed1e0bcc 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -650,6 +650,11 @@ struct net_local { u32 mmc_tx_counters_mask; struct dwceqos_flowcontrol flowcontrol; + + /* Tracks the intermediate state of phy started but hardware + * init not finished yet. + */ + bool phy_defer; }; static void dwceqos_read_mmc_counters(struct net_local *lp, u32 rx_mask, @@ -901,6 +906,9 @@ static void dwceqos_adjust_link(struct net_device *ndev) struct phy_device *phydev = lp->phy_dev; int status_change = 0; + if (lp->phy_defer) + return; + if (phydev->link) { if ((lp->speed != phydev->speed) || (lp->duplex != phydev->duplex)) { @@ -1635,6 +1643,12 @@ static void dwceqos_init_hw(struct net_local *lp) regval = dwceqos_read(lp, REG_DWCEQOS_MAC_CFG); dwceqos_write(lp, REG_DWCEQOS_MAC_CFG, regval | DWCEQOS_MAC_CFG_TE | DWCEQOS_MAC_CFG_RE); + + lp->phy_defer = false; + mutex_lock(&lp->phy_dev->lock); + phy_read_status(lp->phy_dev); + dwceqos_adjust_link(lp->ndev); + mutex_unlock(&lp->phy_dev->lock); } static void dwceqos_tx_reclaim(unsigned long data) @@ -1880,9 +1894,13 @@ static int dwceqos_open(struct net_device *ndev) } netdev_reset_queue(ndev); + /* The dwceqos reset state machine requires all phy clocks to complete, + * hence the unusual init order with phy_start first. + */ + lp->phy_defer = true; + phy_start(lp->phy_dev); dwceqos_init_hw(lp); napi_enable(&lp->napi); - phy_start(lp->phy_dev); netif_start_queue(ndev); tasklet_enable(&lp->tx_bdreclaim_tasklet); @@ -1915,8 +1933,6 @@ static int dwceqos_stop(struct net_device *ndev) { struct net_local *lp = netdev_priv(ndev); - phy_stop(lp->phy_dev); - tasklet_disable(&lp->tx_bdreclaim_tasklet); napi_disable(&lp->napi); @@ -1927,6 +1943,7 @@ static int dwceqos_stop(struct net_device *ndev) dwceqos_drain_dma(lp); dwceqos_reset_hw(lp); + phy_stop(lp->phy_dev); dwceqos_descriptor_free(lp);