Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1448603 > unrolled thread

[PATCH 0/3] drivers: net: cpsw: fix driver loading/unloading

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko <grygorii.strashko@ti.com> - 2016-07-22 16:00 +0200
    [PATCH 2/3] drivers: net: cpsw: fix wrong regs access in cpsw_remove Grygorii Strashko <grygorii.strashko@ti.com> - 2016-07-22 16:00 +0200
    [PATCH 1/3] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko <grygorii.strashko@ti.com> - 2016-07-22 16:10 +0200
      Re: [PATCH 1/3] net: ethernet: ti: cpdma: fix lockup in  cpdma_ctlr_destroy() Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> - 2016-07-23 08:30 +0200

#1448603 — [PATCH 0/3] drivers: net: cpsw: fix driver loading/unloading

This series fixes set of isssues observed when CPSW driver module is unloaded/loaded:
1) rmmod: deadlock in cpdma_ctlr_destroy
2) rmmod: L3 back-trace and crash if all net interfaces are down, because CPSW
can be powerred down by PM runtime in this case.
3) insmod: mdio device is not recreated on next insmod
 - need to use of_platform_depopulate() in cpsw_remove().

Grygorii Strashko (3):
  net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy()
  drivers: net: cpsw: fix wrong regs access in cpsw_remove
  drivers: net: cpsw: use of_platform_depopulate()

 drivers/net/ethernet/ti/cpsw.c          | 19 +++++++++----------
 drivers/net/ethernet/ti/davinci_cpdma.c |  2 --
 2 files changed, 9 insertions(+), 12 deletions(-)

-- 
2.9.2

[toc] | [next] | [standalone]

#1448604 — [PATCH 2/3] drivers: net: cpsw: fix wrong regs access in cpsw_remove

The L3 error will be generated and system will crash during unloading
of CPSW driver if CPSW is used as module and ethX devices are down.
This happens because CPSW can be power off by PM runtime now when ethX
devices are down.

Hence, ensure that CPSW powered up by PM runtime before performing any
deinitialization actions which require CPSW registers access. In case
of PM runtime error just leave cpsw_remove() as we can't do anything
anymore.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 46423dd..a4d6eb5 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2589,6 +2589,13 @@ static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv *priv = netdev_priv(ndev);
+	int ret;
+
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		return ret;
+	}
 
 	if (priv->data.dual_emac)
 		unregister_netdev(cpsw_get_slave_ndev(priv, 1));
@@ -2596,8 +2603,9 @@ static int cpsw_remove(struct platform_device *pdev)
 
 	cpsw_ale_destroy(priv->ale);
 	cpdma_ctlr_destroy(priv->dma);
-	pm_runtime_disable(&pdev->dev);
 	device_for_each_child(&pdev->dev, NULL, cpsw_remove_child_device);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 	if (priv->data.dual_emac)
 		free_netdev(cpsw_get_slave_ndev(priv, 1));
 	free_netdev(ndev);
-- 
2.9.2

[toc] | [prev] | [next] | [standalone]

#1448605 — [PATCH 1/3] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy()

Fix deadlock in cpdma_ctlr_destroy() which is triggered now on
cpsw module removal:
 cpsw_remove()
 - cpdma_ctlr_destroy()
   - spin_lock_irqsave(&ctlr->lock, flags)
   - cpdma_ctlr_stop()
     - spin_lock_irqsave(&ctlr->lock, flags); <- deadlock
   - cpdma_chan_destroy()
     - spin_lock_irqsave(&ctlr->lock, flags); <- deadlock

The issue has not been observed before because CPDMA channels have
been destroyed manually by CPSW until commit d941ebe88a41 ("net:
ethernet: ti: cpsw: use destroy ctlr to destroy channels") was merged.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/davinci_cpdma.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index a68652a..89242e9 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -436,7 +436,6 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
 	if (!ctlr)
 		return -EINVAL;
 
-	spin_lock_irqsave(&ctlr->lock, flags);
 	if (ctlr->state != CPDMA_STATE_IDLE)
 		cpdma_ctlr_stop(ctlr);
 
@@ -444,7 +443,6 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
 		cpdma_chan_destroy(ctlr->channels[i]);
 
 	cpdma_desc_pool_destroy(ctlr->pool);
-	spin_unlock_irqrestore(&ctlr->lock, flags);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(cpdma_ctlr_destroy);
-- 
2.9.2

[toc] | [prev] | [next] | [standalone]

#1448884 — Re: [PATCH 1/3] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy()

On 22.07.16 16:58, Grygorii Strashko wrote:
> Fix deadlock in cpdma_ctlr_destroy() which is triggered now on
> cpsw module removal:
>  cpsw_remove()
>  - cpdma_ctlr_destroy()
>    - spin_lock_irqsave(&ctlr->lock, flags)
>    - cpdma_ctlr_stop()
>      - spin_lock_irqsave(&ctlr->lock, flags); <- deadlock
>    - cpdma_chan_destroy()
>      - spin_lock_irqsave(&ctlr->lock, flags); <- deadlock
>
> The issue has not been observed before because CPDMA channels have
> been destroyed manually by CPSW until commit d941ebe88a41 ("net:
> ethernet: ti: cpsw: use destroy ctlr to destroy channels") was merged.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/net/ethernet/ti/davinci_cpdma.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> index a68652a..89242e9 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> @@ -436,7 +436,6 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
>  	if (!ctlr)
>  		return -EINVAL;
>
> -	spin_lock_irqsave(&ctlr->lock, flags);
Should ctlr->state be checked under lock?
Seems like here should be used unlocked static versions of
cpdma_ctlr_stop() and cpdma_chan_destroy() instead.

>  	if (ctlr->state != CPDMA_STATE_IDLE)
>  		cpdma_ctlr_stop(ctlr);
>
> @@ -444,7 +443,6 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
>  		cpdma_chan_destroy(ctlr->channels[i]);
>
>  	cpdma_desc_pool_destroy(ctlr->pool);
> -	spin_unlock_irqrestore(&ctlr->lock, flags);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(cpdma_ctlr_destroy);
>

-- 
Regards,
Ivan Khoronzhuk

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web