* [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address @ 2019-04-28 12:53 Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 1/4] " Petr Štetiar ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 12:53 UTC (permalink / raw) To: netdev, devicetree, linux-kernel, Matthias Brugger Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Petr Štetiar, linux-arm-kernel, linux-mediatek Hi, this patch series is a continuation of my previous attempt[1], where I've tried to wire MTD layer into of_get_mac_address, so it would be possible to load MAC addresses from various NVMEMs as EEPROMs etc. Predecessor of this patch which used directly MTD layer has originated in OpenWrt some time ago and supports already about 497 use cases in 357 device tree files. During the review process of my 1st attempt I was told, that I shouldn't be using MTD directly, but I should rather use new NVMEM subsystem, so this patch series tries to accommodate that. First patch is wiring NVMEM support directly into of_get_mac_address as it's obvious, that adding support for NVMEM into every other driver would mean adding a lot of repetitive code. This patch allows us to configure MAC addresses in various devices like ethernet and wireless adapters directly from of_get_mac_address, which is used by quite a lot of drivers in the tree already. Second patch is simply updating documentation with NVMEM bits, also adding some missing bits like mac-address and local-mac-address properties, which are currently supported by of_get_mac_address. Third and fourth patches are simply removing duplicate NVMEM code which is no longer needed as the first patch has added NVMEM support directly into of_get_mac_address. Just for a better picture, this patch series and one simple patch[2] on top of it, allows me to configure 8Devices Carambola2 board's MAC addresses with following DTS (simplified): &spi { flash@0 { partitions { art: partition@ff0000 { label = "art"; reg = <0xff0000 0x010000>; read-only; nvmem-cells { compatible = "nvmem-cells"; #address-cells = <1>; #size-cells = <1>; eth0_addr: eth-mac-addr@0 { reg = <0x0 0x6>; }; eth1_addr: eth-mac-addr@6 { reg = <0x6 0x6>; }; wmac_addr: wifi-mac-addr@1002 { reg = <0x1002 0x6>; }; }; }; }; }; }; ð0 { nvmem-cells = <ð0_addr>; nvmem-cell-names = "mac-address"; }; ð1 { nvmem-cells = <ð1_addr>; nvmem-cell-names = "mac-address"; }; &wmac { nvmem-cells = <&wmac_addr>; nvmem-cell-names = "mac-address"; }; 1. https://patchwork.ozlabs.org/patch/1086628/ 2. https://patchwork.ozlabs.org/patch/890738/ -- ynezz Petr Štetiar (4): of_net: Add NVMEM support to of_get_mac_address dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour net: macb: Drop nvmem_get_mac_address usage net: davinci_emac: Drop nvmem_get_mac_address usage .../devicetree/bindings/net/altera_tse.txt | 5 ++- Documentation/devicetree/bindings/net/amd-xgbe.txt | 5 +-- .../devicetree/bindings/net/brcm,amac.txt | 4 +-- Documentation/devicetree/bindings/net/cpsw.txt | 5 +-- .../devicetree/bindings/net/davinci_emac.txt | 5 +-- Documentation/devicetree/bindings/net/dsa/dsa.txt | 13 ++----- Documentation/devicetree/bindings/net/ethernet.txt | 6 ++-- .../devicetree/bindings/net/hisilicon-femac.txt | 6 ++-- .../bindings/net/hisilicon-hix5hd2-gmac.txt | 7 ++-- .../devicetree/bindings/net/keystone-netcp.txt | 8 ++--- Documentation/devicetree/bindings/net/macb.txt | 5 ++- .../devicetree/bindings/net/marvell-pxa168.txt | 5 +-- .../devicetree/bindings/net/microchip,enc28j60.txt | 3 +- .../devicetree/bindings/net/microchip,lan78xx.txt | 5 ++- .../devicetree/bindings/net/qca,qca7000.txt | 4 ++- .../devicetree/bindings/net/samsung-sxgbe.txt | 6 ++-- .../bindings/net/snps,dwc-qos-ethernet.txt | 6 ++-- .../bindings/net/socionext,uniphier-ave4.txt | 4 +-- .../devicetree/bindings/net/socionext-netsec.txt | 7 ++-- .../bindings/net/wireless/mediatek,mt76.txt | 5 +-- .../devicetree/bindings/net/wireless/qca,ath9k.txt | 4 +-- drivers/net/ethernet/cadence/macb_main.c | 12 ++----- drivers/net/ethernet/ti/davinci_emac.c | 14 +++----- drivers/of/of_net.c | 42 ++++++++++++++++++++-- 24 files changed, 102 insertions(+), 84 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/4] of_net: Add NVMEM support to of_get_mac_address 2019-04-28 12:53 [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address Petr Štetiar @ 2019-04-28 12:53 ` Petr Štetiar 2019-05-01 20:19 ` Rob Herring 2019-04-28 12:53 ` [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour Petr Štetiar ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 12:53 UTC (permalink / raw) To: netdev, devicetree, linux-kernel, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand Cc: Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Petr Štetiar, Felix Fietkau, John Crispin Many embedded devices have information such as MAC addresses stored inside NVMEMs like EEPROMs and so on. Currently there are only two drivers in the tree which benefit from NVMEM bindings. Adding support for NVMEM into every other driver would mean adding a lot of repetitive code. This patch allows us to configure MAC addresses in various devices like ethernet and wireless adapters directly from of_get_mac_address, which is already used by almost every driver in the tree. Predecessor of this patch which used directly MTD layer has originated in OpenWrt some time ago and supports already about 497 use cases in 357 device tree files. Cc: Alban Bedel <albeu@free.fr> Signed-off-by: Felix Fietkau <nbd@nbd.name> Signed-off-by: John Crispin <john@phrozen.org> Signed-off-by: Petr Štetiar <ynezz@true.cz> --- Changes since v1: * moved handling of nvmem after mac-address and local-mac-address properties drivers/of/of_net.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c index d820f3e..8ce4f47 100644 --- a/drivers/of/of_net.c +++ b/drivers/of/of_net.c @@ -8,6 +8,7 @@ #include <linux/etherdevice.h> #include <linux/kernel.h> #include <linux/of_net.h> +#include <linux/of_platform.h> #include <linux/phy.h> #include <linux/export.h> @@ -47,12 +48,45 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) return NULL; } +static const void *of_get_mac_addr_nvmem(struct device_node *np) +{ + int r; + u8 mac[ETH_ALEN]; + struct property *pp; + struct platform_device *pdev = of_find_device_by_node(np); + + if (!pdev) + return NULL; + + r = nvmem_get_mac_address(&pdev->dev, &mac); + if (r < 0) + return NULL; + + pp = kzalloc(sizeof(*pp), GFP_KERNEL); + if (!pp) + return NULL; + + pp->name = "nvmem-mac-address"; + pp->length = ETH_ALEN; + pp->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); + if (!pp->value || of_add_property(np, pp)) + goto free; + + return pp->value; +free: + kfree(pp->value); + kfree(pp); + + 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 * address. If that isn't set, then 'local-mac-address' is checked next, - * because that is the default address. If that isn't set, then the obsolete - * 'address' is checked, just in case we're using an old device tree. + * because that is the default address. If that isn't set, try to get MAC + * address from nvmem cell named 'mac-address'. If that isn't set, then the + * obsolete 'address' is checked, just in case we're using an old device tree. * * Note that the 'address' property is supposed to contain a virtual address of * the register set, but some DTS files have redefined that property to be the @@ -77,6 +111,10 @@ const void *of_get_mac_address(struct device_node *np) if (addr) return addr; + addr = of_get_mac_addr_nvmem(np); + if (addr) + return addr; + return of_get_mac_addr(np, "address"); } EXPORT_SYMBOL(of_get_mac_address); -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] of_net: Add NVMEM support to of_get_mac_address 2019-04-28 12:53 ` [PATCH v2 1/4] " Petr Štetiar @ 2019-05-01 20:19 ` Rob Herring 2019-05-02 9:05 ` Petr Štetiar 0 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2019-05-01 20:19 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Felix Fietkau, John Crispin On Sun, Apr 28, 2019 at 02:53:19PM +0200, Petr Štetiar wrote: > Many embedded devices have information such as MAC addresses stored > inside NVMEMs like EEPROMs and so on. Currently there are only two > drivers in the tree which benefit from NVMEM bindings. > > Adding support for NVMEM into every other driver would mean adding a lot > of repetitive code. This patch allows us to configure MAC addresses in > various devices like ethernet and wireless adapters directly from > of_get_mac_address, which is already used by almost every driver in the > tree. > > Predecessor of this patch which used directly MTD layer has originated > in OpenWrt some time ago and supports already about 497 use cases in 357 > device tree files. > > Cc: Alban Bedel <albeu@free.fr> > Signed-off-by: Felix Fietkau <nbd@nbd.name> > Signed-off-by: John Crispin <john@phrozen.org> > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > > Changes since v1: > > * moved handling of nvmem after mac-address and local-mac-address properties > > drivers/of/of_net.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c > index d820f3e..8ce4f47 100644 > --- a/drivers/of/of_net.c > +++ b/drivers/of/of_net.c > @@ -8,6 +8,7 @@ > #include <linux/etherdevice.h> > #include <linux/kernel.h> > #include <linux/of_net.h> > +#include <linux/of_platform.h> > #include <linux/phy.h> > #include <linux/export.h> > > @@ -47,12 +48,45 @@ static const void *of_get_mac_addr(struct device_node *np, const char *name) > return NULL; > } > > +static const void *of_get_mac_addr_nvmem(struct device_node *np) > +{ > + int r; > + u8 mac[ETH_ALEN]; > + struct property *pp; > + struct platform_device *pdev = of_find_device_by_node(np); > + > + if (!pdev) > + return NULL; > + > + r = nvmem_get_mac_address(&pdev->dev, &mac); > + if (r < 0) > + return NULL; > + > + pp = kzalloc(sizeof(*pp), GFP_KERNEL); > + if (!pp) > + return NULL; > + > + pp->name = "nvmem-mac-address"; > + pp->length = ETH_ALEN; > + pp->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); > + if (!pp->value || of_add_property(np, pp)) > + goto free; Why add this to the DT? You have the struct device ptr, so just use devm_kzalloc() if you need an allocation. > + > + return pp->value; > +free: > + kfree(pp->value); > + kfree(pp); > + > + 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 > * address. If that isn't set, then 'local-mac-address' is checked next, > - * because that is the default address. If that isn't set, then the obsolete > - * 'address' is checked, just in case we're using an old device tree. > + * because that is the default address. If that isn't set, try to get MAC > + * address from nvmem cell named 'mac-address'. If that isn't set, then the > + * obsolete 'address' is checked, just in case we're using an old device tree. > * > * Note that the 'address' property is supposed to contain a virtual address of > * the register set, but some DTS files have redefined that property to be the > @@ -77,6 +111,10 @@ const void *of_get_mac_address(struct device_node *np) > if (addr) > return addr; > > + addr = of_get_mac_addr_nvmem(np); > + if (addr) > + return addr; > + > return of_get_mac_addr(np, "address"); > } > EXPORT_SYMBOL(of_get_mac_address); > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] of_net: Add NVMEM support to of_get_mac_address 2019-05-01 20:19 ` Rob Herring @ 2019-05-02 9:05 ` Petr Štetiar 2019-05-07 16:06 ` Rob Herring 0 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-05-02 9:05 UTC (permalink / raw) To: Rob Herring Cc: netdev, devicetree, linux-kernel, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Felix Fietkau, John Crispin Rob Herring <robh@kernel.org> [2019-05-01 15:19:25]: Hi Rob, > > + struct property *pp; ... > > + pp = kzalloc(sizeof(*pp), GFP_KERNEL); > > + if (!pp) > > + return NULL; > > + > > + pp->name = "nvmem-mac-address"; > > + pp->length = ETH_ALEN; > > + pp->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); > > + if (!pp->value || of_add_property(np, pp)) > > + goto free; > > Why add this to the DT? I've just carried it over from v1 ("of_net: add mtd-mac-address support to of_get_mac_address()")[1] as nobody objected about this so far. Honestly I don't know if it's necessary to have it, but so far address, mac-address and local-mac-address properties provide this DT nodes, so I've simply thought, that it would be good to have it for MAC address from NVMEM as well in order to stay consistent. Just FYI, my testing ar9331_8dev_carambola2.dts[2] currently produces following runtime DT content: root@OpenWrt:/# find /sys/firmware/devicetree/ -name *nvmem* -o -name *addr@* /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/eth-mac-addr@0 /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/eth-mac-addr@6 /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/wifi-mac-addr@1002 /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-cells /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-mac-address /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-cell-names /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-cells /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-mac-address /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-cell-names /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-cells /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-mac-address /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-cell-names root@OpenWrt:/# hexdump -C /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-mac-address 00000000 00 03 7f 11 52 da |....R.| 00000006 root@OpenWrt:/# ip addr show wlan0 4: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000 link/ether 00:03:7f:11:52:da brd ff:ff:ff:ff:ff:ff 1. https://patchwork.ozlabs.org/patch/1086628/ 2. https://git.openwrt.org/?p=openwrt/staging/ynezz.git;a=blob;f=target/linux/ath79/dts/ar9331_8dev_carambola2.dts;h=349c91e760ca5a56d65c587c949fed5fb6ea980e;hb=349c91e760ca5a56d65c587c949fed5fb6ea980e > You have the struct device ptr, so just use devm_kzalloc() if you need an > allocation. I'll address this in v3, thanks. -- ynezz ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] of_net: Add NVMEM support to of_get_mac_address 2019-05-02 9:05 ` Petr Štetiar @ 2019-05-07 16:06 ` Rob Herring 2019-05-08 9:02 ` Petr Štetiar 0 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2019-05-07 16:06 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Felix Fietkau, John Crispin On Thu, May 2, 2019 at 4:05 AM Petr Štetiar <ynezz@true.cz> wrote: > > Rob Herring <robh@kernel.org> [2019-05-01 15:19:25]: > > Hi Rob, > > > > + struct property *pp; > > ... > > > > + pp = kzalloc(sizeof(*pp), GFP_KERNEL); > > > + if (!pp) > > > + return NULL; > > > + > > > + pp->name = "nvmem-mac-address"; > > > + pp->length = ETH_ALEN; > > > + pp->value = kmemdup(mac, ETH_ALEN, GFP_KERNEL); > > > + if (!pp->value || of_add_property(np, pp)) > > > + goto free; > > > > Why add this to the DT? > > I've just carried it over from v1 ("of_net: add mtd-mac-address support to > of_get_mac_address()")[1] as nobody objected about this so far. That's not really a reason... > Honestly I don't know if it's necessary to have it, but so far address, > mac-address and local-mac-address properties provide this DT nodes, so I've > simply thought, that it would be good to have it for MAC address from NVMEM as > well in order to stay consistent. If you want to be consistent, then fill in 'local-mac-address' with the value from nvmem. We don't need the same thing with a new name added to DT. (TBC, I'm not suggesting you do that here.) But really, my point with using devm_kzalloc() is just return the data, not store in DT and free it when the driver unbinds. Allocating it with devm_kzalloc AND adding it to DT as you've done in v4 leads to 2 entities refcounting the allocation. If the driver unbinds, the buffer is freed, but DT code is still referencing that memory. Also, what happens the 2 time a driver binds? The property would already be in the DT. > > Just FYI, my testing ar9331_8dev_carambola2.dts[2] currently produces > following runtime DT content: > > root@OpenWrt:/# find /sys/firmware/devicetree/ -name *nvmem* -o -name *addr@* > /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells > /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/eth-mac-addr@0 > /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/eth-mac-addr@6 > /sys/firmware/devicetree/base/ahb/spi@1f000000/flash@0/partitions/partition@ff0000/nvmem-cells/wifi-mac-addr@1002 > /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-cells > /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-mac-address > /sys/firmware/devicetree/base/ahb/wmac@18100000/nvmem-cell-names > /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-cells > /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-mac-address > /sys/firmware/devicetree/base/ahb/eth@1a000000/nvmem-cell-names > /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-cells > /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-mac-address > /sys/firmware/devicetree/base/ahb/eth@19000000/nvmem-cell-names 'nvmem-mac-address' is not a documented property. That would need to be documented before using upstream. Though, for reasons above, I don't think it should be. Rob ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] of_net: Add NVMEM support to of_get_mac_address 2019-05-07 16:06 ` Rob Herring @ 2019-05-08 9:02 ` Petr Štetiar 0 siblings, 0 replies; 19+ messages in thread From: Petr Štetiar @ 2019-05-08 9:02 UTC (permalink / raw) To: Rob Herring Cc: netdev, devicetree, linux-kernel, Andrew Lunn, Florian Fainelli, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Felix Fietkau, John Crispin Rob Herring <robh@kernel.org> [2019-05-07 11:06:43]: Hi, > > Honestly I don't know if it's necessary to have it, but so far address, > > mac-address and local-mac-address properties provide this DT nodes, so I've > > simply thought, that it would be good to have it for MAC address from NVMEM as > > well in order to stay consistent. > > If you want to be consistent, then fill in 'local-mac-address' with > the value from nvmem. We don't need the same thing with a new name > added to DT. (TBC, I'm not suggesting you do that here.) Ok, got it. > But really, my point with using devm_kzalloc() is just return the > data, not store in DT and free it when the driver unbinds. Ok, I've simply misunderstood your point, sorry, I'll handle it in the follow up fix series, along with the DT documentation update. > Allocating it with devm_kzalloc AND adding it to DT as you've done in v4 > leads to 2 entities refcounting the allocation. If the driver unbinds, the > buffer is freed, but DT code is still referencing that memory. Indeed, I did it wrong, will fix that. > 'nvmem-mac-address' is not a documented property. That would need to > be documented before using upstream. Though, for reasons above, I > don't think it should be. Ok, it makes sense now. Thanks for the detailed explanation. -- ynezz ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour 2019-04-28 12:53 [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 1/4] " Petr Štetiar @ 2019-04-28 12:53 ` Petr Štetiar 2019-04-28 16:53 ` Andrew Lunn 2019-04-28 12:53 ` [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage Petr Štetiar 3 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 12:53 UTC (permalink / raw) To: netdev, devicetree, linux-kernel, David S. Miller, Rob Herring, Mark Rutland, Andrew Lunn, Vivien Didelot, Florian Fainelli, Yisen Zhuang, Salil Mehta, Woojung Huh, Microchip Linux Driver Support, Kunihiko Hayashi, Masahiro Yamada, Jassi Brar, Kalle Valo, Matthias Brugger Cc: Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Petr Štetiar, linux-arm-kernel, linux-wireless, linux-mediatek As of_get_mac_address now supports NVMEM under the hood, we need to update the bindings documentation with the new nvmem-cell* properties, which would mean copy&pasting a lot of redundant information to every binding documentation currently referencing some of the MAC address properties. So I've just removed all the references to the optional MAC address properties and replaced them with the reference to the net/ethernet.txt file. While at it, I've also removed other optional Ethernet properties. Signed-off-by: Petr Štetiar <ynezz@true.cz> --- Changes since v1: * instead of updating all bindings documentation with nvmem properties, I've just updated those docs which were already referencing MAC address properties and replaced them with reference to common net/ethernet.txt Documentation/devicetree/bindings/net/altera_tse.txt | 5 ++--- Documentation/devicetree/bindings/net/amd-xgbe.txt | 5 +++-- Documentation/devicetree/bindings/net/brcm,amac.txt | 4 ++-- Documentation/devicetree/bindings/net/cpsw.txt | 5 +++-- Documentation/devicetree/bindings/net/davinci_emac.txt | 5 +++-- Documentation/devicetree/bindings/net/dsa/dsa.txt | 13 ++----------- Documentation/devicetree/bindings/net/ethernet.txt | 6 ++++-- Documentation/devicetree/bindings/net/hisilicon-femac.txt | 6 ++---- .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt | 7 +++---- Documentation/devicetree/bindings/net/keystone-netcp.txt | 8 +++----- Documentation/devicetree/bindings/net/macb.txt | 5 ++--- Documentation/devicetree/bindings/net/marvell-pxa168.txt | 5 +++-- .../devicetree/bindings/net/microchip,enc28j60.txt | 3 ++- Documentation/devicetree/bindings/net/microchip,lan78xx.txt | 5 ++--- Documentation/devicetree/bindings/net/qca,qca7000.txt | 4 +++- Documentation/devicetree/bindings/net/samsung-sxgbe.txt | 6 ++---- .../devicetree/bindings/net/snps,dwc-qos-ethernet.txt | 6 +++--- .../devicetree/bindings/net/socionext,uniphier-ave4.txt | 4 ++-- Documentation/devicetree/bindings/net/socionext-netsec.txt | 7 +++---- .../devicetree/bindings/net/wireless/mediatek,mt76.txt | 5 +++-- .../devicetree/bindings/net/wireless/qca,ath9k.txt | 4 ++-- 21 files changed, 54 insertions(+), 64 deletions(-) diff --git a/Documentation/devicetree/bindings/net/altera_tse.txt b/Documentation/devicetree/bindings/net/altera_tse.txt index 0e21df9..c85a589 100644 --- a/Documentation/devicetree/bindings/net/altera_tse.txt +++ b/Documentation/devicetree/bindings/net/altera_tse.txt @@ -46,9 +46,8 @@ Required properties: - reg: phy id used to communicate to phy. - device_type: Must be "ethernet-phy". -Optional properties: -- local-mac-address: See ethernet.txt in the same directory. -- max-frame-size: See ethernet.txt in the same directory. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example: diff --git a/Documentation/devicetree/bindings/net/amd-xgbe.txt b/Documentation/devicetree/bindings/net/amd-xgbe.txt index 93dcb79..b607765 100644 --- a/Documentation/devicetree/bindings/net/amd-xgbe.txt +++ b/Documentation/devicetree/bindings/net/amd-xgbe.txt @@ -24,8 +24,6 @@ Required properties: - phy-mode: See ethernet.txt file in the same directory Optional properties: -- mac-address: mac address to be assigned to the device. Can be overridden - by UEFI. - dma-coherent: Present if dma operations are coherent - amd,per-channel-interrupt: Indicates that Rx and Tx complete will generate a unique interrupt for each DMA channel - this requires an additional @@ -48,6 +46,9 @@ property is used. - amd,serdes-dfe-tap-config: DFE taps available to run - amd,serdes-dfe-tap-enable: DFE taps to enable +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + Example: xgbe@e0700000 { compatible = "amd,xgbe-seattle-v1a"; diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt index 0bfad65..5e0ba27 100644 --- a/Documentation/devicetree/bindings/net/brcm,amac.txt +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt @@ -16,8 +16,8 @@ Required properties: registers (required for Northstar2) - interrupts: Interrupt number -Optional properties: -- mac-address: See ethernet.txt file in the same directory +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Examples: diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index 3264e19..370161c9 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -49,9 +49,10 @@ Required properties: Optional properties: - dual_emac_res_vlan : Specifies VID to be used to segregate the ports -- mac-address : See ethernet.txt file in the same directory - phy_id : Specifies slave phy id (deprecated, use phy-handle) -- phy-handle : See ethernet.txt file in the same directory + +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Slave sub-nodes: - fixed-link : See fixed-link.txt file in the same directory diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt index ca83dcc..d250c8e 100644 --- a/Documentation/devicetree/bindings/net/davinci_emac.txt +++ b/Documentation/devicetree/bindings/net/davinci_emac.txt @@ -20,11 +20,12 @@ Required properties: Optional properties: - phy-handle: See ethernet.txt file in the same directory. If absent, davinci_emac driver defaults to 100/FULL. -- 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 - ti,davinci-rmii-en: 1 byte, 1 means use RMII - ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM? +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + Example (enbw_cmc board): eth0: emac@1e20000 { compatible = "ti,davinci-dm6467-emac"; diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt index d66a529..6c8da44 100644 --- a/Documentation/devicetree/bindings/net/dsa/dsa.txt +++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt @@ -58,22 +58,13 @@ A user port has the following optional property: Port child nodes may also contain the following optional standardised properties, described in binding documents: -- phy-handle : Phandle to a PHY on an MDIO bus. See - Documentation/devicetree/bindings/net/ethernet.txt - for details. - -- phy-mode : See - Documentation/devicetree/bindings/net/ethernet.txt - for details. - - fixed-link : Fixed-link subnode describing a link to a non-MDIO managed entity. See Documentation/devicetree/bindings/net/fixed-link.txt for details. -- local-mac-address : See - Documentation/devicetree/bindings/net/ethernet.txt - for details. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt index 2974e63..e6e01e0 100644 --- a/Documentation/devicetree/bindings/net/ethernet.txt +++ b/Documentation/devicetree/bindings/net/ethernet.txt @@ -4,12 +4,14 @@ NOTE: All 'phy*' properties documented below are Ethernet specific. For the generic PHY 'phys' property, see Documentation/devicetree/bindings/phy/phy-bindings.txt. -- local-mac-address: array of 6 bytes, specifies the MAC address that was - assigned to the network device; - mac-address: array of 6 bytes, specifies the MAC address that was last used by 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; +- local-mac-address: array of 6 bytes, specifies the MAC address that was + assigned to the network device; +- 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/Documentation/devicetree/bindings/net/hisilicon-femac.txt b/Documentation/devicetree/bindings/net/hisilicon-femac.txt index d11af5e..99ad4d5 100644 --- a/Documentation/devicetree/bindings/net/hisilicon-femac.txt +++ b/Documentation/devicetree/bindings/net/hisilicon-femac.txt @@ -14,15 +14,13 @@ Required properties: the PHY reset signal(optional). - reset-names: should contain the reset signal name "mac"(required) and "phy"(optional). -- mac-address: see ethernet.txt [1]. -- phy-mode: see ethernet.txt [1]. -- phy-handle: see ethernet.txt [1]. - hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given. The 1st cell is reset pre-delay in micro seconds. The 2nd cell is reset pulse in micro seconds. The 3rd cell is reset post-delay in micro seconds. -[1] Documentation/devicetree/bindings/net/ethernet.txt +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example: hisi_femac: ethernet@10090000 { diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt index eea73ad..f4aad2a 100644 --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt @@ -16,9 +16,6 @@ Required properties: - interrupts: should contain the MAC interrupt. - #address-cells: must be <1>. - #size-cells: must be <0>. -- phy-mode: see ethernet.txt [1]. -- phy-handle: see ethernet.txt [1]. -- mac-address: see ethernet.txt [1]. - clocks: clock phandle and specifier pair. - clock-names: contain the clock name "mac_core"(required) and "mac_ifc"(optional). - resets: should contain the phandle to the MAC core reset signal(optional), @@ -31,9 +28,11 @@ Required properties: The 2nd cell is reset pulse in micro seconds. The 3rd cell is reset post-delay in micro seconds. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + - PHY subnode: inherits from phy binding [2] -[1] Documentation/devicetree/bindings/net/ethernet.txt [2] Documentation/devicetree/bindings/net/phy.txt Example: diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt index 04ba1dc..2c9e8ac 100644 --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt @@ -135,14 +135,12 @@ Optional properties: are swapped. The netcp driver will swap the two DWORDs back to the proper order when this property is set to 2 when it obtains the mac address from efuse. -- local-mac-address: the driver is designed to use the of_get_mac_address api - only if efuse-mac is 0. When efuse-mac is 0, the MAC - address is obtained from local-mac-address. If this - attribute is not present, then the driver will use a - random MAC address. - "netcp-device label": phandle to the device specification for each of NetCP sub-module attached to this interface. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + Example binding: netcp: netcp@2000000 { diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt index 8b80515..58923f3 100644 --- a/Documentation/devicetree/bindings/net/macb.txt +++ b/Documentation/devicetree/bindings/net/macb.txt @@ -26,9 +26,8 @@ Required properties: Optional elements: 'tsu_clk' - clocks: Phandles to input clocks. -Optional properties: -- 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 +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Optional properties for PHY child node: - reset-gpios : Should specify the gpio for phy reset diff --git a/Documentation/devicetree/bindings/net/marvell-pxa168.txt b/Documentation/devicetree/bindings/net/marvell-pxa168.txt index 845a148..0d36fcb 100644 --- a/Documentation/devicetree/bindings/net/marvell-pxa168.txt +++ b/Documentation/devicetree/bindings/net/marvell-pxa168.txt @@ -10,8 +10,9 @@ Optional properties: - port-id: Ethernet port number. Should be '0','1' or '2'. - #address-cells: must be 1 when using sub-nodes. - #size-cells: must be 0 when using sub-nodes. -- phy-handle: see ethernet.txt file in the same directory. -- local-mac-address: see ethernet.txt file in the same directory. + +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Sub-nodes: Each PHY can be represented as a sub-node. This is not mandatory. diff --git a/Documentation/devicetree/bindings/net/microchip,enc28j60.txt b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt index 24626e0..dc15eba 100644 --- a/Documentation/devicetree/bindings/net/microchip,enc28j60.txt +++ b/Documentation/devicetree/bindings/net/microchip,enc28j60.txt @@ -21,8 +21,9 @@ Optional properties: - spi-max-frequency: Maximum frequency of the SPI bus when accessing the ENC28J60. According to the ENC28J80 datasheet, the chip allows a maximum of 20 MHz, however, board designs may need to limit this value. -- local-mac-address: See ethernet.txt in the same directory. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example (for NXP i.MX28 with pin control stuff for GPIO irq): diff --git a/Documentation/devicetree/bindings/net/microchip,lan78xx.txt b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt index 76786a0..7824581 100644 --- a/Documentation/devicetree/bindings/net/microchip,lan78xx.txt +++ b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt @@ -7,9 +7,8 @@ The Device Tree properties, if present, override the OTP and EEPROM. Required properties: - compatible: Should be one of "usb424,7800", "usb424,7801" or "usb424,7850". -Optional properties: -- local-mac-address: see ethernet.txt -- mac-address: see ethernet.txt +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Optional properties of the embedded PHY: - microchip,led-modes: a 0..4 element vector, with each element configuring diff --git a/Documentation/devicetree/bindings/net/qca,qca7000.txt b/Documentation/devicetree/bindings/net/qca,qca7000.txt index e4a8a51..26cce1c 100644 --- a/Documentation/devicetree/bindings/net/qca,qca7000.txt +++ b/Documentation/devicetree/bindings/net/qca,qca7000.txt @@ -23,7 +23,6 @@ Optional properties: Numbers smaller than 1000000 or greater than 16000000 are invalid. Missing the property will set the SPI frequency to 8000000 Hertz. -- local-mac-address : see ./ethernet.txt - qca,legacy-mode : Set the SPI data transfer of the QCA7000 to legacy mode. In this mode the SPI master must toggle the chip select between each data word. In burst mode these gaps aren't @@ -31,6 +30,9 @@ Optional properties: the QCA7000 is setup via GPIO pin strapping. If the property is missing the driver defaults to burst mode. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + SPI Example: /* Freescale i.MX28 SPI master*/ diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt index 46e5911..09bd850 100644 --- a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt @@ -20,10 +20,8 @@ Required properties: When fixed length is needed for burst mode, it can be set within allowable range. -Optional properties: -- mac-address: 6 bytes, mac address -- max-frame-size: Maximum Transfer Unit (IEEE defined MTU), rather - than the maximum frame size. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example: diff --git a/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt b/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt index 36f1aef..db4c698 100644 --- a/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt +++ b/Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt @@ -103,8 +103,6 @@ Required properties: Optional properties: - dma-coherent: Present if dma operations are coherent -- mac-address: See ethernet.txt in the same directory -- local-mac-address: See ethernet.txt in the same directory - phy-reset-gpios: Phandle and specifier for any GPIO used to reset the PHY. See ../gpio/gpio.txt. - snps,en-lpi: If present it enables use of the AXI low-power interface @@ -118,7 +116,6 @@ Optional properties: - snps,rxpbl: DMA Programmable burst length for the RX DMA - snps,en-tx-lpi-clockgating: Enable gating of the MAC TX clock during TX low-power mode. -- phy-handle: See ethernet.txt file in the same directory - mdio device tree subnode: When the GMAC has a phy connected to its local mdio, there must be device tree subnode with the following required properties: @@ -133,6 +130,9 @@ Optional properties: - device_type: Must be "ethernet-phy". - fixed-mode device tree subnode: see fixed-link.txt in the same directory +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + Examples: ethernet2@40010000 { clock-names = "phy_ref_clk", "apb_pclk"; diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt index fc8f017..d90d27e 100644 --- a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt @@ -31,8 +31,8 @@ Required properties: - socionext,syscon-phy-mode: A phandle to syscon with one argument that configures phy mode. The argument is the ID of MAC instance. -Optional properties: - - local-mac-address: See ethernet.txt in the same directory. +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Required subnode: - mdio: A container for child nodes representing phy nodes. diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt index 0cff94f..3073c16 100644 --- a/Documentation/devicetree/bindings/net/socionext-netsec.txt +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt @@ -26,10 +26,9 @@ Required properties: Optional properties: (See ethernet.txt file in the same directory) - dma-coherent: Boolean property, must only be present if memory accesses performed by the device are cache coherent. -- local-mac-address: See ethernet.txt in the same directory. -- mac-address: See ethernet.txt in the same directory. -- max-speed: See ethernet.txt in the same directory. -- max-frame-size: See ethernet.txt in the same directory. + +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. Example: eth0: ethernet@522d0000 { diff --git a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt index 7b9a776..bcccbb9 100644 --- a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt @@ -13,11 +13,12 @@ properties: Optional properties: -- mac-address: See ethernet.txt in the parent directory -- local-mac-address: See ethernet.txt in the parent directory - ieee80211-freq-limit: See ieee80211.txt - mediatek,mtd-eeprom: Specify a MTD partition + offset containing EEPROM data +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. + Optional nodes: - led: Properties for a connected LED Optional properties: diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt index b7396c8..d07542f 100644 --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt @@ -34,9 +34,9 @@ Optional properties: ath9k wireless chip (in this case the calibration / EEPROM data will be loaded from userspace using the kernel firmware loader). -- mac-address: See ethernet.txt in the parent directory -- local-mac-address: See ethernet.txt in the parent directory +For all other optional Ethernet properties, please refer to +Documentation/devicetree/bindings/net/ethernet.txt. In this example, the node is defined as child node of the PCI controller: &pci0 { -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour 2019-04-28 12:53 ` [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour Petr Štetiar @ 2019-04-28 16:53 ` Andrew Lunn 2019-05-01 20:22 ` Rob Herring 0 siblings, 1 reply; 19+ messages in thread From: Andrew Lunn @ 2019-04-28 16:53 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, David S. Miller, Rob Herring, Mark Rutland, Vivien Didelot, Florian Fainelli, Yisen Zhuang, Salil Mehta, Woojung Huh, Microchip Linux Driver Support, Kunihiko Hayashi, Masahiro Yamada, Jassi Brar, Kalle Valo, Matthias Brugger, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, linux-arm-kernel, linux-wireless, linux-mediatek On Sun, Apr 28, 2019 at 02:53:20PM +0200, Petr Štetiar wrote: > As of_get_mac_address now supports NVMEM under the hood, we need to update > the bindings documentation with the new nvmem-cell* properties, which would > mean copy&pasting a lot of redundant information to every binding > documentation currently referencing some of the MAC address properties. > > So I've just removed all the references to the optional MAC address > properties and replaced them with the reference to the net/ethernet.txt > file. While at it, I've also removed other optional Ethernet properties. Hi Petr I think each individual binding needs to give a hint if of_get_mac_address() is used, and hence if these optional properties are respected. The same is true for other optional properties. I don't want to have to look at the driver to know which optional properties are implemented, the binding should tell me. What the optional properties mean, and which order they are used in can then be defined in ethernet.txt. So i would suggests something like: The MAC address will be determined using the optional properties defined in ethernet.txt. And leave all the other optional parameters in the bindings. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour 2019-04-28 16:53 ` Andrew Lunn @ 2019-05-01 20:22 ` Rob Herring 0 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2019-05-01 20:22 UTC (permalink / raw) To: Andrew Lunn Cc: Petr Štetiar, netdev, devicetree, linux-kernel, David S. Miller, Mark Rutland, Vivien Didelot, Florian Fainelli, Yisen Zhuang, Salil Mehta, Woojung Huh, Microchip Linux Driver Support, Kunihiko Hayashi, Masahiro Yamada, Jassi Brar, Kalle Valo, Matthias Brugger, Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, linux-arm-kernel, linux-wireless, linux-mediatek On Sun, Apr 28, 2019 at 06:53:26PM +0200, Andrew Lunn wrote: > On Sun, Apr 28, 2019 at 02:53:20PM +0200, Petr Štetiar wrote: > > As of_get_mac_address now supports NVMEM under the hood, we need to update > > the bindings documentation with the new nvmem-cell* properties, which would > > mean copy&pasting a lot of redundant information to every binding > > documentation currently referencing some of the MAC address properties. > > > > So I've just removed all the references to the optional MAC address > > properties and replaced them with the reference to the net/ethernet.txt > > file. While at it, I've also removed other optional Ethernet properties. > > Hi Petr > > I think each individual binding needs to give a hint if > of_get_mac_address() is used, and hence if these optional properties > are respected. The same is true for other optional properties. I don't > want to have to look at the driver to know which optional properties > are implemented, the binding should tell me. What the optional > properties mean, and which order they are used in can then be defined > in ethernet.txt. > > So i would suggests something like: > > The MAC address will be determined using the optional properties > defined in ethernet.txt. > > And leave all the other optional parameters in the bindings. Yes. Generally we need to know which properties from a common pool of properties apply to a specific binding. Also there are typically additional constraints for a specific binding. Rob ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-28 12:53 [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 1/4] " Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour Petr Štetiar @ 2019-04-28 12:53 ` Petr Štetiar 2019-04-28 16:56 ` Andrew Lunn 2019-04-28 12:53 ` [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage Petr Štetiar 3 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 12:53 UTC (permalink / raw) To: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Petr Štetiar of_get_mac_address now uses NVMEM under the hood, so it's not necessary to call it manually anymore. Signed-off-by: Petr Štetiar <ynezz@true.cz> --- drivers/net/ethernet/cadence/macb_main.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 3da2795..1b98bc8 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -4172,16 +4172,10 @@ static int macb_probe(struct platform_device *pdev) bp->rx_intr_mask |= MACB_BIT(RXUBR); mac = of_get_mac_address(np); - if (mac) { + if (mac) ether_addr_copy(bp->dev->dev_addr, mac); - } else { - err = nvmem_get_mac_address(&pdev->dev, bp->dev->dev_addr); - if (err) { - if (err == -EPROBE_DEFER) - goto err_out_free_netdev; - macb_get_hwaddr(bp); - } - } + else + macb_get_hwaddr(bp); err = of_get_phy_mode(np); if (err < 0) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-28 12:53 ` [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage Petr Štetiar @ 2019-04-28 16:56 ` Andrew Lunn 2019-04-28 21:08 ` Petr Štetiar 0 siblings, 1 reply; 19+ messages in thread From: Andrew Lunn @ 2019-04-28 16:56 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel On Sun, Apr 28, 2019 at 02:53:21PM +0200, Petr Štetiar wrote: > of_get_mac_address now uses NVMEM under the hood, so it's not necessary > to call it manually anymore. > > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > drivers/net/ethernet/cadence/macb_main.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index 3da2795..1b98bc8 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -4172,16 +4172,10 @@ static int macb_probe(struct platform_device *pdev) > bp->rx_intr_mask |= MACB_BIT(RXUBR); > > mac = of_get_mac_address(np); > - if (mac) { > + if (mac) > ether_addr_copy(bp->dev->dev_addr, mac); > - } else { > - err = nvmem_get_mac_address(&pdev->dev, bp->dev->dev_addr); > - if (err) { > - if (err == -EPROBE_DEFER) The EPRODE_DEFER is interesting here. your change to of_get_mac_address() does not seem to handle that case. It probably should. We don't want it to fail and end up with a random MAC addresses etc, because the NVMEM has not probed yet. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-28 16:56 ` Andrew Lunn @ 2019-04-28 21:08 ` Petr Štetiar 2019-04-28 21:36 ` Andrew Lunn 0 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 21:08 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel Andrew Lunn <andrew@lunn.ch> [2019-04-28 18:56:37]: Hi Andrew, > On Sun, Apr 28, 2019 at 02:53:21PM +0200, Petr Štetiar wrote: > > of_get_mac_address now uses NVMEM under the hood, so it's not necessary > > to call it manually anymore. > > > > Signed-off-by: Petr Štetiar <ynezz@true.cz> > > --- > > drivers/net/ethernet/cadence/macb_main.c | 12 +++--------- > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > > index 3da2795..1b98bc8 100644 > > --- a/drivers/net/ethernet/cadence/macb_main.c > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > @@ -4172,16 +4172,10 @@ static int macb_probe(struct platform_device *pdev) > > bp->rx_intr_mask |= MACB_BIT(RXUBR); > > > > mac = of_get_mac_address(np); > > - if (mac) { > > + if (mac) > > ether_addr_copy(bp->dev->dev_addr, mac); > > - } else { > > - err = nvmem_get_mac_address(&pdev->dev, bp->dev->dev_addr); > > - if (err) { > > - if (err == -EPROBE_DEFER) > > The EPRODE_DEFER is interesting here. your change to > of_get_mac_address() does not seem to handle that case. It probably > should. We don't want it to fail and end up with a random MAC > addresses etc, because the NVMEM has not probed yet. so if I understand this correctly, it probably means, that this approach with modified of_get_mac_address is dead end as current of_get_mac_address users don't expect and handle possible -EPROBE_DEFER error, so I would need to change all the current users, which is nonsense. -- ynezz ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-28 21:08 ` Petr Štetiar @ 2019-04-28 21:36 ` Andrew Lunn 2019-04-29 7:55 ` Petr Štetiar 0 siblings, 1 reply; 19+ messages in thread From: Andrew Lunn @ 2019-04-28 21:36 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel > so if I understand this correctly, it probably means, that this approach with > modified of_get_mac_address is dead end as current of_get_mac_address users > don't expect and handle possible -EPROBE_DEFER error, so I would need to > change all the current users, which is nonsense. Hi Petr I would not say it is dead, it just needs a bit more work. The current users should always be checking for a NULL pointer. You just need to change that to !IS_ERR(). You can then return ERR_PTR(-PROBE_DEFER) from the NVMEM operation. And i agree, wrapping this all up inside of_get_mac_address() is the correct way to do this. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-28 21:36 ` Andrew Lunn @ 2019-04-29 7:55 ` Petr Štetiar 2019-04-29 13:02 ` Andrew Lunn 0 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-29 7:55 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel Andrew Lunn <andrew@lunn.ch> [2019-04-28 23:36:40]: Hi Andrew, > > so if I understand this correctly, it probably means, that this approach with > > modified of_get_mac_address is dead end as current of_get_mac_address users > > don't expect and handle possible -EPROBE_DEFER error, so I would need to > > change all the current users, which is nonsense. > > I would not say it is dead, it just needs a bit more work. ok, that's good news, I've probably just misunderstood your concern about the random MAC address in case when platform/nvmem subsystem returns -EPROBE_DEFER. > The current users should always be checking for a NULL pointer. You > just need to change that to !IS_ERR(). You can then return > ERR_PTR(-PROBE_DEFER) from the NVMEM operation. I'm more then happy to address this in v3, but I'm still curious, what is it going to change in the current state of the tree. My understanding of -PROBE_DEFER is, that it needs to be propagated back from the driver's probe callback/hook to the upper device/driver subsystem in order to be moved to the list of pending drivers and considered for probe later again. This is not going to happen in any of the current drivers, thus it will probably still always result in random MAC address in case of -EPROBE_DEFER error from the nvmem subsystem. -- ynezz ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage 2019-04-29 7:55 ` Petr Štetiar @ 2019-04-29 13:02 ` Andrew Lunn 2019-04-30 14:13 ` Handling of EPROBE_DEFER in of_get_mac_address [Was: Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage] Petr Štetiar 0 siblings, 1 reply; 19+ messages in thread From: Andrew Lunn @ 2019-04-29 13:02 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel On Mon, Apr 29, 2019 at 09:55:14AM +0200, Petr Štetiar wrote: > Andrew Lunn <andrew@lunn.ch> [2019-04-28 23:36:40]: > > Hi Andrew, > > > > so if I understand this correctly, it probably means, that this approach with > > > modified of_get_mac_address is dead end as current of_get_mac_address users > > > don't expect and handle possible -EPROBE_DEFER error, so I would need to > > > change all the current users, which is nonsense. > > > > I would not say it is dead, it just needs a bit more work. > > ok, that's good news, I've probably just misunderstood your concern about the > random MAC address in case when platform/nvmem subsystem returns -EPROBE_DEFER. > > > The current users should always be checking for a NULL pointer. You > > just need to change that to !IS_ERR(). You can then return > > ERR_PTR(-PROBE_DEFER) from the NVMEM operation. > > I'm more then happy to address this in v3, but I'm still curious, what is it > going to change in the current state of the tree. > > My understanding of -PROBE_DEFER is, that it needs to be propagated back from > the driver's probe callback/hook to the upper device/driver subsystem in order > to be moved to the list of pending drivers and considered for probe later > again. This is not going to happen in any of the current drivers, thus it will > probably still always result in random MAC address in case of -EPROBE_DEFER > error from the nvmem subsystem. Hi Petr All current drivers which don't look in NVMEM don't expect EPROBE_DEFER. So not returning it as the result of the probe is fine. The one driver which does expect EPROBE_DEFER already has the code to handle it. What you have to be careful of, is the return value from your new code looking in NVMEM. It should only return EPROBE_DEFER, or another error if there really is expected to be a value in NVMEM, or getting it from NVMEM resulted in an error. I've not looked at the details of nvmem_get_mac_address(), but it should be a two stage process. The first is to look in device tree to find the properties. Device tree is always accessible. So performing a lookup will never return EPROBE_DEFER. If there are no properties, it probably return -ENODEV. You need to consider that as not being a real error, since these are optional properties. of_get_mac_address() needs to try the next source of the MAC address. The second stage is to look into the NVMEM. That could return -EPROBE_DEFER and you should return that error, or any other error at this stage. The MAC address should exist in NVMEM so we want to know about the error. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Handling of EPROBE_DEFER in of_get_mac_address [Was: Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage] 2019-04-29 13:02 ` Andrew Lunn @ 2019-04-30 14:13 ` Petr Štetiar 2019-05-01 13:54 ` Andrew Lunn 0 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-30 14:13 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel Andrew Lunn <andrew@lunn.ch> [2019-04-29 15:02:48]: Hi Andrew, > > My understanding of -PROBE_DEFER is, that it needs to be propagated back from > > the driver's probe callback/hook to the upper device/driver subsystem in order > > to be moved to the list of pending drivers and considered for probe later > > again. This is not going to happen in any of the current drivers, thus it will > > probably still always result in random MAC address in case of -EPROBE_DEFER > > error from the nvmem subsystem. > > All current drivers which don't look in NVMEM don't expect > EPROBE_DEFER. once there's NVMEM wired in of_get_mac_address, one can simply use it, nothing is going to stop the potential user of doing so and if EPROBE_DEFER isn't propagated from the driver back to the upper device driver subsytem, it's probably going to end with random MAC address in some (very rare?) cases. So if we don't want to properly convert all current of_get_mac_address users, and just selectively upgrade drivers which could support NVMEM (and it makes sense for those drivers) with proper EPROBE_DEFER handling, we should probably just extend of_get_mac_address and convert those drivers to NVMEM one by one, as needed: enum of_mac_addr { OF_MAC_ADDR_DT = 0, OF_MAC_ADDR_DT_NVMEM }; const void *of_get_mac_address(struct device_node *np, enum of_mac_addr type); (just my first rough idea, feel free to suggest something more acceptable) > The one driver which does expect EPROBE_DEFER already has the code to > handle it. I've read the code in that macb driver several times and I don't see any code which is specificaly handling the EPROBE_DEFER, so I'm probably blind or I miss something fundamental ;-) This driver is simply forwarding that EPROBE_DEFER error to the upper layer, nothing specific. The other one, davinci_emac simply doesn't care about the possible EPROBE_DEFER and uses random MAC address in case of any NVMEM error. > What you have to be careful of, is the return value from your new code > looking in NVMEM. It should only return EPROBE_DEFER, or another error > if there really is expected to be a value in NVMEM, or getting it from > NVMEM resulted in an error. Thanks for the hint, I've created of_has_nvmem_mac_addr helper for that and it works fine. > I've not looked at the details of nvmem_get_mac_address(), but it > should be a two stage process. The first is to look in device tree to > find the properties. Device tree is always accessible. So performing a > lookup will never return EPROBE_DEFER. If there are no properties, it > probably return -ENODEV. You need to consider that as not being a real > error, since these are optional properties. of_get_mac_address() needs > to try the next source of the MAC address. The second stage is to look > into the NVMEM. That could return -EPROBE_DEFER and you should return > that error, or any other error at this stage. The MAC address should > exist in NVMEM so we want to know about the error. While looking at all current of_get_mac_address users, I've simply found out, that it would look inconsistent and confusing to propagate back EPROBE_DEFER just in some places, so I've bitten the bullet and converted all current of_get_mac_address users to return EPROBE_DEFER properly (where applicable), see bellow. It has resulted in a bunch of commits, and as I'm not sure how it's going to be received and to avoid sending more nonsense to a lot of lists and people, I've made it available in my GitHub repository (just tell me that it's OK and I'll send it as v3): The following changes since commit 37624b58542fb9f2d9a70e6ea006ef8a5f66c30b: Linux 5.1-rc7 (2019-04-28 17:04:13 -0700) are available in the git repository at: https://github.com/ynezz/linux.git upstream/nvmem-mac-address for you to fetch changes up to 5eb1e8131d3f97a488b74563dd8fefa068218e05: powerpc: tsi108: adjust for of_get_mac_address ERR_PTR encoded error value (2019-04-30 13:02:22 +0200) ---------------------------------------------------------------- Petr Štetiar (20): of_net: add NVMEM support to of_get_mac_address dt-bindings: doc: reflect new NVMEM of_get_mac_address behaviour net: macb: drop nvmem_get_mac_address usage net: davinci_emac: drop nvmem_get_mac_address usage net: ethernet: make eth_platform_get_mac_address probe defer aware net: ethernet: make of_get_mac_address probe defer aware net: usb: make eth_platform_get_mac_address probe defer aware net: usb: make of_get_mac_address probe defer aware net: sh_eth: make of_get_mac_address probe defer aware net: fec: make of_get_mac_address probe defer aware net: fec_mpc52xx: make of_get_mac_address probe defer aware net: hisi_femac: make of_get_mac_address probe defer aware net: sky2: make of_get_mac_address probe defer aware net: ks8851: make of_get_mac_address probe defer aware wireless: ath9k: make of_get_mac_address probe defer aware wireless: mt76: make of_get_mac_address probe defer aware wireless: ralink: make of_get_mac_address probe defer aware staging: octeon-ethernet: make of_get_mac_address probe defer aware ARM: Kirkwood: adjust for of_get_mac_address ERR_PTR encoded error value powerpc: tsi108: adjust for of_get_mac_address ERR_PTR encoded error value .../devicetree/bindings/net/altera_tse.txt | 5 +- Documentation/devicetree/bindings/net/amd-xgbe.txt | 5 +- .../devicetree/bindings/net/brcm,amac.txt | 4 +- Documentation/devicetree/bindings/net/cpsw.txt | 4 +- .../devicetree/bindings/net/davinci_emac.txt | 5 +- Documentation/devicetree/bindings/net/dsa/dsa.txt | 5 +- Documentation/devicetree/bindings/net/ethernet.txt | 6 +- .../devicetree/bindings/net/hisilicon-femac.txt | 4 +- .../bindings/net/hisilicon-hix5hd2-gmac.txt | 4 +- .../devicetree/bindings/net/keystone-netcp.txt | 10 ++-- Documentation/devicetree/bindings/net/macb.txt | 5 +- .../devicetree/bindings/net/marvell-pxa168.txt | 4 +- .../devicetree/bindings/net/microchip,enc28j60.txt | 3 +- .../devicetree/bindings/net/microchip,lan78xx.txt | 5 +- .../devicetree/bindings/net/qca,qca7000.txt | 4 +- .../devicetree/bindings/net/samsung-sxgbe.txt | 4 +- .../bindings/net/snps,dwc-qos-ethernet.txt | 5 +- .../bindings/net/socionext,uniphier-ave4.txt | 4 +- .../devicetree/bindings/net/socionext-netsec.txt | 5 +- .../bindings/net/wireless/mediatek,mt76.txt | 5 +- .../devicetree/bindings/net/wireless/qca,ath9k.txt | 4 +- arch/arm/mach-mvebu/kirkwood.c | 3 +- arch/powerpc/sysdev/tsi108_dev.c | 2 +- drivers/net/ethernet/aeroflex/greth.c | 5 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 9 +-- drivers/net/ethernet/altera/altera_tse_main.c | 8 ++- drivers/net/ethernet/arc/emac_main.c | 9 ++- drivers/net/ethernet/aurora/nb8800.c | 9 ++- drivers/net/ethernet/broadcom/bcmsysport.c | 5 +- drivers/net/ethernet/broadcom/bgmac-bcma.c | 9 ++- drivers/net/ethernet/broadcom/bgmac-platform.c | 4 +- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 7 ++- drivers/net/ethernet/broadcom/tg3.c | 10 +++- drivers/net/ethernet/cadence/macb_main.c | 12 ++-- drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 9 ++- drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 4 +- drivers/net/ethernet/davicom/dm9000.c | 4 +- drivers/net/ethernet/ethoc.c | 6 +- drivers/net/ethernet/ezchip/nps_enet.c | 8 ++- drivers/net/ethernet/freescale/fec_main.c | 17 ++++-- drivers/net/ethernet/freescale/fec_mpc52xx.c | 25 +++++---- drivers/net/ethernet/freescale/fman/mac.c | 5 +- .../net/ethernet/freescale/fs_enet/fs_enet-main.c | 6 +- drivers/net/ethernet/freescale/gianfar.c | 7 ++- drivers/net/ethernet/freescale/ucc_geth.c | 6 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 21 ++++--- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 ++- drivers/net/ethernet/intel/i40e/i40e_main.c | 15 ++++- drivers/net/ethernet/intel/igb/igb_main.c | 5 +- drivers/net/ethernet/intel/igc/igc_main.c | 5 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +- drivers/net/ethernet/lantiq_xrx200.c | 4 +- drivers/net/ethernet/marvell/mv643xx_eth.c | 4 +- drivers/net/ethernet/marvell/mvneta.c | 5 +- drivers/net/ethernet/marvell/pxa168_eth.c | 9 ++- drivers/net/ethernet/marvell/sky2.c | 14 +++-- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +-- drivers/net/ethernet/micrel/ks8851.c | 15 +++-- drivers/net/ethernet/micrel/ks8851_mll.c | 6 +- drivers/net/ethernet/microchip/enc28j60.c | 8 ++- drivers/net/ethernet/nxp/lpc_eth.c | 10 +++- drivers/net/ethernet/qualcomm/qca_spi.c | 9 +-- drivers/net/ethernet/qualcomm/qca_uart.c | 9 +-- drivers/net/ethernet/realtek/r8169.c | 4 +- drivers/net/ethernet/renesas/ravb_main.c | 11 +++- drivers/net/ethernet/renesas/sh_eth.c | 15 ++--- .../net/ethernet/samsung/sxgbe/sxgbe_platform.c | 7 ++- drivers/net/ethernet/socionext/sni_ave.c | 9 +-- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 + drivers/net/ethernet/ti/cpsw.c | 4 +- drivers/net/ethernet/ti/davinci_emac.c | 25 ++++----- drivers/net/ethernet/ti/netcp_core.c | 8 ++- drivers/net/ethernet/wiznet/w5100-spi.c | 6 +- drivers/net/ethernet/wiznet/w5100.c | 2 +- drivers/net/ethernet/xilinx/ll_temac_main.c | 5 +- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 +- drivers/net/ethernet/xilinx/xilinx_emaclite.c | 7 ++- drivers/net/usb/asix_devices.c | 7 ++- drivers/net/usb/lan78xx.c | 13 ++++- drivers/net/usb/smsc75xx.c | 16 ++++-- drivers/net/usb/smsc95xx.c | 16 ++++-- drivers/net/wireless/ath/ath9k/init.c | 4 +- drivers/net/wireless/mediatek/mt76/eeprom.c | 10 +++- drivers/net/wireless/mediatek/mt76/mt76.h | 2 +- drivers/net/wireless/mediatek/mt76/mt7603/eeprom.c | 4 +- drivers/net/wireless/mediatek/mt76/mt76x2/eeprom.c | 6 +- drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 5 +- drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 5 +- drivers/net/wireless/ralink/rt2x00/rt2500usb.c | 5 +- drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 4 +- drivers/net/wireless/ralink/rt2x00/rt2x00.h | 3 +- drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 16 ++++-- drivers/net/wireless/ralink/rt2x00/rt61pci.c | 5 +- drivers/net/wireless/ralink/rt2x00/rt73usb.c | 5 +- drivers/of/of_net.c | 64 +++++++++++++++++++++- drivers/staging/octeon/ethernet.c | 7 ++- net/ethernet/eth.c | 8 ++- 98 files changed, 527 insertions(+), 239 deletions(-) So I'm wondering, is this (probably) proper handling of EPROBE_DEFER overkill? Or should I really just convert all current consumers of of_get_mac_address to IS_ERR_OR_NULL() macro check (as you've suggested) and call it a day? Or should I simply change of_get_mac_address so it would allow for progressive conversion of drivers using of_get_mac_address to NVMEM? Thanks! -- ynezz ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Handling of EPROBE_DEFER in of_get_mac_address [Was: Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage] 2019-04-30 14:13 ` Handling of EPROBE_DEFER in of_get_mac_address [Was: Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage] Petr Štetiar @ 2019-05-01 13:54 ` Andrew Lunn 0 siblings, 0 replies; 19+ messages in thread From: Andrew Lunn @ 2019-05-01 13:54 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Nicolas Ferre, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel On Tue, Apr 30, 2019 at 04:13:35PM +0200, Petr Štetiar wrote: > Andrew Lunn <andrew@lunn.ch> [2019-04-29 15:02:48]: > > Hi Andrew, > > > > My understanding of -PROBE_DEFER is, that it needs to be propagated back from > > > the driver's probe callback/hook to the upper device/driver subsystem in order > > > to be moved to the list of pending drivers and considered for probe later > > > again. This is not going to happen in any of the current drivers, thus it will > > > probably still always result in random MAC address in case of -EPROBE_DEFER > > > error from the nvmem subsystem. > > > > All current drivers which don't look in NVMEM don't expect > > EPROBE_DEFER. > > once there's NVMEM wired in of_get_mac_address, one can simply use it, nothing > is going to stop the potential user of doing so and if EPROBE_DEFER isn't > propagated from the driver back to the upper device driver subsytem, it's > probably going to end with random MAC address in some (very rare?) cases. Hi Petr There is no simple answer here. If we add EPROBE_DEFER support to all the current drivers using of_get_mac_address(), we are likely to break something. Regressions are bad. If somebody does add NVMEM properties to a device which does not currently have them, and it fails, that it just bad testing, not a regressions. So i would keep it KISS. Allow of_get_mac_address() to return an error, but don't modify current drivers to look for it. Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage 2019-04-28 12:53 [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address Petr Štetiar ` (2 preceding siblings ...) 2019-04-28 12:53 ` [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage Petr Štetiar @ 2019-04-28 12:53 ` Petr Štetiar 2019-04-28 16:58 ` Andrew Lunn 3 siblings, 1 reply; 19+ messages in thread From: Petr Štetiar @ 2019-04-28 12:53 UTC (permalink / raw) To: netdev, devicetree, linux-kernel, Grygorii Strashko, David S. Miller Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, Petr Štetiar, linux-omap of_get_mac_address now uses NVMEM under the hood, so it's not necessary to call it manually anymore. Signed-off-by: Petr Štetiar <ynezz@true.cz> --- drivers/net/ethernet/ti/davinci_emac.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c index 57450b1..c1a5526 100644 --- a/drivers/net/ethernet/ti/davinci_emac.c +++ b/drivers/net/ethernet/ti/davinci_emac.c @@ -1912,15 +1912,11 @@ static int davinci_emac_probe(struct platform_device *pdev) ether_addr_copy(ndev->dev_addr, priv->mac_addr); if (!is_valid_ether_addr(priv->mac_addr)) { - /* Try nvmem if MAC wasn't passed over pdata or DT. */ - rc = nvmem_get_mac_address(&pdev->dev, priv->mac_addr); - if (rc) { - /* Use random MAC if still none obtained. */ - eth_hw_addr_random(ndev); - memcpy(priv->mac_addr, ndev->dev_addr, ndev->addr_len); - dev_warn(&pdev->dev, "using random MAC addr: %pM\n", - priv->mac_addr); - } + /* Use random MAC if still none obtained. */ + eth_hw_addr_random(ndev); + memcpy(priv->mac_addr, ndev->dev_addr, ndev->addr_len); + dev_warn(&pdev->dev, "using random MAC addr: %pM\n", + priv->mac_addr); } ndev->netdev_ops = &emac_netdev_ops; -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage 2019-04-28 12:53 ` [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage Petr Štetiar @ 2019-04-28 16:58 ` Andrew Lunn 0 siblings, 0 replies; 19+ messages in thread From: Andrew Lunn @ 2019-04-28 16:58 UTC (permalink / raw) To: Petr Štetiar Cc: netdev, devicetree, linux-kernel, Grygorii Strashko, David S. Miller, Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand, Srinivas Kandagatla, Maxime Ripard, Alban Bedel, linux-omap On Sun, Apr 28, 2019 at 02:53:22PM +0200, Petr Štetiar wrote: > of_get_mac_address now uses NVMEM under the hood, so it's not necessary > to call it manually anymore. > > Signed-off-by: Petr Štetiar <ynezz@true.cz> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-05-08 9:02 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-28 12:53 [PATCH v2 0/4] of_net: Add NVMEM support to of_get_mac_address Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 1/4] " Petr Štetiar 2019-05-01 20:19 ` Rob Herring 2019-05-02 9:05 ` Petr Štetiar 2019-05-07 16:06 ` Rob Herring 2019-05-08 9:02 ` Petr Štetiar 2019-04-28 12:53 ` [PATCH v2 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour Petr Štetiar 2019-04-28 16:53 ` Andrew Lunn 2019-05-01 20:22 ` Rob Herring 2019-04-28 12:53 ` [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage Petr Štetiar 2019-04-28 16:56 ` Andrew Lunn 2019-04-28 21:08 ` Petr Štetiar 2019-04-28 21:36 ` Andrew Lunn 2019-04-29 7:55 ` Petr Štetiar 2019-04-29 13:02 ` Andrew Lunn 2019-04-30 14:13 ` Handling of EPROBE_DEFER in of_get_mac_address [Was: Re: [PATCH v2 3/4] net: macb: Drop nvmem_get_mac_address usage] Petr Štetiar 2019-05-01 13:54 ` Andrew Lunn 2019-04-28 12:53 ` [PATCH v2 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage Petr Štetiar 2019-04-28 16:58 ` Andrew Lunn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).