| Message ID | 1551361951-1595-1-git-send-email-f.suligoi@asem.it |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v3] net: e1000e: add MAC address kernel cmd line parameter | expand |
On Thu, Feb 28, 2019 at 02:52:31PM +0100, Flavio Suligoi wrote: > Sometimes, in some embedded systems boards (i.e. ARM boards), > the NVM eeprom is not mounted, to save cost and space. > > In this case it is necessary to bypass the NVM management > and directly force the MAC address using a kernel command-line > parameter (macaddr). Hi Flavio Why not use device tree, since this is an ARM platform? Andrew
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: giovedì 28 febbraio 2019 16:33 > To: Flavio Suligoi <f.suligoi@asem.it> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>; David S . Miller > <davem@davemloft.net>; intel-wired-lan@lists.osuosl.org; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v3] net: e1000e: add MAC address kernel cmd line > parameter > > On Thu, Feb 28, 2019 at 02:52:31PM +0100, Flavio Suligoi wrote: > > Sometimes, in some embedded systems boards (i.e. ARM boards), > > the NVM eeprom is not mounted, to save cost and space. > > > > In this case it is necessary to bypass the NVM management > > and directly force the MAC address using a kernel command-line > > parameter (macaddr). > > Hi Flavio > > Why not use device tree, since this is an ARM platform? Hi Andrew, we produce a lot of boards and we have to change the MAC address, from u-boot, for every board. So I must save in the u-boot environment (SPI NOR flash) the MAC address for every board. Flavio > > Andrew
> Hi Andrew, > > we produce a lot of boards and we have to change the MAC address, > from u-boot, for every board. So I must save in the u-boot > environment (SPI NOR flash) the MAC address for every board. Hi Flavio u-boot should be able to write the MAC address in the correct part of device tree. Boards have been doing this a long time. Module parameters are considered bad. You should only do it if you have no other option. Here you do have another options, so it is going to be a hard sell getting David to access your patch. You will have more success by adding a call to eth_platform_get_mac_address() to the e1000e driver. Andrew
> > Hi Andrew, > > > > we produce a lot of boards and we have to change the MAC address, > > from u-boot, for every board. So I must save in the u-boot > > environment (SPI NOR flash) the MAC address for every board. > > Hi Flavio > > u-boot should be able to write the MAC address in the correct part of > device tree. Boards have been doing this a long time. > > Module parameters are considered bad. You should only do it if you > have no other option. Here you do have another options, so it is going > to be a hard sell getting David to access your patch. > > You will have more success by adding a call to > eth_platform_get_mac_address() to the e1000e driver. You have right, and thanks for your suggestions, but with a kernel parameter I can use the same method for any board where the NVM is missed, independently of any architecture (with or without the device tree presence - ARM or x86 or others). In fact slso the drivers/net/ethernet/freescale/fec_main.c provides a kernel parameter for the MAC address, in fec_get_mac: /* * try to get mac address in following order: * * 1) module parameter via kernel command line in form * fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0 */ iap = macaddr; Thanks, Flavio > Andrew
On Thu, Feb 28, 2019 at 05:13:27PM +0000, Flavio Suligoi wrote: > > > Hi Andrew, > > > > > > we produce a lot of boards and we have to change the MAC address, > > > from u-boot, for every board. So I must save in the u-boot > > > environment (SPI NOR flash) the MAC address for every board. > > > > Hi Flavio > > > > u-boot should be able to write the MAC address in the correct part of > > device tree. Boards have been doing this a long time. > > > > Module parameters are considered bad. You should only do it if you > > have no other option. Here you do have another options, so it is going > > to be a hard sell getting David to access your patch. > > > > You will have more success by adding a call to > > eth_platform_get_mac_address() to the e1000e driver. > > You have right, and thanks for your suggestions, > but with a kernel parameter I can use the same method > for any board where the NVM is missed, independently of any architecture > (with or without the device tree presence - ARM or x86 or others). Hi Flavio Well, lets wait for David to say what he thinks about the module parameter. Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: Thu, 28 Feb 2019 20:29:58 +0100 > On Thu, Feb 28, 2019 at 05:13:27PM +0000, Flavio Suligoi wrote: >> > > Hi Andrew, >> > > >> > > we produce a lot of boards and we have to change the MAC address, >> > > from u-boot, for every board. So I must save in the u-boot >> > > environment (SPI NOR flash) the MAC address for every board. >> > >> > Hi Flavio >> > >> > u-boot should be able to write the MAC address in the correct part of >> > device tree. Boards have been doing this a long time. >> > >> > Module parameters are considered bad. You should only do it if you >> > have no other option. Here you do have another options, so it is going >> > to be a hard sell getting David to access your patch. >> > >> > You will have more success by adding a call to >> > eth_platform_get_mac_address() to the e1000e driver. >> >> You have right, and thanks for your suggestions, >> but with a kernel parameter I can use the same method >> for any board where the NVM is missed, independently of any architecture >> (with or without the device tree presence - ARM or x86 or others). > > Hi Flavio > > Well, lets wait for David to say what he thinks about the module > parameter. I already rejected this, no way... Drivers that already have the unacceptable module parameter are no an argument for spreading this mistake further.
> >> > Hi Flavio > >> > > >> > u-boot should be able to write the MAC address in the correct part of > >> > device tree. Boards have been doing this a long time. > >> > > >> > Module parameters are considered bad. You should only do it if you > >> > have no other option. Here you do have another options, so it is > going > >> > to be a hard sell getting David to access your patch. > >> > > >> > You will have more success by adding a call to > >> > eth_platform_get_mac_address() to the e1000e driver. > >> > >> You have right, and thanks for your suggestions, > >> but with a kernel parameter I can use the same method > >> for any board where the NVM is missed, independently of any > architecture > >> (with or without the device tree presence - ARM or x86 or others). > > > > Hi Flavio > > > > Well, lets wait for David to say what he thinks about the module > > parameter. > > I already rejected this, no way... Drivers that already have the > unacceptable module parameter are no an argument for spreading this > mistake further. Hi David and Andrew, ok, thank you for your suggestions and your time! Flavio
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 189f231..34ab560 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -39,6 +39,10 @@ static int debug = -1; module_param(debug, int, 0); MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); +static unsigned char macaddr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; +module_param_array(macaddr, byte, NULL, 0); +MODULE_PARM_DESC(macaddr, "e1000e Ethernet MAC address"); + static const struct e1000_info *e1000_info_tbl[] = { [board_82571] = &e1000_82571_info, [board_82572] = &e1000_82572_info, @@ -7232,27 +7236,38 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) */ adapter->hw.mac.ops.reset_hw(&adapter->hw); - /* systems with ASPM and others may see the checksum fail on the first - * attempt. Let's give it a few tries - */ - for (i = 0;; i++) { - if (e1000_validate_nvm_checksum(&adapter->hw) >= 0) - break; - if (i == 2) { - dev_err(&pdev->dev, "The NVM Checksum Is Not Valid\n"); - err = -EIO; - goto err_eeprom; + /* If the MAC address is supplied by the bootloader, use it! */ + if (!is_multicast_ether_addr(macaddr)) { + dev_info(&pdev->dev, + "MAC address from kernel command line argument\n"); + memcpy(adapter->hw.mac.addr, macaddr, netdev->addr_len); + memcpy(netdev->dev_addr, macaddr, netdev->addr_len); + } else { + /* systems with ASPM and others may see the checksum fail on + * the first attempt. Let's give it a few tries + */ + dev_info(&pdev->dev, "MAC address from NVM\n"); + for (i = 0;; i++) { + if (e1000_validate_nvm_checksum(&adapter->hw) >= 0) + break; + if (i == 2) { + dev_err(&pdev->dev, + "The NVM Checksum Is Not Valid\n"); + err = -EIO; + goto err_eeprom; + } } - } - e1000_eeprom_checks(adapter); + e1000_eeprom_checks(adapter); - /* copy the MAC address */ - if (e1000e_read_mac_addr(&adapter->hw)) - dev_err(&pdev->dev, - "NVM Read Error while reading MAC address\n"); + /* copy the MAC address */ + if (e1000e_read_mac_addr(&adapter->hw)) + dev_err(&pdev->dev, + "NVM Read Error while reading MAC address\n"); - memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len); + memcpy(netdev->dev_addr, adapter->hw.mac.addr, + netdev->addr_len); + } if (!is_valid_ether_addr(netdev->dev_addr)) { dev_err(&pdev->dev, "Invalid MAC Address: %pM\n",
Sometimes, in some embedded systems boards (i.e. ARM boards), the NVM eeprom is not mounted, to save cost and space. In this case it is necessary to bypass the NVM management and directly force the MAC address using a kernel command-line parameter (macaddr). Signed-off-by: Flavio Suligoi <f.suligoi@asem.it> --- v3: - move patch changelog in the right place v2: - e1000_probe: remove wrong newline in the middle of the local variable list v1: - written drivers/net/ethernet/intel/e1000e/netdev.c | 49 +++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-)