Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address()

[Andrew Lunn <andrew@lunn.ch>]

On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote:
> of_get_mac_address() returns a "const void*" pointer to a MAC address.
> Lately, support to fetch the MAC address by an NVMEM provider was added.
> But this will only work with platform devices. It will not work with
> PCI devices (e.g. of an integrated root complex) and esp. not with DSA
> ports.
> 
> There is an of_* variant of the nvmem binding which works without
> devices. The returned data of a nvmem_cell_read() has to be freed after
> use. On the other hand the return of_get_mac_address() points to some
> static data without a lifetime. The trick for now, was to allocate a
> device resource managed buffer which is then returned. This will only
> work if we have an actual device.
> 
> Change it, so that the caller of of_get_mac_address() has to supply a
> buffer where the MAC address is written to. Unfortunately, this will
> touch all drivers which use the of_get_mac_address().
> 
> Usually the code looks like:
> 
>   const char *addr;
>   addr = of_get_mac_address(np);
>   if (!IS_ERR(addr))
>     ether_addr_copy(ndev->dev_addr, addr);
> 
> This can then be simply rewritten as:
> 
>   of_get_mac_address(np, ndev->dev_addr);
> 
> Sometimes is_valid_ether_addr() is used to test the MAC address.
> of_get_mac_address() already makes sure, it just returns a valid MAC
> address. Thus we can just test its return code. But we have to be
> careful if there are still other sources for the MAC address before the
> of_get_mac_address(). In this case we have to keep the
> is_valid_ether_addr() call.
> 
> The following coccinelle patch was used to convert common cases to the
> new style. Afterwards, I've manually gone over the drivers and fixed the
> return code variable: either used a new one or if one was already
> available use that. Mansour Moufid, thanks for that coccinelle patch!
> 
> <spml>
> @a@
> identifier x;
> expression y, z;
> @@
> - x = of_get_mac_address(y);
> + x = of_get_mac_address(y, z);
>   <...
> - ether_addr_copy(z, x);
>   ...>
> 
> @@
> identifier a.x;
> @@
> - if (<+... x ...+>) {}
> 
> @@
> identifier a.x;
> @@
>   if (<+... x ...+>) {
>       ...
>   }
> - else {}
> 
> @@
> identifier a.x;
> expression e;
> @@
> - if (<+... x ...+>@e)
> -     {}
> - else
> + if (!(e))
>       {...}
> 
> @@
> expression x, y, z;
> @@
> - x = of_get_mac_address(y, z);
> + of_get_mac_address(y, z);
>   ... when != x
> </spml>
> 
> All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
> compile-time tested.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michael Walle <michael@walle.cc>

I cannot say i looked at all the changes, but the ones i did exam
seemed O.K.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic