diff mbox series

[1/2] net: mdio: switch to scoped device_for_each_child_node()

Message ID 20240930-net-device_for_each_child_node_scoped-v1-1-bbdd7f9fd649@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: switch to scoped device_for_each_child_node() | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 12 this patch: 12
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 22 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Javier Carrasco Sept. 30, 2024, 8:03 p.m. UTC
There has already been an issue with the handling of early exits from
device_for_each_child() in this driver, and it was solved with commit
b1de5c78ebe9 ("net: mdio: thunder: Add missing fwnode_handle_put()") by
adding a call to fwnode_handle_put() right after the loop.

That solution is valid indeed, but if a new error path with a 'return'
is added to the loop, this solution will fail. A more secure approach
is using the scoped variant of the macro, which automatically
decrements the refcount of the child node when it goes out of scope,
removing the need for explicit calls to fwnode_handle_put().

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/net/mdio/mdio-thunder.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Andrew Lunn Sept. 30, 2024, 8:10 p.m. UTC | #1
On Mon, Sep 30, 2024 at 10:03:29PM +0200, Javier Carrasco wrote:
> There has already been an issue with the handling of early exits from
> device_for_each_child() in this driver, and it was solved with commit
> b1de5c78ebe9 ("net: mdio: thunder: Add missing fwnode_handle_put()") by
> adding a call to fwnode_handle_put() right after the loop.
> 
> That solution is valid indeed, but if a new error path with a 'return'
> is added to the loop, this solution will fail. A more secure approach
> is using the scoped variant of the macro, which automatically
> decrements the refcount of the child node when it goes out of scope,
> removing the need for explicit calls to fwnode_handle_put().

Hi Javier

I know you are going across the whole tree, multiple sub systems, and
each has its own rules. I think naming patches is however pretty
uniform across the tree. Do what other patches did:

d84fe6dc7377 net: mdio: thunder: Add missing fwnode_handle_put()
a93a0a15876d net: mdio: thunder: Fix a double free issue in the .remove function

netdev has some additional documentation you should read:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

The change itself looks O.K, its just the processes which need work.

	Andrew
Javier Carrasco Sept. 30, 2024, 8:24 p.m. UTC | #2
On 30/09/2024 22:10, Andrew Lunn wrote:
> On Mon, Sep 30, 2024 at 10:03:29PM +0200, Javier Carrasco wrote:
>> There has already been an issue with the handling of early exits from
>> device_for_each_child() in this driver, and it was solved with commit
>> b1de5c78ebe9 ("net: mdio: thunder: Add missing fwnode_handle_put()") by
>> adding a call to fwnode_handle_put() right after the loop.
>>
>> That solution is valid indeed, but if a new error path with a 'return'
>> is added to the loop, this solution will fail. A more secure approach
>> is using the scoped variant of the macro, which automatically
>> decrements the refcount of the child node when it goes out of scope,
>> removing the need for explicit calls to fwnode_handle_put().
> 
> Hi Javier
> 
> I know you are going across the whole tree, multiple sub systems, and
> each has its own rules. I think naming patches is however pretty
> uniform across the tree. Do what other patches did:
> 
> d84fe6dc7377 net: mdio: thunder: Add missing fwnode_handle_put()
> a93a0a15876d net: mdio: thunder: Fix a double free issue in the .remove function
> 
> netdev has some additional documentation you should read:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> The change itself looks O.K, its just the processes which need work.
> 
> 	Andrew


Hi Andrew,

thanks for your feedback. I used the same name I found in the first two
patches for that file, and I missed the 'thunder:' because probably
those patches applied to more files. And the same applies for
hns_dsaf_mac.c. I will fix that for v2 alongside anything I might find
in the netdev documentation that might be missing too.

Best regards,
Javier Carrasco
diff mbox series

Patch

diff --git a/drivers/net/mdio/mdio-thunder.c b/drivers/net/mdio/mdio-thunder.c
index 6067d96b2b7b..1e1aa72b1eff 100644
--- a/drivers/net/mdio/mdio-thunder.c
+++ b/drivers/net/mdio/mdio-thunder.c
@@ -23,7 +23,6 @@  static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {
 	struct device_node *node;
-	struct fwnode_handle *fwn;
 	struct thunder_mdiobus_nexus *nexus;
 	int err;
 	int i;
@@ -54,7 +53,7 @@  static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 	}
 
 	i = 0;
-	device_for_each_child_node(&pdev->dev, fwn) {
+	device_for_each_child_node_scoped(&pdev->dev, fwn) {
 		struct resource r;
 		struct mii_bus *mii_bus;
 		struct cavium_mdiobus *bus;
@@ -106,7 +105,6 @@  static int thunder_mdiobus_pci_probe(struct pci_dev *pdev,
 		if (i >= ARRAY_SIZE(nexus->buses))
 			break;
 	}
-	fwnode_handle_put(fwn);
 	return 0;
 
 err_release_regions: