[U-Boot] [PATCH] Revert "env: net: Move eth_parse_enetaddr() to net.c/h"

[Ondřej Jirman]

On Fri, Sep 13, 2019 at 01:19:18PM +0200, Heinrich Schuchardt wrote:
> On 9/13/19 1:48 AM, megous at megous.com wrote:
> > From: Ondrej Jirman <megous at megous.com>
> > 
> > The reverted patch causes linking error with disabled CONFIG_NET:
> > 
> >    cmd/built-in.o: In function `eth_env_get_enetaddr':
> >    u-boot-v2019.10/cmd/nvedit.c:363: undefined reference to `eth_parse_enetaddr'
> > 
> > Function setup_environment() in board/sunxi/board.c calls
> > eth_env_set_enetaddr() to setup stable mac address for ethernet interfaces.
> > 
> > This needs to be implemented and succeed even if net is disabled in u-boot,
> > as it ensures Linux will not generate random MAC addresses, and picks the
> > ones provided by u-boot via DT. See fdt_fixup_ethernet().
> > 
> > This feature is independent of the whole network stack and network drivers
> > in u-boot.
> > 
> > This revert fixes the linking error.
> > 
> > Signed-off-by: Ondrej Jirman <megous at megous.com>
> > ---
> >   cmd/nvedit.c           | 12 ++++++++++++
> >   include/env_internal.h | 11 +++++++++++
> >   include/net.h          | 11 -----------
> >   net/net.c              | 12 ------------
> >   4 files changed, 23 insertions(+), 23 deletions(-)
> > 
> > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> > index 1cb0bc1460..399f6d6ce1 100644
> > --- a/cmd/nvedit.c
> > +++ b/cmd/nvedit.c
> > @@ -358,6 +358,18 @@ ulong env_get_hex(const char *varname, ulong default_val)
> >   	return value;
> >   }
> > 
> > +void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr)
> > +{
> > +	char *end;
> > +	int i;
> > +
> > +	for (i = 0; i < 6; ++i) {
> > +		enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> > +		if (addr)
> > +			addr = (*end) ? end + 1 : end;
> > +	}
> > +}
> > +
> >   int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr)
> >   {
> >   	eth_parse_enetaddr(env_get(name), enetaddr);
> > diff --git a/include/env_internal.h b/include/env_internal.h
> > index b1ddcb5adf..27eb5bd1e7 100644
> > --- a/include/env_internal.h
> > +++ b/include/env_internal.h
> 
> Please, don't move the definition to env_internal.h but to env.h as
> board/renesas/sh7753evb/sh7753evb.c and others are using
> eth_parse_enetaddr().
> 
> env_internal.h explicitly states "It should not be included by board files".
> 
> Please, execute Travis CI tests to ensure you do not break any other board.

I haven't found any documentation in the tree or on the u-boot website on
how to do that.

regards,
	o.

> Best regards
> 
> Heinrich
> 
> > @@ -273,6 +273,17 @@ struct env_driver {
> > 
> >   extern struct hsearch_data env_htab;
> > 
> > +/**
> > + * eth_parse_enetaddr() - Parse a MAC address
> > + *
> > + * Convert a string MAC address
> > + *
> > + * @addr: MAC address in aa:bb:cc:dd:ee:ff format, where each part is a 2-digit
> > + *	hex value
> > + * @enetaddr: Place to put MAC address (6 bytes)
> > + */
> > +void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr);
> > +
> >   #endif /* DO_DEPS_ONLY */
> > 
> >   #endif /* _ENV_INTERNAL_H_ */
> > diff --git a/include/net.h b/include/net.h
> > index 75a16e4c8f..e208cc43a0 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -875,15 +875,4 @@ int update_tftp(ulong addr, char *interface, char *devstring);
> > 
> >   /**********************************************************************/
> > 
> > -/**
> > - * eth_parse_enetaddr() - Parse a MAC address
> > - *
> > - * Convert a string MAC address
> > - *
> > - * @addr: MAC address in aa:bb:cc:dd:ee:ff format, where each part is a 2-digit
> > - *	hex value
> > - * @enetaddr: Place to put MAC address (6 bytes)
> > - */
> > -void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr);
> > -
> >   #endif /* __NET_H__ */
> > diff --git a/net/net.c b/net/net.c
> > index ded86e7456..4d2b7ead3b 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1628,15 +1628,3 @@ ushort env_get_vlan(char *var)
> >   {
> >   	return string_to_vlan(env_get(var));
> >   }
> > -
> > -void eth_parse_enetaddr(const char *addr, uint8_t *enetaddr)
> > -{
> > -	char *end;
> > -	int i;
> > -
> > -	for (i = 0; i < 6; ++i) {
> > -		enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> > -		if (addr)
> > -			addr = (*end) ? end + 1 : end;
> > -	}
> > -}
> > 
>