[Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[John Holland]

Hello,

The Intel i211 LOM PCIe Ethernet controllers' iNVM operates as an OTP
and has no external EEPROM interface [1]. The following allows the
driver to pickup the MAC address from a device tree blob when CONFIG_OF
has been enabled.

[1] 
http://www.intel.com/content/www/us/en/embedded/products/networking/i211-ethernet-controller-datasheet.html

Changes V2
- Restrict searching for compatible devices to current pci device.

Changes V3
- Add device tree binding documentation.

Changes V4
- Rebase patch.

Changes V5
- Use eth_platform_get_mac_address() to resolve MAC specified in a dtb.
- Remove now invalid device tree binding documentation specified in V3
   und V4.

Signed-off-by: John Holland<jotihojr@gmail.com>
---
  drivers/net/ethernet/intel/igb/igb_main.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index a98f418..d3e8228 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -50,6 +50,7 @@
  #include <linux/aer.h>
  #include <linux/prefetch.h>
  #include <linux/pm_runtime.h>
+#include <linux/etherdevice.h>
  #ifdef CONFIG_IGB_DCA
  #include <linux/dca.h>
  #endif
@@ -2507,9 +2508,11 @@ static int igb_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
                 break;
         }

-       /* copy the MAC address out of the NVM */
-       if (hw->mac.ops.read_mac_addr(hw))
-               dev_err(&pdev->dev, "NVM Read Error\n");
+       if (eth_platform_get_mac_address(&pdev->dev, hw->mac.addr)) {
+               /* copy the MAC address out of the NVM */
+               if (hw->mac.ops.read_mac_addr(hw))
+                       dev_err(&pdev->dev, "NVM Read Error\n");
+       }

         memcpy(netdev->dev_addr, hw->mac.addr, netdev->addr_len);

Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[John Holland]

On 02/18/2016 12:10 PM, John Holland wrote:
> Hello,
>
> The Intel i211 LOM PCIe Ethernet controllers' iNVM operates as an OTP
> and has no external EEPROM interface [1]. The following allows the
> driver to pickup the MAC address from a device tree blob when CONFIG_OF
> has been enabled.
>
> +       if (eth_platform_get_mac_address(&pdev->dev, hw->mac.addr)) {

For later reference, putting all necessary information in one place.

This requires the dtb for the mac address routing to be properly 
positioned. On an imx6q using U-Boot, that required setting the U-Boot 
environment variable eth1addr and reworking the PCIe tree and allocating 
an alias as such:

1) Set intel,i211 MAC address.

# env set eth1addr <valid-mac-address>

2) Add an alias to pick up the MAC address from U-Boot and route it to 
the intel,i211 PCIe endpoint for an imx6qdl derivative.

/ {
         aliases {
                 ethernet1 = &eth1;
         };

};

&pcie {
         /* soc pcie bridge 00:00.0 */
         pcie@0,0 {
                 reg = <0x000000 0 0 0 0>;
                 #address-cells = <3>;
                 #size-cells = <2>;

                 /* pcie endpoint 01:00.0 */
                 eth1: intel,i211@pcie0,0 {
                         reg = <0x010000 0 0 0 0>;
                 };
         };
};

John

RE: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[Brown, Aaron F]

> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of John Holland
> Sent: Thursday, February 18, 2016 3:11 AM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using
> a device tree blob V5
> 
> Hello,
> 
> The Intel i211 LOM PCIe Ethernet controllers' iNVM operates as an OTP and
> has no external EEPROM interface [1]. The following allows the driver to
> pickup the MAC address from a device tree blob when CONFIG_OF has been
> enabled.
> 
> [1]
> http://www.intel.com/content/www/us/en/embedded/products/networkin
> g/i211-ethernet-controller-datasheet.html
> 
> Changes V2
> - Restrict searching for compatible devices to current pci device.
> 
> Changes V3
> - Add device tree binding documentation.
> 
> Changes V4
> - Rebase patch.
> 
> Changes V5
> - Use eth_platform_get_mac_address() to resolve MAC specified in a dtb.
> - Remove now invalid device tree binding documentation specified in V3
>    und V4.
> 
> Signed-off-by: John Holland<jotihojr@gmail.com>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

