[PATCH net-next 0/3] net: add a helpers for loading netdev->dev_addr from platform

[PATCH net-next 0/3] net: add a helpers for loading netdev->dev_addr from platform

[Jakub Kicinski]

Similarly to recently added device_get_ethdev_address()
and of_get_ethdev_address() create a helper for drivers
loading mac addr from platform data.

nvmem_get_mac_address() does not have driver callers
so instead of adding a helper there unexport it.

Jakub Kicinski (3):
  ethernet: un-export nvmem_get_mac_address()
  eth: platform: add a helper for loading netdev->dev_addr
  ethernet: use platform_get_ethdev_address()

 drivers/net/ethernet/actions/owl-emac.c       |  2 +-
 drivers/net/ethernet/mediatek/mtk_star_emac.c |  2 +-
 drivers/net/usb/smsc75xx.c                    |  3 +--
 drivers/net/usb/smsc95xx.c                    |  3 +--
 include/linux/etherdevice.h                   |  1 +
 net/ethernet/eth.c                            | 21 ++++++++++++++++++-
 6 files changed, 25 insertions(+), 7 deletions(-)

-- 
2.31.1

[PATCH net-next 1/3] ethernet: un-export nvmem_get_mac_address()

[Jakub Kicinski]

nvmem_get_mac_address() is only called from of_net.c
we don't need the export.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ethernet/eth.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index d7b8fa10fabb..182de70ac258 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -558,7 +558,6 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf)
 
 	return 0;
 }
-EXPORT_SYMBOL(nvmem_get_mac_address);
 
 static int fwnode_get_mac_addr(struct fwnode_handle *fwnode,
 			       const char *name, char *addr)
-- 
2.31.1

Re: [PATCH net-next 1/3] ethernet: un-export nvmem_get_mac_address()

[Andrew Lunn]

On Thu, Oct 07, 2021 at 06:25:09AM -0700, Jakub Kicinski wrote:
> nvmem_get_mac_address() is only called from of_net.c
> we don't need the export.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 2/3] eth: platform: add a helper for loading netdev->dev_addr

[Jakub Kicinski]

Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.

There is a handful of drivers which pass netdev->dev_addr as
the destination buffer to eth_platform_get_mac_address().
Add a helper which takes a dev pointer instead, so it can call
an appropriate helper.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/linux/etherdevice.h |  1 +
 net/ethernet/eth.c          | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index e75116f48cd1..3cf546d2ffd1 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -29,6 +29,7 @@ struct device;
 struct fwnode_handle;
 
 int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr);
+int platform_get_ethdev_address(struct device *dev, struct net_device *netdev);
 unsigned char *arch_get_platform_mac_address(void);
 int nvmem_get_mac_address(struct device *dev, void *addrbuf);
 int device_get_mac_address(struct device *dev, char *addr);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 182de70ac258..63683b11ca4b 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -523,6 +523,26 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
 }
 EXPORT_SYMBOL(eth_platform_get_mac_address);
 
