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 <[email protected]>
---
 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 <[email protected]>


Reply via email to