[net-next,v8,09/10] net: gianfar: Use device_get_child_node_count_named()

Commit Message

Matti Vaittinen March 17, 2025, 3:52 p.m. UTC

We can avoid open-coding the loop construct which counts firmware child
nodes with a specific name by using the newly added
device_get_child_node_count_named().

The gianfar driver has such open-coded loop. Replace it with the
device_get_child_node_count_named().

Suggested-by: Andy Shevchenko <[e-mail]>
Signed-off-by: Matti Vaittinen <[e-mail]>
Reviewed-by: Andy Shevchenko <[e-mail]>
---
Revision history:
v6 =>:
 - No changes
v5 (RFC) => v6:
 - Drop RFC
 - Adapt to changed function name.

It's fair to tell the pros and cons of this patch.
The simplification is there, but it's not a big one. It comes with a cost
of getting the property.h included in this driver which currently uses
exclusively the of_* APIs.

NOTE: This patch depends on the patch:
[2/10] "property: Add functions to iterate named child"

Compile-tested only!
---
 drivers/net/ethernet/freescale/gianfar.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

Comments

Simon Horman March 19, 2025, 4:07 p.m. UTC | #1

On Mon, Mar 17, 2025 at 05:52:25PM +0200, Matti Vaittinen wrote:
> We can avoid open-coding the loop construct which counts firmware child
> nodes with a specific name by using the newly added
> device_get_child_node_count_named().
> 
> The gianfar driver has such open-coded loop. Replace it with the
> device_get_child_node_count_named().
> 
> Suggested-by: Andy Shevchenko <[e-mail]>
> Signed-off-by: Matti Vaittinen <[e-mail]>
> Reviewed-by: Andy Shevchenko <[e-mail]>

This patch looks good to me.
But I think it would be best to resubmit it,
as a standalone patch for net-next, once
it's dependencies are present in net-next.

Matti Vaittinen March 20, 2025, 6:12 a.m. UTC | #2

On 19/03/2025 18:07, Simon Horman wrote:
> On Mon, Mar 17, 2025 at 05:52:25PM +0200, Matti Vaittinen wrote:
>> We can avoid open-coding the loop construct which counts firmware child
>> nodes with a specific name by using the newly added
>> device_get_child_node_count_named().
>>
>> The gianfar driver has such open-coded loop. Replace it with the
>> device_get_child_node_count_named().
>>
>> Suggested-by: Andy Shevchenko <[e-mail]>
>> Signed-off-by: Matti Vaittinen <[e-mail]>
>> Reviewed-by: Andy Shevchenko <[e-mail]>
> 
> This patch looks good to me.
> But I think it would be best to resubmit it,
> as a standalone patch for net-next, once
> it's dependencies are present in net-next.

Thanks Simon. I think you're right.

Patch

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 435138f4699d..d4ee0fc843be 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -97,6 +97,7 @@ 
 #include <linux/phy_fixed.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
+#include <linux/property.h>
 
 #include "gianfar.h"
 
@@ -571,18 +572,6 @@  static int gfar_parse_group(struct device_node *np,
 	return 0;
 }
 
-static int gfar_of_group_count(struct device_node *np)
-{
-	struct device_node *child;
-	int num = 0;
-
-	for_each_available_child_of_node(np, child)
-		if (of_node_name_eq(child, "queue-group"))
-			num++;
-
-	return num;
-}
-
 /* Reads the controller's registers to determine what interface
  * connects it to the PHY.
  */
@@ -654,8 +643,10 @@  static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
 		num_rx_qs = 1;
 	} else { /* MQ_MG_MODE */
 		/* get the actual number of supported groups */
-		unsigned int num_grps = gfar_of_group_count(np);
+		unsigned int num_grps;
 
+		num_grps = device_get_named_child_node_count(&ofdev->dev,
+							     "queue-group");
 		if (num_grps == 0 || num_grps > MAXGROUPS) {
 			dev_err(&ofdev->dev, "Invalid # of int groups(%d)\n",
 				num_grps);