[PATCH] ixgbe: Look up MAC address in Open Firmware

[PATCH] ixgbe: Look up MAC address in Open Firmware

[Martin K. Petersen]

Attempt to look up the MAC address in Open Firmware on systems that
support it. If the "local-mac-address" property is not valid resort to
using the IDPROM value.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3d1032..365924124fab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,11 @@
 #include <linux/prefetch.h>
 #include <scsi/fc/fc_fcoe.h>
 
+#ifdef CONFIG_OF
+#include <asm/idprom.h>
+#include <asm/prom.h>
+#endif
+
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
@@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
 	return is_wol_supported;
 }
 
+#ifdef CONFIG_OF
+/**
+ * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
+ * @adapter: Pointer to adapter struct
+ */
+static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
+{
+	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	const unsigned char *addr;
+	int len;
+
+	addr = of_get_property(dp, "local-mac-address", &len);
+	if (addr && len == 6) {
+		e_dev_info("Using OpenPROM MAC address\n");
+		memcpy(hw->mac.perm_addr, addr, 6);
+	}
+
+	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
+		e_dev_info("Using IDPROM MAC address\n");
+		memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
+	}
+}
+#endif
+
 /**
  * ixgbe_probe - Device Initialization Routine
  * @pdev: PCI device information struct
@@ -8223,6 +8253,10 @@ skip_sriov:
 		goto err_sw_init;
 	}
 
+#ifdef CONFIG_OF
+	ixgbe_of_mac_addr(adapter);
+#endif
+
 	memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
 
 	if (!is_valid_ether_addr(netdev->dev_addr)) {

Re: [linux-nics] [PATCH] ixgbe: Look up MAC address in Open Firmware

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Fri, 2014-11-14 at 14:16 -0500, Martin K. Petersen wrote:
> 
> Attempt to look up the MAC address in Open Firmware on systems that
> support it. If the "local-mac-address" property is not valid resort to
> using the IDPROM value.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Thanks Martin, I will add your patch to my queue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[Sergei Shtylyov]

Hello.

On 11/14/2014 10:16 PM, Martin K. Petersen wrote:

> Attempt to look up the MAC address in Open Firmware on systems that
> support it. If the "local-mac-address" property is not valid resort to
> using the IDPROM value.

> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d2df4e3d1032..365924124fab 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
[...]
> @@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
>   	return is_wol_supported;
>   }
>
> +#ifdef CONFIG_OF
> +/**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> +	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	const unsigned char *addr;
> +	int len;
> +
> +	addr = of_get_property(dp, "local-mac-address", &len);
> +	if (addr && len == 6) {
> +		e_dev_info("Using OpenPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, addr, 6);
> +	}
> +
> +	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> +		e_dev_info("Using IDPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
> +	}
> +}
> +#endif
> +
>   /**
>    * ixgbe_probe - Device Initialization Routine
>    * @pdev: PCI device information struct
> @@ -8223,6 +8253,10 @@ skip_sriov:
>   		goto err_sw_init;
>   	}
>
> +#ifdef CONFIG_OF
> +	ixgbe_of_mac_addr(adapter);
> +#endif

    Eww... why not define the following above instead:

#else
static inline void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter) {}
#endif

    This is closer to what Documentation/SubmittingPatches suggests.

WBR, Sergei

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[Martin K. Petersen]

>>>>> "Sergei" == Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

>> +#ifdef CONFIG_OF
>> + ixgbe_of_mac_addr(adapter);
>> +#endif

Sergei>    Eww... why not define the following above instead:

Sergei> #else
Sergei> static inline void ixgbe_of_mac_addr(struct ixgbe_adapter
Sergei> *adapter) {}
Sergei> #endif

I don't care much either way. But we might as well do this, then, and
shave off an ifdef...

commit 01e25f145972563ee87ebf85b7cb02a4ff8fce3b
Author: Martin K. Petersen <martin.petersen@oracle.com>
Date:   Wed Nov 12 20:47:42 2014 -0500

    ixgbe: Look up MAC address in Open Firmware
    
    Attempt to look up the MAC address in Open Firmware on systems that
    support it. If the "local-mac-address" property is not valid resort to
    using the IDPROM value.
    
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3d1032..0e45a43172eb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,11 @@
 #include <linux/prefetch.h>
 #include <scsi/fc/fc_fcoe.h>
 
+#ifdef CONFIG_OF
+#include <asm/idprom.h>
+#include <asm/prom.h>
+#endif
+
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
@@ -7960,6 +7965,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
 }
 
 /**
+ * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
+ * @adapter: Pointer to adapter struct
+ */
+static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
+{
+#ifdef CONFIG_OF
+	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	const unsigned char *addr;
+	int len;
+
+	addr = of_get_property(dp, "local-mac-address", &len);
+	if (addr && len == 6) {
+		e_dev_info("Using OpenPROM MAC address\n");
+		memcpy(hw->mac.perm_addr, addr, 6);
+	}
+
+	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
+		e_dev_info("Using IDPROM MAC address\n");
+		memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
+	}
+#endif
+}
+
+/**
  * ixgbe_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in ixgbe_pci_tbl
@@ -8223,6 +8253,8 @@ skip_sriov:
 		goto err_sw_init;
 	}
 
+	ixgbe_of_mac_addr(adapter);
+
 	memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
 
 	if (!is_valid_ether_addr(netdev->dev_addr)) {

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[Jesse Brandeburg]

On Fri, 14 Nov 2014 15:20:09 -0500
"Martin K. Petersen" <martin.petersen@oracle.com> wrote:
> commit 01e25f145972563ee87ebf85b7cb02a4ff8fce3b
> Author: Martin K. Petersen <martin.petersen@oracle.com>
> Date:   Wed Nov 12 20:47:42 2014 -0500
> 
>     ixgbe: Look up MAC address in Open Firmware
>     
>     Attempt to look up the MAC address in Open Firmware on systems that
>     support it. If the "local-mac-address" property is not valid resort to
>     using the IDPROM value.
>     
>     Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

seems fine, but see below...

> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d2df4e3d1032..0e45a43172eb 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -50,6 +50,11 @@
>  #include <linux/prefetch.h>
>  #include <scsi/fc/fc_fcoe.h>
>  
> +#ifdef CONFIG_OF
> +#include <asm/idprom.h>
> +#include <asm/prom.h>
> +#endif
> +
>  #include "ixgbe.h"
>  #include "ixgbe_common.h"
>  #include "ixgbe_dcb_82599.h"
> @@ -7960,6 +7965,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
>  }
>  
>  /**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> +#ifdef CONFIG_OF
> +	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	const unsigned char *addr;
> +	int len;
> +
> +	addr = of_get_property(dp, "local-mac-address", &len);
> +	if (addr && len == 6) {
> +		e_dev_info("Using OpenPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, addr, 6);
> +	}
> +
> +	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> +		e_dev_info("Using IDPROM MAC address\n");

There really isn't much point to print this message, just delete this
e_dev_info line.

Re: [linux-nics] [PATCH] ixgbe: Look up MAC address in Open Firmware

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

On Fri, 2014-11-14 at 15:20 -0500, Martin K. Petersen wrote:
> >>>>> "Sergei" == Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> writes:
> 
> >> +#ifdef CONFIG_OF
> >> + ixgbe_of_mac_addr(adapter);
> >> +#endif
> 
> Sergei>    Eww... why not define the following above instead:
> 
> Sergei> #else
> Sergei> static inline void ixgbe_of_mac_addr(struct ixgbe_adapter
> Sergei> *adapter) {}
> Sergei> #endif
> 
> I don't care much either way. But we might as well do this, then, and
> shave off an ifdef...
> 
> commit 01e25f145972563ee87ebf85b7cb02a4ff8fce3b
> Author: Martin K. Petersen <martin.petersen@oracle.com>
> Date:   Wed Nov 12 20:47:42 2014 -0500
> 
>     ixgbe: Look up MAC address in Open Firmware
>     
>     Attempt to look up the MAC address in Open Firmware on systems
> that
>     support it. If the "local-mac-address" property is not valid
> resort to
>     using the IDPROM value.
>     
>     Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Was this meant to be a v2 patch???  Yikes!

Based on the feedback from Segei, Dave and Jesse.  Dropping this patch
from my queue and will wait for a properly formatted v2 of this patch.

Please CC me on any future patches against any changes to
drivers/net/ethernet/intel/*

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[Rustad, Mark D]

[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]

On Nov 14, 2014, at 12:20 PM, Martin K. Petersen <martin.petersen@oracle.com> wrote:

> /**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> +#ifdef CONFIG_OF
> +	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	const unsigned char *addr;
> +	int len;
> +
> +	addr = of_get_property(dp, "local-mac-address", &len);
> +	if (addr && len == 6) {
> +		e_dev_info("Using OpenPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, addr, 6);
> +	}
> +
> +	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> +		e_dev_info("Using IDPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
> +	}
> +#endif
> +}

I would prefer that the entire function be wrapped by the #if so that a null implementation can include the prototype. That allows the unused parameter to properly be declared as __always_unused as in:

#else
static void ixgbe_of_mac_addr(struct ixgbe_adapter __always_unused *adapter)
{}
#endif /* CONFIG_OF */

Otherwise, it will throw warnings when compiled with enhanced warning messages.

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[David Miller]

From: "Martin K. Petersen" <martin.petersen@oracle.com>
Date: Fri, 14 Nov 2014 14:16:50 -0500

> +#ifdef CONFIG_OF
> +#include <asm/idprom.h>
> +#include <asm/prom.h>
> +#endif

CONFIG_OF doesn't imply the presence of idprom.

idprom is completely sparc/m68k specific, whereas OF exists on many
platforms other than sparc/m68k.

Re: [PATCH] ixgbe: Look up MAC address in Open Firmware

[Florian Fainelli]

On 11/14/2014 11:16 AM, Martin K. Petersen wrote:

[snip]

> +#ifdef CONFIG_OF
> +/**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> +	struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	const unsigned char *addr;
> +	int len;
> +
> +	addr = of_get_property(dp, "local-mac-address", &len);
> +	if (addr && len == 6) {
> +		e_dev_info("Using OpenPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, addr, 6);
> +	}

Cannot you use of_get_mac_address() here which does iterate through all
the OF standard ways to specify a MAC address: mac-address,
local-mac-address and address? Benefit is that this will work for all
DT-enabled platforms.

> +
> +	if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> +		e_dev_info("Using IDPROM MAC address\n");
> +		memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
> +	}
> +}
> +#endif
> +
>  /**
>   * ixgbe_probe - Device Initialization Routine
>   * @pdev: PCI device information struct
> @@ -8223,6 +8253,10 @@ skip_sriov:
>  		goto err_sw_init;
>  	}
>  
> +#ifdef CONFIG_OF
> +	ixgbe_of_mac_addr(adapter);
> +#endif
> +
>  	memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
>  
>  	if (!is_valid_ether_addr(netdev->dev_addr)) {
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>