* [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
@ 2017-11-06 21:53 ` Christophe JAILLET
0 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
conflicts in my patches waiting for submission. So I took a closer look at
it.
So here is a serie of 4 patches.
The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
done in only 1 error handling path in the function.
The 2nd one removes some devm_iounmap/release/kfree functions which look
useless to me.
The 3rd one fixes a missing of_node_put.
The 4th one is just cosmetic and removes a useless message.
Christophe JAILLET (4):
fsl/fman: Remove a useless call to 'dev_set_drvdata()'
fsl/fman: Remove some useless code
fsl/fman: Add a missing 'of_node_put()' call in an error handling path
fsl/fman: Remove a useless 'dev_err()' call
drivers/net/ethernet/freescale/fman/mac.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/4] fsl/fman: Remove a useless call to 'dev_set_drvdata()'
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-06 21:53 ` Christophe JAILLET
-1 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Commit c6e26ea8c893 ("dpaa_eth: change device used") has removed usage of
'dev_set_drvdata()' in the 'mac_probe() function.
This call should also be axed.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 1d6da1ea7bfb..c27667a005f7 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -713,7 +713,6 @@ static int mac_probe(struct platform_device *_of_dev)
__devm_release_region(dev, fman_get_mem_region(priv->fman),
res.start, res.end + 1 - res.start);
devm_kfree(dev, mac_dev);
- dev_set_drvdata(dev, NULL);
return -ENODEV;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 1/4] fsl/fman: Remove a useless call to 'dev_set_drvdata()'
@ 2017-11-06 21:53 ` Christophe JAILLET
0 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Commit c6e26ea8c893 ("dpaa_eth: change device used") has removed usage of
'dev_set_drvdata()' in the 'mac_probe() function.
This call should also be axed.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 1d6da1ea7bfb..c27667a005f7 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -713,7 +713,6 @@ static int mac_probe(struct platform_device *_of_dev)
__devm_release_region(dev, fman_get_mem_region(priv->fman),
res.start, res.end + 1 - res.start);
devm_kfree(dev, mac_dev);
- dev_set_drvdata(dev, NULL);
return -ENODEV;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] fsl/fman: Remove some useless code
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-06 21:53 ` Christophe JAILLET
-1 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
There is no need to release explicitly some devm_ allocated resources.
If the 'mac_probe()' probe function fails, they will be released
automatically, as already done in the other error handling paths of
this function.
Also goto '_return_of_get_parent' as in the other error handling paths.
This is useless (priv->fixed_link is NULL at this point), but at least
it is consistent.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index c27667a005f7..ca12e28129ed 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -709,11 +709,8 @@ static int mac_probe(struct platform_device *_of_dev)
}
if (!of_device_is_available(mac_node)) {
- devm_iounmap(dev, priv->vaddr);
- __devm_release_region(dev, fman_get_mem_region(priv->fman),
- res.start, res.end + 1 - res.start);
- devm_kfree(dev, mac_dev);
- return -ENODEV;
+ err = -ENODEV;
+ goto _return_of_get_parent;
}
/* Get the cell-index */
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/4] fsl/fman: Remove some useless code
@ 2017-11-06 21:53 ` Christophe JAILLET
0 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
There is no need to release explicitly some devm_ allocated resources.
If the 'mac_probe()' probe function fails, they will be released
automatically, as already done in the other error handling paths of
this function.
Also goto '_return_of_get_parent' as in the other error handling paths.
This is useless (priv->fixed_link is NULL at this point), but at least
it is consistent.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index c27667a005f7..ca12e28129ed 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -709,11 +709,8 @@ static int mac_probe(struct platform_device *_of_dev)
}
if (!of_device_is_available(mac_node)) {
- devm_iounmap(dev, priv->vaddr);
- __devm_release_region(dev, fman_get_mem_region(priv->fman),
- res.start, res.end + 1 - res.start);
- devm_kfree(dev, mac_dev);
- return -ENODEV;
+ err = -ENODEV;
+ goto _return_of_get_parent;
}
/* Get the cell-index */
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/4] fsl/fman: Remove some useless code
2017-11-06 21:53 ` Christophe JAILLET
(?)
@ 2017-11-07 6:29 ` Christophe JAILLET
-1 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-07 6:29 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kernel-janitors
Le 06/11/2017 à 22:53, Christophe JAILLET a écrit :
> There is no need to release explicitly some devm_ allocated resources.
> If the 'mac_probe()' probe function fails, they will be released
> automatically, as already done in the other error handling paths of
> this function.
>
> Also goto '_return_of_get_parent' as in the other error handling paths.
> This is useless (priv->fixed_link is NULL at this point), but at least
> it is consistent.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/net/ethernet/freescale/fman/mac.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index c27667a005f7..ca12e28129ed 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -709,11 +709,8 @@ static int mac_probe(struct platform_device *_of_dev)
> }
>
> if (!of_device_is_available(mac_node)) {
> - devm_iounmap(dev, priv->vaddr);
> - __devm_release_region(dev, fman_get_mem_region(priv->fman),
> - res.start, res.end + 1 - res.start);
> - devm_kfree(dev, mac_dev);
> - return -ENODEV;
> + err = -ENODEV;
> + goto _return_of_get_parent;
> }
>
> /* Get the cell-index */
Or maybe 'return -EPROBE_DEFER' was expected?
CJ
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] fsl/fman: Add a missing 'of_node_put()' call in an error handling path
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-06 21:53 ` Christophe JAILLET
-1 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
If 'of_phy_find_device()' fails, we must undo the previous 'of_node_get()'
call, as done the the following error handling code.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index ca12e28129ed..86c1e69f44d6 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -821,6 +821,7 @@ static int mac_probe(struct platform_device *_of_dev)
phy = of_phy_find_device(mac_dev->phy_node);
if (!phy) {
err = -EINVAL;
+ of_node_put(mac_dev->phy_node);
goto _return_of_get_parent;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/4] fsl/fman: Add a missing 'of_node_put()' call in an error handling path
@ 2017-11-06 21:53 ` Christophe JAILLET
0 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
If 'of_phy_find_device()' fails, we must undo the previous 'of_node_get()'
call, as done the the following error handling code.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index ca12e28129ed..86c1e69f44d6 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -821,6 +821,7 @@ static int mac_probe(struct platform_device *_of_dev)
phy = of_phy_find_device(mac_dev->phy_node);
if (!phy) {
err = -EINVAL;
+ of_node_put(mac_dev->phy_node);
goto _return_of_get_parent;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] fsl/fman: Remove a useless 'dev_err()' call
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-06 21:53 ` Christophe JAILLET
-1 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Memory allocation functions already display some informaton in case of
memory allocation failure. There is no need to add an extra 'dev_err' here.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 86c1e69f44d6..88c0a0636b44 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -615,7 +615,6 @@ static int mac_probe(struct platform_device *_of_dev)
mac_dev = devm_kzalloc(dev, sizeof(*mac_dev), GFP_KERNEL);
if (!mac_dev) {
err = -ENOMEM;
- dev_err(dev, "devm_kzalloc() = %d\n", err);
goto _return;
}
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/4] fsl/fman: Remove a useless 'dev_err()' call
@ 2017-11-06 21:53 ` Christophe JAILLET
0 siblings, 0 replies; 15+ messages in thread
From: Christophe JAILLET @ 2017-11-06 21:53 UTC (permalink / raw)
To: madalin.bucur; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Memory allocation functions already display some informaton in case of
memory allocation failure. There is no need to add an extra 'dev_err' here.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 86c1e69f44d6..88c0a0636b44 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -615,7 +615,6 @@ static int mac_probe(struct platform_device *_of_dev)
mac_dev = devm_kzalloc(dev, sizeof(*mac_dev), GFP_KERNEL);
if (!mac_dev) {
err = -ENOMEM;
- dev_err(dev, "devm_kzalloc() = %d\n", err);
goto _return;
}
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
--
2.14.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-07 7:14 ` Madalin-cristian Bucur
-1 siblings, 0 replies; 15+ messages in thread
From: Madalin-cristian Bucur @ 2017-11-07 7:14 UTC (permalink / raw)
To: Christophe JAILLET; +Cc: netdev, linux-kernel, kernel-janitors
Hi Christophe,
I'll review and test your fixes.
Thank you!
Madalin
> -----Original Message-----
> From: Christophe JAILLET [mailto:christophe.jaillet@wanadoo.fr]
> Sent: Monday, November 06, 2017 11:53 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janitors@vger.kernel.org; Christophe JAILLET
> <christophe.jaillet@wanadoo.fr>
> Subject: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
>
> Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
> conflicts in my patches waiting for submission. So I took a closer look at
> it.
>
>
> So here is a serie of 4 patches.
>
> The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
> done in only 1 error handling path in the function.
>
> The 2nd one removes some devm_iounmap/release/kfree functions which look
> useless to me.
>
> The 3rd one fixes a missing of_node_put.
>
> The 4th one is just cosmetic and removes a useless message.
>
>
> Christophe JAILLET (4):
> fsl/fman: Remove a useless call to 'dev_set_drvdata()'
> fsl/fman: Remove some useless code
> fsl/fman: Add a missing 'of_node_put()' call in an error handling path
> fsl/fman: Remove a useless 'dev_err()' call
>
> drivers/net/ethernet/freescale/fman/mac.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> --
> 2.14.1
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
@ 2017-11-07 7:14 ` Madalin-cristian Bucur
0 siblings, 0 replies; 15+ messages in thread
From: Madalin-cristian Bucur @ 2017-11-07 7:14 UTC (permalink / raw)
To: Christophe JAILLET; +Cc: netdev, linux-kernel, kernel-janitors
Hi Christophe,
I'll review and test your fixes.
Thank you!
Madalin
> -----Original Message-----
> From: Christophe JAILLET [mailto:christophe.jaillet@wanadoo.fr]
> Sent: Monday, November 06, 2017 11:53 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janitors@vger.kernel.org; Christophe JAILLET
> <christophe.jaillet@wanadoo.fr>
> Subject: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
>
> Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
> conflicts in my patches waiting for submission. So I took a closer look at
> it.
>
>
> So here is a serie of 4 patches.
>
> The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
> done in only 1 error handling path in the function.
>
> The 2nd one removes some devm_iounmap/release/kfree functions which look
> useless to me.
>
> The 3rd one fixes a missing of_node_put.
>
> The 4th one is just cosmetic and removes a useless message.
>
>
> Christophe JAILLET (4):
> fsl/fman: Remove a useless call to 'dev_set_drvdata()'
> fsl/fman: Remove some useless code
> fsl/fman: Add a missing 'of_node_put()' call in an error handling path
> fsl/fman: Remove a useless 'dev_err()' call
>
> drivers/net/ethernet/freescale/fman/mac.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> --
> 2.14.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
2017-11-06 21:53 ` Christophe JAILLET
@ 2017-11-08 4:53 ` David Miller
-1 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2017-11-08 4:53 UTC (permalink / raw)
To: christophe.jaillet; +Cc: madalin.bucur, netdev, linux-kernel, kernel-janitors
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Mon, 6 Nov 2017 22:53:28 +0100
> Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
> conflicts in my patches waiting for submission. So I took a closer look at
> it.
>
>
> So here is a serie of 4 patches.
>
> The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
> done in only 1 error handling path in the function.
>
> The 2nd one removes some devm_iounmap/release/kfree functions which look
> useless to me.
>
> The 3rd one fixes a missing of_node_put.
>
> The 4th one is just cosmetic and removes a useless message.
Series applied to net-next, thank you.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe
@ 2017-11-08 4:53 ` David Miller
0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2017-11-08 4:53 UTC (permalink / raw)
To: christophe.jaillet; +Cc: madalin.bucur, netdev, linux-kernel, kernel-janitors
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Mon, 6 Nov 2017 22:53:28 +0100
> Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
> conflicts in my patches waiting for submission. So I took a closer look at
> it.
>
>
> So here is a serie of 4 patches.
>
> The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
> done in only 1 error handling path in the function.
>
> The 2nd one removes some devm_iounmap/release/kfree functions which look
> useless to me.
>
> The 3rd one fixes a missing of_node_put.
>
> The 4th one is just cosmetic and removes a useless message.
Series applied to net-next, thank you.
^ permalink raw reply [flat|nested] 15+ messages in thread