lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 31 Aug 2018 12:46:43 -0700
From:   Brian Norris <>
To:     Bartosz Golaszewski <>
Cc:     Jonathan Corbet <>, Sekhar Nori <>,
        Kevin Hilman <>,
        Russell King <>,
        Arnd Bergmann <>,
        Greg Kroah-Hartman <>,
        David Woodhouse <>,
        Boris Brezillon <>,
        Marek Vasut <>,
        Richard Weinberger <>,
        Grygorii Strashko <>,
        "David S . Miller" <>,
        Srinivas Kandagatla <>,
        Naren <>,
        Mauro Carvalho Chehab <>,
        Andrew Morton <>,
        Lukas Wunner <>,
        Dan Carpenter <>,
        Florian Fainelli <>,
        Ivan Khoronzhuk <>,
        Sven Van Asbroeck <>,
        Paolo Abeni <>, Alban Bedel <>,
        Rob Herring <>,
        David Lechner <>,
        Andrew Lunn <>,,,,,,,,
        Bartosz Golaszewski <>,
Subject: Re: [PATCH v2 00/29] at24: remove at24_platform_data


On Fri, Aug 10, 2018 at 10:04:57AM +0200, Bartosz Golaszewski wrote:
> Most boards use the EEPROM to store the MAC address. This series adds
> support for cell lookups to the nvmem framework, registers relevant
> cells for all users, adds nvmem support to eth_platform_get_mac_address(),
> converts davinci_emac driver to using it and replaces at24_platform_data
> with device properties.

We already have:

of_get_nvmem_mac_address() (which does exactly what you're adding,
except it's DT specific)

and now you've taught me that this exists too:


These mostly don't share code, and with your series, they'll start to
diverge even more as to what they support. Can you please help rectify
that, instead of widening the gap?

For instance, you can delete most of eth_platform_get_mac_address() and
replace it with device_get_mac_address() [1]. And you could add your new
stuff to fwnode_get_mac_address().

And important part to note here is that you code isn't just useful for
ethernet -- it could be useful for Wifi devices too. So IMO, sticking it
only in an "eth" function is the wrong move.


[1] arch_get_platform_mac_address() is the only part I wouldn't want to
replicate into a truly generic helper. The following should be a no-op
refactor, AIUI:

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index ee28440f57c5..b738651f71e1 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -47,12 +47,12 @@
 #include <linux/inet.h>
 #include <linux/ip.h>
 #include <linux/netdevice.h>
+#include <linux/property.h>
 #include <linux/etherdevice.h>
 #include <linux/skbuff.h>
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/if_ether.h>
-#include <linux/of_net.h>
 #include <linux/pci.h>
 #include <net/dst.h>
 #include <net/arp.h>
@@ -528,19 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)
 int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
 	const unsigned char *addr;
-	struct device_node *dp;
-	if (dev_is_pci(dev))
-		dp = pci_device_to_OF_node(to_pci_dev(dev));
-	else
-		dp = dev->of_node;
-	addr = NULL;
-	if (dp)
-		addr = of_get_mac_address(dp);
-	if (!addr)
-		addr = arch_get_platform_mac_address();
+	if (device_get_mac_address(dev, mac_addr, ETH_ALEN))
+		return 0;
+	addr = arch_get_platform_mac_address();
 	if (!addr)
 		return -ENODEV;

Powered by blists - more mailing lists