net: extend eth_platform_get_mac_address()
Subsystems (2)
v2 cover net: extend eth_platform_get_mac_address()
This is a follow-up to a series I posted a while ago the goal of which was to replace the at24 platform data with device properties. To do so we need to somehow remove reading the MAC address from relevant board files.
In my patches I used nvmem and MTD to read the MAC address from within the davinci emac driver. It was suggested that we generalize it further but since MTD doesn't support nvmem yet, the best we can do is to move this code over to net core code.
The following patches modify the eth_platform_get_mac_address() function which seems to be the best candidate for this code.
The first patch calls is_valid_ether_addr() on the read address so that we're sure it's correct.
The last two patches add nvmem and MTD support to the function. In order to stay compatible with existing users, nvmem and MTD will be tried last - after device tree and arch-specific callback.
If this series gets accepted I will modify my previous patches to use it instead of handcoding the same operations in davinci_emac.
v1 -> v2:
- dropped patches 1 & 2
- improved the MAC address verification and fixed a potential buffer overflow in patch 2/3
Bartosz Golaszewski (3):
net: fortify eth_platform_get_mac_address()
net: add support for nvmem to eth_platform_get_mac_address()
net: add MTD support to eth_platform_get_mac_address()
net/ethernet/eth.c | 47 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)
Signature · 1 line
2.17.1
PATCH v2 1/3 net: fortify eth_platform_get_mac_address()
From: Bartosz Golaszewski <[email protected]>
We'll soon have more sources from which to read the MAC address in this routine. Make sure the address is correct before returning it.
--- net/ethernet/eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/ethernet/eth.c +1 -1
@@ -541,7 +541,7 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)if (!addr) addr =arch_get_platform_mac_address (); -if (!addr) +if (!addr || !is_valid_ether_addr (addr))return -ENODEV ;ether_addr_copy (mac_addr, addr);
Signature · 1 line
2.17.1
PATCH v2 2/3 net: add support for nvmem to eth_platform_get_mac_address()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Many non-DT platforms read the MAC address from EEPROM. Usually it's either done with callbacks defined in board files or from SoC-specific ethernet drivers.
In order to generalize this, try to read the MAC from nvmem in eth_platform_get_mac_address() using a standard lookup name:
"mac-address".
--- net/ethernet/eth.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
net/ethernet/eth.c +27 -0
@@ -54,6 +54,7 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h>
@@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) { +unsigned char addrbuf[ETH_ALEN ];const unsigned char *addr; +struct nvmem_cell *nvmem;struct device_node *dp; +size_t alen;if (dev_is_pci (dev)) dp =pci_device_to_OF_node (to_pci_dev (dev));
@@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)if (!addr) addr =arch_get_platform_mac_address (); +if (!addr) { + nvmem =nvmem_cell_get (dev,"mac-address" ); +if (IS_ERR (nvmem) &&PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but +* the corresponding nvmem provider hasn't been +* registered yet. +*/ +return -EPROBE_DEFER ; + +if (!IS_ERR (nvmem)) { + addr =nvmem_cell_read (nvmem, &alen); +if (!IS_ERR (addr)) { +if (alen ==ETH_ALEN ) +ether_addr_copy (addrbuf, addr); + +kfree (addr); + addr = alen ==ETH_ALEN ? addrbuf :NULL ; + } + +nvmem_cell_put (nvmem); + } + } +if (!addr || !is_valid_ether_addr (addr))return -ENODEV ;
Signature · 1 line
2.17.1
PATCH v2 3/3 net: add MTD support to eth_platform_get_mac_address()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
MTD doesn't support nvmem yet. Some platforms use MTD to read the MAC address from SPI flash. If we want this function to generalize reading the MAC address, we need to separately try to use MTD.
--- net/ethernet/eth.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
net/ethernet/eth.c +18 -0
@@ -55,6 +55,7 @@#include <linux/of_net.h> #include <linux/pci.h> #include <linux/nvmem-consumer.h> +#include <linux/mtd/mtd.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h>
@@ -568,6 +569,23 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) } } +#ifdef CONFIG_MTD +/* NOTE: this should go away as soon as MTD gets nvmem support. */ +if (!addr) { +struct mtd_info *mtd; +int rv; + + mtd =get_mtd_device_nm ("MAC-Address" ); +if (!IS_ERR (mtd)) { + rv =mtd_read (mtd,0 ,ETH_ALEN , &alen, addrbuf); +if (rv ==0 ) + addr = addrbuf; + +put_mtd_device (mtd); + } + } +#endif /* CONFIG_MTD */ +if (!addr || !is_valid_ether_addr (addr))return -ENODEV ;
Signature · 1 line
2.17.1
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
In reply to Bartosz GolaszewskiQuoted 45 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Many non-DT platforms read the MAC address from EEPROM. Usually it's either done with callbacks defined in board files or from SoC-specific ethernet drivers.
In order to generalize this, try to read the MAC from nvmem in
eth_platform_get_mac_address()using a standard lookup name:
"mac-address".Signed-off-by Bartosz Golaszewski <[email protected]>--- net/ethernet/eth.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)net/ethernet/eth.c +6
-0@@ -54,6 +54,7 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h> @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) { +unsigned char addrbuf[ETH_ALEN ];const unsigned char *addr; +struct nvmem_cell *nvmem;struct device_node *dp; +size_t alen;if (dev_is_pci (dev)) dp =pci_device_to_OF_node (to_pci_dev (dev));@@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)if (!addr) addr =arch_get_platform_mac_address (); +if (!addr) { + nvmem =nvmem_cell_get (dev,"mac-address" );
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 559
Quoted 10 lines
net/ethernet/eth.c +10
-0+if (IS_ERR (nvmem) &&PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but +* the corresponding nvmem provider hasn't been +* registered yet. +*/ +return -EPROBE_DEFER ; + +if (!IS_ERR (nvmem)) { + addr =nvmem_cell_read (nvmem, &alen); +if (!IS_ERR (addr)) {
^^^^
Never do success handling. Always error handling. Otherwise the code is indent a lot and the error handling is far from the call.
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 571
Quoted 12 lines
net/ethernet/eth.c +11
-0+if (alen ==ETH_ALEN ) +ether_addr_copy (addrbuf, addr); + +kfree (addr); + addr = alen ==ETH_ALEN ? addrbuf :NULL ; + } + +nvmem_cell_put (nvmem); + } + } +if (!addr || !is_valid_ether_addr (addr))
^^^^
Instead of handling the error we dereference the error pointer here.
*frowny face*
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
Quoted 3 lines
net/ethernet/eth.c +0
-1return -ENODEV ; --
Maybe this?
if (!addr) {
nvmem = nvmem_cell_get(dev, "mac-address");
if (PTR_ERR(nvmem) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (IS_ERR(nvmem))
return -ENODEV;
addr = nvmem_cell_read(nvmem, &alen);
if (IS_ERR(addr))
return PTR_ERR(addr);
if (alen != ETH_ALEN) {
kfree(addr);
return -ENODEV;
}
ether_addr_copy(addrbuf, addr);
kfree(addr);
addr = addrbuf;
}
if (!is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
return 0;regards,
dan carpenter
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
2018-07-19 10:48 GMT+02:00 Dan Carpenter <[email protected]>:
Quoted 73 lines · Bartosz Golaszewski
Quoted 45 lines
From: Bartosz Golaszewski <[email protected]>
Many non-DT platforms read the MAC address from EEPROM. Usually it's either done with callbacks defined in board files or from SoC-specific ethernet drivers.
In order to generalize this, try to read the MAC from nvmem in
eth_platform_get_mac_address()using a standard lookup name:
"mac-address".Signed-off-by Bartosz Golaszewski <[email protected]>--- net/ethernet/eth.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)net/ethernet/eth.c +6
-0@@ -54,6 +54,7 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h> @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) { +unsigned char addrbuf[ETH_ALEN ];const unsigned char *addr; +struct nvmem_cell *nvmem;struct device_node *dp; +size_t alen;if (dev_is_pci (dev)) dp =pci_device_to_OF_node (to_pci_dev (dev));@@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)if (!addr) addr =arch_get_platform_mac_address (); +if (!addr) { + nvmem =nvmem_cell_get (dev,"mac-address" );net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 559
Quoted 10 lines
net/ethernet/eth.c +10
-0+if (IS_ERR (nvmem) &&PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but +* the corresponding nvmem provider hasn't been +* registered yet. +*/ +return -EPROBE_DEFER ; + +if (!IS_ERR (nvmem)) { + addr =nvmem_cell_read (nvmem, &alen); +if (!IS_ERR (addr)) {^^^^Never do success handling. Always error handling. Otherwise the code is indent a lot and the error handling is far from the call.
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 571
Quoted 12 lines
net/ethernet/eth.c +11
-0+if (alen ==ETH_ALEN ) +ether_addr_copy (addrbuf, addr); + +kfree (addr); + addr = alen ==ETH_ALEN ? addrbuf :NULL ; + } + +nvmem_cell_put (nvmem); + } + } +if (!addr || !is_valid_ether_addr (addr))^^^^
Quoted 2 lines
Instead of handling the error we dereference the error pointer here.
True - we should add a check for IS_ERR(addr) here.
Quoted 21 lines
*frowny face*
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
Quoted 3 lines
net/ethernet/eth.c +0
-1return -ENODEV ; --Maybe this?
if (!addr) { nvmem = nvmem_cell_get(dev, "mac-address"); if (PTR_ERR(nvmem) == -EPROBE_DEFER) return -EPROBE_DEFER; if (IS_ERR(nvmem)) return -ENODEV; addr = nvmem_cell_read(nvmem, &alen); if (IS_ERR(addr)) return PTR_ERR(addr); if (alen != ETH_ALEN) { kfree(addr); return -ENODEV; }
Quoted 9 lines
ether_addr_copy(addrbuf, addr);
kfree(addr);
addr = addrbuf;
}
if (!is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
return 0;I would normally go this way but here we don't want to bail out when we encounter an error but rather continue on to the next possible source of a MAC address. We'll get -ENODEV from nvmem_cell_get() if the lookup fails for "mac-address" but instead of returning an error code we should then check if we can read the MAC from MTD.
Bart
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
Maybe it would be simpler as three separate functions:
int of_eth_get_mac_address() <- rename existing function to this int nvmem_eth_get_mac_address() <- patch 2
int mtd_eth_nvmem_get_mac_address() patch 3
int ret; ret = of_eth_get_mac_address(dev, mac_addr); if (!ret) return 0; ret = nvmem_eth_get_mac_address(dev, mac_addr); if (ret == -EPROBEDEFER) return ret; if (!ret) return 0; ret = mtd_eth_nvmem_get_mac_address(dev, mac_addr); if (!ret) return 0; return -ENODEV;
regards,
dan carpenter
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
2018-07-19 11:09 GMT+02:00 Dan Carpenter <[email protected]>:
Quoted 23 lines
Maybe it would be simpler as three separate functions:
int
of_eth_get_mac_address()<- rename existing function to this intnvmem_eth_get_mac_address()<- patch 2
intmtd_eth_nvmem_get_mac_address()patch 3int ret; ret = of_eth_get_mac_address(dev, mac_addr); if (!ret) return 0; ret = nvmem_eth_get_mac_address(dev, mac_addr); if (ret == -EPROBEDEFER) return ret; if (!ret) return 0; ret = mtd_eth_nvmem_get_mac_address(dev, mac_addr); if (!ret) return 0; return -ENODEV;
Quoted 2 lines
regards,
dan carpenter
It looks simpler as long as you don't add all the new routines resulting from this approach. I've just tried to quickly implement this solution and it resulted in much bigger and duplicated code (checking the validity of mac_addr, copying it etc.). I would prefer the current approach and would like to read someone else's opinion on that.
Thanks,
Bart
Quoted 1 line
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
In reply to Dan CarpenterQuoted 83 lines · Dan Carpenter
Quoted 45 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Many non-DT platforms read the MAC address from EEPROM. Usually it's either done with callbacks defined in board files or from SoC-specific ethernet drivers.
In order to generalize this, try to read the MAC from nvmem in
eth_platform_get_mac_address()using a standard lookup name:
"mac-address".Signed-off-by Bartosz Golaszewski <[email protected]>--- net/ethernet/eth.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)net/ethernet/eth.c +6
-0@@ -54,6 +54,7 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h> @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) { +unsigned char addrbuf[ETH_ALEN ];const unsigned char *addr; +struct nvmem_cell *nvmem;struct device_node *dp; +size_t alen;if (dev_is_pci (dev)) dp =pci_device_to_OF_node (to_pci_dev (dev));@@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)if (!addr) addr =arch_get_platform_mac_address (); +if (!addr) { + nvmem =nvmem_cell_get (dev,"mac-address" );net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 559
Quoted 10 lines
net/ethernet/eth.c +10
-0+if (IS_ERR (nvmem) &&PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but +* the corresponding nvmem provider hasn't been +* registered yet. +*/ +return -EPROBE_DEFER ; + +if (!IS_ERR (nvmem)) { + addr =nvmem_cell_read (nvmem, &alen); +if (!IS_ERR (addr)) {^^^^Never do success handling. Always error handling. Otherwise the code is indent a lot and the error handling is far from the call.
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 571
Quoted 12 lines
net/ethernet/eth.c +11
-0+if (alen ==ETH_ALEN ) +ether_addr_copy (addrbuf, addr); + +kfree (addr); + addr = alen ==ETH_ALEN ? addrbuf :NULL ; + } + +nvmem_cell_put (nvmem); + } + } +if (!addr || !is_valid_ether_addr (addr))^^^^Instead of handling the error we dereference the error pointer here.
*frowny face*
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
Quoted 3 lines
net/ethernet/eth.c +0
-1return -ENODEV ; --Maybe this?
Quoted 9 lines
if (!addr) {
nvmem = nvmem_cell_get(dev, "mac-address");
if (PTR_ERR(nvmem) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (IS_ERR(nvmem))
return -ENODEV;
addr = nvmem_cell_read(nvmem, &alen);
if (IS_ERR(addr))
return PTR_ERR(addr);The problem with doing it this way is... error handling is Hard(tm). You missed the call to nvmem_cell_put() here.
Quoted 2 lines
if (alen != ETH_ALEN) {
kfree(addr);and again here.
Quoted 5 lines
return -ENODEV; } ether_addr_copy(addrbuf, addr); kfree(addr); addr = addrbuf;
and here.
Quoted 5 lines
} if (!is_valid_ether_addr(addr)) return -ENODEV; ether_addr_copy(mac_addr, addr); return 0;
Without checking the semantics, a possible solution to that could be:
if (!addr) {
nvmem = nvmem_cell_get(dev, "mac-address");
if (PTR_ERR(nvmem) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (IS_ERR(nvmem))
return -ENODEV;
addr = nvmem_cell_read(nvmem, &alen);
+ nvmem_cell_put(nvmem);
if (IS_ERR(addr))
return PTR_ERR(addr);
if (alen != ETH_ALEN) {
kfree(addr);
return -ENODEV;
}
ether_addr_copy(addrbuf, addr);
kfree(addr);
addr = addrbuf;
}
if (!is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
return 0;A potential better solution would be to put this code in a separate function, and then do:
if (!addr) addr = eth_platform_get_nvmem_mac_address(dev, addrbuf);
which returns either NULL or addrbuf depending on whether it failed or succeeded, which would probably end up with cleaner code.
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up According to speedtest.net: 13Mbps down 490kbps up
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
In reply to Bartosz GolaszewskiQuoted 6 lines · Bartosz Golaszewski
It looks simpler as long as you don't add all the new routines resulting from this approach. I've just tried to quickly implement this solution and it resulted in much bigger and duplicated code (checking the validity of
mac_addr, copying it etc.). I would prefer the current approach and would like to read someone else's opinion on that.
It's not too bad. There is two extra ether_addr_copy() calls and one extra is_valid_ether_addr(). There are two extra alen declarations and one extra addr declaration. The functions don't share *that* much code.
regards,
dan carpenter
net/ethernet/eth.c +81 -1
@@ -54,6 +54,8 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> +#include <linux/mtd/mtd.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h>
@@ -525,7 +527,7 @@ unsigned char * __weak arch_get_platform_mac_address(void)return NULL ; } -int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) +static int of_eth_get_mac_address (struct device *dev,u8 *mac_addr) {const unsigned char *addr;struct device_node *dp;
@@ -547,4 +549,82 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)ether_addr_copy (mac_addr, addr);return 0 ; } + +static int nvmem_eth_get_mac_address (struct device *dev,u8 *mac_addr) +{ +struct nvmem_cell *nvmem; +unsigned char *addr; +size_t alen; +int ret =0 ; + + nvmem =nvmem_cell_get (dev,"mac-address" ); +if (PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but the +* corresponding nvmem provider hasn't been registered yet. +*/ +return -EPROBE_DEFER ; +if (IS_ERR (nvmem)) +return PTR_ERR (nvmem); + addr =nvmem_cell_read (nvmem, &alen); +if (IS_ERR (addr)) { + ret =PTR_ERR (addr); + goto put_nvmem; + } +if (alen !=ETH_ALEN || !is_valid_ether_addr (addr)) { + ret = -EINVAL ; + goto free_addr; + } +ether_addr_copy (mac_addr, addr); +free_addr: +kfree (addr); +put_nvmem: +nvmem_cell_put (nvmem); +return ret; +} + +static int mtd_eth_get_mac_address (struct device *dev,u8 *mac_addr) +{ +struct mtd_info *mtd; +u8 addrbuf[ETH_ALEN ]; +size_t alen; +int ret; + +/* This function should go away as soon as MTD gets nvmem support. */ +if (!IS_ENABLED (CONFIG_MTD )) +return -ENODEV ; + + mtd =get_mtd_device_nm ("MAC-Address" ); +if (IS_ERR (mtd)) +return PTR_ERR (mtd); + ret =mtd_read (mtd,0 ,ETH_ALEN , &alen, addrbuf); +if (ret) + goto put_mtd; +if (!is_valid_ether_addr (addrbuf)) { + ret = -EINVAL ; + goto put_mtd; + } +ether_addr_copy (mac_addr, addrbuf); +put_mtd: +put_mtd_device (mtd); +return ret; +} + +int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) +{ +int ret; + + ret =of_eth_get_mac_address (dev, mac_addr); +if (!ret) +return 0 ; + ret =nvmem_eth_get_mac_address (dev, mac_addr); +if (ret == -EPROBE_DEFER ) +return -EPROBE_DEFER ; +if (!ret) +return 0 ; + ret =mtd_eth_get_mac_address (dev, mac_addr); +if (!ret) +return 0 ; + +return -ENODEV ; +}EXPORT_SYMBOL (eth_platform_get_mac_address);
Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
2018-07-19 14:37 GMT+02:00 Dan Carpenter <[email protected]>:
Quoted 110 lines · Bartosz Golaszewski
Quoted 6 lines
It looks simpler as long as you don't add all the new routines resulting from this approach. I've just tried to quickly implement this solution and it resulted in much bigger and duplicated code (checking the validity of
mac_addr, copying it etc.). I would prefer the current approach and would like to read someone else's opinion on that.It's not too bad. There is two extra
ether_addr_copy()calls and one extrais_valid_ether_addr(). There are two extra alen declarations and one extra addr declaration. The functions don't share *that* much code.regards,
dan carpenternet/ethernet/eth.c +72
-1@@ -54,6 +54,8 @@#include <linux/if_ether.h> #include <linux/of_net.h> #include <linux/pci.h> +#include <linux/nvmem-consumer.h> +#include <linux/mtd/mtd.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h> @@ -525,7 +527,7 @@ unsigned char * __weak arch_get_platform_mac_address(void)return NULL ; } -int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) +static int of_eth_get_mac_address (struct device *dev,u8 *mac_addr) {const unsigned char *addr;struct device_node *dp;@@ -547,4 +549,82 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)ether_addr_copy (mac_addr, addr);return 0 ; } + +static int nvmem_eth_get_mac_address (struct device *dev,u8 *mac_addr) +{ +struct nvmem_cell *nvmem; +unsigned char *addr; +size_t alen; +int ret =0 ; + + nvmem =nvmem_cell_get (dev,"mac-address" ); +if (PTR_ERR (nvmem) == -EPROBE_DEFER ) +/* We may have a lookup registered for MAC address but the +* corresponding nvmem provider hasn't been registered yet. +*/ +return -EPROBE_DEFER ; +if (IS_ERR (nvmem)) +return PTR_ERR (nvmem); + addr =nvmem_cell_read (nvmem, &alen); +if (IS_ERR (addr)) { + ret =PTR_ERR (addr); + goto put_nvmem; + } +if (alen !=ETH_ALEN || !is_valid_ether_addr (addr)) { + ret = -EINVAL ; + goto free_addr; + } +ether_addr_copy (mac_addr, addr); +free_addr: +kfree (addr); +put_nvmem: +nvmem_cell_put (nvmem); +return ret; +} + +static int mtd_eth_get_mac_address (struct device *dev,u8 *mac_addr) +{ +struct mtd_info *mtd; +u8 addrbuf[ETH_ALEN ]; +size_t alen; +int ret; + +/* This function should go away as soon as MTD gets nvmem support. */ +if (!IS_ENABLED (CONFIG_MTD )) +return -ENODEV ; + + mtd =get_mtd_device_nm ("MAC-Address" ); +if (IS_ERR (mtd)) +return PTR_ERR (mtd); + ret =mtd_read (mtd,0 ,ETH_ALEN , &alen, addrbuf); +if (ret) + goto put_mtd; +if (!is_valid_ether_addr (addrbuf)) { + ret = -EINVAL ; + goto put_mtd; + } +ether_addr_copy (mac_addr, addrbuf); +put_mtd: +put_mtd_device (mtd); +return ret; +} + +int eth_platform_get_mac_address (struct device *dev,u8 *mac_addr) +{ +int ret; + + ret =of_eth_get_mac_address (dev, mac_addr); +if (!ret) +return 0 ; + ret =nvmem_eth_get_mac_address (dev, mac_addr); +if (ret == -EPROBE_DEFER )
net/ethernet/eth.c int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) · line 630
Quoted 10 lines
net/ethernet/eth.c +9
-0+return -EPROBE_DEFER ; +if (!ret) +return 0 ; + ret =mtd_eth_get_mac_address (dev, mac_addr); +if (!ret) +return 0 ; + +return -ENODEV ; +}EXPORT_SYMBOL (eth_platform_get_mac_address);
Please take a look at v3 - I did it a bit differently but the idea is the same.
Bart