* [PATCH net 0/7] net: cpsw: fix leaks and probe deferral
@ 2016-11-16 14:35 Johan Hovold
2016-11-16 14:35 ` [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path Johan Hovold
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
This series fixes as number of leaks and issues in the cpsw probe-error
and driver-unbind paths, some which specifically prevented deferred
probing.
Johan
Johan Hovold (7):
net: ethernet: ti: cpsw: fix bad register access in probe error path
net: ethernet: ti: cpsw: fix mdio device reference leak
net: ethernet: ti: cpsw: fix deferred probe
net: ethernet: ti: cpsw: fix of_node and phydev leaks
net: ethernet: ti: cpsw: fix secondary-emac probe error path
net: ethernet: ti: cpsw: add missing sanity check
net: ethernet: ti: cpsw: fix fixed-link phy probe deferral
drivers/net/ethernet/ti/cpsw.c | 88 ++++++++++++++++++++++++++++++++++--------
1 file changed, 71 insertions(+), 17 deletions(-)
--
2.7.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 20:33 ` Grygorii Strashko
2016-11-16 14:35 ` [PATCH net 2/7] net: ethernet: ti: cpsw: fix mdio device reference leak Johan Hovold
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to resume the platform device to enable clocks before
accessing the CPSW registers in the probe error path (e.g. for deferred
probe).
Unhandled fault: external abort on non-linefetch (0x1008) at 0xd0872d08
...
[<c04fabcc>] (cpsw_ale_control_set) from [<c04fb8b4>] (cpsw_ale_destroy+0x2c/0x44)
[<c04fb8b4>] (cpsw_ale_destroy) from [<c04fea58>] (cpsw_probe+0xbd0/0x10c4)
[<c04fea58>] (cpsw_probe) from [<c047b2a0>] (platform_drv_probe+0x5c/0xc0)
Note that in the unlikely event of a runtime-resume failure, we'll leak
the ale struct.
Fixes: df828598a755 ("netdev: driver: ethernet: Add TI CPSW driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index c6cff3d2ff05..5bc5e6189661 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2818,7 +2818,12 @@ static int cpsw_probe(struct platform_device *pdev)
return 0;
clean_ale_ret:
- cpsw_ale_destroy(cpsw->ale);
+ if (pm_runtime_get_sync(&pdev->dev) < 0) {
+ pm_runtime_put_noidle(&pdev->dev);
+ } else {
+ cpsw_ale_destroy(cpsw->ale);
+ pm_runtime_put_sync(&pdev->dev);
+ }
clean_dma_ret:
cpdma_ctlr_destroy(cpsw->dma);
clean_runtime_disable_ret:
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 2/7] net: ethernet: ti: cpsw: fix mdio device reference leak
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
2016-11-16 14:35 ` [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 3/7] net: ethernet: ti: cpsw: fix deferred probe Johan Hovold
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to drop the reference taken by of_find_device_by_node() when
looking up an mdio device from a phy_id property during probe.
Fixes: 549985ee9c72 ("cpsw: simplify the setup of the register
pointers")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 5bc5e6189661..ee1fae914abc 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2397,6 +2397,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
PHY_ID_FMT, mdio->name, phyid);
+ put_device(&mdio->dev);
} else {
dev_err(&pdev->dev,
"No slave[%d] phy_id, phy-handle, or fixed-link property\n",
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 3/7] net: ethernet: ti: cpsw: fix deferred probe
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
2016-11-16 14:35 ` [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path Johan Hovold
2016-11-16 14:35 ` [PATCH net 2/7] net: ethernet: ti: cpsw: fix mdio device reference leak Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 4/7] net: ethernet: ti: cpsw: fix of_node and phydev leaks Johan Hovold
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to deregister all child devices also on probe errors to avoid
leaks and to fix probe deferral.
cpsw 4a100000.ethernet: omap_device: omap_device_enable() called from invalid state 1
cpsw 4a100000.ethernet: use pm_runtime_put_sync_suspend() in driver?
cpsw: probe of 4a100000.ethernet failed with error -22
Add generic helper to undo the effects of cpsw_probe_dt(), which will
also be used in a follow-on patch to fix further leaks that have been
introduced more recently.
Fixes: 1fb19aa730e4 ("net: cpsw: Add parent<->child relation support
between cpsw and mdio")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ee1fae914abc..4374ba05610b 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2441,6 +2441,11 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
return 0;
}
+static void cpsw_remove_dt(struct platform_device *pdev)
+{
+ of_platform_depopulate(&pdev->dev);
+}
+
static int cpsw_probe_dual_emac(struct cpsw_priv *priv)
{
struct cpsw_common *cpsw = priv->cpsw;
@@ -2588,7 +2593,7 @@ static int cpsw_probe(struct platform_device *pdev)
if (cpsw_probe_dt(&cpsw->data, pdev)) {
dev_err(&pdev->dev, "cpsw: platform data missing\n");
ret = -ENODEV;
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
data = &cpsw->data;
cpsw->rx_ch_num = 1;
@@ -2609,7 +2614,7 @@ static int cpsw_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!cpsw->slaves) {
ret = -ENOMEM;
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
for (i = 0; i < data->slaves; i++)
cpsw->slaves[i].slave_num = i;
@@ -2621,7 +2626,7 @@ static int cpsw_probe(struct platform_device *pdev)
if (IS_ERR(clk)) {
dev_err(priv->dev, "fck is not found\n");
ret = -ENODEV;
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
cpsw->bus_freq_mhz = clk_get_rate(clk) / 1000000;
@@ -2629,7 +2634,7 @@ static int cpsw_probe(struct platform_device *pdev)
ss_regs = devm_ioremap_resource(&pdev->dev, ss_res);
if (IS_ERR(ss_regs)) {
ret = PTR_ERR(ss_regs);
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
cpsw->regs = ss_regs;
@@ -2639,7 +2644,7 @@ static int cpsw_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0) {
pm_runtime_put_noidle(&pdev->dev);
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
cpsw->version = readl(&cpsw->regs->id_ver);
pm_runtime_put_sync(&pdev->dev);
@@ -2648,7 +2653,7 @@ static int cpsw_probe(struct platform_device *pdev)
cpsw->wr_regs = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(cpsw->wr_regs)) {
ret = PTR_ERR(cpsw->wr_regs);
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
memset(&dma_params, 0, sizeof(dma_params));
@@ -2685,7 +2690,7 @@ static int cpsw_probe(struct platform_device *pdev)
default:
dev_err(priv->dev, "unknown version 0x%08x\n", cpsw->version);
ret = -ENODEV;
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
for (i = 0; i < cpsw->data.slaves; i++) {
struct cpsw_slave *slave = &cpsw->slaves[i];
@@ -2714,7 +2719,7 @@ static int cpsw_probe(struct platform_device *pdev)
if (!cpsw->dma) {
dev_err(priv->dev, "error initializing dma\n");
ret = -ENOMEM;
- goto clean_runtime_disable_ret;
+ goto clean_dt_ret;
}
cpsw->txch[0] = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0);
@@ -2827,7 +2832,8 @@ static int cpsw_probe(struct platform_device *pdev)
}
clean_dma_ret:
cpdma_ctlr_destroy(cpsw->dma);
-clean_runtime_disable_ret:
+clean_dt_ret:
+ cpsw_remove_dt(pdev);
pm_runtime_disable(&pdev->dev);
clean_ndev_ret:
free_netdev(priv->ndev);
@@ -2852,7 +2858,7 @@ static int cpsw_remove(struct platform_device *pdev)
cpsw_ale_destroy(cpsw->ale);
cpdma_ctlr_destroy(cpsw->dma);
- of_platform_depopulate(&pdev->dev);
+ cpsw_remove_dt(pdev);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
if (cpsw->data.dual_emac)
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 4/7] net: ethernet: ti: cpsw: fix of_node and phydev leaks
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
` (2 preceding siblings ...)
2016-11-16 14:35 ` [PATCH net 3/7] net: ethernet: ti: cpsw: fix deferred probe Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 5/7] net: ethernet: ti: cpsw: fix secondary-emac probe error path Johan Hovold
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to drop references taken and deregister devices registered
during probe on probe errors (including deferred probe) and driver
unbind.
Specifically, PHY of-node references were never released and fixed-link
PHY devices were never deregistered.
Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
Fixes: 1f71e8c96fc6 ("drivers: net: cpsw: Add support for fixed-link
PHY")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 4374ba05610b..4bd96732291c 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2443,6 +2443,41 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
static void cpsw_remove_dt(struct platform_device *pdev)
{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
+ struct cpsw_platform_data *data = &cpsw->data;
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *slave_node;
+ int i = 0;
+
+ for_each_available_child_of_node(node, slave_node) {
+ struct cpsw_slave_data *slave_data = &data->slave_data[i];
+
+ if (strcmp(slave_node->name, "slave"))
+ continue;
+
+ if (of_phy_is_fixed_link(slave_node)) {
+ struct phy_device *phydev;
+
+ phydev = of_phy_find_device(slave_node);
+ if (phydev) {
+ fixed_phy_unregister(phydev);
+ /* Put references taken by
+ * of_phy_find_device() and
+ * of_phy_register_fixed_link().
+ */
+ phy_device_free(phydev);
+ phy_device_free(phydev);
+ }
+ }
+
+ of_node_put(slave_data->phy_node);
+
+ i++;
+ if (i == data->slaves)
+ break;
+ }
+
of_platform_depopulate(&pdev->dev);
}
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 5/7] net: ethernet: ti: cpsw: fix secondary-emac probe error path
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
` (3 preceding siblings ...)
2016-11-16 14:35 ` [PATCH net 4/7] net: ethernet: ti: cpsw: fix of_node and phydev leaks Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 6/7] net: ethernet: ti: cpsw: add missing sanity check Johan Hovold
2016-11-16 14:35 ` [PATCH net 7/7] net: ethernet: ti: cpsw: fix fixed-link phy probe deferral Johan Hovold
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to deregister the primary device in case the secondary emac
fails to probe.
kernel BUG at /home/johan/work/omicron/src/linux/net/core/dev.c:7743!
...
[<c05b3dec>] (free_netdev) from [<c04fe6c0>] (cpsw_probe+0x9cc/0xe50)
[<c04fe6c0>] (cpsw_probe) from [<c047b28c>] (platform_drv_probe+0x5c/0xc0)
Fixes: d9ba8f9e6298 ("driver: net: ethernet: cpsw: dual emac interface
implementation")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 4bd96732291c..421da97a5be6 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2852,12 +2852,14 @@ static int cpsw_probe(struct platform_device *pdev)
ret = cpsw_probe_dual_emac(priv);
if (ret) {
cpsw_err(priv, probe, "error probe slave 2 emac interface\n");
- goto clean_ale_ret;
+ goto clean_unregister_netdev_ret;
}
}
return 0;
+clean_unregister_netdev_ret:
+ unregister_netdev(ndev);
clean_ale_ret:
if (pm_runtime_get_sync(&pdev->dev) < 0) {
pm_runtime_put_noidle(&pdev->dev);
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 6/7] net: ethernet: ti: cpsw: add missing sanity check
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
` (4 preceding siblings ...)
2016-11-16 14:35 ` [PATCH net 5/7] net: ethernet: ti: cpsw: fix secondary-emac probe error path Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 7/7] net: ethernet: ti: cpsw: fix fixed-link phy probe deferral Johan Hovold
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to check for allocation failures before dereferencing a
NULL-pointer during probe.
Fixes: 649a1688c960 ("net: ethernet: ti: cpsw: create common struct to
hold shared driver data")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 421da97a5be6..411be060fee6 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2588,6 +2588,9 @@ static int cpsw_probe(struct platform_device *pdev)
int irq;
cpsw = devm_kzalloc(&pdev->dev, sizeof(struct cpsw_common), GFP_KERNEL);
+ if (!cpsw)
+ return -ENOMEM;
+
cpsw->dev = &pdev->dev;
ndev = alloc_etherdev_mq(sizeof(struct cpsw_priv), CPSW_MAX_QUEUES);
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net 7/7] net: ethernet: ti: cpsw: fix fixed-link phy probe deferral
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
` (5 preceding siblings ...)
2016-11-16 14:35 ` [PATCH net 6/7] net: ethernet: ti: cpsw: add missing sanity check Johan Hovold
@ 2016-11-16 14:35 ` Johan Hovold
6 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-16 14:35 UTC (permalink / raw)
To: Mugunthan V N
Cc: Grygorii Strashko, linux-omap, netdev, linux-kernel, Johan Hovold
Make sure to propagate errors from of_phy_register_fixed_link() which
can fail with -EPROBE_DEFER.
Fixes: 1f71e8c96fc6 ("drivers: net: cpsw: Add support for fixed-link
PHY")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 411be060fee6..cf8f1afa46bf 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2375,8 +2375,11 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
* to the PHY is the Ethernet MAC DT node.
*/
ret = of_phy_register_fixed_link(slave_node);
- if (ret)
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to register fixed-link phy: %d\n", ret);
return ret;
+ }
slave_data->phy_node = of_node_get(slave_node);
} else if (parp) {
u32 phyid;
@@ -2628,11 +2631,10 @@ static int cpsw_probe(struct platform_device *pdev)
/* Select default pin state */
pinctrl_pm_select_default_state(&pdev->dev);
- if (cpsw_probe_dt(&cpsw->data, pdev)) {
- dev_err(&pdev->dev, "cpsw: platform data missing\n");
- ret = -ENODEV;
+ ret = cpsw_probe_dt(&cpsw->data, pdev);
+ if (ret)
goto clean_dt_ret;
- }
+
data = &cpsw->data;
cpsw->rx_ch_num = 1;
cpsw->tx_ch_num = 1;
--
2.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path
2016-11-16 14:35 ` [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path Johan Hovold
@ 2016-11-16 20:33 ` Grygorii Strashko
2016-11-17 9:41 ` Johan Hovold
0 siblings, 1 reply; 10+ messages in thread
From: Grygorii Strashko @ 2016-11-16 20:33 UTC (permalink / raw)
To: Johan Hovold, Mugunthan V N; +Cc: linux-omap, netdev, linux-kernel
On 11/16/2016 08:35 AM, Johan Hovold wrote:
> Make sure to resume the platform device to enable clocks before
> accessing the CPSW registers in the probe error path (e.g. for deferred
> probe).
>
> Unhandled fault: external abort on non-linefetch (0x1008) at 0xd0872d08
> ...
> [<c04fabcc>] (cpsw_ale_control_set) from [<c04fb8b4>] (cpsw_ale_destroy+0x2c/0x44)
> [<c04fb8b4>] (cpsw_ale_destroy) from [<c04fea58>] (cpsw_probe+0xbd0/0x10c4)
> [<c04fea58>] (cpsw_probe) from [<c047b2a0>] (platform_drv_probe+0x5c/0xc0)
>
> Note that in the unlikely event of a runtime-resume failure, we'll leak
> the ale struct.
>
> Fixes: df828598a755 ("netdev: driver: ethernet: Add TI CPSW driver")
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/net/ethernet/ti/cpsw.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index c6cff3d2ff05..5bc5e6189661 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -2818,7 +2818,12 @@ static int cpsw_probe(struct platform_device *pdev)
> return 0;
>
> clean_ale_ret:
> - cpsw_ale_destroy(cpsw->ale);
> + if (pm_runtime_get_sync(&pdev->dev) < 0) {
> + pm_runtime_put_noidle(&pdev->dev);
> + } else {
> + cpsw_ale_destroy(cpsw->ale);
> + pm_runtime_put_sync(&pdev->dev);
> + }
> clean_dma_ret:
> cpdma_ctlr_destroy(cpsw->dma);
> clean_runtime_disable_ret:
>
I think, wouldn't it be logically more simple to just keep CPSW PM runtime enabled during probe?
Like in below diff (not tested):
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 0548e56..deaac1b 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2657,13 +2657,12 @@ static int cpsw_probe(struct platform_device *pdev)
goto clean_runtime_disable_ret;
}
cpsw->version = readl(&cpsw->regs->id_ver);
- pm_runtime_put_sync(&pdev->dev);
res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
cpsw->wr_regs = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(cpsw->wr_regs)) {
ret = PTR_ERR(cpsw->wr_regs);
- goto clean_runtime_disable_ret;
+ goto clean_runtime_put_ret;
}
memset(&dma_params, 0, sizeof(dma_params));
@@ -2700,7 +2699,7 @@ static int cpsw_probe(struct platform_device *pdev)
default:
dev_err(priv->dev, "unknown version 0x%08x\n", cpsw->version);
ret = -ENODEV;
- goto clean_runtime_disable_ret;
+ goto clean_runtime_put_ret;
}
for (i = 0; i < cpsw->data.slaves; i++) {
struct cpsw_slave *slave = &cpsw->slaves[i];
@@ -2729,7 +2728,7 @@ static int cpsw_probe(struct platform_device *pdev)
if (!cpsw->dma) {
dev_err(priv->dev, "error initializing dma\n");
ret = -ENOMEM;
- goto clean_runtime_disable_ret;
+ goto clean_runtime_put_ret;
}
cpsw->txch[0] = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0);
@@ -2831,12 +2830,16 @@ static int cpsw_probe(struct platform_device *pdev)
}
}
+ pm_runtime_put(&pdev->dev);
+
return 0;
clean_ale_ret:
cpsw_ale_destroy(cpsw->ale);
clean_dma_ret:
cpdma_ctlr_destroy(cpsw->dma);
+clean_runtime_put_ret:
+ pm_runtime_put_sync(&pdev->dev);
clean_runtime_disable_ret:
pm_runtime_disable(&pdev->dev);
clean_ndev_ret:
--
regards,
-grygorii
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path
2016-11-16 20:33 ` Grygorii Strashko
@ 2016-11-17 9:41 ` Johan Hovold
0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2016-11-17 9:41 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Johan Hovold, Mugunthan V N, linux-omap, netdev, linux-kernel
On Wed, Nov 16, 2016 at 02:33:18PM -0600, Grygorii Strashko wrote:
> On 11/16/2016 08:35 AM, Johan Hovold wrote:
> > Make sure to resume the platform device to enable clocks before
> > accessing the CPSW registers in the probe error path (e.g. for deferred
> > probe).
> >
> > Unhandled fault: external abort on non-linefetch (0x1008) at 0xd0872d08
> > ...
> > [<c04fabcc>] (cpsw_ale_control_set) from [<c04fb8b4>] (cpsw_ale_destroy+0x2c/0x44)
> > [<c04fb8b4>] (cpsw_ale_destroy) from [<c04fea58>] (cpsw_probe+0xbd0/0x10c4)
> > [<c04fea58>] (cpsw_probe) from [<c047b2a0>] (platform_drv_probe+0x5c/0xc0)
> >
> > Note that in the unlikely event of a runtime-resume failure, we'll leak
> > the ale struct.
> >
> > Fixes: df828598a755 ("netdev: driver: ethernet: Add TI CPSW driver")
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> > drivers/net/ethernet/ti/cpsw.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> > index c6cff3d2ff05..5bc5e6189661 100644
> > --- a/drivers/net/ethernet/ti/cpsw.c
> > +++ b/drivers/net/ethernet/ti/cpsw.c
> > @@ -2818,7 +2818,12 @@ static int cpsw_probe(struct platform_device *pdev)
> > return 0;
> >
> > clean_ale_ret:
> > - cpsw_ale_destroy(cpsw->ale);
> > + if (pm_runtime_get_sync(&pdev->dev) < 0) {
> > + pm_runtime_put_noidle(&pdev->dev);
> > + } else {
> > + cpsw_ale_destroy(cpsw->ale);
> > + pm_runtime_put_sync(&pdev->dev);
> > + }
> > clean_dma_ret:
> > cpdma_ctlr_destroy(cpsw->dma);
> > clean_runtime_disable_ret:
> >
>
> I think, wouldn't it be logically more simple to just keep CPSW PM
> runtime enabled during probe?
Indeed it would. I'll do that in a v2.
Thanks,
Johan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-17 9:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-16 14:35 [PATCH net 0/7] net: cpsw: fix leaks and probe deferral Johan Hovold
2016-11-16 14:35 ` [PATCH net 1/7] net: ethernet: ti: cpsw: fix bad register access in probe error path Johan Hovold
2016-11-16 20:33 ` Grygorii Strashko
2016-11-17 9:41 ` Johan Hovold
2016-11-16 14:35 ` [PATCH net 2/7] net: ethernet: ti: cpsw: fix mdio device reference leak Johan Hovold
2016-11-16 14:35 ` [PATCH net 3/7] net: ethernet: ti: cpsw: fix deferred probe Johan Hovold
2016-11-16 14:35 ` [PATCH net 4/7] net: ethernet: ti: cpsw: fix of_node and phydev leaks Johan Hovold
2016-11-16 14:35 ` [PATCH net 5/7] net: ethernet: ti: cpsw: fix secondary-emac probe error path Johan Hovold
2016-11-16 14:35 ` [PATCH net 6/7] net: ethernet: ti: cpsw: add missing sanity check Johan Hovold
2016-11-16 14:35 ` [PATCH net 7/7] net: ethernet: ti: cpsw: fix fixed-link phy probe deferral Johan Hovold
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).