| Message ID | CAAMvbhES0YLfp__ErFxivfMHhpCrOq0z47a4w84JP547VBhd_Q@mail.gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Patch for igb_main.c | expand |
Please mail patches to intel-wired-lan@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@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
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
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 <div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Mon, 26 Mar 2018, 16:39 Duyck, Alexander H, <<a href="mailto:alexander.h.duyck@intel.com" target="_blank" rel="noreferrer">alexander.h.duyck@intel.com</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> Any idea why the MAC address is invalid? Does the part not have an NVM,<br> or is the NVM not populated correctly?<br> <br> I'm really not a fan of us enabling a random MAC address for the device<br> if a valid one is not present. I would like to know more about why a<br> valid one is not present and see if we can address that before we start<br> working around it in the driver.<br> <br> Thanks.<br> <br> - Alex</blockquote></div></div><div dir="auto"><br></div><div dir="auto">Hi Alex,</div><div dir="auto"><br></div><div dir="auto">The intention is that it only chooses a random mac addres if all other options are not available.</div><div dir="auto">It is better for the driver to load than not at all.</div><div dir="auto"><br></div><div dir="auto">Kind regards</div><div dir="auto"><br></div><div dir="auto">James</div><div dir="auto"></div></div>
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
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@intel.com -----Original Message----- From: Intel-wired-lan [mailto:intel-wired-lan-bounces@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@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
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@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
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@intel.com -----Original Message----- From: James Courtier-Dutton [mailto:james.dutton@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@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@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
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@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
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);
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> if (!is_valid_ether_addr(netdev->dev_addr)) {