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

[Joe Hershberger]

On Thu, Oct 25, 2018 at 9:26 AM Hannes Schmelzer
<hannes.schmelzer at 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 at 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot