diff mbox

[1/2] net: ethernet: cpsw: don't claim IRQs with devm_request_irq()

Message ID 1409676245-13897-1-git-send-email-zonque@gmail.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Daniel Mack Sept. 2, 2014, 4:44 p.m. UTC 
Julia Lawall spotted a problem with aa1a15e ("net: ethernet: cpsw:
switch to devres allocations") which introduced a race condition in
cpsw_probe() by removing explicit interrupt disable calls before
calling free_netdev(). Hence, an interrupt can fire after free_netdev()
was called. The same problem exists in cpsw_remove().

Fix this by reverting the IRQ part of the aforementioned patch and
handle those resources explicitly.

Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/net/ethernet/ti/cpsw.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Mugunthan V N Sept. 3, 2014, 7:27 a.m. UTC | #1 
On Tuesday 02 September 2014 10:14 PM, Daniel Mack wrote:
> Julia Lawall spotted a problem with aa1a15e ("net: ethernet: cpsw:
> switch to devres allocations") which introduced a race condition in
> cpsw_probe() by removing explicit interrupt disable calls before
> calling free_netdev(). Hence, an interrupt can fire after free_netdev()
> was called. The same problem exists in cpsw_remove().
> 
> Fix this by reverting the IRQ part of the aforementioned patch and
> handle those resources explicitly.
> 
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Daniel Mack <zonque@gmail.com>

CPSW interrupts cannot be triggered as the interrupts are disabled in
priv->wr_regs->tx_en and priv->wr_regs->rx_en inside CPSW module and
these interrupts are enabled only when the device is opened.

In cpsw_remove, CPDMA controller is stopped and interrupts are disabled
in cpsw_ndo_stop(), so there is no chance that an interrupt can occur
during cpsw_remove().

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 999fb72..cdbbb58 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2233,10 +2233,10 @@  static int cpsw_probe(struct platform_device *pdev)
 
 	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
-			if (devm_request_irq(&pdev->dev, i, cpsw_interrupt, 0,
-					     dev_name(&pdev->dev), priv)) {
+			if (request_irq(i, cpsw_interrupt, 0,
+					dev_name(&pdev->dev), priv)) {
 				dev_err(priv->dev, "error attaching irq\n");
-				goto clean_ale_ret;
+				goto clean_irq_ret;
 			}
 			priv->irqs_table[k] = i;
 			priv->num_irqs = k + 1;
@@ -2256,7 +2256,7 @@  static int cpsw_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(priv->dev, "error registering net device\n");
 		ret = -ENODEV;
-		goto clean_ale_ret;
+		goto clean_irq_ret;
 	}
 
 	cpsw_notice(priv, probe, "initialized device (regs %pa, irq %d)\n",
@@ -2266,12 +2266,15 @@  static int cpsw_probe(struct platform_device *pdev)
 		ret = cpsw_probe_dual_emac(pdev, priv);
 		if (ret) {
 			cpsw_err(priv, probe, "error probe slave 2 emac interface\n");
-			goto clean_ale_ret;
+			goto clean_irq_ret;
 		}
 	}
 
 	return 0;
 
+clean_irq_ret:
+	for (i = 0; i < priv->num_irqs; i++)
+		free_irq(priv->irqs_table[i], priv);
 clean_ale_ret:
 	cpsw_ale_destroy(priv->ale);
 clean_dma_ret:
@@ -2289,11 +2292,15 @@  static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv *priv = netdev_priv(ndev);
+	int i;
 
 	if (priv->data.dual_emac)
 		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
 	unregister_netdev(ndev);
 
+	for (i = 0; i < priv->num_irqs; i++)
+		free_irq(priv->irqs_table[i], priv);
+
 	cpsw_ale_destroy(priv->ale);
 	cpdma_chan_destroy(priv->txch);
 	cpdma_chan_destroy(priv->rxch);