| Message ID | 1555445100-30936-1-git-send-email-ynezz@true.cz |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of_net: add mtd-mac-address support to of_get_mac_address() | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 122 lines checked" |
On 16/04/2019 13:05, Petr Štetiar wrote: > From: John Crispin <john@phrozen.org> > > Many embedded devices have information such as MAC addresses stored > inside MTD devices. This patch allows us to add a property inside a node > describing a network interface. The new property points at a MTD > partition with an offset where the MAC address can be found. > > This patch has originated in OpenWrt some time ago, so in order to > consider usefulness of this patch, here are some real-world numbers > which hopefully speak for themselves: > > * mtd-mac-address used 497 times in 357 device tree files > * mtd-mac-address-increment used 74 times in 58 device tree files > * mtd-mac-address-increment-byte used 1 time in 1 device tree file > > Signed-off-by: John Crispin <john@phrozen.org> > Signed-off-by: Felix Fietkau <nbd@nbd.name> > [cleanup of the patch for upstream submission] > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- [snip] > +static const void *of_get_mac_address_mtd(struct device_node *np) > +{ > +#ifdef CONFIG_MTD > + void *addr; > + size_t retlen; > + int size, ret; > + u8 mac[ETH_ALEN]; > + phandle phandle; > + const char *part; > + const __be32 *list; > + struct mtd_info *mtd; > + struct property *prop; > + u32 mac_inc = 0; > + u32 inc_idx = ETH_ALEN-1; > + struct device_node *mtd_np = NULL; Reverse christmas tree would look a bit nicer here. > + > + list = of_get_property(np, "mtd-mac-address", &size); > + if (!list || (size != (2 * sizeof(*list)))) > + return NULL; > + > + phandle = be32_to_cpup(list++); > + if (phandle) > + mtd_np = of_find_node_by_phandle(phandle); > + > + if (!mtd_np) > + return NULL; > + > + part = of_get_property(mtd_np, "label", NULL); > + if (!part) > + part = mtd_np->name; > + > + mtd = get_mtd_device_nm(part); > + if (IS_ERR(mtd)) > + return NULL; > + > + ret = mtd_read(mtd, be32_to_cpup(list), ETH_ALEN, &retlen, mac); > + put_mtd_device(mtd); > + > + of_property_read_u32(np, "mtd-mac-address-increment-byte", &inc_idx); of_property_read_u8() would probably be good here since this can't be bigger than 5 anyway. > + if (inc_idx > ETH_ALEN-1) > + return NULL; > > + if (!of_property_read_u32(np, "mtd-mac-address-increment", &mac_inc)) > + mac[inc_idx] += mac_inc; If I use a number greater than and included 128; this will cause a roll over, should this be range checked? Similarly, using of_property_read_u8() might be a better fit? Other than those, LGTM
Hi Rob, On 4/16/19 5:29 PM, Florian Fainelli wrote: > > > On 16/04/2019 13:05, Petr Štetiar wrote: >> From: John Crispin <john@phrozen.org> >> >> Many embedded devices have information such as MAC addresses stored >> inside MTD devices. This patch allows us to add a property inside a node >> describing a network interface. The new property points at a MTD >> partition with an offset where the MAC address can be found. >> >> This patch has originated in OpenWrt some time ago, so in order to >> consider usefulness of this patch, here are some real-world numbers >> which hopefully speak for themselves: >> >> * mtd-mac-address used 497 times in 357 device tree files >> * mtd-mac-address-increment used 74 times in 58 device tree files >> * mtd-mac-address-increment-byte used 1 time in 1 device tree file >> >> Signed-off-by: John Crispin <john@phrozen.org> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> >> [cleanup of the patch for upstream submission] >> Signed-off-by: Petr Štetiar <ynezz@true.cz> >> --- > > [snip] > >> +static const void *of_get_mac_address_mtd(struct device_node *np) >> +{ >> +#ifdef CONFIG_MTD >> + void *addr; >> + size_t retlen; >> + int size, ret; >> + u8 mac[ETH_ALEN]; >> + phandle phandle; >> + const char *part; >> + const __be32 *list; >> + struct mtd_info *mtd; >> + struct property *prop; >> + u32 mac_inc = 0; >> + u32 inc_idx = ETH_ALEN-1; >> + struct device_node *mtd_np = NULL; > > Reverse christmas tree would look a bit nicer here. Do we a variable declaration format preference for drivers/of/*? -Frank > >> + >> + list = of_get_property(np, "mtd-mac-address", &size); >> + if (!list || (size != (2 * sizeof(*list)))) >> + return NULL; >> + >> + phandle = be32_to_cpup(list++); >> + if (phandle) >> + mtd_np = of_find_node_by_phandle(phandle); >> + >> + if (!mtd_np) >> + return NULL; >> + >> + part = of_get_property(mtd_np, "label", NULL); >> + if (!part) >> + part = mtd_np->name; >> + >> + mtd = get_mtd_device_nm(part); >> + if (IS_ERR(mtd)) >> + return NULL; >> + >> + ret = mtd_read(mtd, be32_to_cpup(list), ETH_ALEN, &retlen, mac); >> + put_mtd_device(mtd); >> + >> + of_property_read_u32(np, "mtd-mac-address-increment-byte", &inc_idx); > > of_property_read_u8() would probably be good here since this can't be bigger than 5 anyway. > >> + if (inc_idx > ETH_ALEN-1) >> + return NULL; > >> + if (!of_property_read_u32(np, "mtd-mac-address-increment", &mac_inc)) >> + mac[inc_idx] += mac_inc; > > If I use a number greater than and included 128; this will cause a roll over, should this be range checked? Similarly, using of_property_read_u8() might be a better fit? > > Other than those, LGTM
On 16.04.2019 22:05, Petr Štetiar wrote: > From: John Crispin <john@phrozen.org> > > Many embedded devices have information such as MAC addresses stored > inside MTD devices. This patch allows us to add a property inside a node > describing a network interface. The new property points at a MTD > partition with an offset where the MAC address can be found. > > This patch has originated in OpenWrt some time ago, so in order to > consider usefulness of this patch, here are some real-world numbers > which hopefully speak for themselves: > > * mtd-mac-address used 497 times in 357 device tree files > * mtd-mac-address-increment used 74 times in 58 device tree files > * mtd-mac-address-increment-byte used 1 time in 1 device tree file > > Signed-off-by: John Crispin <john@phrozen.org> > Signed-off-by: Felix Fietkau <nbd@nbd.name> > [cleanup of the patch for upstream submission] > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > Documentation/devicetree/bindings/net/ethernet.txt | 5 ++ > drivers/of/of_net.c | 82 +++++++++++++++++++++- > 2 files changed, 86 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt > index cfc376b..8a7891e 100644 > --- a/Documentation/devicetree/bindings/net/ethernet.txt > +++ b/Documentation/devicetree/bindings/net/ethernet.txt > @@ -10,6 +10,11 @@ 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; > +- mtd-mac-address: specify a MTD partition + offset containing array of 6 bytes > +- mtd-mac-address-increment: specify number by which we should increment the > + MAC address stored in the MTD partition > +- mtd-mac-address-increment-byte: specify octet/byte(0-5) in the MAC address, > + where we should increment the value, defaults to octet 5 (the last one) > - 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; > diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c > index 810ab0f..01b24d6 100644 > --- a/drivers/of/of_net.c > +++ b/drivers/of/of_net.c > @@ -11,6 +11,7 @@ > #include <linux/of_net.h> > #include <linux/phy.h> > #include <linux/export.h> > +#include <linux/mtd/mtd.h> > > /** > * of_get_phy_mode - Get phy mode for given device_node > @@ -39,7 +40,7 @@ int of_get_phy_mode(struct device_node *np) > } > EXPORT_SYMBOL_GPL(of_get_phy_mode); > > -static const void *of_get_mac_addr(struct device_node *np, const char *name) > +static void *of_get_mac_addr(struct device_node *np, const char *name) > { > struct property *pp = of_find_property(np, name, NULL); > > @@ -48,6 +49,78 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) > return NULL; > } > > +static const void *of_get_mac_address_mtd(struct device_node *np) > +{ > +#ifdef CONFIG_MTD > + void *addr; > + size_t retlen; > + int size, ret; > + u8 mac[ETH_ALEN]; > + phandle phandle; > + const char *part; > + const __be32 *list; > + struct mtd_info *mtd; > + struct property *prop; > + u32 mac_inc = 0; > + u32 inc_idx = ETH_ALEN-1; > + struct device_node *mtd_np = NULL; > + > + list = of_get_property(np, "mtd-mac-address", &size); > + if (!list || (size != (2 * sizeof(*list)))) > + return NULL; > + > + phandle = be32_to_cpup(list++); > + if (phandle) > + mtd_np = of_find_node_by_phandle(phandle); > + > + if (!mtd_np) > + return NULL; > + > + part = of_get_property(mtd_np, "label", NULL); > + if (!part) > + part = mtd_np->name; > + > + mtd = get_mtd_device_nm(part); > + if (IS_ERR(mtd)) > + return NULL; > + > + ret = mtd_read(mtd, be32_to_cpup(list), ETH_ALEN, &retlen, mac); ret doesn't seem to be checked. And checking retlen wouldn't be bad too. > + put_mtd_device(mtd); > + > + of_property_read_u32(np, "mtd-mac-address-increment-byte", &inc_idx); > + if (inc_idx > ETH_ALEN-1) > + return NULL; > + > + if (!of_property_read_u32(np, "mtd-mac-address-increment", &mac_inc)) > + mac[inc_idx] += mac_inc; > + > + if (!is_valid_ether_addr(mac)) > + return NULL; > + > + addr = of_get_mac_addr(np, "mac-address"); > + if (addr) { > + memcpy(addr, mac, ETH_ALEN); > + return addr; > + } > + > + prop = kzalloc(sizeof(*prop), GFP_KERNEL); > + if (!prop) > + return NULL; > + > + prop->name = "mac-address"; > + prop->length = ETH_ALEN; > + prop->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); > + if (!prop->value || of_add_property(np, prop)) > + goto free; > + > + return prop->value; > +free: > + kfree(prop->value); > + kfree(prop); > +#endif > + return NULL; > +} > + > /** > * Search the device tree for the best MAC address to use. 'mac-address' is > * checked first, because that is supposed to contain to "most recent" MAC > @@ -65,11 +138,18 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) > * addresses. Some older U-Boots only initialized 'local-mac-address'. In > * this case, the real MAC is in 'local-mac-address', and 'mac-address' exists > * but is all zeros. > + * > + * If a mtd-mac-address property exists, try to fetch the MAC address from the > + * specified MTD device, and store it as a 'mac-address' property. > */ > const void *of_get_mac_address(struct device_node *np) > { > const void *addr; > > + addr = of_get_mac_address_mtd(np); > + if (addr) > + return addr; > + > addr = of_get_mac_addr(np, "mac-address"); > if (addr) > return addr; >
On Tue, Apr 16, 2019 at 10:05:00PM +0200, Petr Štetiar wrote: > From: John Crispin <john@phrozen.org> > > Many embedded devices have information such as MAC addresses stored > inside MTD devices. This patch allows us to add a property inside a node > describing a network interface. The new property points at a MTD > partition with an offset where the MAC address can be found. > > This patch has originated in OpenWrt some time ago, so in order to > consider usefulness of this patch, here are some real-world numbers > which hopefully speak for themselves: > > * mtd-mac-address used 497 times in 357 device tree files > * mtd-mac-address-increment used 74 times in 58 device tree files > * mtd-mac-address-increment-byte used 1 time in 1 device tree file > > Signed-off-by: John Crispin <john@phrozen.org> > Signed-off-by: Felix Fietkau <nbd@nbd.name> > [cleanup of the patch for upstream submission] > Signed-off-by: Petr Štetiar <ynezz@true.cz> NVMEM is supported by of_net already and there's an MTD-to-nvmem bridge already, so it doesn't look really necessary to create additional properties that cover the same use case. (or at least, you should explain why nvmem doesn't work there) Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Maxime Ripard <maxime.ripard@bootlin.com> [2019-04-17 10:06:14]: Hi Maxime, > On Tue, Apr 16, 2019 at 10:05:00PM +0200, Petr Štetiar wrote: > > From: John Crispin <john@phrozen.org> > > > > Many embedded devices have information such as MAC addresses stored > > inside MTD devices. This patch allows us to add a property inside a node > > describing a network interface. The new property points at a MTD > > partition with an offset where the MAC address can be found. > > > > This patch has originated in OpenWrt some time ago, so in order to > > consider usefulness of this patch, here are some real-world numbers > > which hopefully speak for themselves: > > > > * mtd-mac-address used 497 times in 357 device tree files > > * mtd-mac-address-increment used 74 times in 58 device tree files > > * mtd-mac-address-increment-byte used 1 time in 1 device tree file > > NVMEM is supported by of_net already and there's an MTD-to-nvmem > bridge already, so it doesn't look really necessary to create > additional properties that cover the same use case. if those use cases could be handled with NVMEM, then I'm all in. As I can't find any example in some device tree file in the kernel tree yet and the documentation isn't clear to me about this topic either, could you please help me and provide me with some NVMEM based example for the following simplified use case? Or what do I need to bend/patch in order to support this within NVME. &spi { flash@0 { compatible = "jedec,spi-nor"; mtdparts: partitions { compatible = "fixed-partitions"; art: partition@50000 { label = "factory"; reg = <0x050000 0x010000>; read-only; }; }; }; }; ð0 { mtd-mac-address = <&art 0x4>; }; ð1 { mtd-mac-address = <&art 0x04>; mtd-mac-address-increment = <1>; }; &wmac { mtd-mac-address = <&art 0x04>; mtd-mac-address-increment = <2>; }; I would like to point out, that this proposed patch is currently handling only some of the use cases within OpenWrt tree as well, merely those where the vendor of the device is providing MAC address in the NVMEM as an array of 6 bytes. Unfortunately there are creative vendors which provide MAC addresses in the NVMEM as ASCII text in following two formats (hexdump -C /dev/mtdX): 00000090 57 2e 4c 41 4e 2e 4d 41 43 2e 41 64 64 72 65 73 |W.LAN.MAC.Addres| 000000a0 73 3d 30 30 39 30 46 45 43 39 43 42 45 35 00 48 |s=0090FEC9CBE5.H| 000000b0 57 2e 4c 41 4e 2e 32 47 2e 30 2e 4d 41 43 2e 41 |W.LAN.2G.0.MAC.A| 000000c0 64 64 72 65 73 73 3d 30 30 39 30 46 45 43 39 43 |ddress=0090FEC9C| 000000d0 42 45 36 00 48 57 2e 4c 41 4e 2e 35 47 2e 30 2e |BE6.HW.LAN.5G.0.| 000000e0 4d 41 43 2e 41 64 64 72 65 73 73 3d 30 30 39 30 |MAC.Address=0090| 000000f0 46 45 43 39 43 42 45 37 00 48 57 2e 57 41 4e 2e |FEC9CBE7.HW.WAN.| 00000100 4d 41 43 2e 41 64 64 72 65 73 73 3d 30 30 39 30 |MAC.Address=0090| 00000110 46 45 43 39 43 42 45 34 00 00 00 00 00 00 00 00 |FEC9CBE4........| (From https://github.com/openwrt/openwrt/pull/1448#issuecomment-442476695) 00000170 39 7a 71 05 83 6b bd 9c a7 45 fb 69 6e 27 a2 56 |9zq..k...E.in'.V| 00000180 66 61 63 5f 6d 61 63 20 3d 20 44 34 3a 45 45 3a |fac_mac = D4:EE:| 00000190 30 37 3a 33 33 3a 36 43 3a 32 30 0a 42 44 49 4e |07:33:6C:20.BDIN| 000001a0 46 4f 5f 45 4e 44 0a 00 ff ff ff ff ff ff ff ff |FO_END..........| (From https://github.com/openwrt/openwrt/pull/1906#issuecomment-483881911) And those use cases are currently handled within OpenWrt by workarounds in the user space[1] like this one for example: echo $(macaddr_add "$(mtd_get_mac_ascii u-boot-env ethaddr)" 1) > /sys${DEVPATH}/macaddress So I'm wondering how this could be handled with NVMEM as well. I've simply though, that I would try to fix this with some device tree based solution as I think, that this information belongs to the device tree and shouldn't be handled in the user space. I was thinking about adding `mtd-mac-address-ascii` (or similar) device tree based solution to handle the MAC addresses in 0090FEC9CBE4 and 00:90:FE:C9:CB:E4 ascii/text formats as well. 1. https://github.com/openwrt/openwrt/blob/master/target/linux/ath79/base-files/etc/hotplug.d/ieee80211/10_fix_wifi_mac -- ynezz
On Wed, Apr 17, 2019 at 11:49:21AM +0200, Petr Štetiar wrote: > Maxime Ripard <maxime.ripard@bootlin.com> [2019-04-17 10:06:14]: > > Hi Maxime, > > > On Tue, Apr 16, 2019 at 10:05:00PM +0200, Petr Štetiar wrote: > > > From: John Crispin <john@phrozen.org> > > > > > > Many embedded devices have information such as MAC addresses stored > > > inside MTD devices. This patch allows us to add a property inside a node > > > describing a network interface. The new property points at a MTD > > > partition with an offset where the MAC address can be found. > > > > > > This patch has originated in OpenWrt some time ago, so in order to > > > consider usefulness of this patch, here are some real-world numbers > > > which hopefully speak for themselves: > > > > > > * mtd-mac-address used 497 times in 357 device tree files > > > * mtd-mac-address-increment used 74 times in 58 device tree files > > > * mtd-mac-address-increment-byte used 1 time in 1 device tree file > > > > NVMEM is supported by of_net already and there's an MTD-to-nvmem > > bridge already, so it doesn't look really necessary to create > > additional properties that cover the same use case. > > if those use cases could be handled with NVMEM, then I'm all in. As I can't > find any example in some device tree file in the kernel tree yet and the > documentation isn't clear to me about this topic either, could you please help > me and provide me with some NVMEM based example for the following simplified > use case? Or what do I need to bend/patch in order to support this within > NVME. https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/nvmem/nvmem.txt and https://elixir.bootlin.com/linux/latest/source/net/ethernet/eth.c#L564 Should get you started Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Maxime Ripard <maxime.ripard@bootlin.com> [2019-04-17 12:15:57]:
> https://elixir.bootlin.com/linux/latest/source/net/ethernet/eth.c#L564
Thanks, so if I parse those 2 use cases of nvmem_get_mac_address correctly:
drivers/net/ethernet/cadence/macb_main.c: err = nvmem_get_mac_address(&pdev->dev, bp->dev->dev_addr);
drivers/net/ethernet/ti/davinci_emac.c: rc = nvmem_get_mac_address(&pdev->dev, priv->mac_addr);
We would need to spill `nvmem_get_mac_address` into every single driver on
earth (as it's done now in those two drivers I've mentioned above) which would
like to use MAC address stored in NVME or wire `nvmem_get_mac_address` into
`of_get_mac_address`, right?
-- ynezz
Maxime Ripard <maxime.ripard@bootlin.com> [2019-04-17 10:06:14]: > NVMEM is supported by of_net already Well, not anymore: commit afa64a72b862a7a9d04f8d07fba632eaf06b23f8 Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> Date: Fri Nov 30 09:20:59 2018 +0100 of: net: kill of_get_nvmem_mac_address() Now, I'm really confused. Documentation/devicetree/bindings/net/ethernet.txt states following: - 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; which is actually misleading and confusing. There are only two ethernet drivers in the tree cadence and davinci which support this properties. Well there's nixge, but this one is special, because it has renamed `mac-address` to `address` with it's own implementation in `nixge_get_nvmem_address`. All other ethernet drivers (and few others) simply use `of_get_mac_address` which doesn't support NVMEM and the documented nvmem-cells* properties. > so it doesn't look really necessary to create additional properties that > cover the same use case. NVMEM could be reused for this purpose and it seems like a way to go, probably if we could wire `nvmem_get_mac_address` into `of_get_mac_address` and find a way how to handle the remaining use cases currently not handled in NVMEM: * MAC address (octet/byte) incrementation (already handled by the proposed patch) * MAC address stored as ascii/text (0090FEC9CBE4 and 00:90:FE:C9:CB:E4 formats) which is currently missing but would be nice to have I can prepare patches for that, just don't want to waste more time then really necessary, so it would really help me if someone could tell me how this should be implemented in NVMEM and I'll simply do it in this acceptable way and call it a day. Thanks! -- ynezz
On Wed, Apr 17, 2019 at 06:06:00PM +0200, Petr Štetiar wrote: > Maxime Ripard <maxime.ripard@bootlin.com> [2019-04-17 10:06:14]: > > > NVMEM is supported by of_net already > > Well, not anymore: > > commit afa64a72b862a7a9d04f8d07fba632eaf06b23f8 > Author: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Date: Fri Nov 30 09:20:59 2018 +0100 > > of: net: kill of_get_nvmem_mac_address() > > Now, I'm really confused. > > Documentation/devicetree/bindings/net/ethernet.txt states following: > > - 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; > > which is actually misleading and confusing. There are only two ethernet > drivers in the tree cadence and davinci which support this properties. Well > there's nixge, but this one is special, because it has renamed `mac-address` > to `address` with it's own implementation in `nixge_get_nvmem_address`. > > All other ethernet drivers (and few others) simply use `of_get_mac_address` > which doesn't support NVMEM and the documented nvmem-cells* properties. > > > so it doesn't look really necessary to create additional properties that > > cover the same use case. > > NVMEM could be reused for this purpose and it seems like a way to go, probably > if we could wire `nvmem_get_mac_address` into `of_get_mac_address` and find a > way how to handle the remaining use cases currently not handled in NVMEM: > > * MAC address (octet/byte) incrementation (already handled by the proposed patch) > * MAC address stored as ascii/text (0090FEC9CBE4 and 00:90:FE:C9:CB:E4 > formats) which is currently missing but would be nice to have > > I can prepare patches for that, just don't want to waste more time then really > necessary, so it would really help me if someone could tell me how this should > be implemented in NVMEM and I'll simply do it in this acceptable way and call > it a day. Just send whatever you have in mind with the nvmem developpers as recepients. They are not in this thread, so I'm not sure we can point you in the direction they want Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, Apr 16, 2019 at 08:01:56PM -0700, Frank Rowand wrote: > Hi Rob, > > On 4/16/19 5:29 PM, Florian Fainelli wrote: > > > > > > On 16/04/2019 13:05, Petr Štetiar wrote: > >> From: John Crispin <john@phrozen.org> > >> > >> Many embedded devices have information such as MAC addresses stored > >> inside MTD devices. This patch allows us to add a property inside a node > >> describing a network interface. The new property points at a MTD > >> partition with an offset where the MAC address can be found. > >> > >> This patch has originated in OpenWrt some time ago, so in order to > >> consider usefulness of this patch, here are some real-world numbers > >> which hopefully speak for themselves: > >> > >> * mtd-mac-address used 497 times in 357 device tree files > >> * mtd-mac-address-increment used 74 times in 58 device tree files > >> * mtd-mac-address-increment-byte used 1 time in 1 device tree file > >> > >> Signed-off-by: John Crispin <john@phrozen.org> > >> Signed-off-by: Felix Fietkau <nbd@nbd.name> > >> [cleanup of the patch for upstream submission] > >> Signed-off-by: Petr Štetiar <ynezz@true.cz> > >> --- > > > > [snip] > > > >> +static const void *of_get_mac_address_mtd(struct device_node *np) > >> +{ > >> +#ifdef CONFIG_MTD > >> + void *addr; > >> + size_t retlen; > >> + int size, ret; > >> + u8 mac[ETH_ALEN]; > >> + phandle phandle; > >> + const char *part; > >> + const __be32 *list; > >> + struct mtd_info *mtd; > >> + struct property *prop; > >> + u32 mac_inc = 0; > >> + u32 inc_idx = ETH_ALEN-1; > >> + struct device_node *mtd_np = NULL; > > > > Reverse christmas tree would look a bit nicer here. > > Do we a variable declaration format preference for drivers/of/*? We'd better get one. It's all the rage. How about fallen Christmas tree: int a; bool fallen; char christmas_tree; int for_our; int dt; Rob
On 4/29/19 5:48 PM, Rob Herring wrote: > On Tue, Apr 16, 2019 at 08:01:56PM -0700, Frank Rowand wrote: >> Hi Rob, >> >> On 4/16/19 5:29 PM, Florian Fainelli wrote: >>> >>> >>> On 16/04/2019 13:05, Petr Štetiar wrote: >>>> From: John Crispin <john@phrozen.org> >>>> >>>> Many embedded devices have information such as MAC addresses stored >>>> inside MTD devices. This patch allows us to add a property inside a node >>>> describing a network interface. The new property points at a MTD >>>> partition with an offset where the MAC address can be found. >>>> >>>> This patch has originated in OpenWrt some time ago, so in order to >>>> consider usefulness of this patch, here are some real-world numbers >>>> which hopefully speak for themselves: >>>> >>>> * mtd-mac-address used 497 times in 357 device tree files >>>> * mtd-mac-address-increment used 74 times in 58 device tree files >>>> * mtd-mac-address-increment-byte used 1 time in 1 device tree file >>>> >>>> Signed-off-by: John Crispin <john@phrozen.org> >>>> Signed-off-by: Felix Fietkau <nbd@nbd.name> >>>> [cleanup of the patch for upstream submission] >>>> Signed-off-by: Petr Štetiar <ynezz@true.cz> >>>> --- >>> >>> [snip] >>> >>>> +static const void *of_get_mac_address_mtd(struct device_node *np) >>>> +{ >>>> +#ifdef CONFIG_MTD >>>> + void *addr; >>>> + size_t retlen; >>>> + int size, ret; >>>> + u8 mac[ETH_ALEN]; >>>> + phandle phandle; >>>> + const char *part; >>>> + const __be32 *list; >>>> + struct mtd_info *mtd; >>>> + struct property *prop; >>>> + u32 mac_inc = 0; >>>> + u32 inc_idx = ETH_ALEN-1; >>>> + struct device_node *mtd_np = NULL; >>> >>> Reverse christmas tree would look a bit nicer here. >> >> Do we a variable declaration format preference for drivers/of/*? > > We'd better get one. It's all the rage. > > How about fallen Christmas tree: > > int a; > bool fallen; > char christmas_tree; > int for_our; > int dt; Nice! That is actually the most aesthetically pleasing method I have seen. :-) In the future I will tell people to ignore devicetree review comments that espouse a declaration religion. As long as the declarations are within reason (and sort of follow whatever style is present elsewhere in the same file). > > Rob >
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt index cfc376b..8a7891e 100644 --- a/Documentation/devicetree/bindings/net/ethernet.txt +++ b/Documentation/devicetree/bindings/net/ethernet.txt @@ -10,6 +10,11 @@ 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; +- mtd-mac-address: specify a MTD partition + offset containing array of 6 bytes +- mtd-mac-address-increment: specify number by which we should increment the + MAC address stored in the MTD partition +- mtd-mac-address-increment-byte: specify octet/byte(0-5) in the MAC address, + where we should increment the value, defaults to octet 5 (the last one) - 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; diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c index 810ab0f..01b24d6 100644 --- a/drivers/of/of_net.c +++ b/drivers/of/of_net.c @@ -11,6 +11,7 @@ #include <linux/of_net.h> #include <linux/phy.h> #include <linux/export.h> +#include <linux/mtd/mtd.h> /** * of_get_phy_mode - Get phy mode for given device_node @@ -39,7 +40,7 @@ int of_get_phy_mode(struct device_node *np) } EXPORT_SYMBOL_GPL(of_get_phy_mode); -static const void *of_get_mac_addr(struct device_node *np, const char *name) +static void *of_get_mac_addr(struct device_node *np, const char *name) { struct property *pp = of_find_property(np, name, NULL); @@ -48,6 +49,78 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) return NULL; } +static const void *of_get_mac_address_mtd(struct device_node *np) +{ +#ifdef CONFIG_MTD + void *addr; + size_t retlen; + int size, ret; + u8 mac[ETH_ALEN]; + phandle phandle; + const char *part; + const __be32 *list; + struct mtd_info *mtd; + struct property *prop; + u32 mac_inc = 0; + u32 inc_idx = ETH_ALEN-1; + struct device_node *mtd_np = NULL; + + list = of_get_property(np, "mtd-mac-address", &size); + if (!list || (size != (2 * sizeof(*list)))) + return NULL; + + phandle = be32_to_cpup(list++); + if (phandle) + mtd_np = of_find_node_by_phandle(phandle); + + if (!mtd_np) + return NULL; + + part = of_get_property(mtd_np, "label", NULL); + if (!part) + part = mtd_np->name; + + mtd = get_mtd_device_nm(part); + if (IS_ERR(mtd)) + return NULL; + + ret = mtd_read(mtd, be32_to_cpup(list), ETH_ALEN, &retlen, mac); + put_mtd_device(mtd); + + of_property_read_u32(np, "mtd-mac-address-increment-byte", &inc_idx); + if (inc_idx > ETH_ALEN-1) + return NULL; + + if (!of_property_read_u32(np, "mtd-mac-address-increment", &mac_inc)) + mac[inc_idx] += mac_inc; + + if (!is_valid_ether_addr(mac)) + return NULL; + + addr = of_get_mac_addr(np, "mac-address"); + if (addr) { + memcpy(addr, mac, ETH_ALEN); + return addr; + } + + prop = kzalloc(sizeof(*prop), GFP_KERNEL); + if (!prop) + return NULL; + + prop->name = "mac-address"; + prop->length = ETH_ALEN; + prop->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); + if (!prop->value || of_add_property(np, prop)) + goto free; + + return prop->value; +free: + kfree(prop->value); + kfree(prop); +#endif + return NULL; +} + /** * Search the device tree for the best MAC address to use. 'mac-address' is * checked first, because that is supposed to contain to "most recent" MAC @@ -65,11 +138,18 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) * addresses. Some older U-Boots only initialized 'local-mac-address'. In * this case, the real MAC is in 'local-mac-address', and 'mac-address' exists * but is all zeros. + * + * If a mtd-mac-address property exists, try to fetch the MAC address from the + * specified MTD device, and store it as a 'mac-address' property. */ const void *of_get_mac_address(struct device_node *np) { const void *addr; + addr = of_get_mac_address_mtd(np); + if (addr) + return addr; + addr = of_get_mac_addr(np, "mac-address"); if (addr) return addr;