This throws a few checkpatch warnings, but I won't withhold my tested by for these:
-----------------------------------------------------
u1463:[0]/usr/src/kernels/next-queue> git format-patch $item -1 --stdout|./scripts/checkpatch.pl -
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
http://www.intel.com/content/www/us/en/embedded/products/networking/i211-ethernet-controller-datasheet.html

WARNING: email address 'John Holland<jotihojr@gmail.com>' might be better as 'John Holland <jotihojr@gmail.com>'
#30:
Signed-off-by: John Holland<jotihojr@gmail.com>

total: 0 errors, 2 warnings, 0 checks, 21 lines checked

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1463:[0]/usr/src/kernels/next-queue>
-----------------------------------------------------

I do not seem to have hardware that uses device tree, so my testing is relegated to regression tests with my existing set of chipsets.

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[John Holland]

On Mar 1, 2016, at 03:52, Brown, Aaron F <aaron.f.brown@intel.com> wrote:

> This throws a few checkpatch warnings, but I won't withhold my tested by for these:
> 
> total: 0 errors, 2 warnings, 0 checks, 21 lines checked
> 
> Your patch has style problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>      them to the maintainer, see CHECKPATCH in MAINTAINERS.
> u1463:[0]/usr/src/kernels/next-queue>

Thanks for testing...

Do you require me to reformat the patch text? And won't that break the link?

John

RE: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[Brown, Aaron F]

> From: John Holland [mailto:jotihojr@gmail.com]
> Sent: Monday, February 29, 2016 10:56 PM
> To: Brown, Aaron F <aaron.f.brown@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211
> using a device tree blob V5
> 
> On Mar 1, 2016, at 03:52, Brown, Aaron F <aaron.f.brown@intel.com>
> wrote:
> 
> > This throws a few checkpatch warnings, but I won't withhold my tested by
> for these:
> >
> > total: 0 errors, 2 warnings, 0 checks, 21 lines checked
> >
> > Your patch has style problems, please review.
> >
> > NOTE: If any of the errors are false positives, please report
> >      them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > u1463:[0]/usr/src/kernels/next-queue>
> 
> Thanks for testing...
> 
> Do you require me to reformat the patch text? And won't that break the link?

Unless Jeff Kirsher wants you to resubmit it for the space between your name and email address I don't think resubmitting should be necessary.  Yes, breaking the URL up would break the URL and for that reason I think it is appropriate to ignore that (Possible unwrapped commit description) warning.

Thanks,
Aaron
> 
> John

Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[Jeff Kirsher]

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

On Tue, 2016-03-01 at 07:56 +0100, John Holland wrote:
> On Mar 1, 2016, at 03:52, Brown, Aaron F <aaron.f.brown@intel.com>
> wrote:
> 
> > This throws a few checkpatch warnings, but I won't withhold my
> tested by for these:
> > 
> > total: 0 errors, 2 warnings, 0 checks, 21 lines checked
> > 
> > Your patch has style problems, please review.
> > 
> > NOTE: If any of the errors are false positives, please report
> >      them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > u1463:[0]/usr/src/kernels/next-queue>
> 
> Thanks for testing...
> 
> Do you require me to reformat the patch text? And won't that break
> the link?

No, that checkpatch.pl warning on the link you provided is fine,
although I will clean up your signed-off-by when I go to push it
upstream. :-)

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

Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

[John Holland]

> On 02.03.2016, at 00:55, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
>> On Tue, 2016-03-01 at 07:56 +0100, John Holland wrote:
>> On Mar 1, 2016, at 03:52, Brown, Aaron F <aaron.f.brown@intel.com>
>> wrote:
>> 
>>> This throws a few checkpatch warnings, but I won't withhold my
>> tested by for these:
>>>  
>>> total: 0 errors, 2 warnings, 0 checks, 21 lines checked
>>>  
>>> Your patch has style problems, please review.
>>>  
>>> NOTE: If any of the errors are false positives, please report
>>>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>> u1463:[0]/usr/src/kernels/next-queue>
>> 
>> Thanks for testing...
>> 
>> Do you require me to reformat the patch text? And won't that break
>> the link?
> 
> No, that checkpatch.pl warning on the link you provided is fine,
> although I will clean up your signed-off-by when I go to push it
> upstream. :-)

Thank you very much!