| Message ID | 1522046489-19652-1-git-send-email-mike.looijmans@topic.nl |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] of_net: Implement of_get_nvmem_mac_address helper | expand |
On Mon, Mar 26, 2018 at 08:41:29AM +0200, Mike Looijmans wrote: > It's common practice to store MAC addresses for network interfaces into > nvmem devices. However the code to actually do this in the kernel lacks, > so this patch adds of_get_nvmem_mac_address() for drivers to obtain the > address from an nvmem cell provider. > > This is particulary useful on devices where the ethernet interface cannot > be configured by the bootloader, for example because it's in an FPGA. > > Tested by adapting the cadence macb driver to call this instead of > of_get_mac_address(). Hi Mike I can understand you not wanting to modify all the call sites for of_get_mac_address(). However, the name of_get_nvmem_mac_address() suggests it gets the MAC address from NVMEM. I think people are going to be surprised when they find it first tries for a MAC address directly in device tree. I would drop the call to of_get_mac_address(), and have the MAC driver call both. You could also maybe take a look at fwnode_get_mac_address(). It should work for both OF and ACPI. It fits better because is passes a char * for the address. You could make that do both, and call it from the macb driver. dev_fwnode() probably does what you want. Andrew -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Mike Looijmans <mike.looijmans@topic.nl> Date: Mon, 26 Mar 2018 08:41:29 +0200 > It's common practice to store MAC addresses for network interfaces into > nvmem devices. However the code to actually do this in the kernel lacks, > so this patch adds of_get_nvmem_mac_address() for drivers to obtain the > address from an nvmem cell provider. > > This is particulary useful on devices where the ethernet interface cannot > be configured by the bootloader, for example because it's in an FPGA. > > Tested by adapting the cadence macb driver to call this instead of > of_get_mac_address(). > > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> > --- > v2: Use of_nvmem_cell_get to avoid needing the assiciated device > Use void* instead of char* > Add devicetree binding doc Like Andrew, I think you should add a new interface for getting the MAC address from nvmem. And drivers can call both of them if they want OF device tree and NVMEM probing for MAC addresses. Later you can add a consolidated interface, if necessary, which does both and also take a reference to the MAC address buffer of the driver in order to deal with the resource allocation issues. Thanks. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/26/2018 09:58 AM, David Miller wrote: > From: Mike Looijmans <mike.looijmans@topic.nl> > Date: Mon, 26 Mar 2018 08:41:29 +0200 > >> It's common practice to store MAC addresses for network interfaces into >> nvmem devices. However the code to actually do this in the kernel lacks, >> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the >> address from an nvmem cell provider. >> >> This is particulary useful on devices where the ethernet interface cannot >> be configured by the bootloader, for example because it's in an FPGA. >> >> Tested by adapting the cadence macb driver to call this instead of >> of_get_mac_address(). >> >> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> >> --- >> v2: Use of_nvmem_cell_get to avoid needing the assiciated device >> Use void* instead of char* >> Add devicetree binding doc > > Like Andrew, I think you should add a new interface for getting the MAC > address from nvmem. > > And drivers can call both of them if they want OF device tree and > NVMEM probing for MAC addresses. > > Later you can add a consolidated interface, if necessary, which does > both and also take a reference to the MAC address buffer of the driver > in order to deal with the resource allocation issues. Agreed, also, how does this fit with Alban's patch series here: https://lkml.org/lkml/2018/3/24/312 do you depend on those changes at all?
On 26-03-18 19:05, Florian Fainelli wrote: > On 03/26/2018 09:58 AM, David Miller wrote: >> From: Mike Looijmans <mike.looijmans@topic.nl> >> Date: Mon, 26 Mar 2018 08:41:29 +0200 >> >>> It's common practice to store MAC addresses for network interfaces into >>> nvmem devices. However the code to actually do this in the kernel lacks, >>> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the >>> address from an nvmem cell provider. >>> >>> This is particulary useful on devices where the ethernet interface cannot >>> be configured by the bootloader, for example because it's in an FPGA. >>> >>> Tested by adapting the cadence macb driver to call this instead of >>> of_get_mac_address(). >>> >>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> >>> --- >>> v2: Use of_nvmem_cell_get to avoid needing the assiciated device >>> Use void* instead of char* >>> Add devicetree binding doc >> >> Like Andrew, I think you should add a new interface for getting the MAC >> address from nvmem. >> >> And drivers can call both of them if they want OF device tree and >> NVMEM probing for MAC addresses. >> >> Later you can add a consolidated interface, if necessary, which does >> both and also take a reference to the MAC address buffer of the driver >> in order to deal with the resource allocation issues. > > Agreed, also, how does this fit with Alban's patch series here: Ok, makes sense. I'll cook up a v3. > > https://lkml.org/lkml/2018/3/24/312 > > do you depend on those changes at all? > As far as I can tell, there's no dependency, I'm adding an nvmem "consumer" while Alban's patch adds a "provider". I'm mainly interested in storing the MAC address into an I2C EEPROM, which you can also buy with a pre-programmed MAC address in the first 6 bytes, so there's no production cost for managing that. Alban's patch might some day collide with my idea of making hardware that has some unique ID into nvmem providers, like 1-wire chips, some NOR flash chips (there's the conflict) and various other devices. That would allow a board to obtain a random yet constant MAC address without any additional hardware. I'll cross that bridge when I find it.
On 26-03-18 17:50, Andrew Lunn wrote: > On Mon, Mar 26, 2018 at 08:41:29AM +0200, Mike Looijmans wrote: >> It's common practice to store MAC addresses for network interfaces into >> nvmem devices. However the code to actually do this in the kernel lacks, >> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the >> address from an nvmem cell provider. >> >> This is particulary useful on devices where the ethernet interface cannot >> be configured by the bootloader, for example because it's in an FPGA. >> >> Tested by adapting the cadence macb driver to call this instead of >> of_get_mac_address(). > > Hi Mike > > I can understand you not wanting to modify all the call sites for > of_get_mac_address(). > > However, the name of_get_nvmem_mac_address() suggests it gets the MAC > address from NVMEM. I think people are going to be surprised when they > find it first tries for a MAC address directly in device tree. I would > drop the call to of_get_mac_address(), and have the MAC driver call > both. > > You could also maybe take a look at fwnode_get_mac_address(). It > should work for both OF and ACPI. It fits better because is passes a > char * for the address. You could make that do both, and call it from > the macb driver. dev_fwnode() probably does what you want. fwnode_get_mac_address looks really new, there's only one user so far. Is it the intention that all drivers eventually migrate to that? (It also means I cannot backport it to the kernel I'm actually using, 'cause I haven't got the Zynq to work with the mainline macb driver yet. But that's just my problem...)
> fwnode_get_mac_address looks really new, there's only one user so far. Is it > the intention that all drivers eventually migrate to that? Hi Mike Probably not. But any driver which needs to work with both ACPI and OF is likely to use this API. So server class ARM64 chips for example. Andrew -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt index 2974e63..cfc376b 100644 --- a/Documentation/devicetree/bindings/net/ethernet.txt +++ b/Documentation/devicetree/bindings/net/ethernet.txt @@ -10,6 +10,8 @@ Documentation/devicetree/bindings/phy/phy-bindings.txt. the boot program; should be used in cases where the MAC address assigned to the device by the boot program is different from the "local-mac-address" property; +- nvmem-cells: phandle, reference to an nvmem node for the MAC address; +- nvmem-cell-names: string, should be "mac-address" if nvmem is to be used; - max-speed: number, specifies maximum speed in Mbit/s supported by the device; - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than the maximum frame size (there's contradiction in the Devicetree diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c index d820f3e..8999745 100644 --- a/drivers/of/of_net.c +++ b/drivers/of/of_net.c @@ -7,6 +7,7 @@ */ #include <linux/etherdevice.h> #include <linux/kernel.h> +#include <linux/nvmem-consumer.h> #include <linux/of_net.h> #include <linux/phy.h> #include <linux/export.h> @@ -80,3 +81,49 @@ const void *of_get_mac_address(struct device_node *np) return of_get_mac_addr(np, "address"); } EXPORT_SYMBOL(of_get_mac_address); + +/** + * Search the device tree for a MAC address, by calling of_get_mac_address + * and if that doesn't provide an address, fetch it from an nvmem provider + * using the name 'mac-address'. + * On success, copies the new address is into memory pointed to by addr and + * returns 0. Returns a negative error code otherwise. + * @dev: Pointer to the device containing the device_node + * @addr: Pointer to receive the MAC address using ether_addr_copy() + */ +int of_get_nvmem_mac_address(struct device_node *np, void *addr) +{ + const void *mac; + struct nvmem_cell *cell; + size_t len; + int ret; + + mac = of_get_mac_address(np); + if (mac) { + ether_addr_copy(addr, mac); + return 0; + } + + cell = of_nvmem_cell_get(np, "mac-address"); + if (IS_ERR(cell)) + return PTR_ERR(cell); + + mac = nvmem_cell_read(cell, &len); + + nvmem_cell_put(cell); + + if (IS_ERR(mac)) + return PTR_ERR(mac); + + if (len < 6 || !is_valid_ether_addr(mac)) { + ret = -EINVAL; + } else { + ether_addr_copy(addr, mac); + ret = 0; + } + + kfree(mac); + + return ret; +} +EXPORT_SYMBOL(of_get_nvmem_mac_address); diff --git a/include/linux/of_net.h b/include/linux/of_net.h index 9cd72aa..90d81ee 100644 --- a/include/linux/of_net.h +++ b/include/linux/of_net.h @@ -13,6 +13,7 @@ struct net_device; extern int of_get_phy_mode(struct device_node *np); extern const void *of_get_mac_address(struct device_node *np); +extern int of_get_nvmem_mac_address(struct device_node *np, void *addr); extern struct net_device *of_find_net_device_by_node(struct device_node *np); #else static inline int of_get_phy_mode(struct device_node *np) @@ -25,6 +26,11 @@ static inline const void *of_get_mac_address(struct device_node *np) return NULL; } +static inline int of_get_nvmem_mac_address(struct device_node *np, void *addr) +{ + return -ENODEV; +} + static inline struct net_device *of_find_net_device_by_node(struct device_node *np) { return NULL;
It's common practice to store MAC addresses for network interfaces into nvmem devices. However the code to actually do this in the kernel lacks, so this patch adds of_get_nvmem_mac_address() for drivers to obtain the address from an nvmem cell provider. This is particulary useful on devices where the ethernet interface cannot be configured by the bootloader, for example because it's in an FPGA. Tested by adapting the cadence macb driver to call this instead of of_get_mac_address(). Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> --- v2: Use of_nvmem_cell_get to avoid needing the assiciated device Use void* instead of char* Add devicetree binding doc Documentation/devicetree/bindings/net/ethernet.txt | 2 + drivers/of/of_net.c | 47 ++++++++++++++++++++++ include/linux/of_net.h | 6 +++ 3 files changed, 55 insertions(+)