diff mbox

[net-next,v2,07/10] net/faraday: Read MAC address from chip

Message ID 1468579460-13768-8-git-send-email-gwshan@linux.vnet.ibm.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Gavin Shan July 15, 2016, 10:44 a.m. UTC 
The device is assigned with random MAC address. It isn't reasonable.
An valid MAC address might have been provided by (uboot) firmware by
device-tree or in chip. It's reasonable to use it to maintain consistency.

This uses the MAC address from device-tree or that in the chip if it's
valid. Otherwise, a random MAC address is given as before.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Acked-by: Joel Stanley <joel@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 69 ++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 7 deletions(-)

Comments

David Laight July 19, 2016, 10:50 a.m. UTC | #1 
From: Gavin Shan
> Sent: 15 July 2016 11:44
> The device is assigned with random MAC address. It isn't reasonable.
> An valid MAC address might have been provided by (uboot) firmware by
> device-tree or in chip. It's reasonable to use it to maintain consistency.
> 
> This uses the MAC address from device-tree or that in the chip if it's
> valid. Otherwise, a random MAC address is given as before.
...
> +	m = ioread32(priv->base + FTGMAC100_OFFSET_MAC_MADR);
> +	l = ioread32(priv->base + FTGMAC100_OFFSET_MAC_LADR);
> +
> +	mac[0] = (m >> 8) & 0xff;
> +	mac[1] = m & 0xff;
> +	mac[2] = (l >> 24) & 0xff;
> +	mac[3] = (l >> 16) & 0xff;
> +	mac[4] = (l >> 8) & 0xff;
> +	mac[5] = l & 0xff;
> +
> +	if (!is_valid_ether_addr(mac)) {
> +		mac[5] = (m >> 8) & 0xff;
> +		mac[4] = m & 0xff;
> +		mac[3] = (l >> 24) & 0xff;
> +		mac[2] = (l >> 16) & 0xff;
> +		mac[1] = (l >>  8) & 0xff;
> +		mac[0] = l & 0xff;
> +	}
...

That is horrid, not all byte reversed addresses will be invalid.

	David
Benjamin Herrenschmidt July 19, 2016, 10:57 a.m. UTC | #2 
On Tue, 2016-07-19 at 10:50 +0000, David Laight wrote:
> > +     if (!is_valid_ether_addr(mac)) {
> > +             mac[5] = (m >> 8) & 0xff;
> > +             mac[4] = m & 0xff;
> > +             mac[3] = (l >> 24) & 0xff;
> > +             mac[2] = (l >> 16) & 0xff;
> > +             mac[1] = (l >>  8) & 0xff;
> > +             mac[0] = l & 0xff;
> > +     }
> ...
> 
> That is horrid, not all byte reversed addresses will be invalid.

Right, that's just a hack for a broken vendor uboot we had here, Gavin,
drop that part of the patch please.

Cheers,
Ben.
Gavin Shan July 19, 2016, 11:40 p.m. UTC | #3 
On Tue, Jul 19, 2016 at 08:57:53PM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2016-07-19 at 10:50 +0000, David Laight wrote:
>> > +     if (!is_valid_ether_addr(mac)) {
>> > +             mac[5] = (m >> 8) & 0xff;
>> > +             mac[4] = m & 0xff;
>> > +             mac[3] = (l >> 24) & 0xff;
>> > +             mac[2] = (l >> 16) & 0xff;
>> > +             mac[1] = (l >>  8) & 0xff;
>> > +             mac[0] = l & 0xff;
>> > +     }
>> ...
>> 
>> That is horrid, not all byte reversed addresses will be invalid.
>
>Right, that's just a hack for a broken vendor uboot we had here, Gavin,
>drop that part of the patch please.
>

Sure, I'll drop it in v4 or a followup patch. v3 is being reviewed this
moment.

Thanks,
Gavin
diff mbox

Patch

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 9b09493..2c3f656 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -141,6 +141,64 @@  static void ftgmac100_set_mac(struct ftgmac100 *priv, const unsigned char *mac)
 	iowrite32(laddr, priv->base + FTGMAC100_OFFSET_MAC_LADR);
 }
 
+static void ftgmac100_setup_mac(struct ftgmac100 *priv)
+{
+	u8 mac[ETH_ALEN];
+	unsigned int m;
+	unsigned int l;
+	void *addr;
+
+	addr = device_get_mac_address(priv->dev, mac, ETH_ALEN);
+	if (addr) {
+		ether_addr_copy(priv->netdev->dev_addr, mac);
+		dev_info(priv->dev, "Read MAC address %pM from device tree\n",
+			 mac);
+		return;
+	}
+
+	m = ioread32(priv->base + FTGMAC100_OFFSET_MAC_MADR);
+	l = ioread32(priv->base + FTGMAC100_OFFSET_MAC_LADR);
+
+	mac[0] = (m >> 8) & 0xff;
+	mac[1] = m & 0xff;
+	mac[2] = (l >> 24) & 0xff;
+	mac[3] = (l >> 16) & 0xff;
+	mac[4] = (l >> 8) & 0xff;
+	mac[5] = l & 0xff;
+
+	if (!is_valid_ether_addr(mac)) {
+		mac[5] = (m >> 8) & 0xff;
+		mac[4] = m & 0xff;
+		mac[3] = (l >> 24) & 0xff;
+		mac[2] = (l >> 16) & 0xff;
+		mac[1] = (l >>  8) & 0xff;
+		mac[0] = l & 0xff;
+	}
+
+	if (is_valid_ether_addr(mac)) {
+		ether_addr_copy(priv->netdev->dev_addr, mac);
+		dev_info(priv->dev, "Read MAC address %pM from chip\n", mac);
+	} else {
+		eth_hw_addr_random(priv->netdev);
+		dev_info(priv->dev, "Generated random MAC address %pM\n",
+			 priv->netdev->dev_addr);
+	}
+}
+
+static int ftgmac100_set_mac_addr(struct net_device *dev, void *p)
+{
+	int ret;
+
+	ret = eth_prepare_mac_addr_change(dev, p);
+	if (ret < 0)
+		return ret;
+
+	eth_commit_mac_addr_change(dev, p);
+	ftgmac100_set_mac(netdev_priv(dev), dev->dev_addr);
+
+	return 0;
+}
+
 static void ftgmac100_init_hw(struct ftgmac100 *priv)
 {
 	/* setup ring buffer base registers */
@@ -1141,7 +1199,7 @@  static const struct net_device_ops ftgmac100_netdev_ops = {
 	.ndo_open		= ftgmac100_open,
 	.ndo_stop		= ftgmac100_stop,
 	.ndo_start_xmit		= ftgmac100_hard_start_xmit,
-	.ndo_set_mac_address	= eth_mac_addr,
+	.ndo_set_mac_address	= ftgmac100_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl		= ftgmac100_do_ioctl,
 };
@@ -1265,6 +1323,9 @@  static int ftgmac100_probe(struct platform_device *pdev)
 
 	priv->irq = irq;
 
+	/* MAC address from chip or random one */
+	ftgmac100_setup_mac(priv);
+
 	err = ftgmac100_setup_mdio(netdev);
 	if (err)
 		goto err_setup_mdio;
@@ -1278,12 +1339,6 @@  static int ftgmac100_probe(struct platform_device *pdev)
 
 	netdev_info(netdev, "irq %d, mapped at %p\n", priv->irq, priv->base);
 
-	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		eth_hw_addr_random(netdev);
-		netdev_info(netdev, "generated random MAC address %pM\n",
-			    netdev->dev_addr);
-	}
-
 	return 0;
 
 err_register_netdev: