diff mbox series

[5/6] net: bcmgenet: Fetch MAC address from the adapter

Message ID 20200201074625.8698-6-jeremy.linton@arm.com
State Deferred
Delegated to: David Miller
Headers show
Series Add ACPI bindings to the genet | expand

Commit Message

Jeremy Linton Feb. 1, 2020, 7:46 a.m. UTC
ARM/ACPI machines should utilize self describing hardware
when possible. The MAC address on the BCMGENET can be
read from the adapter if a full featured firmware has already
programmed it. Lets try using the address already programmed,
if it appears to be valid.

It should be noted that while we move the macaddr logic below
the clock and power logic in the driver, none of that code will
ever be active in an ACPI environment as the device will be
attached to the acpi power domain, and brought to full power with
all clocks enabled immediately before the device probe routine is
called.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../net/ethernet/broadcom/genet/bcmgenet.c    | 43 ++++++++++++++-----
 1 file changed, 32 insertions(+), 11 deletions(-)

Comments

Andrew Lunn Feb. 1, 2020, 3:37 p.m. UTC | #1
> @@ -3601,6 +3605,23 @@ static int bcmgenet_probe(struct platform_device *pdev)
>  	    !strcasecmp(phy_mode_str, "internal"))
>  		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
>  
> +	if (dn)
> +		macaddr = of_get_mac_address(dn);
> +	else if (pd)
> +		macaddr = pd->mac_address;
> +
> +	if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
> +		if (has_acpi_companion(&pdev->dev))
> +			bcmgenet_get_hw_addr(priv, dev->dev_addr);
> +
> +		if (!is_valid_ether_addr(dev->dev_addr)) {
> +			dev_warn(&pdev->dev, "using random Ethernet MAC\n");
> +			eth_hw_addr_random(dev);
> +		}
> +	} else {
> +		ether_addr_copy(dev->dev_addr, macaddr);
> +	}
> +

Could you also maybe put in here somewhere a call to
device_get_mac_address(), to support getting the MAC address out of
ACPI?

	Andrew
Jeremy Linton Feb. 1, 2020, 7:20 p.m. UTC | #2
Hi,

On 2/1/20 9:37 AM, Andrew Lunn wrote:
>> @@ -3601,6 +3605,23 @@ static int bcmgenet_probe(struct platform_device *pdev)
>>   	    !strcasecmp(phy_mode_str, "internal"))
>>   		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
>>   
>> +	if (dn)
>> +		macaddr = of_get_mac_address(dn);
>> +	else if (pd)
>> +		macaddr = pd->mac_address;
>> +
>> +	if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
>> +		if (has_acpi_companion(&pdev->dev))
>> +			bcmgenet_get_hw_addr(priv, dev->dev_addr);
>> +
>> +		if (!is_valid_ether_addr(dev->dev_addr)) {
>> +			dev_warn(&pdev->dev, "using random Ethernet MAC\n");
>> +			eth_hw_addr_random(dev);
>> +		}
>> +	} else {
>> +		ether_addr_copy(dev->dev_addr, macaddr);
>> +	}
>> +
> 
> Could you also maybe put in here somewhere a call to
> device_get_mac_address(), to support getting the MAC address out of
> ACPI?

I had that here until right before I posted it, mostly because I was 
trying to consolidate the DT/ACPI paths. I pulled it out because it 
wasn't making the code any clearer, and as I mentioned in my response to 
the general _DSD properties I would rather entirely depend on non DSD 
properties if possible.

I will put it back in, but IMHO we shouldn't be finding firmware using 
it. Since the discussion a few years back, its become clearer to me its 
not usually needed. As in this example, the addresses can usually be 
picked off the adapter if the firmware bothers to set them up.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index c736700f829e..dbf96fc96689 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2779,6 +2779,21 @@  static void bcmgenet_set_hw_addr(struct bcmgenet_priv *priv,
 	bcmgenet_umac_writel(priv, (addr[4] << 8) | addr[5], UMAC_MAC1);
 }
 
+static void bcmgenet_get_hw_addr(struct bcmgenet_priv *priv,
+				 unsigned char *addr)
+{
+	u32 addr_tmp;
+
+	addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC0);
+	addr[0] = addr_tmp >> 24;
+	addr[1] = (addr_tmp >> 16) & 0xff;
+	addr[2] = (addr_tmp >>	8) & 0xff;
+	addr[3] = addr_tmp & 0xff;
+	addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC1);
+	addr[4] = (addr_tmp >> 8) & 0xff;
+	addr[5] = addr_tmp & 0xff;
+}
+
 /* Returns a reusable dma control register value */
 static u32 bcmgenet_dma_disable(struct bcmgenet_priv *priv)
 {
@@ -3509,11 +3524,6 @@  static int bcmgenet_probe(struct platform_device *pdev)
 	}
 	priv->wol_irq = platform_get_irq_optional(pdev, 2);
 
-	if (dn)
-		macaddr = of_get_mac_address(dn);
-	else if (pd)
-		macaddr = pd->mac_address;
-
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base)) {
 		err = PTR_ERR(priv->base);
@@ -3524,12 +3534,6 @@  static int bcmgenet_probe(struct platform_device *pdev)
 
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	dev_set_drvdata(&pdev->dev, dev);
-	if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
-		dev_warn(&pdev->dev, "using random Ethernet MAC\n");
-		eth_hw_addr_random(dev);
-	} else {
-		ether_addr_copy(dev->dev_addr, macaddr);
-	}
 	dev->watchdog_timeo = 2 * HZ;
 	dev->ethtool_ops = &bcmgenet_ethtool_ops;
 	dev->netdev_ops = &bcmgenet_netdev_ops;
@@ -3601,6 +3605,23 @@  static int bcmgenet_probe(struct platform_device *pdev)
 	    !strcasecmp(phy_mode_str, "internal"))
 		bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
+	if (dn)
+		macaddr = of_get_mac_address(dn);
+	else if (pd)
+		macaddr = pd->mac_address;
+
+	if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
+		if (has_acpi_companion(&pdev->dev))
+			bcmgenet_get_hw_addr(priv, dev->dev_addr);
+
+		if (!is_valid_ether_addr(dev->dev_addr)) {
+			dev_warn(&pdev->dev, "using random Ethernet MAC\n");
+			eth_hw_addr_random(dev);
+		}
+	} else {
+		ether_addr_copy(dev->dev_addr, macaddr);
+	}
+
 	reset_umac(priv);
 
 	err = bcmgenet_mii_init(dev);