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@xxxxxxx> > Signed-off-by: Michael Walle <michael@xxxxxxxx> I cannot say i looked at all the changes, but the ones i did exam seemed O.K. Reviewed-by: Andrew Lunn <andrew@xxxxxxx> Andrew