Re: [PATCH u-boot 5/5] net: aspeednic: Do not fill in a random MAC address

[Cyril Bur <cyrilbur@gmail.com>]

On Fri, 11 Mar 2016 12:40:44 -0600
OpenBMC Patches <openbmc-patches@stwcx.xyz> wrote:

> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 

Hi Milton,

Series looks nice.

One question about this patch, should you also have removed the call to
aspeednick_write_hwaddr() from aspeednic_initialize() since the eth layer will
do it for you? Not a bit deal at all, the eth layer will also detect that it's
been done and just skip it?



> Do not check the u-boot environemnt or fill in a random address
> in the write_hwaddr hook, instead rely on the framework to do so.
> 
> The doc/README.ethaddr specifically states that random addresses
> are only to be assigned as part of a emergency such as a netboot
> recovery command.
> 
> The upstream commit created a config variable to assign a
> random mac when none is set leaving it zero and that has now
> been backported.
> 
> Note: The hardware address is reset to 0 as part of the ethernet
> reset performed at boot.  If no valid MAC address is found in
> the environment the hardware will contain zeros and the operating
> system will assign a valid random MAC address if u-boot is
> configured not to.
> 
> The net effect is an attempt to use the network will result in
> the ethernet address not set warning being printed if the ethaddr
> variable is not set, and a warning iwth the random mac address
> if the config is set.  If a valid ethernet address is set
> in the environment it will be programmed in the hardware and
> used by the operating system.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  drivers/net/aspeednic.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/net/aspeednic.c b/drivers/net/aspeednic.c
> index 841df8b..e80561f 100644
> --- a/drivers/net/aspeednic.c
> +++ b/drivers/net/aspeednic.c
> @@ -1363,10 +1363,6 @@ static void aspeednic_halt(struct eth_device* dev)
>  
>  static int aspeednic_write_hwaddr(struct eth_device* dev)
>  {
> -  if (!eth_getenv_enetaddr_by_index("eth", 0, dev->enetaddr)) {
> -    eth_random_enetaddr(dev->enetaddr);
> -  }
> -
>    OUTL(dev, ((dev->enetaddr[2] << 24) | (dev->enetaddr[3] << 16)
>               | (dev->enetaddr[4] << 8) | dev->enetaddr[5]), MAC_LADR_REG);
>    OUTL(dev, ((dev->enetaddr[0] << 8) | dev->enetaddr[1]), MAC_MADR_REG);