[Intel-wired-lan] Patch for igb_main.c

[Intel-wired-lan] Patch for igb_main.c

[James Courtier-Dutton]

Hi,

I have a patch for igb_main.c that helps bring up the igb NIC on the
Utilite Pro  embedded ARM PC.
How do I get this added to the kernel?
Summary below:

Kind Regards

James


Author: James Courtier-Dutton <James.Dutton@gmail.com>
Date:   Sat Jul 1 19:27:51 2017 +0100

    net: igb: This helps to bring up the igb NIC by using a random MAC
address when no other sources of MAC addres are available.

    This help bring up the igb NIC on the Utilite.

    Signed-off-by: James Courtier-Dutton <James@superbug.co.uk>

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
b/drivers/net/ethernet/intel/igb/igb_main.c
index be456ba..d7c2c99 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2509,6 +2509,11 @@ static int igb_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
                        dev_err(&pdev->dev, "NVM Read Error\n");
        }

+       if (!is_valid_ether_addr(hw->mac.addr)) {
+               dev_info(&pdev->dev, "Random MAC Address\n");
+               random_ether_addr(hw->mac.addr);
+       }
+
        memcpy(netdev->dev_addr, hw->mac.addr, netdev->addr_len);

        if (!is_valid_ether_addr(netdev->dev_addr)) {

[Intel-wired-lan] Patch for igb_main.c

[Kirsher, Jeffrey T]

Please mail patches to intel-wired-lan at lists.osuosl.org, that is how.

Sent from my iPhone

> On Mar 25, 2018, at 16:47, James Courtier-Dutton <james.dutton@gmail.com> wrote:
> 
> Hi,
> 
> I have a patch for igb_main.c that helps bring up the igb NIC on the
> Utilite Pro  embedded ARM PC.
> How do I get this added to the kernel?
> Summary below:
> 
> Kind Regards
> 
> James
> 
> 
> Author: James Courtier-Dutton <James.Dutton@gmail.com>
> Date:   Sat Jul 1 19:27:51 2017 +0100
> 
>    net: igb: This helps to bring up the igb NIC by using a random MAC
> address when no other sources of MAC addres are available.
> 
>    This help bring up the igb NIC on the Utilite.
> 
>    Signed-off-by: James Courtier-Dutton <James@superbug.co.uk>
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index be456ba..d7c2c99 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2509,6 +2509,11 @@ static int igb_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>                        dev_err(&pdev->dev, "NVM Read Error\n");
>        }
> 
> +       if (!is_valid_ether_addr(hw->mac.addr)) {
> +               dev_info(&pdev->dev, "Random MAC Address\n");
> +               random_ether_addr(hw->mac.addr);
> +       }
> +
>        memcpy(netdev->dev_addr, hw->mac.addr, netdev->addr_len);
> 
>        if (!is_valid_ether_addr(netdev->dev_addr)) {
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] Patch for igb_main.c

[Duyck, Alexander H]

On Mon, 2018-03-26 at 00:46 +0100, James Courtier-Dutton wrote:
> Hi,
> 
> I have a patch for igb_main.c that helps bring up the igb NIC on the
> Utilite Pro  embedded ARM PC.
> How do I get this added to the kernel?
> Summary below:
> 
> Kind Regards
> 
> James
> 
> 
> Author: James Courtier-Dutton <James.Dutton@gmail.com>
> Date:   Sat Jul 1 19:27:51 2017 +0100
> 
>     net: igb: This helps to bring up the igb NIC by using a random MAC
> address when no other sources of MAC addres are available.
> 
>     This help bring up the igb NIC on the Utilite.
> 
>     Signed-off-by: James Courtier-Dutton <James@superbug.co.uk>
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index be456ba..d7c2c99 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2509,6 +2509,11 @@ static int igb_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>                         dev_err(&pdev->dev, "NVM Read Error\n");
>         }
> 
> +       if (!is_valid_ether_addr(hw->mac.addr)) {
> +               dev_info(&pdev->dev, "Random MAC Address\n");
> +               random_ether_addr(hw->mac.addr);
> +       }
> +
>         memcpy(netdev->dev_addr, hw->mac.addr, netdev->addr_len);
> 
>         if (!is_valid_ether_addr(netdev->dev_addr)) {

Any idea why the MAC address is invalid? Does the part not have an NVM,
or is the NVM not populated correctly?

I'm really not a fan of us enabling a random MAC address for the device
if a valid one is not present. I would like to know more about why a
valid one is not present and see if we can address that before we start
working around it in the driver.

Thanks.

- Alex

[Intel-wired-lan] Patch for igb_main.c

[James Courtier-Dutton]

On Mon, 26 Mar 2018, 16:39 Duyck, Alexander H, <alexander.h.duyck@intel.com>
wrote:

>
> Any idea why the MAC address is invalid? Does the part not have an NVM,
> or is the NVM not populated correctly?
>
> I'm really not a fan of us enabling a random MAC address for the device
> if a valid one is not present. I would like to know more about why a
> valid one is not present and see if we can address that before we start
> working around it in the driver.
>
> Thanks.
>
> - Alex


Hi Alex,

The intention is that it only chooses a random mac addres if all other
options are not available.
It is better for the driver to load than not at all.

Kind regards

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20180326/f7a8ba7d/attachment.html>

[Intel-wired-lan] Patch for igb_main.c

[Alexander Duyck]

On Mon, Mar 26, 2018 at 12:00 PM, James Courtier-Dutton
<james.dutton@gmail.com> wrote:
>
>
> On Mon, 26 Mar 2018, 16:39 Duyck, Alexander H, <alexander.h.duyck@intel.com>
> wrote:
>>
>>
>> Any idea why the MAC address is invalid? Does the part not have an NVM,
>> or is the NVM not populated correctly?
>>
>> I'm really not a fan of us enabling a random MAC address for the device
>> if a valid one is not present. I would like to know more about why a
>> valid one is not present and see if we can address that before we start
>> working around it in the driver.
>>
>> Thanks.
>>
>> - Alex
>
>
> Hi Alex,
>
> The intention is that it only chooses a random mac addres if all other
> options are not available.
> It is better for the driver to load than not at all.
>
> Kind regards
>
> James

Hi James,

I disagree. If we have an NVM issue it is better for us to fail
loading the driver than for us to load and just have random data
populating the fields that are supposed to be configured by the NVM.
You changes impacts all devices supported by the driver. Not just the
one system you care about. We need to take all of them into account.

I would much rather have a driver fail to load than open us up to the
potential for things like a kernel panic.

Thanks.

- Alex

[Intel-wired-lan] Patch for igb_main.c

[Fujinaka, Todd]

Is this a board you designed? Why doesn't it have a properly programmed NVM?

Todd Fujinaka
Software Application Engineer
Datacenter Engineering Group
Intel Corporation
todd.fujinaka at intel.com

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alexander Duyck
Sent: Monday, March 26, 2018 1:10 PM
To: James Courtier-Dutton <james.dutton@gmail.com>
Cc: intel-wired-lan at osuosl.org
Subject: Re: [Intel-wired-lan] Patch for igb_main.c

On Mon, Mar 26, 2018 at 12:00 PM, James Courtier-Dutton <james.dutton@gmail.com> wrote:
>
>
> On Mon, 26 Mar 2018, 16:39 Duyck, Alexander H, 
> <alexander.h.duyck@intel.com>
> wrote:
>>
>>
>> Any idea why the MAC address is invalid? Does the part not have an 
>> NVM, or is the NVM not populated correctly?
>>
>> I'm really not a fan of us enabling a random MAC address for the 
>> device if a valid one is not present. I would like to know more about 
>> why a valid one is not present and see if we can address that before 
>> we start working around it in the driver.
>>
>> Thanks.
>>
>> - Alex
>
>
> Hi Alex,
>
> The intention is that it only chooses a random mac addres if all other 
> options are not available.
> It is better for the driver to load than not at all.
>
> Kind regards
>
> James

Hi James,

I disagree. If we have an NVM issue it is better for us to fail loading the driver than for us to load and just have random data populating the fields that are supposed to be configured by the NVM.
You changes impacts all devices supported by the driver. Not just the one system you care about. We need to take all of them into account.

I would much rather have a driver fail to load than open us up to the potential for things like a kernel panic.

Thanks.

- Alex
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] Patch for igb_main.c

[James Courtier-Dutton]

On 26 March 2018 at 21:29, Fujinaka, Todd <todd.fujinaka@intel.com> wrote:
> Is this a board you designed? Why doesn't it have a properly programmed NVM?
>
> Todd Fujinaka
> Software Application Engineer
> Datacenter Engineering Group
> Intel Corporation
> todd.fujinaka at intel.com
>

Hi,

No. I have not designed the board.

It is for this device:
http://www.compulab.co.il/utilite-computer/web/utilite-overview

The manufacturer of the device only has a very old kernel for the device.
I have some patches that can make it work with the latest Linux kernel.
See here:
https://github.com/jcdutton/linux-utilite/tree/v4.15.7-utilite

Of those patches, one of them is for the igb-main.c  file.

My aim is to get all the patches into mainline, at which point, I will
not need to manage my own kernel for this device.
I only have 4 remaining patches to go, one of which is this igb-main.c one.

Kind Regards

James

[Intel-wired-lan] Patch for igb_main.c

[Fujinaka, Todd]

I think you need to contact the manufacturer if the NVM doesn?t have a MAC address programmed correctly.

Todd Fujinaka
Software Application Engineer
Datacenter Engineering Group
Intel Corporation
todd.fujinaka at intel.com


-----Original Message-----
From: James Courtier-Dutton [mailto:james.dutton at gmail.com] 
Sent: Monday, March 26, 2018 1:43 PM
To: Fujinaka, Todd <todd.fujinaka@intel.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>; intel-wired-lan at osuosl.org
Subject: Re: [Intel-wired-lan] Patch for igb_main.c

On 26 March 2018 at 21:29, Fujinaka, Todd <todd.fujinaka@intel.com> wrote:
> Is this a board you designed? Why doesn't it have a properly programmed NVM?
>
> Todd Fujinaka
> Software Application Engineer
> Datacenter Engineering Group
> Intel Corporation
> todd.fujinaka at intel.com
>

Hi,

No. I have not designed the board.

It is for this device:
http://www.compulab.co.il/utilite-computer/web/utilite-overview

The manufacturer of the device only has a very old kernel for the device.
I have some patches that can make it work with the latest Linux kernel.
See here:
https://github.com/jcdutton/linux-utilite/tree/v4.15.7-utilite

Of those patches, one of them is for the igb-main.c  file.

My aim is to get all the patches into mainline, at which point, I will not need to manage my own kernel for this device.
I only have 4 remaining patches to go, one of which is this igb-main.c one.

Kind Regards

James

[Intel-wired-lan] Patch for igb_main.c

[Alexander Duyck]

On Mon, Mar 26, 2018 at 1:42 PM, James Courtier-Dutton
<james.dutton@gmail.com> wrote:
> On 26 March 2018 at 21:29, Fujinaka, Todd <todd.fujinaka@intel.com> wrote:
>> Is this a board you designed? Why doesn't it have a properly programmed NVM?
>>
>> Todd Fujinaka
>> Software Application Engineer
>> Datacenter Engineering Group
>> Intel Corporation
>> todd.fujinaka at intel.com
>>
>
> Hi,
>
> No. I have not designed the board.
>
> It is for this device:
> http://www.compulab.co.il/utilite-computer/web/utilite-overview
>
> The manufacturer of the device only has a very old kernel for the device.
> I have some patches that can make it work with the latest Linux kernel.
> See here:
> https://github.com/jcdutton/linux-utilite/tree/v4.15.7-utilite
>
> Of those patches, one of them is for the igb-main.c  file.

Have you done any work to root cause why the MAC address is invalid?
For example is it all 0's or by chance is it all 1's?

I ask because there appears to be a known method for addressing an
issue of getting the MAC address configured for these parts via
u-boot:
http://www.compulab.co.il/utilite-computer/forum/viewtopic.php?f=66&t=1986

> My aim is to get all the patches into mainline, at which point, I will
> not need to manage my own kernel for this device.
> I only have 4 remaining patches to go, one of which is this igb-main.c one.
>
> Kind Regards
>
> James

I'm not sure this needs patch needs to be committed. If I am
understanding things correctly this can just be setup correctly in
u-boot and the problem is solved. We already have code in the igb
driver to handle platform provided MAC addresses and it seems like
this is how this is supposed to be configured.

Thanks.

- Alex