Skip to contentSkip to conversation

of_net: factor out repetitive code from of_get_mac_address()

PATCH In review Sergei Shtylyov 4 participants 6 messages Mar 18, 2015 – Mar 28, 2015 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 of_net: factor out repetitive code from of_get_mac_address()

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

The resulting ARM object file is 100 bytes less in size than before the patch.

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

 drivers/of/of_net.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)
drivers/of/of_net.c +17 -12
Index: linux/drivers/of/of_net.c
===================================================================
@@ -38,6 +38,15 @@ int of_get_phy_mode(struct device_node *
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
+static const void *of_get_mac_addr(struct device_node *np, const char *name)
+{
+	struct property *pp = of_find_property(np, name, NULL);
+
+	if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
+		return pp->value;
+	return NULL;
+}
+
 /**
  * Search the device tree for the best MAC address to use.  'mac-address' is
  * checked first, because that is supposed to contain to "most recent" MAC
@@ -58,20 +67,16 @@ EXPORT_SYMBOL_GPL(of_get_phy_mode);
 */
 const void *of_get_mac_address(struct device_node *np)
 {
-	struct property *pp;
+	const void *addr;
 
-	pp = of_find_property(np, "mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "local-mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "local-mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
-
-	return NULL;
+	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
maintainer Acked-by on v1 1/1
#1 lore raw

From: Sergei Shtylyov <[email protected]>
Date: Wed, 18 Mar 2015 22:25:46 +0300

Quoted 11 lines

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

The resulting ARM object file is 100 bytes less in size than before the patch.

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

#4 lore raw
In reply to a message not in the archive
Quoted 8 lines · Sergei Shtylyov

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

The resulting ARM object file is 100 bytes less in size than before the patch.

Quoted 57 lines

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

 drivers/of/of_net.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Index: linux/drivers/of/of_net.c
===================================================================

drivers/of/of_net.c +17 -12
@@ -38,6 +38,15 @@ int of_get_phy_mode(struct device_node *
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
+static const void *of_get_mac_addr(struct device_node *np, const char *name)
+{
+	struct property *pp = of_find_property(np, name, NULL);
+
+	if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
+		return pp->value;
+	return NULL;
+}
+
 /**
  * Search the device tree for the best MAC address to use.  'mac-address' is
  * checked first, because that is supposed to contain to "most recent" MAC
@@ -58,20 +67,16 @@ EXPORT_SYMBOL_GPL(of_get_phy_mode);
 */
 const void *of_get_mac_address(struct device_node *np)
 {
-	struct property *pp;
+	const void *addr;
 
-	pp = of_find_property(np, "mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "local-mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "local-mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
-
-	return NULL;
+	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
Signature · 5 lines
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw
In reply to David Miller
Quoted 8 lines · David Miller

From: Sergei Shtylyov <[email protected]>
Date: Wed, 18 Mar 2015 22:25:46 +0300

Quoted 5 lines

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

Quoted 8 lines
Quoted 6 lines

The resulting ARM object file is 100 bytes less in size than before the patch.

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

Applied for v4.1, thanks.

g.

#5 lore raw
In reply to Sergei Shtylyov
Quoted 11 lines · Sergei Shtylyov

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

The resulting ARM object file is 100 bytes less in size than before the patch.

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

Thanks for looking at this, I've merged it.

Since you're already playing with this, could you also write a unittest of of_get_mac_addr()? (drivers/of/unittest.c & drivers/of/unittest-data)

g.

Quoted 54 lines
 drivers/of/of_net.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Index: linux/drivers/of/of_net.c
===================================================================

drivers/of/of_net.c +17 -12
@@ -38,6 +38,15 @@ int of_get_phy_mode(struct device_node *
 }
 EXPORT_SYMBOL_GPL(of_get_phy_mode);
 
+static const void *of_get_mac_addr(struct device_node *np, const char *name)
+{
+	struct property *pp = of_find_property(np, name, NULL);
+
+	if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
+		return pp->value;
+	return NULL;
+}
+
 /**
  * Search the device tree for the best MAC address to use.  'mac-address' is
  * checked first, because that is supposed to contain to "most recent" MAC
@@ -58,20 +67,16 @@ EXPORT_SYMBOL_GPL(of_get_phy_mode);
 */
 const void *of_get_mac_address(struct device_node *np)
 {
-	struct property *pp;
+	const void *addr;
 
-	pp = of_find_property(np, "mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "local-mac-address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
+	addr = of_get_mac_addr(np, "local-mac-address");
+	if (addr)
+		return addr;
 
-	pp = of_find_property(np, "address", NULL);
-	if (pp && (pp->length == 6) && is_valid_ether_addr(pp->value))
-		return pp->value;
-
-	return NULL;
+	return of_get_mac_addr(np, "address");
 }
 EXPORT_SYMBOL(of_get_mac_address);
#6 lore raw

Hello.

Quoted 9 lines · Grant Likely
Quoted 8 lines

of_get_mac_address() basically does the same thing thrice, every time with a different property name, so it makes sense to factor out the repetitive code into separate function. While at it, we can start using ETH_ALEN instead of the bare number and drop unnecessary parens around the property length check.

The resulting ARM object file is 100 bytes less in size than before the patch.

Quoted 7 lines
Quoted 2 lines

---
The patch is against Grant Likely's 'linux.git' repo's 'devicetree/next' branch.

Thanks for looking at this, I've merged it.

Since you're already playing with this, could you also write a unittest of of_get_mac_addr()? (drivers/of/unittest.c & drivers/of/unittest-data)

Unfortunately, I'm constantly low on time. Even this patch was done on
weekend.

Quoted 1 line

g.

WBR, Sergei