* [PATCH 1/6] fec: Do not enable/disable optional clocks unconditionally
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
@ 2013-07-21 16:24 ` Fabio Estevam
2013-07-21 16:25 ` [PATCH 2/6] fec: Fix the order for enabling/disabling the clocks Fabio Estevam
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:24 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
clk_enet_out and clk_ptp are optional clocks, so we should not enable/disable
them unconditionally.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f0f0e96..de9bf7a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2105,8 +2105,10 @@ fec_probe(struct platform_device *pdev)
clk_prepare_enable(fep->clk_ahb);
clk_prepare_enable(fep->clk_ipg);
- clk_prepare_enable(fep->clk_enet_out);
- clk_prepare_enable(fep->clk_ptp);
+ if (fep->clk_enet_out)
+ clk_prepare_enable(fep->clk_enet_out);
+ if (fep->clk_ptp)
+ clk_prepare_enable(fep->clk_ptp);
fep->reg_phy = devm_regulator_get(&pdev->dev, "phy");
if (!IS_ERR(fep->reg_phy)) {
@@ -2179,8 +2181,10 @@ failed_init:
failed_regulator:
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
- clk_disable_unprepare(fep->clk_enet_out);
- clk_disable_unprepare(fep->clk_ptp);
+ if (fep->clk_enet_out)
+ clk_disable_unprepare(fep->clk_enet_out);
+ if (fep->clk_ptp)
+ clk_disable_unprepare(fep->clk_ptp);
failed_clk:
failed_ioremap:
free_netdev(ndev);
@@ -2206,10 +2210,12 @@ fec_drv_remove(struct platform_device *pdev)
}
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
- clk_disable_unprepare(fep->clk_ptp);
+ if (fep->clk_ptp)
+ clk_disable_unprepare(fep->clk_ptp);
if (fep->ptp_clock)
ptp_clock_unregister(fep->ptp_clock);
- clk_disable_unprepare(fep->clk_enet_out);
+ if (fep->clk_enet_out)
+ clk_disable_unprepare(fep->clk_enet_out);
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
free_netdev(ndev);
@@ -2228,7 +2234,8 @@ fec_suspend(struct device *dev)
fec_stop(ndev);
netif_device_detach(ndev);
}
- clk_disable_unprepare(fep->clk_enet_out);
+ if (fep->clk_enet_out)
+ clk_disable_unprepare(fep->clk_enet_out);
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
@@ -2251,7 +2258,8 @@ fec_resume(struct device *dev)
return ret;
}
- clk_prepare_enable(fep->clk_enet_out);
+ if (fep->clk_enet_out)
+ clk_prepare_enable(fep->clk_enet_out);
clk_prepare_enable(fep->clk_ahb);
clk_prepare_enable(fep->clk_ipg);
if (netif_running(ndev)) {
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/6] fec: Fix the order for enabling/disabling the clocks
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
2013-07-21 16:24 ` [PATCH 1/6] fec: Do not enable/disable optional clocks unconditionally Fabio Estevam
@ 2013-07-21 16:25 ` Fabio Estevam
2013-07-21 16:25 ` [PATCH 3/6] fec: Enable/disable clk_ptp in suspend/resume Fabio Estevam
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:25 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
On fec_probe the clocks are enabled in the following order:
clk_ahb -> clk_ipg -> clk_enet_out -> clk_ptp
, so in the error and remove paths we should disabled them in the opposite
order.
Also fix the order in the suspend/resume functions.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index de9bf7a..5e82200 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2179,12 +2179,12 @@ failed_init:
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
failed_regulator:
- clk_disable_unprepare(fep->clk_ahb);
- clk_disable_unprepare(fep->clk_ipg);
- if (fep->clk_enet_out)
- clk_disable_unprepare(fep->clk_enet_out);
if (fep->clk_ptp)
clk_disable_unprepare(fep->clk_ptp);
+ if (fep->clk_enet_out)
+ clk_disable_unprepare(fep->clk_enet_out);
+ clk_disable_unprepare(fep->clk_ipg);
+ clk_disable_unprepare(fep->clk_ahb);
failed_clk:
failed_ioremap:
free_netdev(ndev);
@@ -2216,8 +2216,8 @@ fec_drv_remove(struct platform_device *pdev)
ptp_clock_unregister(fep->ptp_clock);
if (fep->clk_enet_out)
clk_disable_unprepare(fep->clk_enet_out);
- clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
+ clk_disable_unprepare(fep->clk_ahb);
free_netdev(ndev);
return 0;
@@ -2236,8 +2236,8 @@ fec_suspend(struct device *dev)
}
if (fep->clk_enet_out)
clk_disable_unprepare(fep->clk_enet_out);
- clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
+ clk_disable_unprepare(fep->clk_ahb);
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
@@ -2258,10 +2258,10 @@ fec_resume(struct device *dev)
return ret;
}
- if (fep->clk_enet_out)
- clk_prepare_enable(fep->clk_enet_out);
clk_prepare_enable(fep->clk_ahb);
clk_prepare_enable(fep->clk_ipg);
+ if (fep->clk_enet_out)
+ clk_prepare_enable(fep->clk_enet_out);
if (netif_running(ndev)) {
fec_restart(ndev, fep->full_duplex);
netif_device_attach(ndev);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/6] fec: Enable/disable clk_ptp in suspend/resume
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
2013-07-21 16:24 ` [PATCH 1/6] fec: Do not enable/disable optional clocks unconditionally Fabio Estevam
2013-07-21 16:25 ` [PATCH 2/6] fec: Fix the order for enabling/disabling the clocks Fabio Estevam
@ 2013-07-21 16:25 ` Fabio Estevam
2013-07-21 16:25 ` [PATCH 4/6] fec: Check the return value from clk_prepare_enable() Fabio Estevam
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:25 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
clk_ptp should also be enabled in fec_resume() and disabled in fec_suspend().
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5e82200..886a372 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2234,6 +2234,8 @@ fec_suspend(struct device *dev)
fec_stop(ndev);
netif_device_detach(ndev);
}
+ if (fep->clk_ptp)
+ clk_disable_unprepare(fep->clk_ptp);
if (fep->clk_enet_out)
clk_disable_unprepare(fep->clk_enet_out);
clk_disable_unprepare(fep->clk_ipg);
@@ -2262,6 +2264,8 @@ fec_resume(struct device *dev)
clk_prepare_enable(fep->clk_ipg);
if (fep->clk_enet_out)
clk_prepare_enable(fep->clk_enet_out);
+ if (fep->clk_ptp)
+ clk_prepare_enable(fep->clk_ptp);
if (netif_running(ndev)) {
fec_restart(ndev, fep->full_duplex);
netif_device_attach(ndev);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] fec: Check the return value from clk_prepare_enable()
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
` (2 preceding siblings ...)
2013-07-21 16:25 ` [PATCH 3/6] fec: Enable/disable clk_ptp in suspend/resume Fabio Estevam
@ 2013-07-21 16:25 ` Fabio Estevam
2013-07-21 16:25 ` [PATCH 5/6] fec: Remove unneeded check in platform_get_resource() Fabio Estevam
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:25 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
clk_prepare_enable() may fail, so let's check its return value and propagate it
in the case of error.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 66 +++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 886a372..acc1850 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2103,12 +2103,25 @@ fec_probe(struct platform_device *pdev)
fep->bufdesc_ex = 0;
}
- clk_prepare_enable(fep->clk_ahb);
- clk_prepare_enable(fep->clk_ipg);
- if (fep->clk_enet_out)
- clk_prepare_enable(fep->clk_enet_out);
- if (fep->clk_ptp)
- clk_prepare_enable(fep->clk_ptp);
+ ret = clk_prepare_enable(fep->clk_ahb);
+ if (ret)
+ goto failed_clk;
+
+ ret = clk_prepare_enable(fep->clk_ipg);
+ if (ret)
+ goto failed_clk_ipg;
+
+ if (fep->clk_enet_out) {
+ ret = clk_prepare_enable(fep->clk_enet_out);
+ if (ret)
+ goto failed_clk_enet_out;
+ }
+
+ if (fep->clk_ptp) {
+ ret = clk_prepare_enable(fep->clk_ptp);
+ if (ret)
+ goto failed_clk_ptp;
+ }
fep->reg_phy = devm_regulator_get(&pdev->dev, "phy");
if (!IS_ERR(fep->reg_phy)) {
@@ -2181,9 +2194,12 @@ failed_init:
failed_regulator:
if (fep->clk_ptp)
clk_disable_unprepare(fep->clk_ptp);
+failed_clk_ptp:
if (fep->clk_enet_out)
clk_disable_unprepare(fep->clk_enet_out);
+failed_clk_enet_out:
clk_disable_unprepare(fep->clk_ipg);
+failed_clk_ipg:
clk_disable_unprepare(fep->clk_ahb);
failed_clk:
failed_ioremap:
@@ -2260,18 +2276,44 @@ fec_resume(struct device *dev)
return ret;
}
- clk_prepare_enable(fep->clk_ahb);
- clk_prepare_enable(fep->clk_ipg);
- if (fep->clk_enet_out)
- clk_prepare_enable(fep->clk_enet_out);
- if (fep->clk_ptp)
- clk_prepare_enable(fep->clk_ptp);
+ ret = clk_prepare_enable(fep->clk_ahb);
+ if (ret)
+ goto failed_clk_ahb;
+
+ ret = clk_prepare_enable(fep->clk_ipg);
+ if (ret)
+ goto failed_clk_ipg;
+
+ if (fep->clk_enet_out) {
+ ret = clk_prepare_enable(fep->clk_enet_out);
+ if (ret)
+ goto failed_clk_enet_out;
+ }
+
+ if (fep->clk_ptp) {
+ ret = clk_prepare_enable(fep->clk_ptp);
+ if (ret)
+ goto failed_clk_ptp;
+ }
+
if (netif_running(ndev)) {
fec_restart(ndev, fep->full_duplex);
netif_device_attach(ndev);
}
return 0;
+
+failed_clk_ptp:
+ if (fep->clk_enet_out)
+ clk_disable_unprepare(fep->clk_enet_out);
+failed_clk_enet_out:
+ clk_disable_unprepare(fep->clk_ipg);
+failed_clk_ipg:
+ clk_disable_unprepare(fep->clk_ahb);
+failed_clk_ahb:
+ if (fep->reg_phy)
+ regulator_disable(fep->reg_phy);
+ return ret;
}
#endif /* CONFIG_PM_SLEEP */
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/6] fec: Remove unneeded check in platform_get_resource()
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
` (3 preceding siblings ...)
2013-07-21 16:25 ` [PATCH 4/6] fec: Check the return value from clk_prepare_enable() Fabio Estevam
@ 2013-07-21 16:25 ` Fabio Estevam
2013-07-21 16:25 ` [PATCH 6/6] fec: Use devm_request_irq() Fabio Estevam
2013-07-22 4:21 ` [PATCH 0/6] fec: Improve resources handling Frank Li
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:25 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
As devm_ioremap_resource() is used, there is no need to explicitely check the
return value from platform_get_resource(), as this is something that
devm_ioremap_resource() takes care by itself.
Also, place platform_get_resource() prior to devm_ioremap_resource() for
better code readability.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index acc1850..03028c4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2033,10 +2033,6 @@ fec_probe(struct platform_device *pdev)
if (of_id)
pdev->id_entry = of_id->data;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!r)
- return -ENXIO;
-
/* Init network device */
ndev = alloc_etherdev(sizeof(struct fec_enet_private));
if (!ndev)
@@ -2054,6 +2050,7 @@ fec_probe(struct platform_device *pdev)
fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
#endif
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
fep->hwp = devm_ioremap_resource(&pdev->dev, r);
if (IS_ERR(fep->hwp)) {
ret = PTR_ERR(fep->hwp);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/6] fec: Use devm_request_irq()
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
` (4 preceding siblings ...)
2013-07-21 16:25 ` [PATCH 5/6] fec: Remove unneeded check in platform_get_resource() Fabio Estevam
@ 2013-07-21 16:25 ` Fabio Estevam
2013-07-22 4:21 ` [PATCH 0/6] fec: Improve resources handling Frank Li
6 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2013-07-21 16:25 UTC (permalink / raw)
To: davem; +Cc: kernel, Frank.Li, shawn.guo, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
Using devm_request_irq() can make the code smaller and cleaner.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 03028c4..0642006 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2149,14 +2149,10 @@ fec_probe(struct platform_device *pdev)
ret = irq;
goto failed_irq;
}
- ret = request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
- if (ret) {
- while (--i >= 0) {
- irq = platform_get_irq(pdev, i);
- free_irq(irq, ndev);
- }
+ ret = devm_request_irq(&pdev->dev, irq, fec_enet_interrupt,
+ IRQF_DISABLED, pdev->name, ndev);
+ if (ret)
goto failed_irq;
- }
}
ret = fec_enet_mii_init(pdev);
@@ -2180,11 +2176,6 @@ failed_register:
fec_enet_mii_remove(fep);
failed_mii_init:
failed_irq:
- for (i = 0; i < FEC_IRQ_NUM; i++) {
- irq = platform_get_irq(pdev, i);
- if (irq > 0)
- free_irq(irq, ndev);
- }
failed_init:
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
@@ -2210,17 +2201,11 @@ fec_drv_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
- int i;
cancel_delayed_work_sync(&(fep->delay_work.delay_work));
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
del_timer_sync(&fep->time_keep);
- for (i = 0; i < FEC_IRQ_NUM; i++) {
- int irq = platform_get_irq(pdev, i);
- if (irq > 0)
- free_irq(irq, ndev);
- }
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
if (fep->clk_ptp)
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] fec: Improve resources handling
2013-07-21 16:24 [PATCH 0/6] fec: Improve resources handling Fabio Estevam
` (5 preceding siblings ...)
2013-07-21 16:25 ` [PATCH 6/6] fec: Use devm_request_irq() Fabio Estevam
@ 2013-07-22 4:21 ` Frank Li
2013-07-22 23:51 ` David Miller
6 siblings, 1 reply; 9+ messages in thread
From: Frank Li @ 2013-07-22 4:21 UTC (permalink / raw)
To: Fabio Estevam; +Cc: David Miller, kernel, Frank.Li, Shawn Guo, netdev
2013/7/22 Fabio Estevam <festevam@gmail.com>:
> This series improves clock handling in the driver by not enabling/disabling
> the optional ptp and enet_out clocks unconditionally, check for the return value
> of clk_prepare_enable and also handle clk_ptp in suspend/resume.
>
> Remove an unneeded check in platform_get_resource() and also use
> devm_request_irq() that can help to simplify the code.
>
> Fabio Estevam (6):
> fec: Do not enable/disable optional clocks unconditionally
> fec: Fix the order for enabling/disabling the clocks
> fec: Enable/disable clk_ptp in suspend/resume
> fec: Check the return value from clk_prepare_enable()
> fec: Remove unneeded check in platform_get_resource()
> fec: Use devm_request_irq()
>
> drivers/net/ethernet/freescale/fec_main.c | 110 ++++++++++++++++++++----------
> 1 file changed, 73 insertions(+), 37 deletions(-)
Acked
>
> --
> 1.8.1.2
>
> --
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] fec: Improve resources handling
2013-07-22 4:21 ` [PATCH 0/6] fec: Improve resources handling Frank Li
@ 2013-07-22 23:51 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-07-22 23:51 UTC (permalink / raw)
To: lznuaa; +Cc: festevam, kernel, Frank.Li, shawn.guo, netdev
From: Frank Li <lznuaa@gmail.com>
Date: Mon, 22 Jul 2013 12:21:58 +0800
> 2013/7/22 Fabio Estevam <festevam@gmail.com>:
>> This series improves clock handling in the driver by not enabling/disabling
>> the optional ptp and enet_out clocks unconditionally, check for the return value
>> of clk_prepare_enable and also handle clk_ptp in suspend/resume.
>>
>> Remove an unneeded check in platform_get_resource() and also use
>> devm_request_irq() that can help to simplify the code.
>>
>> Fabio Estevam (6):
>> fec: Do not enable/disable optional clocks unconditionally
>> fec: Fix the order for enabling/disabling the clocks
>> fec: Enable/disable clk_ptp in suspend/resume
>> fec: Check the return value from clk_prepare_enable()
>> fec: Remove unneeded check in platform_get_resource()
>> fec: Use devm_request_irq()
>>
>> drivers/net/ethernet/freescale/fec_main.c | 110 ++++++++++++++++++++----------
>> 1 file changed, 73 insertions(+), 37 deletions(-)
>
> Acked
Series applied to net-next, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread