2019-04-26 23:10:21

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH 0/4] of_net: Add NVMEM support to of_get_mac_address

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>;
};
};
};
};
};
};

&eth0 {
nvmem-cells = <&eth0_addr>;
nvmem-cell-names = "mac-address";
};

&eth1 {
nvmem-cells = <&eth1_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 | 3 ++
Documentation/devicetree/bindings/net/arc_emac.txt | 4 ++
.../devicetree/bindings/net/brcm,amac.txt | 3 ++
.../devicetree/bindings/net/brcm,bcmgenet.txt | 5 +++
.../devicetree/bindings/net/brcm,systemport.txt | 4 ++
.../devicetree/bindings/net/cavium-mix.txt | 7 +++-
.../devicetree/bindings/net/cavium-pip.txt | 5 +++
Documentation/devicetree/bindings/net/cpsw.txt | 3 ++
.../devicetree/bindings/net/davicom-dm9000.txt | 4 ++
.../devicetree/bindings/net/davinci_emac.txt | 2 +
Documentation/devicetree/bindings/net/dsa/dsa.txt | 7 ++--
.../devicetree/bindings/net/emac_rockchip.txt | 4 ++
Documentation/devicetree/bindings/net/ethernet.txt | 2 +
.../devicetree/bindings/net/ezchip_enet.txt | 6 +++
Documentation/devicetree/bindings/net/fsl-fec.txt | 4 ++
.../devicetree/bindings/net/fsl-tsec-phy.txt | 4 ++
.../devicetree/bindings/net/hisilicon-femac.txt | 3 ++
.../bindings/net/hisilicon-hix5hd2-gmac.txt | 3 ++
.../devicetree/bindings/net/keystone-netcp.txt | 9 ++--
Documentation/devicetree/bindings/net/macb.txt | 8 ++--
.../devicetree/bindings/net/marvell-orion-net.txt | 3 ++
.../devicetree/bindings/net/marvell-pxa168.txt | 3 ++
.../devicetree/bindings/net/microchip,enc28j60.txt | 3 ++
.../devicetree/bindings/net/microchip,lan78xx.txt | 2 +
.../devicetree/bindings/net/opencores-ethoc.txt | 4 ++
.../devicetree/bindings/net/oxnas-dwmac.txt | 6 +++
.../devicetree/bindings/net/qca,qca7000.txt | 3 ++
.../devicetree/bindings/net/samsung-sxgbe.txt | 3 ++
.../bindings/net/snps,dwc-qos-ethernet.txt | 2 +
.../devicetree/bindings/net/socfpga-dwmac.txt | 4 ++
.../bindings/net/socionext,uniphier-ave4.txt | 3 ++
.../devicetree/bindings/net/socionext-netsec.txt | 2 +
Documentation/devicetree/bindings/net/stmmac.txt | 4 ++
.../bindings/net/wireless/mediatek,mt76.txt | 2 +
.../devicetree/bindings/net/wireless/qca,ath9k.txt | 2 +
.../devicetree/bindings/soc/fsl/cpm_qe/qe/ucc.txt | 3 ++
drivers/net/ethernet/cadence/macb_main.c | 12 ++----
drivers/net/ethernet/ti/davinci_emac.c | 14 +++----
drivers/of/of_net.c | 48 +++++++++++++++++++---
39 files changed, 179 insertions(+), 34 deletions(-)

--
1.9.1


2019-04-26 23:09:30

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH 1/4] of_net: Add NVMEM support to of_get_mac_address

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 <[email protected]>
Signed-off-by: Felix Fietkau <[email protected]>
Signed-off-by: John Crispin <[email protected]>
Signed-off-by: Petr Štetiar <[email protected]>
---
drivers/of/of_net.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index d820f3e..a3d6773 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.
+ * Search the device tree for the best MAC address to use. Check NVME first as
+ * it should contain the proper MAC address, then 'mac-address' is checked
+ * next, 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.
*
* 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
@@ -69,6 +103,10 @@ const void *of_get_mac_address(struct device_node *np)
{
const void *addr;

+ addr = of_get_mac_addr_nvmem(np);
+ if (addr)
+ return addr;
+
addr = of_get_mac_addr(np, "mac-address");
if (addr)
return addr;
--
1.9.1

2019-04-26 23:09:30

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH 3/4] net: macb: Drop nvmem_get_mac_address usage

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 <[email protected]>
---
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

2019-04-26 23:11:00

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage

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 <[email protected]>
---
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

2019-04-27 16:31:05

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 1/4] of_net: Add NVMEM support to of_get_mac_address

> /**
> - * 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.
> + * Search the device tree for the best MAC address to use. Check NVME first as
> + * it should contain the proper MAC address, then 'mac-address' is checked
> + * next, 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.

Hi Petr

I'm not sure this is the correct order. I would actually put NVMEM
after mac-address and local-mac-address. These are well established
and used. We don't want to break existing boards with a new
property. By putting NVMEM later, in order to make it used, any
existing mac-address and local-mac-address need to be removed, making
the developers actually think about what they are doing.

Andrew

2019-04-27 16:46:30

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/4] net: macb: Drop nvmem_get_mac_address usage

On Sat, Apr 27, 2019 at 01:06:41AM +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 <[email protected]>
> ---
> 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);
> - }
> - }

Here we have an ordering problem. The current code looks for NVMEM
after local-mac-address and mac-address. You change will make it look
for NVMEM first.

Andrew

2019-04-27 16:57:15

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 4/4] net: davinci_emac: Drop nvmem_get_mac_address usage

On Sat, Apr 27, 2019 at 01:06:42AM +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 <[email protected]>
> ---
> 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);

Hi Petr

There is the same ordering issue here as with the previous patch.

Andrew