+/**
+ * platform_get_ethdev_address - Set netdev's MAC address from a given device
+ * @dev:	Pointer to the device
+ * @netdev:	Pointer to netdev to write the address to
+ *
+ * Wrapper around eth_platform_get_mac_address() which writes the address
+ * directly to netdev->dev_addr.
+ */
+int platform_get_ethdev_address(struct device *dev, struct net_device *netdev)
+{
+	u8 addr[ETH_ALEN];
+	int ret;
+
+	ret = eth_platform_get_mac_address(dev, addr);
+	if (!ret)
+		eth_hw_addr_set(netdev, addr);
+	return ret;
+}
+EXPORT_SYMBOL(platform_get_ethdev_address);
+
 /**
  * nvmem_get_mac_address - Obtain the MAC address from an nvmem cell named
  * 'mac-address' associated with given device.
-- 
2.31.1

Re: [PATCH net-next 2/3] eth: platform: add a helper for loading netdev->dev_addr

[Michael Walle]

Am 2021-10-07 15:25, schrieb Jakub Kicinski:
> Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
> of VLANs...") introduced a rbtree for faster Ethernet address look
> up. To maintain netdev->dev_addr in this tree we need to make all
> the writes to it got through appropriate helpers.
> 
> There is a handful of drivers which pass netdev->dev_addr as
> the destination buffer to eth_platform_get_mac_address().
> Add a helper which takes a dev pointer instead, so it can call
> an appropriate helper.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
..

> +/**
> + * platform_get_ethdev_address - Set netdev's MAC address from a given 
> device
> + * @dev:	Pointer to the device
> + * @netdev:	Pointer to netdev to write the address to
> + *
> + * Wrapper around eth_platform_get_mac_address() which writes the 
> address
> + * directly to netdev->dev_addr.
> + */
> +int platform_get_ethdev_address(struct device *dev, struct net_device 
> *netdev)
> +{
> +	u8 addr[ETH_ALEN];
> +	int ret;
> +
> +	ret = eth_platform_get_mac_address(dev, addr);

this eventually calls ether_addr_copy(), which has a note:
   Please note: dst & src must both be aligned to u16.

Is this true for this addr on the stack?

-michael

Re: [PATCH net-next 2/3] eth: platform: add a helper for loading netdev->dev_addr

[Jakub Kicinski]

On Thu, 07 Oct 2021 15:42:17 +0200 Michael Walle wrote:
> > +int platform_get_ethdev_address(struct device *dev, struct net_device *netdev)
> > +{
> > +	u8 addr[ETH_ALEN];
> > +	int ret;
> > +
> > +	ret = eth_platform_get_mac_address(dev, addr);  
> 
> this eventually calls ether_addr_copy(), which has a note:
>    Please note: dst & src must both be aligned to u16.
> 
> Is this true for this addr on the stack?

It will but I don't think there's anything in the standard that
requires it. Let me slap __aligned(2) on it to be sure.

Re: [PATCH net-next 2/3] eth: platform: add a helper for loading netdev->dev_addr

[Andrew Lunn]

On Thu, Oct 07, 2021 at 06:57:01AM -0700, Jakub Kicinski wrote:
> On Thu, 07 Oct 2021 15:42:17 +0200 Michael Walle wrote:
> > > +int platform_get_ethdev_address(struct device *dev, struct net_device *netdev)
> > > +{
> > > +	u8 addr[ETH_ALEN];
> > > +	int ret;
> > > +
> > > +	ret = eth_platform_get_mac_address(dev, addr);  
> > 
> > this eventually calls ether_addr_copy(), which has a note:
> >    Please note: dst & src must both be aligned to u16.
> > 
> > Is this true for this addr on the stack?
> 
> It will but I don't think there's anything in the standard that
> requires it. Let me slap __aligned(2) on it to be sure.

Hi Jakub

I though you changed ether_addr_copy() to be a memcpy?
Or was that some other helper?

	Andrew

Re: [PATCH net-next 2/3] eth: platform: add a helper for loading netdev->dev_addr

[Jakub Kicinski]

On Thu, 7 Oct 2021 16:10:27 +0200 Andrew Lunn wrote:
> On Thu, Oct 07, 2021 at 06:57:01AM -0700, Jakub Kicinski wrote:
> > > this eventually calls ether_addr_copy(), which has a note:
> > >    Please note: dst & src must both be aligned to u16.
> > > 
> > > Is this true for this addr on the stack?  
> > 
> > It will but I don't think there's anything in the standard that
> > requires it. Let me slap __aligned(2) on it to be sure.  
> 
> Hi Jakub
> 
> I though you changed ether_addr_copy() to be a memcpy?
> Or was that some other helper?

I changed eth_hw_addr_set(), I think Michael is referring to 
a ether_addr_copy() inside eth_platform_get_mac_address().

[PATCH net-next 3/3] ethernet: use platform_get_ethdev_address()

[Jakub Kicinski]

Use the new platform_get_ethdev_address() helper for the cases
where dev->dev_addr is passed in directly as the destination.

  @@
  expression dev, net;
  @@
  - eth_platform_get_mac_address(dev, net->dev_addr)
  + platform_get_ethdev_address(dev, net)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/actions/owl-emac.c       | 2 +-
 drivers/net/ethernet/mediatek/mtk_star_emac.c | 2 +-
 drivers/net/usb/smsc75xx.c                    | 3 +--
 drivers/net/usb/smsc95xx.c                    | 3 +--
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/actions/owl-emac.c b/drivers/net/ethernet/actions/owl-emac.c
index 2c25ff3623cd..dce93acd1644 100644
--- a/drivers/net/ethernet/actions/owl-emac.c
+++ b/drivers/net/ethernet/actions/owl-emac.c
@@ -1385,7 +1385,7 @@ static void owl_emac_get_mac_addr(struct net_device *netdev)
 	struct device *dev = netdev->dev.parent;
 	int ret;
 
-	ret = eth_platform_get_mac_address(dev, netdev->dev_addr);
+	ret = platform_get_ethdev_address(dev, netdev);
 	if (!ret && is_valid_ether_addr(netdev->dev_addr))
 		return;
 
diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/ethernet/mediatek/mtk_star_emac.c
index 1d5dd2015453..e2ebfd8115a0 100644
--- a/drivers/net/ethernet/mediatek/mtk_star_emac.c
+++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c
@@ -1544,7 +1544,7 @@ static int mtk_star_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = eth_platform_get_mac_address(dev, ndev->dev_addr);
+	ret = platform_get_ethdev_address(dev, ndev);
 	if (ret || !is_valid_ether_addr(ndev->dev_addr))
 		eth_hw_addr_random(ndev);
 
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 76f7af161313..3b6987bb4fbe 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -758,8 +758,7 @@ static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 static void smsc75xx_init_mac_address(struct usbnet *dev)
 {
 	/* maybe the boot loader passed the MAC address in devicetree */
-	if (!eth_platform_get_mac_address(&dev->udev->dev,
-			dev->net->dev_addr)) {
+	if (!platform_get_ethdev_address(&dev->udev->dev, dev->net)) {
 		if (is_valid_ether_addr(dev->net->dev_addr)) {
 			/* device tree values are valid so use them */
 			netif_dbg(dev, ifup, dev->net, "MAC address read from the device tree\n");
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 26b1bd8e845b..21a42a6527dc 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -756,8 +756,7 @@ static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
 static void smsc95xx_init_mac_address(struct usbnet *dev)
 {
 	/* maybe the boot loader passed the MAC address in devicetree */
-	if (!eth_platform_get_mac_address(&dev->udev->dev,
-			dev->net->dev_addr)) {
+	if (!platform_get_ethdev_address(&dev->udev->dev, dev->net)) {
 		if (is_valid_ether_addr(dev->net->dev_addr)) {
 			/* device tree values are valid so use them */
 			netif_dbg(dev, ifup, dev->net, "MAC address read from the device tree\n");
-- 
2.31.1

Re: [PATCH net-next 3/3] ethernet: use platform_get_ethdev_address()

[Andrew Lunn]

On Thu, Oct 07, 2021 at 06:25:11AM -0700, Jakub Kicinski wrote:
> Use the new platform_get_ethdev_address() helper for the cases
> where dev->dev_addr is passed in directly as the destination.
> 
>   @@
>   expression dev, net;
>   @@
>   - eth_platform_get_mac_address(dev, net->dev_addr)
>   + platform_get_ethdev_address(dev, net)
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew