[PATCH] common: tlv: Correct eth address list fixup

[PATCH] common: tlv: Correct eth address list fixup

[Jonas Rebmann]

The tlv_format_mac() function was written as if it would fixup a list of
MAC addresses, however that implementation was broken as it always
exited after adding the first MAC due to interpretation of any nonzero
exit value from of_property_sprintf() as an error, although it returns
the length of the resulting string upon success.

This was worked around by tlv_handle_eth_address_seq() invoking
tlv_format_mac() in a loop however tlv_handle_eth_address(), which is
specified to support an array of MAC addresses invoked tlv_format_mac()
on the list resulting in only the first MAC of that list ending up in
the devicetree fixup.

Remove the broken loop from tlv_format_mac() and clarify that it only
adds a single MAC. Fix and clean up the usage of tlv_format_mac() in
tlv_handle_eth_address() and tlv_handle_eth_address_seq().

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 common/tlv/barebox.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
index 6fdfe94028..986b655a6f 100644
--- a/common/tlv/barebox.c
+++ b/common/tlv/barebox.c
@@ -22,21 +22,26 @@ int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len,
 
 int tlv_handle_eth_address(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
 {
+	int ret;
 	int i;
 
 	if (len % ETH_ALEN != 0)
 		return -EINVAL;
 
-	for (i = 0; i < len / ETH_ALEN; i++)
+	for (i = 0; i < len / ETH_ALEN; i++) {
 		eth_register_ethaddr(i, val + i * ETH_ALEN);
-
-	return tlv_format_mac(dev, map, len, val);
+		ret = tlv_format_mac(dev, map, ETH_ALEN, val + i * ETH_ALEN);
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
 }
 
 int tlv_handle_eth_address_seq(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
 {
 	u8 eth_addr[ETH_ALEN];
 	int eth_count;
+	int ret;
 
 	eth_count = *val;
 
@@ -47,7 +52,9 @@ int tlv_handle_eth_address_seq(struct tlv_device *dev, struct tlv_mapping *map,
 
 	for (int i = 0; i < eth_count; i++, eth_addr_inc(eth_addr)) {
 		eth_register_ethaddr(i, eth_addr);
-		tlv_format_mac(dev, map, ETH_ALEN, eth_addr);
+		ret = tlv_format_mac(dev, map, ETH_ALEN, eth_addr);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -111,14 +118,15 @@ static struct device_node *of_append_node(struct device_node *root, const char *
 	return of_new_node(root, name);
 }
 
+/* add a single address-<num> entry to the property */
 int tlv_format_mac(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
 {
 	struct device_node *np = tlv_of_node(dev);
 	struct property *pp;
 	char propname[sizeof("address-4294967295")];
-	int base = 0, i, ret;
+	int base = 0, ret;
 
-	if (len % 6 != 0)
+	if (len != ETH_ALEN)
 		return -EINVAL;
 
 	np = of_append_node(np, map->prop);
@@ -128,12 +136,10 @@ int tlv_format_mac(struct tlv_device *dev, struct tlv_mapping *map, u16 len, con
 	for_each_property_of_node(np, pp)
 		base++;
 
-	for (i = base; i < base + len / 6; i++) {
-		snprintf(propname, sizeof(propname), "address-%u", i);
-		ret = of_property_sprintf(np, propname, "%*phC", 6, val);
-		if (ret)
-			return ret;
-	}
+	snprintf(propname, sizeof(propname), "address-%u", base);
+	ret = of_property_sprintf(np, propname, "%*phC", ETH_ALEN, val);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }

---
base-commit: 89ed19d5c673ad0ab1851ed62ece38d30e96b412
change-id: 20260205-tlv-eth-list-fixup-ec8af6bb9ebf

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

Re: [PATCH] common: tlv: Correct eth address list fixup

[Sascha Hauer]

On Thu, 05 Feb 2026 14:55:46 +0100, Jonas Rebmann wrote:
> The tlv_format_mac() function was written as if it would fixup a list of
> MAC addresses, however that implementation was broken as it always
> exited after adding the first MAC due to interpretation of any nonzero
> exit value from of_property_sprintf() as an error, although it returns
> the length of the resulting string upon success.
> 
> This was worked around by tlv_handle_eth_address_seq() invoking
> tlv_format_mac() in a loop however tlv_handle_eth_address(), which is
> specified to support an array of MAC addresses invoked tlv_format_mac()
> on the list resulting in only the first MAC of that list ending up in
> the devicetree fixup.
> 
> [...]

Applied, thanks!

[1/1] common: tlv: Correct eth address list fixup
      https://git.pengutronix.de/cgit/barebox/commit/?id=925dba9ba412 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>