Message-ID: <YHTsGXbbr8mkifDo@lunn.ch>
Date:   Tue, 13 Apr 2021 02:55:53 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Walle <michael@...le.cc>
Cc:     ath9k-devel@....qualcomm.com, UNGLinuxDriver@...rochip.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, netdev@...r.kernel.org,
        linux-mediatek@...ts.infradead.org,
        linux-renesas-soc@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-amlogic@...ts.infradead.org, linux-oxnas@...ups.io,
        linux-omap@...r.kernel.org, linux-wireless@...r.kernel.org,
        devicetree@...r.kernel.org, linux-staging@...ts.linux.dev,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Andreas Larsson <andreas@...sler.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Maxime Ripard <mripard@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Joyce Ooi <joyce.ooi@...el.com>,
        Chris Snook <chris.snook@...il.com>,
        Rafał Miłecki <rafal@...ecki.pl>,
        bcm-kernel-feedback-list@...adcom.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Sunil Goutham <sgoutham@...vell.com>,
        Fugang Duan <fugang.duan@....com>,
        Madalin Bucur <madalin.bucur@....com>,
        Pantelis Antoniou <pantelis.antoniou@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Li Yang <leoyang.li@....com>,
        Yisen Zhuang <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Vadym Kochan <vkochan@...vell.com>,
        Taras Chornyi <tchornyi@...vell.com>,
        Mirko Lindner <mlindner@...vell.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
        Sean Wang <sean.wang@...iatek.com>,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Bryan Whitehead <bryan.whitehead@...rochip.com>,
        Vladimir Zapolskiy <vz@...ia.com>,
        Sergei Shtylyov <sergei.shtylyov@...il.com>,
        Byungho An <bh74.an@...sung.com>,
        Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Kevin Hilman <khilman@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Vinod Koul <vkoul@...nel.org>,
        Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...hiba.co.jp>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Wingman Kwok <w-kwok2@...com>,
        Murali Karicheri <m-karicheri2@...com>,
        Michal Simek <michal.simek@...inx.com>,
        Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Lorenzo Bianconi <lorenzo.bianconi83@...il.com>,
        Ryder Lee <ryder.lee@...iatek.com>,
        Stanislaw Gruszka <stf_xl@...pl>,
        Helmut Schaa <helmut.schaa@...glemail.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jérôme Pouiller <jerome.pouiller@...abs.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to
 of_get_mac_address()

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@...n.ch>
> Signed-off-by: Michael Walle <michael@...le.cc>

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

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

    Andrew

Powered by blists - more mailing lists