| Message ID | 20200305123414.5010-1-s.hauer@pengutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] fsl/fman: Use random MAC address when none is given | expand |
> -----Original Message----- > From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Thursday, March 5, 2020 2:34 PM > To: netdev@vger.kernel.org > Cc: Madalin Bucur <madalin.bucur@nxp.com>; Sascha Hauer > <s.hauer@pengutronix.de> > Subject: [PATCH v2] fsl/fman: Use random MAC address when none is given > > There's no need to fail probing when no MAC address is given in the > device tree, just use a random MAC address. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > > Changes since v1: > - Remove printing of permanent MAC address > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++-- > drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- > drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- > 3 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > index fd93d542f497..fc117eab788d 100644 > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > @@ -233,8 +233,15 @@ 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)) { > + dev_info(dev, "FMan MAC address: %pM\n", 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); > + dev_info(dev, "Using random MAC address: %pM\n", > + net_dev->dev_addr); > + } To make the HW MAC aware of the new random address you set in the dpaa_eth, you also need to call mac_dev->change_addr() after eth_hw_addr_random(), like it's done in dpaa_set_mac_address(): err = mac_dev->change_addr(mac_dev->fman_mac, (enet_addr_t *)net_dev->dev_addr); This will write the new MAC address in the MAC HW registers. > > net_dev->ethtool_ops = &dpaa_ethtool_ops; > > diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c > b/drivers/net/ethernet/freescale/fman/fman_memac.c > index e1901874c19f..9e2372688405 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; > diff --git a/drivers/net/ethernet/freescale/fman/mac.c > b/drivers/net/ethernet/freescale/fman/mac.c > index 55f2122c3217..9de76bc4ebde 100644 > --- a/drivers/net/ethernet/freescale/fman/mac.c > +++ b/drivers/net/ethernet/freescale/fman/mac.c > @@ -724,12 +724,8 @@ 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)) > + 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,8 +851,6 @@ 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); > - > priv->eth_dev = dpaa_eth_add_device(fman_id, mac_dev); > if (IS_ERR(priv->eth_dev)) { > dev_err(dev, "failed to add Ethernet platform device for MAC > %d\n", > -- > 2.25.1
On Thu, Mar 05, 2020 at 01:38:01PM +0000, Madalin Bucur wrote: > > -----Original Message----- > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Thursday, March 5, 2020 2:34 PM > > To: netdev@vger.kernel.org > > Cc: Madalin Bucur <madalin.bucur@nxp.com>; Sascha Hauer > > <s.hauer@pengutronix.de> > > Subject: [PATCH v2] fsl/fman: Use random MAC address when none is given > > > > There's no need to fail probing when no MAC address is given in the > > device tree, just use a random MAC address. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > > > Changes since v1: > > - Remove printing of permanent MAC address > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++-- > > drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- > > drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > index fd93d542f497..fc117eab788d 100644 > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > @@ -233,8 +233,15 @@ 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)) { > > + dev_info(dev, "FMan MAC address: %pM\n", 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); > > + dev_info(dev, "Using random MAC address: %pM\n", > > + net_dev->dev_addr); > > + } > > To make the HW MAC aware of the new random address you set in the dpaa_eth, > you also need to call mac_dev->change_addr() after eth_hw_addr_random(), like > it's done in dpaa_set_mac_address(): > > err = mac_dev->change_addr(mac_dev->fman_mac, > (enet_addr_t *)net_dev->dev_addr); > > This will write the new MAC address in the MAC HW registers. Ok, I see. So when I call mac_dev->change_addr() here in dpaa_netdev_init() it means I can remove all the code setting the MAC address in hardware before this point, right? Sascha
> -----Original Message----- > From: Sascha Hauer <s.hauer@pengutronix.de> > Sent: Thursday, March 5, 2020 4:48 PM > To: Madalin Bucur <madalin.bucur@nxp.com> > Cc: netdev@vger.kernel.org > Subject: Re: [PATCH v2] fsl/fman: Use random MAC address when none is > given > > On Thu, Mar 05, 2020 at 01:38:01PM +0000, Madalin Bucur wrote: > > > -----Original Message----- > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Thursday, March 5, 2020 2:34 PM > > > To: netdev@vger.kernel.org > > > Cc: Madalin Bucur <madalin.bucur@nxp.com>; Sascha Hauer > > > <s.hauer@pengutronix.de> > > > Subject: [PATCH v2] fsl/fman: Use random MAC address when none is > given > > > > > > There's no need to fail probing when no MAC address is given in the > > > device tree, just use a random MAC address. > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > > > > Changes since v1: > > > - Remove printing of permanent MAC address > > > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++-- > > > drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- > > > drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > index fd93d542f497..fc117eab788d 100644 > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > @@ -233,8 +233,15 @@ 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)) { > > > + dev_info(dev, "FMan MAC address: %pM\n", 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); > > > + dev_info(dev, "Using random MAC address: %pM\n", > > > + net_dev->dev_addr); > > > + } > > > > To make the HW MAC aware of the new random address you set in the > dpaa_eth, > > you also need to call mac_dev->change_addr() after > eth_hw_addr_random(), like > > it's done in dpaa_set_mac_address(): > > > > err = mac_dev->change_addr(mac_dev->fman_mac, > > (enet_addr_t *)net_dev->dev_addr); > > > > This will write the new MAC address in the MAC HW registers. > > Ok, I see. > > So when I call mac_dev->change_addr() here in dpaa_netdev_init() it > means I can remove all the code setting the MAC address in hardware > before this point, right? It's not an issue if you just write it here for the random address and leave the previous functionality in place. Do you have a board to validate your changes? > Sascha > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.peng > utronix.de%2F&data=02%7C01%7Cmadalin.bucur%40nxp.com%7C4b875830989945 > 006ae908d7c11439bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637190164 > 922597543&sdata=48VF83dAoB1m5B3I0jQIf0KwhO9au4gJVIieGqt1G8c%3D&re > served=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 > |
On Thu, Mar 05, 2020 at 03:03:48PM +0000, Madalin Bucur wrote: > > -----Original Message----- > > From: Sascha Hauer <s.hauer@pengutronix.de> > > Sent: Thursday, March 5, 2020 4:48 PM > > To: Madalin Bucur <madalin.bucur@nxp.com> > > Cc: netdev@vger.kernel.org > > Subject: Re: [PATCH v2] fsl/fman: Use random MAC address when none is > > given > > > > On Thu, Mar 05, 2020 at 01:38:01PM +0000, Madalin Bucur wrote: > > > > -----Original Message----- > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Sent: Thursday, March 5, 2020 2:34 PM > > > > To: netdev@vger.kernel.org > > > > Cc: Madalin Bucur <madalin.bucur@nxp.com>; Sascha Hauer > > > > <s.hauer@pengutronix.de> > > > > Subject: [PATCH v2] fsl/fman: Use random MAC address when none is > > given > > > > > > > > There's no need to fail probing when no MAC address is given in the > > > > device tree, just use a random MAC address. > > > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > --- > > > > > > > > Changes since v1: > > > > - Remove printing of permanent MAC address > > > > > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++-- > > > > drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- > > > > drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- > > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > index fd93d542f497..fc117eab788d 100644 > > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > @@ -233,8 +233,15 @@ 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)) { > > > > + dev_info(dev, "FMan MAC address: %pM\n", 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); > > > > + dev_info(dev, "Using random MAC address: %pM\n", > > > > + net_dev->dev_addr); > > > > + } > > > > > > To make the HW MAC aware of the new random address you set in the > > dpaa_eth, > > > you also need to call mac_dev->change_addr() after > > eth_hw_addr_random(), like > > > it's done in dpaa_set_mac_address(): > > > > > > err = mac_dev->change_addr(mac_dev->fman_mac, > > > (enet_addr_t *)net_dev->dev_addr); > > > > > > This will write the new MAC address in the MAC HW registers. > > > > Ok, I see. > > > > So when I call mac_dev->change_addr() here in dpaa_netdev_init() it > > means I can remove all the code setting the MAC address in hardware > > before this point, right? > > It's not an issue if you just write it here for the random address and leave > the previous functionality in place. No, it's not. It's only that the code becomes rather useless once we overwrite the MAC address here anyway. > Do you have a board to validate your changes? I have a LS1046a based board, yes. Sascha
> -----Original Message----- > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > Behalf Of Sascha Hauer > Sent: Thursday, March 5, 2020 5:36 PM > To: Madalin Bucur <madalin.bucur@nxp.com> > Cc: netdev@vger.kernel.org > Subject: Re: [PATCH v2] fsl/fman: Use random MAC address when none is > given > > On Thu, Mar 05, 2020 at 03:03:48PM +0000, Madalin Bucur wrote: > > > -----Original Message----- > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > Sent: Thursday, March 5, 2020 4:48 PM > > > To: Madalin Bucur <madalin.bucur@nxp.com> > > > Cc: netdev@vger.kernel.org > > > Subject: Re: [PATCH v2] fsl/fman: Use random MAC address when none is > > > given > > > > > > On Thu, Mar 05, 2020 at 01:38:01PM +0000, Madalin Bucur wrote: > > > > > -----Original Message----- > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > Sent: Thursday, March 5, 2020 2:34 PM > > > > > To: netdev@vger.kernel.org > > > > > Cc: Madalin Bucur <madalin.bucur@nxp.com>; Sascha Hauer > > > > > <s.hauer@pengutronix.de> > > > > > Subject: [PATCH v2] fsl/fman: Use random MAC address when none is > > > given > > > > > > > > > > There's no need to fail probing when no MAC address is given in > the > > > > > device tree, just use a random MAC address. > > > > > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > > --- > > > > > > > > > > Changes since v1: > > > > > - Remove printing of permanent MAC address > > > > > > > > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++- > - > > > > > drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- > > > > > drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- > > > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > > > > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > > index fd93d542f497..fc117eab788d 100644 > > > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > > > @@ -233,8 +233,15 @@ 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)) { > > > > > + dev_info(dev, "FMan MAC address: %pM\n", 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); > > > > > + dev_info(dev, "Using random MAC address: %pM\n", > > > > > + net_dev->dev_addr); > > > > > + } > > > > > > > > To make the HW MAC aware of the new random address you set in the > > > dpaa_eth, > > > > you also need to call mac_dev->change_addr() after > > > eth_hw_addr_random(), like > > > > it's done in dpaa_set_mac_address(): > > > > > > > > err = mac_dev->change_addr(mac_dev->fman_mac, > > > > (enet_addr_t *)net_dev- > >dev_addr); > > > > > > > > This will write the new MAC address in the MAC HW registers. > > > > > > Ok, I see. > > > > > > So when I call mac_dev->change_addr() here in dpaa_netdev_init() it > > > means I can remove all the code setting the MAC address in hardware > > > before this point, right? > > > > It's not an issue if you just write it here for the random address and > leave > > the previous functionality in place. > > No, it's not. It's only that the code becomes rather useless once we > overwrite the MAC address here anyway. > > > Do you have a board to validate your changes? > > I have a LS1046a based board, yes. > > Sascha I'll send a v3 addressing all MAC types.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index fd93d542f497..fc117eab788d 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -233,8 +233,15 @@ 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)) { + dev_info(dev, "FMan MAC address: %pM\n", 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); + dev_info(dev, "Using random MAC address: %pM\n", + net_dev->dev_addr); + } net_dev->ethtool_ops = &dpaa_ethtool_ops; diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c index e1901874c19f..9e2372688405 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; diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c index 55f2122c3217..9de76bc4ebde 100644 --- a/drivers/net/ethernet/freescale/fman/mac.c +++ b/drivers/net/ethernet/freescale/fman/mac.c @@ -724,12 +724,8 @@ 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)) + 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,8 +851,6 @@ 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); - priv->eth_dev = dpaa_eth_add_device(fman_id, mac_dev); if (IS_ERR(priv->eth_dev)) { dev_err(dev, "failed to add Ethernet platform device for MAC %d\n",
There's no need to fail probing when no MAC address is given in the device tree, just use a random MAC address. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- Changes since v1: - Remove printing of permanent MAC address drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 11 +++++++++-- drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ---- drivers/net/ethernet/freescale/fman/mac.c | 10 ++-------- 3 files changed, 11 insertions(+), 14 deletions(-)