linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading
@ 2016-07-28 17:50 Grygorii Strashko
  2016-07-28 17:50 ` [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-28 17:50 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko

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().
4) rmmod: system crash on omap_device removal

Tested on: am437x-idk, am57xx-beagle-x15

Changes in v2:
- build warning fixed
- added fix for correct omap_device removal

Link on v1:
 https://lkml.org/lkml/2016/7/22/240

Grygorii Strashko (4):
  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()
  ARM: OMAP2+: omap_device: fix crash on omap_device removal

 arch/arm/mach-omap2/omap_device.c       |  2 +-
 drivers/net/ethernet/ti/cpsw.c          | 19 +++++++++----------
 drivers/net/ethernet/ti/davinci_cpdma.c |  3 ---
 3 files changed, 10 insertions(+), 14 deletions(-)

-- 
2.9.2

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy()
  2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
@ 2016-07-28 17:50 ` Grygorii Strashko
  2016-07-29  8:16   ` Mugunthan V N
  2016-07-28 17:50 ` [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove Grygorii Strashko
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-28 17:50 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko

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);
   - cpdma_chan_destroy()
     - spin_lock_irqsave(&ctlr->lock, flags);

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 | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index c8154ff..fdc0f4f 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -357,13 +357,11 @@ EXPORT_SYMBOL_GPL(cpdma_ctlr_stop);
 
 int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
 {
-	unsigned long flags;
 	int ret = 0, i;
 
 	if (!ctlr)
 		return -EINVAL;
 
-	spin_lock_irqsave(&ctlr->lock, flags);
 	if (ctlr->state != CPDMA_STATE_IDLE)
 		cpdma_ctlr_stop(ctlr);
 
@@ -371,7 +369,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove
  2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
  2016-07-28 17:50 ` [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko
@ 2016-07-28 17:50 ` Grygorii Strashko
  2016-07-29  8:31   ` Mugunthan V N
  2016-07-28 17:50 ` [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate() Grygorii Strashko
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-28 17:50 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko

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 8f1eab9..ec6f473 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2579,6 +2579,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));
@@ -2586,8 +2593,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate()
  2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
  2016-07-28 17:50 ` [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko
  2016-07-28 17:50 ` [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove Grygorii Strashko
@ 2016-07-28 17:50 ` Grygorii Strashko
  2016-07-29  8:36   ` Mugunthan V N
  2016-07-28 17:50 ` [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal Grygorii Strashko
  2016-07-31  4:00 ` [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading David Miller
  4 siblings, 1 reply; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-28 17:50 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko

Use of_platform_depopulate() in cpsw_remove() instead of
of_device_unregister(), because CSPW child devices will not be
recreated otherwise on next insmod. of_platform_depopulate() is
correct way now as it will ensure that all steps done in
of_platform_populate() are reverted, including cleaning up of
OF_POPULATED flag.

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

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ec6f473..f0ed470 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2566,15 +2566,6 @@ clean_ndev_ret:
 	return ret;
 }
 
-static int cpsw_remove_child_device(struct device *dev, void *c)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-
-	of_device_unregister(pdev);
-
-	return 0;
-}
-
 static int cpsw_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
@@ -2593,7 +2584,7 @@ static int cpsw_remove(struct platform_device *pdev)
 
 	cpsw_ale_destroy(priv->ale);
 	cpdma_ctlr_destroy(priv->dma);
-	device_for_each_child(&pdev->dev, NULL, cpsw_remove_child_device);
+	of_platform_depopulate(&pdev->dev);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	if (priv->data.dual_emac)
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal
  2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
                   ` (2 preceding siblings ...)
  2016-07-28 17:50 ` [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate() Grygorii Strashko
@ 2016-07-28 17:50 ` Grygorii Strashko
  2016-07-29  5:15   ` Peter Ujfalusi
  2016-07-31  4:00 ` [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading David Miller
  4 siblings, 1 reply; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-28 17:50 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Grygorii Strashko,
	Tony Lindgren, Tero Kristo

Below call chain causes system crash when OMAP device is
removed by calling of_platform_depopulate()/device_del():

device_del()
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 			     BUS_NOTIFY_DEL_DEVICE, dev);
  - _omap_device_notifier_call()
    - omap_device_delete()
      - od->pdev->archdata.od = NULL;
	kfree(od->hwmods);
	kfree(od);
  - bus_remove_device()
    - device_release_driver()
      - __device_release_driver()
	- pm_runtime_get_sync()
	   - _od_runtime_resume()
	     - omap_hwmod_enable() <- OOPS od's delted already

Backtrace:
Unable to handle kernel NULL pointer dereference at virtual address 0000000d
pgd = eb100000
[0000000d] *pgd=ad6e1831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] PREEMPT SMP ARM
CPU: 1 PID: 1273 Comm: modprobe Not tainted 4.4.15-rt19-00115-ge4d3cd3-dirty #68
Hardware name: Generic DRA74X (Flattened Device Tree)
task: eb1ee800 ti: ec962000 task.ti: ec962000
PC is at omap_device_enable+0x10/0x90
LR is at _od_runtime_resume+0x10/0x24
[...]
[<c00299dc>] (omap_device_enable) from [<c0029a6c>] (_od_runtime_resume+0x10/0x24)
[<c0029a6c>] (_od_runtime_resume) from [<c04ad404>] (__rpm_callback+0x20/0x34)
[<c04ad404>] (__rpm_callback) from [<c04ad438>] (rpm_callback+0x20/0x80)
[<c04ad438>] (rpm_callback) from [<c04aee28>] (rpm_resume+0x48c/0x964)
[<c04aee28>] (rpm_resume) from [<c04af360>] (__pm_runtime_resume+0x60/0x88)
[<c04af360>] (__pm_runtime_resume) from [<c04a4974>] (__device_release_driver+0x30/0x100)
[<c04a4974>] (__device_release_driver) from [<c04a4a60>] (device_release_driver+0x1c/0x28)
[<c04a4a60>] (device_release_driver) from [<c04a38c0>] (bus_remove_device+0xec/0x144)
[<c04a38c0>] (bus_remove_device) from [<c04a0764>] (device_del+0x10c/0x210)
[<c04a0764>] (device_del) from [<c04a67b0>] (platform_device_del+0x18/0x84)
[<c04a67b0>] (platform_device_del) from [<c04a6828>] (platform_device_unregister+0xc/0x20)
[<c04a6828>] (platform_device_unregister) from [<c05adcfc>] (of_platform_device_destroy+0x8c/0x90)
[<c05adcfc>] (of_platform_device_destroy) from [<c04a02f0>] (device_for_each_child+0x4c/0x78)
[<c04a02f0>] (device_for_each_child) from [<c05adc5c>] (of_platform_depopulate+0x30/0x44)
[<c05adc5c>] (of_platform_depopulate) from [<bf123920>] (cpsw_remove+0x68/0xf4 [ti_cpsw])
[<bf123920>] (cpsw_remove [ti_cpsw]) from [<c04a68d8>] (platform_drv_remove+0x24/0x3c)
[<c04a68d8>] (platform_drv_remove) from [<c04a49c8>] (__device_release_driver+0x84/0x100)
[<c04a49c8>] (__device_release_driver) from [<c04a4b20>] (driver_detach+0xac/0xb0)
[<c04a4b20>] (driver_detach) from [<c04a3be8>] (bus_remove_driver+0x60/0xd4)
[<c04a3be8>] (bus_remove_driver) from [<c00d9870>] (SyS_delete_module+0x184/0x20c)
[<c00d9870>] (SyS_delete_module) from [<c0010540>] (ret_fast_syscall+0x0/0x1c)
Code: e3500000 e92d4070 1590630c 01a06000 (e5d6300d)

Hence, fix it by using BUS_NOTIFY_REMOVED_DEVICE event for OMAP device
deletion which is sent when DD has finished processing of device
deletion.

Cc: Tony Lindgren <tony@atomide.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-omap2/omap_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index f7ff3b9..208f115 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -194,7 +194,7 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
 	int err;
 
 	switch (event) {
-	case BUS_NOTIFY_DEL_DEVICE:
+	case BUS_NOTIFY_REMOVED_DEVICE:
 		if (pdev->archdata.od)
 			omap_device_delete(pdev->archdata.od);
 		break;
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal
  2016-07-28 17:50 ` [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal Grygorii Strashko
@ 2016-07-29  5:15   ` Peter Ujfalusi
  2016-07-29 12:52     ` Grygorii Strashko
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Ujfalusi @ 2016-07-29  5:15 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Tony Lindgren, Tero Kristo

On 07/28/16 20:50, Grygorii Strashko wrote:
> Below call chain causes system crash when OMAP device is
> removed by calling of_platform_depopulate()/device_del():

Should you swap 3 <-> 4 in the series?
Currently patch 3 will introduce the crash you are fixing in patch 4...

> 
> device_del()
> - blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>  			     BUS_NOTIFY_DEL_DEVICE, dev);
>   - _omap_device_notifier_call()
>     - omap_device_delete()
>       - od->pdev->archdata.od = NULL;
> 	kfree(od->hwmods);
> 	kfree(od);
>   - bus_remove_device()
>     - device_release_driver()
>       - __device_release_driver()
> 	- pm_runtime_get_sync()
> 	   - _od_runtime_resume()
> 	     - omap_hwmod_enable() <- OOPS od's delted already
> 
> Backtrace:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000d
> pgd = eb100000
> [0000000d] *pgd=ad6e1831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> CPU: 1 PID: 1273 Comm: modprobe Not tainted 4.4.15-rt19-00115-ge4d3cd3-dirty #68
> Hardware name: Generic DRA74X (Flattened Device Tree)
> task: eb1ee800 ti: ec962000 task.ti: ec962000
> PC is at omap_device_enable+0x10/0x90
> LR is at _od_runtime_resume+0x10/0x24
> [...]
> [<c00299dc>] (omap_device_enable) from [<c0029a6c>] (_od_runtime_resume+0x10/0x24)
> [<c0029a6c>] (_od_runtime_resume) from [<c04ad404>] (__rpm_callback+0x20/0x34)
> [<c04ad404>] (__rpm_callback) from [<c04ad438>] (rpm_callback+0x20/0x80)
> [<c04ad438>] (rpm_callback) from [<c04aee28>] (rpm_resume+0x48c/0x964)
> [<c04aee28>] (rpm_resume) from [<c04af360>] (__pm_runtime_resume+0x60/0x88)
> [<c04af360>] (__pm_runtime_resume) from [<c04a4974>] (__device_release_driver+0x30/0x100)
> [<c04a4974>] (__device_release_driver) from [<c04a4a60>] (device_release_driver+0x1c/0x28)
> [<c04a4a60>] (device_release_driver) from [<c04a38c0>] (bus_remove_device+0xec/0x144)
> [<c04a38c0>] (bus_remove_device) from [<c04a0764>] (device_del+0x10c/0x210)
> [<c04a0764>] (device_del) from [<c04a67b0>] (platform_device_del+0x18/0x84)
> [<c04a67b0>] (platform_device_del) from [<c04a6828>] (platform_device_unregister+0xc/0x20)
> [<c04a6828>] (platform_device_unregister) from [<c05adcfc>] (of_platform_device_destroy+0x8c/0x90)
> [<c05adcfc>] (of_platform_device_destroy) from [<c04a02f0>] (device_for_each_child+0x4c/0x78)
> [<c04a02f0>] (device_for_each_child) from [<c05adc5c>] (of_platform_depopulate+0x30/0x44)
> [<c05adc5c>] (of_platform_depopulate) from [<bf123920>] (cpsw_remove+0x68/0xf4 [ti_cpsw])
> [<bf123920>] (cpsw_remove [ti_cpsw]) from [<c04a68d8>] (platform_drv_remove+0x24/0x3c)
> [<c04a68d8>] (platform_drv_remove) from [<c04a49c8>] (__device_release_driver+0x84/0x100)
> [<c04a49c8>] (__device_release_driver) from [<c04a4b20>] (driver_detach+0xac/0xb0)
> [<c04a4b20>] (driver_detach) from [<c04a3be8>] (bus_remove_driver+0x60/0xd4)
> [<c04a3be8>] (bus_remove_driver) from [<c00d9870>] (SyS_delete_module+0x184/0x20c)
> [<c00d9870>] (SyS_delete_module) from [<c0010540>] (ret_fast_syscall+0x0/0x1c)
> Code: e3500000 e92d4070 1590630c 01a06000 (e5d6300d)
> 
> Hence, fix it by using BUS_NOTIFY_REMOVED_DEVICE event for OMAP device
> deletion which is sent when DD has finished processing of device
> deletion.
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  arch/arm/mach-omap2/omap_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index f7ff3b9..208f115 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -194,7 +194,7 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
>  	int err;
>  
>  	switch (event) {
> -	case BUS_NOTIFY_DEL_DEVICE:
> +	case BUS_NOTIFY_REMOVED_DEVICE:
>  		if (pdev->archdata.od)
>  			omap_device_delete(pdev->archdata.od);
>  		break;
> 


-- 
Péter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy()
  2016-07-28 17:50 ` [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko
@ 2016-07-29  8:16   ` Mugunthan V N
  0 siblings, 0 replies; 11+ messages in thread
From: Mugunthan V N @ 2016-07-29  8:16 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev
  Cc: Sekhar Nori, linux-kernel, linux-omap

On Thursday 28 July 2016 11:20 PM, 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);
>    - cpdma_chan_destroy()
>      - spin_lock_irqsave(&ctlr->lock, flags);
> 
> 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>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove
  2016-07-28 17:50 ` [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove Grygorii Strashko
@ 2016-07-29  8:31   ` Mugunthan V N
  0 siblings, 0 replies; 11+ messages in thread
From: Mugunthan V N @ 2016-07-29  8:31 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev
  Cc: Sekhar Nori, linux-kernel, linux-omap

On Thursday 28 July 2016 11:20 PM, Grygorii Strashko wrote:
> 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>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate()
  2016-07-28 17:50 ` [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate() Grygorii Strashko
@ 2016-07-29  8:36   ` Mugunthan V N
  0 siblings, 0 replies; 11+ messages in thread
From: Mugunthan V N @ 2016-07-29  8:36 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev
  Cc: Sekhar Nori, linux-kernel, linux-omap

On Thursday 28 July 2016 11:20 PM, Grygorii Strashko wrote:
> Use of_platform_depopulate() in cpsw_remove() instead of
> of_device_unregister(), because CSPW child devices will not be
> recreated otherwise on next insmod. of_platform_depopulate() is
> correct way now as it will ensure that all steps done in
> of_platform_populate() are reverted, including cleaning up of
> OF_POPULATED flag.
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal
  2016-07-29  5:15   ` Peter Ujfalusi
@ 2016-07-29 12:52     ` Grygorii Strashko
  0 siblings, 0 replies; 11+ messages in thread
From: Grygorii Strashko @ 2016-07-29 12:52 UTC (permalink / raw)
  To: Peter Ujfalusi, David S. Miller, netdev, Mugunthan V N
  Cc: Sekhar Nori, linux-kernel, linux-omap, Tony Lindgren, Tero Kristo

On 07/29/2016 08:15 AM, Peter Ujfalusi wrote:
> On 07/28/16 20:50, Grygorii Strashko wrote:
>> Below call chain causes system crash when OMAP device is
>> removed by calling of_platform_depopulate()/device_del():
>
> Should you swap 3 <-> 4 in the series?
> Currently patch 3 will introduce the crash you are fixing in patch 4...

No. The key function here is device_del() - of_device_unregister(), 
which was used previously has the same issue:
   of_device_unregister() -> device_unregister() ->device_del()

In general all these patches are independent and they were created just 
in bugs detection order.

>
>>
>> device_del()
>> - blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>>  			     BUS_NOTIFY_DEL_DEVICE, dev);
>>   - _omap_device_notifier_call()
>>     - omap_device_delete()
>>       - od->pdev->archdata.od = NULL;
>> 	kfree(od->hwmods);
>> 	kfree(od);
>>   - bus_remove_device()
>>     - device_release_driver()
>>       - __device_release_driver()
>> 	- pm_runtime_get_sync()
>> 	   - _od_runtime_resume()
>> 	     - omap_hwmod_enable() <- OOPS od's delted already
>>
>> Backtrace:
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000d
>> pgd = eb100000
>> [0000000d] *pgd=ad6e1831, *pte=00000000, *ppte=00000000
>> Internal error: Oops: 17 [#1] PREEMPT SMP ARM
>> CPU: 1 PID: 1273 Comm: modprobe Not tainted 4.4.15-rt19-00115-ge4d3cd3-dirty #68
>> Hardware name: Generic DRA74X (Flattened Device Tree)
>> task: eb1ee800 ti: ec962000 task.ti: ec962000
>> PC is at omap_device_enable+0x10/0x90
>> LR is at _od_runtime_resume+0x10/0x24
>> [...]
>> [<c00299dc>] (omap_device_enable) from [<c0029a6c>] (_od_runtime_resume+0x10/0x24)
>> [<c0029a6c>] (_od_runtime_resume) from [<c04ad404>] (__rpm_callback+0x20/0x34)
>> [<c04ad404>] (__rpm_callback) from [<c04ad438>] (rpm_callback+0x20/0x80)
>> [<c04ad438>] (rpm_callback) from [<c04aee28>] (rpm_resume+0x48c/0x964)
>> [<c04aee28>] (rpm_resume) from [<c04af360>] (__pm_runtime_resume+0x60/0x88)
>> [<c04af360>] (__pm_runtime_resume) from [<c04a4974>] (__device_release_driver+0x30/0x100)
>> [<c04a4974>] (__device_release_driver) from [<c04a4a60>] (device_release_driver+0x1c/0x28)
>> [<c04a4a60>] (device_release_driver) from [<c04a38c0>] (bus_remove_device+0xec/0x144)
>> [<c04a38c0>] (bus_remove_device) from [<c04a0764>] (device_del+0x10c/0x210)
>> [<c04a0764>] (device_del) from [<c04a67b0>] (platform_device_del+0x18/0x84)
>> [<c04a67b0>] (platform_device_del) from [<c04a6828>] (platform_device_unregister+0xc/0x20)
>> [<c04a6828>] (platform_device_unregister) from [<c05adcfc>] (of_platform_device_destroy+0x8c/0x90)
>> [<c05adcfc>] (of_platform_device_destroy) from [<c04a02f0>] (device_for_each_child+0x4c/0x78)
>> [<c04a02f0>] (device_for_each_child) from [<c05adc5c>] (of_platform_depopulate+0x30/0x44)
>> [<c05adc5c>] (of_platform_depopulate) from [<bf123920>] (cpsw_remove+0x68/0xf4 [ti_cpsw])
>> [<bf123920>] (cpsw_remove [ti_cpsw]) from [<c04a68d8>] (platform_drv_remove+0x24/0x3c)
>> [<c04a68d8>] (platform_drv_remove) from [<c04a49c8>] (__device_release_driver+0x84/0x100)
>> [<c04a49c8>] (__device_release_driver) from [<c04a4b20>] (driver_detach+0xac/0xb0)
>> [<c04a4b20>] (driver_detach) from [<c04a3be8>] (bus_remove_driver+0x60/0xd4)
>> [<c04a3be8>] (bus_remove_driver) from [<c00d9870>] (SyS_delete_module+0x184/0x20c)
>> [<c00d9870>] (SyS_delete_module) from [<c0010540>] (ret_fast_syscall+0x0/0x1c)
>> Code: e3500000 e92d4070 1590630c 01a06000 (e5d6300d)
>>
>> Hence, fix it by using BUS_NOTIFY_REMOVED_DEVICE event for OMAP device
>> deletion which is sent when DD has finished processing of device
>> deletion.
>>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Tero Kristo <t-kristo@ti.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap_device.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
>> index f7ff3b9..208f115 100644
>> --- a/arch/arm/mach-omap2/omap_device.c
>> +++ b/arch/arm/mach-omap2/omap_device.c
>> @@ -194,7 +194,7 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
>>  	int err;
>>
>>  	switch (event) {
>> -	case BUS_NOTIFY_DEL_DEVICE:
>> +	case BUS_NOTIFY_REMOVED_DEVICE:
>>  		if (pdev->archdata.od)
>>  			omap_device_delete(pdev->archdata.od);
>>  		break;
>>
>
>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading
  2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
                   ` (3 preceding siblings ...)
  2016-07-28 17:50 ` [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal Grygorii Strashko
@ 2016-07-31  4:00 ` David Miller
  4 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2016-07-31  4:00 UTC (permalink / raw)
  To: grygorii.strashko; +Cc: netdev, mugunthanvnm, nsekhar, linux-kernel, linux-omap

From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Thu, 28 Jul 2016 20:50:33 +0300

> 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().
> 4) rmmod: system crash on omap_device removal
> 
> Tested on: am437x-idk, am57xx-beagle-x15
> 
> Changes in v2:
> - build warning fixed
> - added fix for correct omap_device removal
> 
> Link on v1:
>  https://lkml.org/lkml/2016/7/22/240

Series applied, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-07-31  4:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 17:50 [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading Grygorii Strashko
2016-07-28 17:50 ` [PATCH v2 1/4] net: ethernet: ti: cpdma: fix lockup in cpdma_ctlr_destroy() Grygorii Strashko
2016-07-29  8:16   ` Mugunthan V N
2016-07-28 17:50 ` [PATCH v2 2/4] drivers: net: cpsw: fix wrong regs access in cpsw_remove Grygorii Strashko
2016-07-29  8:31   ` Mugunthan V N
2016-07-28 17:50 ` [PATCH v2 3/4] drivers: net: cpsw: use of_platform_depopulate() Grygorii Strashko
2016-07-29  8:36   ` Mugunthan V N
2016-07-28 17:50 ` [PATCH v2 4/4] ARM: OMAP2+: omap_device: fix crash on omap_device removal Grygorii Strashko
2016-07-29  5:15   ` Peter Ujfalusi
2016-07-29 12:52     ` Grygorii Strashko
2016-07-31  4:00 ` [PATCH v2 0/4] drivers: net: cpsw: fix driver loading/unloading David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).