Re: [PATCH] of: of_net: add support to parse ASCII encoded mac-addresses

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Marco,

On 07.08.23 19:07, Marco Felsch wrote:
> Some vendors like Polyhex store the MAC address ASCII encoded instead of
> using the plain 6-byte MAC address. This commit adds the support to
> decode the 12-byte ASCII encoded MAC addresses.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

FYI, the upstream device tree binding for this is NVMEM layout, which was only
recently added to Linux and for which barebox has no support yet.

I can understand that porting NVMEM layouts, just to get a MAC address assigned
might not be an attractive proposition, but I don't think that adding a new
barebox-specific binding is the right way here. I'd suggest, you get the
nvmem cell in board code and assign it there. There's readily available API
for that. If you are interested in a generic solution, NVMEM layouts are
the way to go IMO.

> ---
>  drivers/of/of_net.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
> index 75a24073da51..4e74986cdda8 100644
> --- a/drivers/of/of_net.c
> +++ b/drivers/of/of_net.c
> @@ -79,6 +79,8 @@ static int of_get_mac_addr(struct device_node *np, const char *name, u8 *addr)
>  	return -ENODEV;
>  }
>  
> +#define ETH_ALEN_ASCII	12
> +
>  int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
>  {
>  	struct nvmem_cell *cell;
> @@ -98,6 +100,23 @@ int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
>  	if (IS_ERR(mac))
>  		return PTR_ERR(mac);
>  
> +	if (len == ETH_ALEN_ASCII) {
> +		u8 *mac_new;
> +		int ret;
> +
> +		mac_new = kzalloc(sizeof("xx:xx:xx:xx:xx:xx"), GFP_KERNEL);
> +		ret = hex2bin(mac_new, mac, ETH_ALEN);

Why not parse into a fixed size local buffer and then copy it? Would save
you the extra allocation.

> +		if (ret) {
> +			kfree(mac_new);
> +			kfree(mac);
> +			return ret;
> +		}
> +
> +		kfree(mac);
> +		mac = mac_new;
> +		len = ETH_ALEN;
> +	}
> +
>  	if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
>  		kfree(mac);
>  		return -EINVAL;

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |