[PATCH net-next v3 0/3] QorIQ DPAA: Use random MAC address when none is given

[PATCH net-next v3 0/3] QorIQ DPAA: Use random MAC address when none is given

[Madalin Bucur]

From: Sascha Hauer <s.hauer@pengutronix.de>

Use random MAC addresses when they are not provided in the device tree.
Tested on LS1046ARDB.

Changes in v3:
 addressed all MAC types, removed some redundant code in dtsec in
 the process

Madalin Bucur (3):
  fsl/fman: reuse set_mac_address() in dtsec init()
  fsl/fman: tolerate missing MAC address in device tree
  dpaa_eth: Use random MAC address when none is given

 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c   | 16 +++++++-
 drivers/net/ethernet/freescale/fman/fman_dtsec.c | 50 ++++++++++--------------
 drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++---
 drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++---
 drivers/net/ethernet/freescale/fman/mac.c        | 13 +++---
 5 files changed, 49 insertions(+), 50 deletions(-)

-- 
2.1.0

[PATCH net-next v3 1/3] fsl/fman: reuse set_mac_address() in dtsec init()

[Madalin Bucur]

Reuse the set_mac_address() in the init() function.

Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman_dtsec.c | 44 ++++++++++--------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index 1ca543ac8f2c..f7aec507787f 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
@@ -366,13 +366,26 @@ static void set_dflts(struct dtsec_cfg *cfg)
 	cfg->maximum_frame = DEFAULT_MAXIMUM_FRAME;
 }
 
+static void set_mac_address(struct dtsec_regs __iomem *regs, u8 *adr)
+{
+	u32 tmp;
+
+	tmp = (u32)((adr[5] << 24) |
+		    (adr[4] << 16) | (adr[3] << 8) | adr[2]);
+	iowrite32be(tmp, &regs->macstnaddr1);
+
+	tmp = (u32)((adr[1] << 24) | (adr[0] << 16));
+	iowrite32be(tmp, &regs->macstnaddr2);
+}
+
 static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
-		phy_interface_t iface, u16 iface_speed, u8 *macaddr,
+		phy_interface_t iface, u16 iface_speed, u64 addr,
 		u32 exception_mask, u8 tbi_addr)
 {
 	bool is_rgmii, is_sgmii, is_qsgmii;
-	int i;
+	enet_addr_t eth_addr;
 	u32 tmp;
+	int i;
 
 	/* Soft reset */
 	iowrite32be(MACCFG1_SOFT_RESET, &regs->maccfg1);
@@ -501,12 +514,8 @@ static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
 
 	iowrite32be(0xffffffff, &regs->ievent);
 
-	tmp = (u32)((macaddr[5] << 24) |
-		    (macaddr[4] << 16) | (macaddr[3] << 8) | macaddr[2]);
-	iowrite32be(tmp, &regs->macstnaddr1);
-
-	tmp = (u32)((macaddr[1] << 24) | (macaddr[0] << 16));
-	iowrite32be(tmp, &regs->macstnaddr2);
+	MAKE_ENET_ADDR_FROM_UINT64(addr, eth_addr);
+	set_mac_address(regs, (u8 *)eth_addr);
 
 	/* HASH */
 	for (i = 0; i < NUM_OF_HASH_REGS; i++) {
@@ -519,18 +528,6 @@ static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
 	return 0;
 }
 
-static void set_mac_address(struct dtsec_regs __iomem *regs, u8 *adr)
-{
-	u32 tmp;
-
-	tmp = (u32)((adr[5] << 24) |
-		    (adr[4] << 16) | (adr[3] << 8) | adr[2]);
-	iowrite32be(tmp, &regs->macstnaddr1);
-
-	tmp = (u32)((adr[1] << 24) | (adr[0] << 16));
-	iowrite32be(tmp, &regs->macstnaddr2);
-}
-
 static void set_bucket(struct dtsec_regs __iomem *regs, int bucket,
 		       bool enable)
 {
@@ -1391,9 +1388,8 @@ int dtsec_init(struct fman_mac *dtsec)
 {
 	struct dtsec_regs __iomem *regs = dtsec->regs;
 	struct dtsec_cfg *dtsec_drv_param;
-	int err;
 	u16 max_frm_ln;
-	enet_addr_t eth_addr;
+	int err;
 
 	if (is_init_done(dtsec->dtsec_drv_param))
 		return -EINVAL;
@@ -1410,10 +1406,8 @@ int dtsec_init(struct fman_mac *dtsec)
 
 	dtsec_drv_param = dtsec->dtsec_drv_param;
 
-	MAKE_ENET_ADDR_FROM_UINT64(dtsec->addr, eth_addr);
-
 	err = init(dtsec->regs, dtsec_drv_param, dtsec->phy_if,
-		   dtsec->max_speed, (u8 *)eth_addr, dtsec->exceptions,
+		   dtsec->max_speed, dtsec->addr, dtsec->exceptions,
 		   dtsec->tbiphy->mdio.addr);
 	if (err) {
 		free_init_resources(dtsec);
-- 
2.1.0

[PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Madalin Bucur]

Allow the initialization of the MAC to be performed even if the
device tree does not provide a valid MAC address. Later a random
MAC address should be assigned by the Ethernet driver.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
 drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
 drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
 drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++-------
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index f7aec507787f..004c266802a8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
@@ -514,8 +514,10 @@ static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
 
 	iowrite32be(0xffffffff, &regs->ievent);
 
-	MAKE_ENET_ADDR_FROM_UINT64(addr, eth_addr);
-	set_mac_address(regs, (u8 *)eth_addr);
+	if (addr) {
+		MAKE_ENET_ADDR_FROM_UINT64(addr, eth_addr);
+		set_mac_address(regs, (u8 *)eth_addr);
+	}
 
 	/* HASH */
 	for (i = 0; i < NUM_OF_HASH_REGS; i++) {
@@ -553,10 +555,6 @@ static int check_init_parameters(struct fman_mac *dtsec)
 		pr_err("1G MAC driver supports 1G or lower speeds\n");
 		return -EINVAL;
 	}
-	if (dtsec->addr == 0) {
-		pr_err("Ethernet MAC Must have a valid MAC Address\n");
-		return -EINVAL;
-	}
 	if ((dtsec->dtsec_drv_param)->rx_prepend >
 	    MAX_PACKET_ALIGNMENT) {
 		pr_err("packetAlignmentPadding can't be > than %d\n",
diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index e1901874c19f..f2b2bfcbb529 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -596,10 +596,6 @@ static void setup_sgmii_internal_phy_base_x(struct fman_mac *memac)
 
 static int check_init_parameters(struct fman_mac *memac)
 {
-	if (memac->addr == 0) {
-		pr_err("Ethernet MAC must have a valid MAC address\n");
-		return -EINVAL;
-	}
 	if (!memac->exception_cb) {
 		pr_err("Uninitialized exception handler\n");
 		return -EINVAL;
@@ -1057,8 +1053,10 @@ int memac_init(struct fman_mac *memac)
 	}
 
 	/* MAC Address */
-	MAKE_ENET_ADDR_FROM_UINT64(memac->addr, eth_addr);
-	add_addr_in_paddr(memac->regs, (u8 *)eth_addr, 0);
+	if (memac->addr != 0) {
+		MAKE_ENET_ADDR_FROM_UINT64(memac->addr, eth_addr);
+		add_addr_in_paddr(memac->regs, (u8 *)eth_addr, 0);
+	}
 
 	fixed_link = memac_drv_param->fixed_link;
 
diff --git a/drivers/net/ethernet/freescale/fman/fman_tgec.c b/drivers/net/ethernet/freescale/fman/fman_tgec.c
index f75b9c11b2d2..8c7eb878d5b4 100644
--- a/drivers/net/ethernet/freescale/fman/fman_tgec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_tgec.c
@@ -273,10 +273,6 @@ static int check_init_parameters(struct fman_mac *tgec)
 		pr_err("10G MAC driver only support 10G speed\n");
 		return -EINVAL;
 	}
-	if (tgec->addr == 0) {
-		pr_err("Ethernet 10G MAC Must have valid MAC Address\n");
-		return -EINVAL;
-	}
 	if (!tgec->exception_cb) {
 		pr_err("uninitialized exception_cb\n");
 		return -EINVAL;
@@ -706,8 +702,10 @@ int tgec_init(struct fman_mac *tgec)
 
 	cfg = tgec->cfg;
 
-	MAKE_ENET_ADDR_FROM_UINT64(tgec->addr, eth_addr);
-	set_mac_address(tgec->regs, (u8 *)eth_addr);
+	if (tgec->addr) {
+		MAKE_ENET_ADDR_FROM_UINT64(tgec->addr, eth_addr);
+		set_mac_address(tgec->regs, (u8 *)eth_addr);
+	}
 
 	/* interrupts */
 	/* FM_10G_REM_N_LCL_FLT_EX_10GMAC_ERRATA_SW005 Errata workaround */
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 55f2122c3217..43427c5b9396 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -724,12 +724,10 @@ static int mac_probe(struct platform_device *_of_dev)
 
 	/* Get the MAC address */
 	mac_addr = of_get_mac_address(mac_node);
-	if (IS_ERR(mac_addr)) {
-		dev_err(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
-		err = -EINVAL;
-		goto _return_of_get_parent;
-	}
-	ether_addr_copy(mac_dev->addr, mac_addr);
+	if (IS_ERR(mac_addr))
+		dev_warn(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
+	else
+		ether_addr_copy(mac_dev->addr, mac_addr);
 
 	/* Get the port handles */
 	nph = of_count_phandle_with_args(mac_node, "fsl,fman-ports", NULL);
@@ -855,7 +853,8 @@ static int mac_probe(struct platform_device *_of_dev)
 	if (err < 0)
 		dev_err(dev, "fman_set_mac_active_pause() = %d\n", err);
 
-	dev_info(dev, "FMan MAC address: %pM\n", mac_dev->addr);
+	if (!IS_ERR(mac_addr))
+		dev_info(dev, "FMan MAC address: %pM\n", mac_dev->addr);
 
 	priv->eth_dev = dpaa_eth_add_device(fman_id, mac_dev);
 	if (IS_ERR(priv->eth_dev)) {
-- 
2.1.0

[PATCH net-next v3 3/3] dpaa_eth: Use random MAC address when none is given

[Madalin Bucur]

If there is no valid MAC address in the device tree, use a random
MAC address.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index e3ac9ec54c7c..190e4478128a 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -259,8 +259,20 @@ static int dpaa_netdev_init(struct net_device *net_dev,
 	net_dev->features |= net_dev->hw_features;
 	net_dev->vlan_features = net_dev->features;
 
-	memcpy(net_dev->perm_addr, mac_addr, net_dev->addr_len);
-	memcpy(net_dev->dev_addr, mac_addr, net_dev->addr_len);
+	if (is_valid_ether_addr(mac_addr)) {
+		memcpy(net_dev->perm_addr, mac_addr, net_dev->addr_len);
+		memcpy(net_dev->dev_addr, mac_addr, net_dev->addr_len);
+	} else {
+		eth_hw_addr_random(net_dev);
+		err = priv->mac_dev->change_addr(priv->mac_dev->fman_mac,
+			(enet_addr_t *)net_dev->dev_addr);
+		if (err) {
+			dev_err(dev, "Failed to set random MAC address\n");
+			return -EINVAL;
+		}
+		dev_info(dev, "Using random MAC address: %pM\n",
+			 net_dev->dev_addr);
+	}
 
 	net_dev->ethtool_ops = &dpaa_ethtool_ops;
 
-- 
2.1.0

Re: [PATCH net-next v3 0/3] QorIQ DPAA: Use random MAC address when none is given

[David Miller]

From: Madalin Bucur <madalin.bucur@oss.nxp.com>
Date: Thu,  5 Mar 2020 19:08:55 +0200

> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Use random MAC addresses when they are not provided in the device tree.
> Tested on LS1046ARDB.
> 
> Changes in v3:
>  addressed all MAC types, removed some redundant code in dtsec in
>  the process

Series applied, thanks.

Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Sascha Hauer]

On Thu, Mar 05, 2020 at 07:08:57PM +0200, Madalin Bucur wrote:
> Allow the initialization of the MAC to be performed even if the
> device tree does not provide a valid MAC address. Later a random
> MAC address should be assigned by the Ethernet driver.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> ---
>  drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
>  drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
>  drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
>  drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++-------
>  4 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> index f7aec507787f..004c266802a8 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> @@ -514,8 +514,10 @@ static int init(struct dtsec_regs __iomem *regs, struct dtsec_cfg *cfg,
>  
>  	iowrite32be(0xffffffff, &regs->ievent);
>  
> -	MAKE_ENET_ADDR_FROM_UINT64(addr, eth_addr);
> -	set_mac_address(regs, (u8 *)eth_addr);
> +	if (addr) {
> +		MAKE_ENET_ADDR_FROM_UINT64(addr, eth_addr);
> +		set_mac_address(regs, (u8 *)eth_addr);
> +	}
>  
>  	/* HASH */
>  	for (i = 0; i < NUM_OF_HASH_REGS; i++) {
> @@ -553,10 +555,6 @@ static int check_init_parameters(struct fman_mac *dtsec)
>  		pr_err("1G MAC driver supports 1G or lower speeds\n");
>  		return -EINVAL;
>  	}
> -	if (dtsec->addr == 0) {
> -		pr_err("Ethernet MAC Must have a valid MAC Address\n");
> -		return -EINVAL;
> -	}
>  	if ((dtsec->dtsec_drv_param)->rx_prepend >
>  	    MAX_PACKET_ALIGNMENT) {
>  		pr_err("packetAlignmentPadding can't be > than %d\n",
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index e1901874c19f..f2b2bfcbb529 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -596,10 +596,6 @@ static void setup_sgmii_internal_phy_base_x(struct fman_mac *memac)
>  
>  static int check_init_parameters(struct fman_mac *memac)
>  {
> -	if (memac->addr == 0) {
> -		pr_err("Ethernet MAC must have a valid MAC address\n");
> -		return -EINVAL;
> -	}
>  	if (!memac->exception_cb) {
>  		pr_err("Uninitialized exception handler\n");
>  		return -EINVAL;
> @@ -1057,8 +1053,10 @@ int memac_init(struct fman_mac *memac)
>  	}
>  
>  	/* MAC Address */
> -	MAKE_ENET_ADDR_FROM_UINT64(memac->addr, eth_addr);
> -	add_addr_in_paddr(memac->regs, (u8 *)eth_addr, 0);
> +	if (memac->addr != 0) {
> +		MAKE_ENET_ADDR_FROM_UINT64(memac->addr, eth_addr);
> +		add_addr_in_paddr(memac->regs, (u8 *)eth_addr, 0);
> +	}
>  
>  	fixed_link = memac_drv_param->fixed_link;
>  
> diff --git a/drivers/net/ethernet/freescale/fman/fman_tgec.c b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> index f75b9c11b2d2..8c7eb878d5b4 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_tgec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_tgec.c
> @@ -273,10 +273,6 @@ static int check_init_parameters(struct fman_mac *tgec)
>  		pr_err("10G MAC driver only support 10G speed\n");
>  		return -EINVAL;
>  	}
> -	if (tgec->addr == 0) {
> -		pr_err("Ethernet 10G MAC Must have valid MAC Address\n");
> -		return -EINVAL;
> -	}
>  	if (!tgec->exception_cb) {
>  		pr_err("uninitialized exception_cb\n");
>  		return -EINVAL;
> @@ -706,8 +702,10 @@ int tgec_init(struct fman_mac *tgec)
>  
>  	cfg = tgec->cfg;
>  
> -	MAKE_ENET_ADDR_FROM_UINT64(tgec->addr, eth_addr);
> -	set_mac_address(tgec->regs, (u8 *)eth_addr);
> +	if (tgec->addr) {
> +		MAKE_ENET_ADDR_FROM_UINT64(tgec->addr, eth_addr);
> +		set_mac_address(tgec->regs, (u8 *)eth_addr);
> +	}
>  
>  	/* interrupts */
>  	/* FM_10G_REM_N_LCL_FLT_EX_10GMAC_ERRATA_SW005 Errata workaround */
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index 55f2122c3217..43427c5b9396 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -724,12 +724,10 @@ static int mac_probe(struct platform_device *_of_dev)
>  
>  	/* Get the MAC address */
>  	mac_addr = of_get_mac_address(mac_node);
> -	if (IS_ERR(mac_addr)) {
> -		dev_err(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
> -		err = -EINVAL;
> -		goto _return_of_get_parent;
> -	}
> -	ether_addr_copy(mac_dev->addr, mac_addr);
> +	if (IS_ERR(mac_addr))
> +		dev_warn(dev, "of_get_mac_address(%pOF) failed\n", mac_node);

Why this warning? There's nothing wrong with not providing the MAC in
the device tree.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, March 9, 2020 8:47 AM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> address in device tree
> 
> On Thu, Mar 05, 2020 at 07:08:57PM +0200, Madalin Bucur wrote:
> > Allow the initialization of the MAC to be performed even if the
> > device tree does not provide a valid MAC address. Later a random
> > MAC address should be assigned by the Ethernet driver.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
> >  drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
> >  drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
> >  drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++-------
> >  4 files changed, 18 insertions(+), 25 deletions(-)
> >
<snip>
> >  	/* Get the MAC address */
> >  	mac_addr = of_get_mac_address(mac_node);
> > -	if (IS_ERR(mac_addr)) {
> > -		dev_err(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
> > -		err = -EINVAL;
> > -		goto _return_of_get_parent;
> > -	}
> > -	ether_addr_copy(mac_dev->addr, mac_addr);
> > +	if (IS_ERR(mac_addr))
> > +		dev_warn(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
> 
> Why this warning? There's nothing wrong with not providing the MAC in
> the device tree.
> 
> Sascha

Actually, there is, most likely it's the result of misconfiguration so one
must be made aware of it.

Madalin

Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Sascha Hauer]

On Mon, Mar 09, 2020 at 10:17:36AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Monday, March 9, 2020 8:47 AM
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Cc: davem@davemloft.net; netdev@vger.kernel.org
> > Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> > address in device tree
> > 
> > On Thu, Mar 05, 2020 at 07:08:57PM +0200, Madalin Bucur wrote:
> > > Allow the initialization of the MAC to be performed even if the
> > > device tree does not provide a valid MAC address. Later a random
> > > MAC address should be assigned by the Ethernet driver.
> > >
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > > ---
> > >  drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
> > >  drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
> > >  drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
> > >  drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++-------
> > >  4 files changed, 18 insertions(+), 25 deletions(-)
> > >
> <snip>
> > >  	/* Get the MAC address */
> > >  	mac_addr = of_get_mac_address(mac_node);
> > > -	if (IS_ERR(mac_addr)) {
> > > -		dev_err(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
> > > -		err = -EINVAL;
> > > -		goto _return_of_get_parent;
> > > -	}
> > > -	ether_addr_copy(mac_dev->addr, mac_addr);
> > > +	if (IS_ERR(mac_addr))
> > > +		dev_warn(dev, "of_get_mac_address(%pOF) failed\n", mac_node);
> > 
> > Why this warning? There's nothing wrong with not providing the MAC in
> > the device tree.
> > 
> > Sascha
> 
> Actually, there is, most likely it's the result of misconfiguration so one
> must be made aware of it.

In my case it's not, that's why I wanted to allow random MACs in the
first place ;)

On our hardware the MAC addresses are stored in some flash in a special
format. There's no need to port parsing of that format into the
bootloader, the existing userspace code does that well and sets the
desired MAC addresses, but only if the devices do not fail during probe
due to the lack of valid MAC addresses.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, March 9, 2020 3:10 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> address in device tree
> 
> On Mon, Mar 09, 2020 at 10:17:36AM +0000, Madalin Bucur (OSS) wrote:
> > > -----Original Message-----
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > Sent: Monday, March 9, 2020 8:47 AM
> > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > Cc: davem@davemloft.net; netdev@vger.kernel.org
> > > Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> > > address in device tree
> > >
> > > On Thu, Mar 05, 2020 at 07:08:57PM +0200, Madalin Bucur wrote:
> > > > Allow the initialization of the MAC to be performed even if the
> > > > device tree does not provide a valid MAC address. Later a random
> > > > MAC address should be assigned by the Ethernet driver.
> > > >
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > > > ---
> > > >  drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
> > > >  drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
> > > >  drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
> > > >  drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++------
> -
> > > >  4 files changed, 18 insertions(+), 25 deletions(-)
> > > >
> > <snip>
> > > >  	/* Get the MAC address */
> > > >  	mac_addr = of_get_mac_address(mac_node);
> > > > -	if (IS_ERR(mac_addr)) {
> > > > -		dev_err(dev, "of_get_mac_address(%pOF) failed\n",
> mac_node);
> > > > -		err = -EINVAL;
> > > > -		goto _return_of_get_parent;
> > > > -	}
> > > > -	ether_addr_copy(mac_dev->addr, mac_addr);
> > > > +	if (IS_ERR(mac_addr))
> > > > +		dev_warn(dev, "of_get_mac_address(%pOF) failed\n",
> mac_node);
> > >
> > > Why this warning? There's nothing wrong with not providing the MAC in
> > > the device tree.
> > >
> > > Sascha
> >
> > Actually, there is, most likely it's the result of misconfiguration so
> one
> > must be made aware of it.
> 
> In my case it's not, that's why I wanted to allow random MACs in the
> first place ;)
> 
> On our hardware the MAC addresses are stored in some flash in a special
> format. There's no need to port parsing of that format into the
> bootloader, the existing userspace code does that well and sets the
> desired MAC addresses, but only if the devices do not fail during probe
> due to the lack of valid MAC addresses.
> 
> Sascha

What MAC address does the bootloader use then?

Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC address in device tree

[Sascha Hauer]

On Mon, Mar 09, 2020 at 01:21:16PM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Monday, March 9, 2020 3:10 PM
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Cc: davem@davemloft.net; netdev@vger.kernel.org
> > Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> > address in device tree
> > 
> > On Mon, Mar 09, 2020 at 10:17:36AM +0000, Madalin Bucur (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Sent: Monday, March 9, 2020 8:47 AM
> > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > > Cc: davem@davemloft.net; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH net-next v3 2/3] fsl/fman: tolerate missing MAC
> > > > address in device tree
> > > >
> > > > On Thu, Mar 05, 2020 at 07:08:57PM +0200, Madalin Bucur wrote:
> > > > > Allow the initialization of the MAC to be performed even if the
> > > > > device tree does not provide a valid MAC address. Later a random
> > > > > MAC address should be assigned by the Ethernet driver.
> > > > >
> > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > > > > ---
> > > > >  drivers/net/ethernet/freescale/fman/fman_dtsec.c | 10 ++++------
> > > > >  drivers/net/ethernet/freescale/fman/fman_memac.c | 10 ++++------
> > > > >  drivers/net/ethernet/freescale/fman/fman_tgec.c  | 10 ++++------
> > > > >  drivers/net/ethernet/freescale/fman/mac.c        | 13 ++++++------
> > -
> > > > >  4 files changed, 18 insertions(+), 25 deletions(-)
> > > > >
> > > <snip>
> > > > >  	/* Get the MAC address */
> > > > >  	mac_addr = of_get_mac_address(mac_node);
> > > > > -	if (IS_ERR(mac_addr)) {
> > > > > -		dev_err(dev, "of_get_mac_address(%pOF) failed\n",
> > mac_node);
> > > > > -		err = -EINVAL;
> > > > > -		goto _return_of_get_parent;
> > > > > -	}
> > > > > -	ether_addr_copy(mac_dev->addr, mac_addr);
> > > > > +	if (IS_ERR(mac_addr))
> > > > > +		dev_warn(dev, "of_get_mac_address(%pOF) failed\n",
> > mac_node);
> > > >
> > > > Why this warning? There's nothing wrong with not providing the MAC in
> > > > the device tree.
> > > >
> > > > Sascha
> > >
> > > Actually, there is, most likely it's the result of misconfiguration so
> > one
> > > must be made aware of it.
> > 
> > In my case it's not, that's why I wanted to allow random MACs in the
> > first place ;)
> > 
> > On our hardware the MAC addresses are stored in some flash in a special
> > format. There's no need to port parsing of that format into the
> > bootloader, the existing userspace code does that well and sets the
> > desired MAC addresses, but only if the devices do not fail during probe
> > due to the lack of valid MAC addresses.
> > 
> > Sascha
> 
> What MAC address does the bootloader use then?

None. The ethernet controllers are unused in the bootloader. They are
connected to the FPGA only anyway, which doesn't even have firmware
loaded in the bootloader. We have a USB Ethernet dongle for use in the
bootloader.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |