dwc_eth_qos: stability fixes and support for CMA
cover dwc_eth_qos: stability fixes and support for CMA
This series has bug fixes for the dwc_eth_qos ethernet driver.
Mainly two stability fixes for problems found by Rabin Vincent:
- Successive starts and stops of the interface would trigger a DMA reset timeout.
- A race condition in the TX DMA handling could trigger a netdev watchdog timeout.
The memory allocation was improved to support use of the CMA as DMA allocator backend.
Lars Persson (2):
dwc_eth_qos: release descriptors outside netif_tx_lock
dwc_eth_qos: do phy_start before resetting hardware
Rabin Vincent (3):
dwc_eth_qos: fix race condition in dwceqos_start_xmit
dwc_eth_qos: use GFP_KERNEL in dma_alloc_coherent()
dwc_eth_qos: use DWCEQOS_MSG_DEFAULT
drivers/net/ethernet/synopsys/dwc_eth_qos.c | 45 ++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 14 deletions(-)
Signature · 1 line
2.1.4
PATCH 1/5 dwc_eth_qos: fix race condition in dwceqos_start_xmit
From: Rabin Vincent <[email protected]>
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.
--- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/net/ethernet/synopsys/dwc_eth_qos.c +2 -4
@@ -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;
Signature · 1 line
2.1.4
PATCH 2/5 dwc_eth_qos: release descriptors outside netif_tx_lock
In reply to Lars PerssonTo 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.
--- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/synopsys/dwc_eth_qos.c +6 -4
@@ -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 ; }
Signature · 1 line
2.1.4
PATCH 3/5 dwc_eth_qos: use GFP_KERNEL in dma_alloc_coherent()
In reply to Lars PerssonFrom: Rabin Vincent <[email protected]>
Since we are in non-atomic context here we can pass GFP_KERNEL to dma_alloc_coherent(). This enables use of the CMA.
--- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/synopsys/dwc_eth_qos.c +2 -2
@@ -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 +
Signature · 1 line
2.1.4
PATCH 4/5 dwc_eth_qos: use DWCEQOS_MSG_DEFAULT
In reply to Lars PerssonFrom: Rabin Vincent <[email protected]>
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.
--- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/synopsys/dwc_eth_qos.c +1 -1
@@ -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)" );
Signature · 1 line
2.1.4
PATCH 5/5 dwc_eth_qos: do phy_start before resetting hardware
In reply to Lars PerssonThis 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.
--- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
drivers/net/ethernet/synopsys/dwc_eth_qos.c +20 -3
@@ -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);
Signature · 1 line
2.1.4
David Miller
maintainerFrom: Lars Persson <[email protected]>
Date: Mon, 29 Feb 2016 16:22:29 +0100
Quoted 9 lines
This series has bug fixes for the
dwc_eth_qosethernet driver.Mainly two stability fixes for problems found by Rabin Vincent:
- Successive starts and stops of the interface would trigger a DMA reset timeout.
- A race condition in the TX DMA handling could trigger a netdev watchdog timeout.The memory allocation was improved to support use of the CMA as DMA allocator backend.
Series applied, thanks.