[U-Boot] [PATCH v2] net: e1000: support 'write_hwaddr' in DM

This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com ---
Changes in v2: - fix build for non-DM board - rebase on current master
drivers/net/e1000.c | 89 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index a34f697..3df9999 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -862,7 +862,6 @@ e1000_read_eeprom(struct e1000_hw *hw, uint16_t offset, return E1000_SUCCESS; }
-#ifndef CONFIG_DM_ETH /****************************************************************************** * e1000_write_eeprom_srwr - Write to Shadow Ram using EEWR * @hw: pointer to the HW structure @@ -1028,7 +1027,6 @@ static int32_t e1000_update_eeprom_checksum_i210(struct e1000_hw *hw) out: return ret_val; } -#endif
/****************************************************************************** * Verifies that the EEPROM has a valid checksum @@ -5488,6 +5486,53 @@ void e1000_get_bus_type(struct e1000_hw *hw) }
#ifndef CONFIG_DM_ETH +static int e1000_write_hwaddr(struct eth_device *dev) +#else +static int e1000_write_hwaddr(struct udevice *dev) +#endif +{ +#ifndef CONFIG_E1000_NO_NVM +#ifndef CONFIG_DM_ETH + unsigned char *mac = dev->enetaddr; + struct e1000_hw *hw = dev->priv; +#else + struct eth_pdata *plat = dev_get_platdata(dev); + unsigned char *mac = plat->enetaddr; + struct e1000_hw *hw = dev_get_priv(dev); +#endif + unsigned char current_mac[6]; + u16 data[3]; + int ret_val, i; + + DEBUGOUT("%s: mac=%pM\n", __func__, mac); + + memset(current_mac, 0, 6); + + /* Read from EEPROM, not from registers, to make sure + * the address is persistently configured + */ + ret_val = e1000_read_mac_addr_from_eeprom(hw, current_mac); + DEBUGOUT("%s: current mac=%pM\n", __func__, current_mac); + + /* Only write to EEPROM if the given address is different or + * reading the current address failed + */ + if (!ret_val && memcmp(current_mac, mac, 6) == 0) + return 0; + + for (i = 0; i < 3; ++i) + data[i] = mac[i * 2 + 1] << 8 | mac[i * 2]; + + ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data); + if (!ret_val) + ret_val = e1000_update_eeprom_checksum_i210(hw); + return ret_val; +#else /* CONFIG_E1000_NO_NVM */ + return 0; +#endif +} + +#ifndef CONFIG_DM_ETH /* A list of all registered e1000 devices */ static LIST_HEAD(e1000_hw_list); #endif @@ -5649,45 +5694,6 @@ e1000_poll(struct eth_device *nic) return len ? 1 : 0; }
-static int e1000_write_hwaddr(struct eth_device *dev) -{ -#ifndef CONFIG_E1000_NO_NVM - unsigned char *mac = dev->enetaddr; - unsigned char current_mac[6]; - struct e1000_hw *hw = dev->priv; - uint16_t data[3]; - int ret_val, i; - - DEBUGOUT("%s: mac=%pM\n", __func__, mac); - - memset(current_mac, 0, 6); - - /* Read from EEPROM, not from registers, to make sure - * the address is persistently configured - */ - ret_val = e1000_read_mac_addr_from_eeprom(hw, current_mac); - DEBUGOUT("%s: current mac=%pM\n", __func__, current_mac); - - /* Only write to EEPROM if the given address is different or - * reading the current address failed - */ - if (!ret_val && memcmp(current_mac, mac, 6) == 0) - return 0; - - for (i = 0; i < 3; ++i) - data[i] = mac[i * 2 + 1] << 8 | mac[i * 2]; - - ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data); - - if (!ret_val) - ret_val = e1000_update_eeprom_checksum_i210(hw); - - return ret_val; -#else - return 0; -#endif -} - /************************************************************************** PROBE - Look for an adapter, this routine's visible to the outside You should omit the last argument struct pci_device * for a non-PCI NIC @@ -5914,6 +5920,7 @@ static const struct eth_ops e1000_eth_ops = { .recv = e1000_eth_recv, .stop = e1000_eth_stop, .free_pkt = e1000_free_pkt, + .write_hwaddr = e1000_write_hwaddr, };
static const struct udevice_id e1000_eth_ids[] = {

On Thu, Oct 25, 2018 at 9:26 AM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Changes in v2:
- fix build for non-DM board
- rebase on current master
drivers/net/e1000.c | 89 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index a34f697..3df9999 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -862,7 +862,6 @@ e1000_read_eeprom(struct e1000_hw *hw, uint16_t offset, return E1000_SUCCESS; }
-#ifndef CONFIG_DM_ETH /******************************************************************************
- e1000_write_eeprom_srwr - Write to Shadow Ram using EEWR
- @hw: pointer to the HW structure
@@ -1028,7 +1027,6 @@ static int32_t e1000_update_eeprom_checksum_i210(struct e1000_hw *hw) out: return ret_val; } -#endif
/******************************************************************************
- Verifies that the EEPROM has a valid checksum
@@ -5488,6 +5486,53 @@ void e1000_get_bus_type(struct e1000_hw *hw) }
#ifndef CONFIG_DM_ETH
Please don't use negative logic when an else is present. Switch the cases and use #ifdef.
+static int e1000_write_hwaddr(struct eth_device *dev) +#else +static int e1000_write_hwaddr(struct udevice *dev) +#endif +{ +#ifndef CONFIG_E1000_NO_NVM
Same here
+#ifndef CONFIG_DM_ETH
Same here
unsigned char *mac = dev->enetaddr;
struct e1000_hw *hw = dev->priv;
+#else
struct eth_pdata *plat = dev_get_platdata(dev);
unsigned char *mac = plat->enetaddr;
struct e1000_hw *hw = dev_get_priv(dev);
+#endif
unsigned char current_mac[6];
u16 data[3];
int ret_val, i;
DEBUGOUT("%s: mac=%pM\n", __func__, mac);
memset(current_mac, 0, 6);
/* Read from EEPROM, not from registers, to make sure
Please use a /* on a blank line for multi-line comments.
* the address is persistently configured
*/
ret_val = e1000_read_mac_addr_from_eeprom(hw, current_mac);
DEBUGOUT("%s: current mac=%pM\n", __func__, current_mac);
/* Only write to EEPROM if the given address is different or
Please use a /* on a blank line for multi-line comments.
* reading the current address failed
*/
if (!ret_val && memcmp(current_mac, mac, 6) == 0)
return 0;
for (i = 0; i < 3; ++i)
data[i] = mac[i * 2 + 1] << 8 | mac[i * 2];
ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data);
This is a pretty uncommon implementation of this driver operation. Does the i120 in this case read the MAC directly from the EEPROM?
Usually this function is used to write the hwaddr into the registers of the MAC. The .read_rom_hwaddr is used to fetch the hwaddr from the EEPROM and that address is used in the case that the U-Boot env has not set one.
if (!ret_val)
ret_val = e1000_update_eeprom_checksum_i210(hw);
return ret_val;
+#else /* CONFIG_E1000_NO_NVM */
return 0;
+#endif +}
+#ifndef CONFIG_DM_ETH /* A list of all registered e1000 devices */ static LIST_HEAD(e1000_hw_list); #endif @@ -5649,45 +5694,6 @@ e1000_poll(struct eth_device *nic) return len ? 1 : 0; }
-static int e1000_write_hwaddr(struct eth_device *dev) -{ -#ifndef CONFIG_E1000_NO_NVM
unsigned char *mac = dev->enetaddr;
unsigned char current_mac[6];
struct e1000_hw *hw = dev->priv;
uint16_t data[3];
int ret_val, i;
DEBUGOUT("%s: mac=%pM\n", __func__, mac);
memset(current_mac, 0, 6);
/* Read from EEPROM, not from registers, to make sure
* the address is persistently configured
*/
ret_val = e1000_read_mac_addr_from_eeprom(hw, current_mac);
DEBUGOUT("%s: current mac=%pM\n", __func__, current_mac);
/* Only write to EEPROM if the given address is different or
* reading the current address failed
*/
if (!ret_val && memcmp(current_mac, mac, 6) == 0)
return 0;
for (i = 0; i < 3; ++i)
data[i] = mac[i * 2 + 1] << 8 | mac[i * 2];
ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data);
if (!ret_val)
ret_val = e1000_update_eeprom_checksum_i210(hw);
return ret_val;
-#else
return 0;
-#endif -}
/************************************************************************** PROBE - Look for an adapter, this routine's visible to the outside You should omit the last argument struct pci_device * for a non-PCI NIC @@ -5914,6 +5920,7 @@ static const struct eth_ops e1000_eth_ops = { .recv = e1000_eth_recv, .stop = e1000_eth_stop, .free_pkt = e1000_free_pkt,
.write_hwaddr = e1000_write_hwaddr,
};
static const struct udevice_id e1000_eth_ids[] = {
2.7.4
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On 10/25/2018 08:41 PM, Joe Hershberger wrote:
On Thu, Oct 25, 2018 at 9:26 AM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
Hi Joe,
* reading the current address failed
*/
if (!ret_val && memcmp(current_mac, mac, 6) == 0)
return 0;
for (i = 0; i < 3; ++i)
data[i] = mac[i * 2 + 1] << 8 | mac[i * 2];
ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data);
This is a pretty uncommon implementation of this driver operation. Does the i120 in this case read the MAC directly from the EEPROM?
Usually this function is used to write the hwaddr into the registers of the MAC. The .read_rom_hwaddr is used to fetch the hwaddr from the EEPROM and that address is used in the case that the U-Boot env has not set one.
This implementation is not new, just moved a bit upward in the file to avoid forward declaration for non-DM build. I will double check it, but i guess there will be no change until we've no fail behavior of the existing code.
I didn't imagine that this little change would become so big ;-) I will provide some v3 within next days with the cosmetic cleanup and other requested changes.
cheers, Hannes

On Thu, Oct 25, 2018 at 2:34 PM Hannes Schmelzer hannes@schmelzer.or.at wrote:
On 10/25/2018 08:41 PM, Joe Hershberger wrote:
On Thu, Oct 25, 2018 at 9:26 AM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
Hi Joe,
* reading the current address failed
*/
if (!ret_val && memcmp(current_mac, mac, 6) == 0)
return 0;
for (i = 0; i < 3; ++i)
data[i] = mac[i * 2 + 1] << 8 | mac[i * 2];
ret_val = e1000_write_eeprom_srwr(hw, 0x0, 3, data);
This is a pretty uncommon implementation of this driver operation. Does the i120 in this case read the MAC directly from the EEPROM?
Usually this function is used to write the hwaddr into the registers of the MAC. The .read_rom_hwaddr is used to fetch the hwaddr from the EEPROM and that address is used in the case that the U-Boot env has not set one.
This implementation is not new, just moved a bit upward in the file to avoid forward declaration for non-DM build.
Yes, I understand... I just brought it up since I was looking at it and maybe you had some insight.
I will double check it, but i guess there will be no change until we've no fail behavior of the existing code.
What do you mean here?
I didn't imagine that this little change would become so big ;-)
:) Sorry, wasn't meaning for it to be big... my comment about the current implementation is not meant to block this change.
I will provide some v3 within next days with the cosmetic cleanup and other requested changes.
Thanks!
cheers, Hannes
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Thu, Oct 25, 2018 at 10:25 PM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Changes in v2:
- fix build for non-DM board
- rebase on current master
drivers/net/e1000.c | 89 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
FYI
I was trying to port the most recent Linux kernel e1000e driver to U-Boot. Current e1000 driver in U-Boot is way out of sync to Linux kernel and it has many many ad-hoc hacks here and there to support newer Intel NICs.
Regards, Bin

On 10/30/2018 09:13 AM, Bin Meng wrote:
On Thu, Oct 25, 2018 at 10:25 PM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Changes in v2:
fix build for non-DM board
rebase on current master
drivers/net/e1000.c | 89 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
FYI
I was trying to port the most recent Linux kernel e1000e driver to U-Boot. Current e1000 driver in U-Boot is way out of sync to Linux kernel and it has many many ad-hoc hacks here and there to support newer Intel NICs.
Regards, Bin
good to know. is this work still in progress ? maybe i should wait with my changes. pls let me know if there will be some progress next time, otherwise i will move on to bring in my changes as planned.
cheers, Hannes

On 30.10.2018 13:06, Hannes Schmelzer wrote:
On 10/30/2018 09:13 AM, Bin Meng wrote:
On Thu, Oct 25, 2018 at 10:25 PM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Changes in v2:
- fix build for non-DM board
- rebase on current master
drivers/net/e1000.c | 89 +++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
FYI
I was trying to port the most recent Linux kernel e1000e driver to U-Boot. Current e1000 driver in U-Boot is way out of sync to Linux kernel and it has many many ad-hoc hacks here and there to support newer Intel NICs.
Regards, Bin
good to know. is this work still in progress ? maybe i should wait with my changes. pls let me know if there will be some progress next time, otherwise i will move on to bring in my changes as planned.
cheers, Hannes
Bin,
do you plan to have some progress next time or should i move on with my changes? cheers, Hannes

Hi Hannes,
On Fri, Nov 23, 2018 at 3:34 PM Hannes Schmelzer hannes@schmelzer.or.at wrote:
On 30.10.2018 13:06, Hannes Schmelzer wrote:
On 10/30/2018 09:13 AM, Bin Meng wrote:
On Thu, Oct 25, 2018 at 10:25 PM Hannes Schmelzer hannes.schmelzer@br-automation.com wrote:
This commit ports the existing (non-DM) function for writing the MAC- address into the shadow ram (and flash) for DM.
Signed-off-by: Hannes Schmelzer hannes.schmelzer@br-automation.com
Changes in v2:
fix build for non-DM board
rebase on current master
drivers/net/e1000.c | 89
+++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 41 deletions(-)
FYI
I was trying to port the most recent Linux kernel e1000e driver to U-Boot. Current e1000 driver in U-Boot is way out of sync to Linux kernel and it has many many ad-hoc hacks here and there to support newer Intel NICs.
Regards, Bin
good to know. is this work still in progress ? maybe i should wait with my changes. pls let me know if there will be some progress next time, otherwise i will move on to bring in my changes as planned.
cheers, Hannes
Bin,
do you plan to have some progress next time or should i move on with my changes?
Please move on with your changes. I did not have enough time to continue the e1000e driver port.
Regards, Bin
participants (4)
-
Bin Meng
-
Hannes Schmelzer
-
Hannes Schmelzer
-
Joe Hershberger