* [PATCH net-next 1/3] fec: Do not set fep->clk_ptp to NULL on error
2014-09-02 1:12 [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error Fabio Estevam
@ 2014-09-02 1:12 ` Fabio Estevam
2014-09-02 1:12 ` [PATCH net-next 2/3] fec: Do not set fep->clk_enet_out " Fabio Estevam
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2014-09-02 1:12 UTC (permalink / raw)
To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
There is no need to set fep->clk_ptp to NULL when devm_clk_get() returns an
error. We can simply use IS_ERR() instead, which makes the code simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 89355a7..76e4f56 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1610,7 +1610,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
if (ret)
goto failed_clk_enet_out;
}
- if (fep->clk_ptp) {
+ if (!IS_ERR(fep->clk_ptp)) {
mutex_lock(&fep->ptp_clk_mutex);
ret = clk_prepare_enable(fep->clk_ptp);
if (ret) {
@@ -1626,7 +1626,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
clk_disable_unprepare(fep->clk_ipg);
if (fep->clk_enet_out)
clk_disable_unprepare(fep->clk_enet_out);
- if (fep->clk_ptp) {
+ if (!IS_ERR(fep->clk_ptp)) {
mutex_lock(&fep->ptp_clk_mutex);
clk_disable_unprepare(fep->clk_ptp);
fep->ptp_clk_on = false;
@@ -2640,10 +2640,8 @@ fec_probe(struct platform_device *pdev)
fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");
fep->bufdesc_ex =
pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
- if (IS_ERR(fep->clk_ptp)) {
- fep->clk_ptp = NULL;
+ if (IS_ERR(fep->clk_ptp))
fep->bufdesc_ex = 0;
- }
ret = fec_enet_clk_enable(ndev, true);
if (ret)
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 2/3] fec: Do not set fep->clk_enet_out to NULL on error
2014-09-02 1:12 [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error Fabio Estevam
2014-09-02 1:12 ` [PATCH net-next 1/3] fec: Do not set fep->clk_ptp to NULL on error Fabio Estevam
@ 2014-09-02 1:12 ` Fabio Estevam
2014-09-02 1:12 ` [PATCH net-next 3/3] fec: Do not set fep->reg_phy " Fabio Estevam
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2014-09-02 1:12 UTC (permalink / raw)
To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
There is no need to set fep->clk_enet_out to NULL when devm_clk_get() returns an
error. We can simply use IS_ERR() instead, which makes the code simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 76e4f56..e87cc66 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1605,7 +1605,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
ret = clk_prepare_enable(fep->clk_ipg);
if (ret)
goto failed_clk_ipg;
- if (fep->clk_enet_out) {
+ if (!IS_ERR(fep->clk_enet_out)) {
ret = clk_prepare_enable(fep->clk_enet_out);
if (ret)
goto failed_clk_enet_out;
@@ -1624,7 +1624,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
} else {
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
- if (fep->clk_enet_out)
+ if (!IS_ERR(fep->clk_enet_out))
clk_disable_unprepare(fep->clk_enet_out);
if (!IS_ERR(fep->clk_ptp)) {
mutex_lock(&fep->ptp_clk_mutex);
@@ -1636,7 +1636,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
return 0;
failed_clk_ptp:
- if (fep->clk_enet_out)
+ if (!IS_ERR(fep->clk_enet_out))
clk_disable_unprepare(fep->clk_enet_out);
failed_clk_enet_out:
clk_disable_unprepare(fep->clk_ipg);
@@ -2632,8 +2632,6 @@ fec_probe(struct platform_device *pdev)
/* enet_out is optional, depends on board */
fep->clk_enet_out = devm_clk_get(&pdev->dev, "enet_out");
- if (IS_ERR(fep->clk_enet_out))
- fep->clk_enet_out = NULL;
fep->ptp_clk_on = false;
mutex_init(&fep->ptp_clk_mutex);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 3/3] fec: Do not set fep->reg_phy to NULL on error
2014-09-02 1:12 [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error Fabio Estevam
2014-09-02 1:12 ` [PATCH net-next 1/3] fec: Do not set fep->clk_ptp to NULL on error Fabio Estevam
2014-09-02 1:12 ` [PATCH net-next 2/3] fec: Do not set fep->clk_enet_out " Fabio Estevam
@ 2014-09-02 1:12 ` Fabio Estevam
2014-09-02 1:37 ` [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error fugang.duan
2014-09-02 20:48 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2014-09-02 1:12 UTC (permalink / raw)
To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
There is no need to set fep->reg_phy to NULL when devm_regulator_get()
returns an error. We can simply use IS_ERR() instead, which makes the code
simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e87cc66..39c28d1 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2653,8 +2653,6 @@ fec_probe(struct platform_device *pdev)
"Failed to enable phy regulator: %d\n", ret);
goto failed_regulator;
}
- } else {
- fep->reg_phy = NULL;
}
fec_reset_phy(pdev);
@@ -2704,7 +2702,7 @@ failed_register:
failed_mii_init:
failed_irq:
failed_init:
- if (fep->reg_phy)
+ if (!IS_ERR(fep->reg_phy))
regulator_disable(fep->reg_phy);
failed_regulator:
fec_enet_clk_enable(ndev, false);
@@ -2727,7 +2725,7 @@ fec_drv_remove(struct platform_device *pdev)
cancel_work_sync(&fep->tx_timeout_work);
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
- if (fep->reg_phy)
+ if (!IS_ERR(fep->reg_phy))
regulator_disable(fep->reg_phy);
if (fep->ptp_clock)
ptp_clock_unregister(fep->ptp_clock);
@@ -2757,7 +2755,7 @@ static int __maybe_unused fec_suspend(struct device *dev)
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(&fep->pdev->dev);
- if (fep->reg_phy)
+ if (!IS_ERR(fep->reg_phy))
regulator_disable(fep->reg_phy);
return 0;
@@ -2769,7 +2767,7 @@ static int __maybe_unused fec_resume(struct device *dev)
struct fec_enet_private *fep = netdev_priv(ndev);
int ret;
- if (fep->reg_phy) {
+ if (!IS_ERR(fep->reg_phy)) {
ret = regulator_enable(fep->reg_phy);
if (ret)
return ret;
@@ -2794,7 +2792,7 @@ static int __maybe_unused fec_resume(struct device *dev)
return 0;
failed_clk:
- if (fep->reg_phy)
+ if (!IS_ERR(fep->reg_phy))
regulator_disable(fep->reg_phy);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error
2014-09-02 1:12 [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error Fabio Estevam
` (2 preceding siblings ...)
2014-09-02 1:12 ` [PATCH net-next 3/3] fec: Do not set fep->reg_phy " Fabio Estevam
@ 2014-09-02 1:37 ` fugang.duan
2014-09-02 20:48 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: fugang.duan @ 2014-09-02 1:37 UTC (permalink / raw)
To: Fabio Estevam, davem@davemloft.net
Cc: linux@arm.linux.org.uk, netdev@vger.kernel.org,
Fabio.Estevam@freescale.com
From: Fabio Estevam <festevam@gmail.com> Sent: Tuesday, September 02, 2014 9:13 AM
>To: davem@davemloft.net
>Cc: linux@arm.linux.org.uk; Duan Fugang-B38611; netdev@vger.kernel.org;
>Estevam Fabio-R49496
>Subject: [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate
>error
>
>From: Fabio Estevam <fabio.estevam@freescale.com>
>
>Using IS_ERR() to indicate error can simplify the code a bit, so let's use
>it when appropriate.
>
>Fabio Estevam (3):
> fec: Do not set fep->clk_ptp to NULL on error
> fec: Do not set fep->clk_enet_out to NULL on error
> fec: Do not set fep->reg_phy to NULL on error
>
> drivers/net/ethernet/freescale/fec_main.c | 28 +++++++++++---------------
>--
> 1 file changed, 11 insertions(+), 17 deletions(-)
>
>--
>1.9.1
The patch serial are fine.
Acked-by: Fugang Duan <B38611@freescale.com>
Thanks,
Andy
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error
2014-09-02 1:12 [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error Fabio Estevam
` (3 preceding siblings ...)
2014-09-02 1:37 ` [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error fugang.duan
@ 2014-09-02 20:48 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-09-02 20:48 UTC (permalink / raw)
To: festevam; +Cc: linux, B38611, netdev, fabio.estevam
From: Fabio Estevam <festevam@gmail.com>
Date: Mon, 1 Sep 2014 22:12:52 -0300
> Using IS_ERR() to indicate error can simplify the code a bit, so let's use
> it when appropriate.
I would rather you not do this.
Generally speaking we should not leave pointer error values in
structure pointers.
It's just asking for someone to later accidently kfree or
release the "object" because pointer errors evaluate to
non-NULL.
^ permalink raw reply [flat|nested] 6+ messages in thread