Re: [PATCH] net: ethernet: Fixe issue in nvmem_get_mac_address() where invalid mac addresses

[Andrew Lunn]

On Thu, May 08, 2025 at 02:14:00AM +0000, Ozgur Kara wrote:
> From: Ozgur Karatas <ozgur@xxxxxxxxxx>
> 
> it's necessary to log error returned from
> fwnode_property_read_u8_array because there is no detailed information
> when addr returns an invalid mac address.
> 
> kfree(mac) should actually be marked as kfree((void *)mac) because mac
> pointer is of type const void * and type conversion is required so
> data returned from nvmem_cell_read() is of same type.

What warning do you see from the compiler?

> @@ -565,11 +565,16 @@ static int fwnode_get_mac_addr(struct
> fwnode_handle *fwnode,
>         int ret;
> 
>         ret = fwnode_property_read_u8_array(fwnode, name, addr, ETH_ALEN);
> -       if (ret)
> +       if (ret) {
> +               pr_err("Failed to read MAC address property %s\n", name);
>                 return ret;
> +        }
> 
> -       if (!is_valid_ether_addr(addr))
> +       if (!is_valid_ether_addr(addr)) {
> +               pr_err("Invalid MAC address read for %s\n", name);
>                 return -EINVAL;
> +        }
> +
>         return 0;
>  }

Look at how it is used:

int of_get_mac_address(struct device_node *np, u8 *addr)
{
	int ret;

	if (!np)
		return -ENODEV;

	ret = of_get_mac_addr(np, "mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "local-mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "address", addr);
	if (!ret)
		return 0;

	return of_get_mac_address_nvmem(np, addr);
}

We keep trying until we find a MAC address. It is not an error if a
source does not have a MAC address, we just keep going and try the
next.

So you should not print an message if the property does not
exist. Other errors, EIO, EINVAL, etc, are O.K. to print a warning.

    Andrew

---
pw-bot: cr