Hi,

On Fri, Sep 13, 2019 at 07:40:22PM -0500, Joe Hershberger wrote:
> Part of the env cleanup moved this out of the environment code and into
> the net code. However, this helper is sometimes needed even when the net
> stack isn't included.
> 
> Move the helper to lib/net_utils.c like it's similarly-purposed
> string_to_ip(). Also rename the moved function to similar naming.
> 
> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com>
> Reported-by: Ondrej Jirman <meg...@megous.com>

I've tested the patch and it works, but I'be found other related issue, where
u-boot thinks %pM will format a MAC address string, but it does just
print out the pointer due to relevant functions being gated by CONFIG_CMD_NET
guard in lib/vsprintf.c.

The gating should probably be done so that it panics/halts the u-boot if gated
pointer flags are used by u-boot code, because that will clearly be incorrect,
without calling code ever knowing. This way the user will know that something
is wrong and will have to fix the code.

I noticed be cause produced mac address is wrong.

Quick fix is to remove the CONFIG_CMD_NET gate.

regards,
        o.

> 
> ---
> 
>  arch/arm/mach-tegra/cboot.c         |  2 +-
>  board/renesas/sh7752evb/sh7752evb.c |  2 +-
>  board/renesas/sh7753evb/sh7753evb.c |  2 +-
>  board/renesas/sh7757lcr/sh7757lcr.c |  4 ++--
>  cmd/ethsw.c                         |  2 +-
>  cmd/nvedit.c                        |  2 +-
>  doc/README.enetaddr                 |  4 ++--
>  include/net.h                       | 24 +++++++++++++-----------
>  lib/net_utils.c                     | 15 +++++++++++++++
>  net/eth-uclass.c                    |  2 +-
>  net/eth_legacy.c                    |  2 +-
>  net/net.c                           | 12 ------------
>  12 files changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/cboot.c b/arch/arm/mach-tegra/cboot.c
> index 0433081c6c..0762144ecf 100644
> --- a/arch/arm/mach-tegra/cboot.c
> +++ b/arch/arm/mach-tegra/cboot.c
> @@ -495,7 +495,7 @@ static int cboot_get_ethaddr_legacy(const void *fdt, 
> uint8_t mac[ETH_ALEN])
>               return -ENOENT;
>       }
>  
> -     eth_parse_enetaddr(prop, mac);
> +     string_to_enetaddr(prop, mac);
>  
>       if (!is_valid_ethaddr(mac)) {
>               printf("Invalid MAC address: %s\n", prop);
> diff --git a/board/renesas/sh7752evb/sh7752evb.c 
> b/board/renesas/sh7752evb/sh7752evb.c
> index d0b850f35d..b4292b22e7 100644
> --- a/board/renesas/sh7752evb/sh7752evb.c
> +++ b/board/renesas/sh7752evb/sh7752evb.c
> @@ -93,7 +93,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, 
> char *mac_string)
>       unsigned char mac[6];
>       unsigned long val;
>  
> -     eth_parse_enetaddr(mac_string, mac);
> +     string_to_enetaddr(mac_string, mac);
>  
>       if (!channel)
>               ether = GETHER0_MAC_BASE;
> diff --git a/board/renesas/sh7753evb/sh7753evb.c 
> b/board/renesas/sh7753evb/sh7753evb.c
> index e1bed7dcc3..5aebb041b0 100644
> --- a/board/renesas/sh7753evb/sh7753evb.c
> +++ b/board/renesas/sh7753evb/sh7753evb.c
> @@ -100,7 +100,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, 
> char *mac_string)
>       unsigned char mac[6];
>       unsigned long val;
>  
> -     eth_parse_enetaddr(mac_string, mac);
> +     string_to_enetaddr(mac_string, mac);
>  
>       if (!channel)
>               ether = GETHER0_MAC_BASE;
> diff --git a/board/renesas/sh7757lcr/sh7757lcr.c 
> b/board/renesas/sh7757lcr/sh7757lcr.c
> index d2671202e9..662c435f7a 100644
> --- a/board/renesas/sh7757lcr/sh7757lcr.c
> +++ b/board/renesas/sh7757lcr/sh7757lcr.c
> @@ -140,7 +140,7 @@ static void set_mac_to_sh_eth_register(int channel, char 
> *mac_string)
>       unsigned char mac[6];
>       unsigned long val;
>  
> -     eth_parse_enetaddr(mac_string, mac);
> +     string_to_enetaddr(mac_string, mac);
>  
>       if (!channel)
>               ether = ETHER0_MAC_BASE;
> @@ -159,7 +159,7 @@ static void set_mac_to_sh_giga_eth_register(int channel, 
> char *mac_string)
>       unsigned char mac[6];
>       unsigned long val;
>  
> -     eth_parse_enetaddr(mac_string, mac);
> +     string_to_enetaddr(mac_string, mac);
>  
>       if (!channel)
>               ether = GETHER0_MAC_BASE;
> diff --git a/cmd/ethsw.c b/cmd/ethsw.c
> index 8846805799..8d271ce1f3 100644
> --- a/cmd/ethsw.c
> +++ b/cmd/ethsw.c
> @@ -864,7 +864,7 @@ static int keyword_match_mac_addr(enum ethsw_keyword_id 
> key_id, int argc,
>               return 0;
>       }
>  
> -     eth_parse_enetaddr(argv[*argc_nr + 1], parsed_cmd->ethaddr);
> +     string_to_enetaddr(argv[*argc_nr + 1], parsed_cmd->ethaddr);
>  
>       if (is_broadcast_ethaddr(parsed_cmd->ethaddr)) {
>               memset(parsed_cmd->ethaddr, 0xFF, sizeof(parsed_cmd->ethaddr));
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 1cb0bc1460..1e4b225a94 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -360,7 +360,7 @@ ulong env_get_hex(const char *varname, ulong default_val)
>  
>  int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr)
>  {
> -     eth_parse_enetaddr(env_get(name), enetaddr);
> +     string_to_enetaddr(env_get(name), enetaddr);
>       return is_valid_ethaddr(enetaddr);
>  }
>  
> diff --git a/doc/README.enetaddr b/doc/README.enetaddr
> index f926485986..5baa9f2179 100644
> --- a/doc/README.enetaddr
> +++ b/doc/README.enetaddr
> @@ -76,12 +76,12 @@ To assist in the management of these layers, a few helper 
> functions exist.  You
>  should use these rather than attempt to do any kind of parsing/manipulation
>  yourself as many common errors have arisen in the past.
>  
> -     * void eth_parse_enetaddr(const char *addr, uchar *enetaddr);
> +     * void string_to_enetaddr(const char *addr, uchar *enetaddr);
>  
>  Convert a string representation of a MAC address to the binary version.
>  char *addr = "00:11:22:33:44:55";
>  uchar enetaddr[6];
> -eth_parse_enetaddr(addr, enetaddr);
> +string_to_enetaddr(addr, enetaddr);
>  /* enetaddr now equals { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 } */
>  
>       * int eth_env_get_enetaddr(char *name, uchar *enetaddr);
> diff --git a/include/net.h b/include/net.h
> index 75a16e4c8f..d054c5f694 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -825,6 +825,19 @@ static inline void net_random_ethaddr(uchar *addr)
>       addr[0] |= 0x02;        /* set local assignment bit (IEEE802) */
>  }
>  
> +/**
> + * string_to_enetaddr() - Parse a MAC address
> + *
> + * Convert a string MAC address
> + *
> + * Implemented in lib/net_utils.c (built unconditionally)
> + *
> + * @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 string_to_enetaddr(const char *addr, uint8_t *enetaddr);
> +
>  /* Convert an IP address to a string */
>  void ip_to_string(struct in_addr x, char *s);
>  
> @@ -875,15 +888,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/lib/net_utils.c b/lib/net_utils.c
> index 9fb9d4a4b0..ed5044c3de 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -41,3 +41,18 @@ struct in_addr string_to_ip(const char *s)
>       addr.s_addr = htonl(addr.s_addr);
>       return addr;
>  }
> +
> +void string_to_enetaddr(const char *addr, uint8_t *enetaddr)
> +{
> +     char *end;
> +     int i;
> +
> +     if (!enetaddr)
> +             return;
> +
> +     for (i = 0; i < 6; ++i) {
> +             enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> +             if (addr)
> +                     addr = (*end) ? end + 1 : end;
> +     }
> +}
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 3bd98b01ad..9fe4096120 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -227,7 +227,7 @@ static int on_ethaddr(const char *name, const char 
> *value, enum env_op op,
>               switch (op) {
>               case env_op_create:
>               case env_op_overwrite:
> -                     eth_parse_enetaddr(value, pdata->enetaddr);
> +                     string_to_enetaddr(value, pdata->enetaddr);
>                       eth_write_hwaddr(dev);
>                       break;
>               case env_op_delete:
> diff --git a/net/eth_legacy.c b/net/eth_legacy.c
> index 41f5263526..5d6b0d7d7f 100644
> --- a/net/eth_legacy.c
> +++ b/net/eth_legacy.c
> @@ -117,7 +117,7 @@ static int on_ethaddr(const char *name, const char 
> *value, enum env_op op,
>                       switch (op) {
>                       case env_op_create:
>                       case env_op_overwrite:
> -                             eth_parse_enetaddr(value, dev->enetaddr);
> +                             string_to_enetaddr(value, dev->enetaddr);
>                               eth_write_hwaddr(dev, "eth", dev->index);
>                               break;
>                       case env_op_delete:
> 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;
> -     }
> -}
> -- 
> 2.11.0
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to