drivers/net/vsc9953: Fix missing reserved register

These new operations allow manipulation of bitfields within a word by using a mask instead of width and shift values to extract/replace the bitfields.

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none; new patch;

 include/bitfield.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

At startup, the default configuration should be:
- enable HW learning on all ports (HW default);
- all ports are VLAN aware;
- all ports are members of VLAN 1;
- all ports have Port-based VLAN 1;
- on all ports, the switch is allowed to remove maximum one VLAN tag,
- on egress, the switch should add a VLAN tag if the frame is classified to a different VLAN than the port's Port-based VLAN;

Signed-off-by Johnson Leung <[email protected]>

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v2:
- removed Change-id field;

Changes for v3:
- moved the copyright update in the last patch;
- return -EBUSY if the VLAN table is not ready;
- each variable is declared on a new line, without tabs;
- renamed variable "set" to "set_member";
- removed unnecessary brackets;
- renamed some VSC9953_TAG_CFG_* macros;
- removed field_set() and field_get() macros;
- removed "CONFIG_" from some macros' name;
- removed "port_' from members of struct vsc9953_rew_port;
- modified a comment describing struct vsc9953_rew_reg;

 drivers/net/vsc9953.c | 253 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/vsc9953.h     |  56 +++++++++++
 2 files changed, 309 insertions(+)

This patch creates a flexible parser for Ethernet Switch configurations that should support complex commands. The parser searches for predefined keywords in the command and calls the proper function when a match is found. Also, the parser allows for optional keywords, such as "port", to apply the command on a port
or on all ports. For now, the defined commands are:
ethsw [port <port_no>] { enable | disable | show }

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- parser removed from previous patch:
"drivers/net/vsc9953: Refractor the parser for VSC9953 commands";
- removed member "err" from struct command_def;
- using CMD_RET_* macros instead of magic numbers;
- moved each variable declaration on a separate line, with a single space;
- the code from functions cmd_keywords*_check() should be more clear now;
- removed unused function command_def_cleanup();

 common/Makefile    |   1 +
 common/cmd_ethsw.c | 346 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/ethsw.h    |  48 ++++++++
 3 files changed, 395 insertions(+)
 create mode 100644 common/cmd_ethsw.c
 create mode 100644 include/ethsw.h

common/Makefile +1 -0

index d6c1d48..f0b4eec 100644
@@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o
 obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
 obj-$(CONFIG_CMD_DFU) += cmd_dfu.o
 obj-$(CONFIG_CMD_GPT) += cmd_gpt.o
+obj-$(CONFIG_CMD_ETHSW) += cmd_ethsw.o
 
 # Power
 obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o

common/cmd_ethsw.c new file +191 -0

new file mode 100644
index 0000000..ebeaae0
@@ -0,0 +1,346 @@
+/*
+ * Copyright 2015 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:      GPL-2.0+
+ *
+ * Ethernet Switch commands
+ */
+
+#include <common.h>
+#include <command.h>
+#include <errno.h>
+#include <ethsw.h>
+
+static const char *ethsw_name;
+
+static struct keywords_to_function {
+       enum ethsw_keyword_id cmd_keyword[ETHSW_MAX_CMD_PARAMS];
+       int cmd_func_offset;
+       int (*keyword_function)(struct ethsw_command_def *parsed_cmd);
+} ethsw_cmd_def[] = {
+               {
+                       .cmd_keyword = {
+                                       ethsw_id_enable,
+                                       ethsw_id_key_end,
+                       },
+                       .cmd_func_offset = offsetof(struct ethsw_command_func,
+                                                   port_enable),
+                       .keyword_function = NULL,
+               }, {
+                       .cmd_keyword = {
+                                       ethsw_id_disable,
+                                       ethsw_id_key_end,
+                       },
+                       .cmd_func_offset = offsetof(struct ethsw_command_func,
+                                                   port_disable),
+                       .keyword_function = NULL,
+               }, {
+                       .cmd_keyword = {
+                                       ethsw_id_show,
+                                       ethsw_id_key_end,
+                       },
+                       .cmd_func_offset = offsetof(struct ethsw_command_func,
+                                                   port_show),
+                       .keyword_function = NULL,
+               },
+};
+
+struct keywords_optional {
+       int cmd_keyword[ETHSW_MAX_CMD_PARAMS];
+} cmd_opt_def[] = {
+               {
+                               .cmd_keyword = {
+                                               ethsw_id_port,
+                                               ethsw_id_port_no,
+                                               ethsw_id_key_end,
+                               },
+               },
+};
+
+static int keyword_match_gen(enum ethsw_keyword_id key_id, int argc, char
+                            *const argv[], int *argc_nr,
+                            struct ethsw_command_def *parsed_cmd);
+static int keyword_match_port(enum ethsw_keyword_id key_id, int argc,
+                             char *const argv[], int *argc_nr,
+                             struct ethsw_command_def *parsed_cmd);
+
+/*
+ * Define properties for each keyword;
+ * keep the order synced with enum ethsw_keyword_id
+ */
+struct keyword_def {
+       const char *keyword_name;
+       int (*match)(enum ethsw_keyword_id key_id, int argc, char *const argv[],
+                    int *argc_nr, struct ethsw_command_def *parsed_cmd);
+} keyword[] = {
+               {
+                               .keyword_name = "help",
+                               .match = &keyword_match_gen,
+               }, {
+                               .keyword_name = "show",
+                               .match = &keyword_match_gen,
+               }, {
+                               .keyword_name = "port",
+                               .match = &keyword_match_port
+               },  {
+                               .keyword_name = "enable",
+                               .match = &keyword_match_gen,
+               }, {
+                               .keyword_name = "disable",
+                               .match = &keyword_match_gen,
+               },
+};
+
+/*
+ * Function used by an Ethernet Switch driver to set the functions
+ * that must be called by the parser when an ethsw command is given
+ */
+int ethsw_define_functions(const struct ethsw_command_func *cmd_func)
+{
+       int i;
+       void **aux_p;
+       int (*cmd_func_aux)(struct ethsw_command_def *);
+
+       if (!cmd_func->ethsw_name)
+               return -EINVAL;
+
+       ethsw_name = cmd_func->ethsw_name;
+
+       for (i = 0; i < ARRAY_SIZE(ethsw_cmd_def); i++) {
+               /*
+                * get the pointer to the function send by the Ethernet Switch
+                * driver that corresponds to the proper ethsw command
+                */
+               if (ethsw_cmd_def[i].keyword_function)
+                       continue;
+
+               aux_p = (void *)cmd_func + ethsw_cmd_def[i].cmd_func_offset;
+
+               cmd_func_aux = (int (*)(struct ethsw_command_def *)) *aux_p;
+               ethsw_cmd_def[i].keyword_function = cmd_func_aux;
+       }
+
+       return 0;
+}
+
+/* Generic function used to match a keyword only by a string */
+static int keyword_match_gen(enum ethsw_keyword_id key_id, int argc,
+                            char *const argv[], int *argc_nr,
+                            struct ethsw_command_def *parsed_cmd)
+{
+       if (strcmp(argv[*argc_nr], keyword[key_id].keyword_name) == 0) {
+               parsed_cmd->cmd_to_keywords[*argc_nr] = key_id;
+
+               return 1;
+       }
+       return 0;
+}
+
+/* Function used to match the command's port */
+static int keyword_match_port(enum ethsw_keyword_id key_id, int argc,
+                             char *const argv[], int *argc_nr,
+                             struct ethsw_command_def *parsed_cmd)
+{
+       unsigned long val;
+
+       if (!keyword_match_gen(key_id, argc, argv, argc_nr, parsed_cmd))
+               return 0;
+
+       if (*argc_nr + 1 >= argc)
+               return 0;
+
+       if (strict_strtoul(argv[*argc_nr + 1], 10, &val) != -EINVAL) {
+               parsed_cmd->port = val;
+               (*argc_nr)++;
+               parsed_cmd->cmd_to_keywords[*argc_nr] = ethsw_id_port_no;
+               return 1;
+       }
+
+       return 0;
+}
+
+/* Finds optional keywords and modifies *argc_va to skip them */
+static void cmd_keywords_opt_check(const struct ethsw_command_def *parsed_cmd,
+                                  int *argc_val)
+{
+       int i;
+       int keyw_opt_matched;
+       int argc_val_max;
+       int const *cmd_keyw_p;
+       int const *cmd_keyw_opt_p;
+
+       /* remember the best match */
+       argc_val_max = *argc_val;
+
+       /*
+        * check if our command's optional keywords match the optional
+        * keywords of an available command
+        */
+       for (i = 0; i < ARRAY_SIZE(ethsw_cmd_def); i++) {
+               keyw_opt_matched = 0;
+               cmd_keyw_p = &parsed_cmd->cmd_to_keywords[keyw_opt_matched];
+               cmd_keyw_opt_p = &cmd_opt_def[i].cmd_keyword[keyw_opt_matched];
+
+               /*
+                * increase the number of keywords that
+                * matched with a command
+                */
+               while (keyw_opt_matched + *argc_val <
+                      parsed_cmd->cmd_keywords_nr &&
+                      *cmd_keyw_opt_p != ethsw_id_key_end &&
+                      *(cmd_keyw_p + *argc_val) == *cmd_keyw_opt_p) {

common/cmd_ethsw.c new file +145 -0

+               if (keyw_opt_matched + *argc_val <=
+                   parsed_cmd->cmd_keywords_nr &&
+                   *cmd_keyw_opt_p == ethsw_id_key_end &&
+                   *argc_val + keyw_opt_matched > argc_val_max)
+                       argc_val_max = *argc_val + keyw_opt_matched;
+       }
+
+       *argc_val = argc_val_max;
+}
+
+/*
+ * Finds the function to call based on keywords and
+ * modifies *argc_va to skip them
+ */
+static void cmd_keywords_check(struct ethsw_command_def *parsed_cmd,
+                              int *argc_val)
+{
+       int i;
+       int keyw_matched;
+       int *cmd_keyw_p;
+       int *cmd_keyw_def_p;
+
+       /*
+        * check if our command's keywords match the
+        * keywords of an available command
+        */
+       for (i = 0; i < ARRAY_SIZE(ethsw_cmd_def); i++) {
+               keyw_matched = 0;
+               cmd_keyw_p = &parsed_cmd->cmd_to_keywords[keyw_matched];
+               cmd_keyw_def_p = &ethsw_cmd_def[i].cmd_keyword[keyw_matched];
+
+               /*
+                * increase the number of keywords that
+                * matched with a command
+                */
+               while (keyw_matched + *argc_val < parsed_cmd->cmd_keywords_nr &&
+                      *cmd_keyw_def_p != ethsw_id_key_end &&
+                      *(cmd_keyw_p + *argc_val) == *cmd_keyw_def_p) {
+                       keyw_matched++;
+                       cmd_keyw_p++;
+                       cmd_keyw_def_p++;
+               }
+
+               /*
+                * if all our command's keywords perfectly match an
+                * available command, then we get the function we need to call
+                * to configure the Ethernet Switch
+                */
+               if (keyw_matched && keyw_matched + *argc_val ==
+                   parsed_cmd->cmd_keywords_nr &&
+                   *cmd_keyw_def_p == ethsw_id_key_end) {
+                       *argc_val += keyw_matched;
+                       parsed_cmd->cmd_function =
+                                       ethsw_cmd_def[i].keyword_function;
+                       return;
+               }
+       }
+}
+
+/* find all the keywords in the command */
+static int keywords_find(int argc, char * const argv[],
+                        struct ethsw_command_def *parsed_cmd)
+{
+       int i;
+       int j;
+       int argc_val;
+       int rc = CMD_RET_SUCCESS;
+
+       for (i = 1; i < argc; i++) {
+               for (j = 0; j < ethsw_id_count; j++) {
+                       if (keyword[j].match(j, argc, argv, &i, parsed_cmd))
+                               break;
+               }
+       }
+
+       /* if there is no keyword match for a word, the command is invalid */
+       for (i = 1; i < argc; i++)
+               if (parsed_cmd->cmd_to_keywords[i] == ethsw_id_key_end)
+                       rc = CMD_RET_USAGE;
+
+       parsed_cmd->cmd_keywords_nr = argc;
+       argc_val = 1;
+
+       /* get optional parameters first */
+       cmd_keywords_opt_check(parsed_cmd, &argc_val);
+
+       if (argc_val == parsed_cmd->cmd_keywords_nr)
+               return CMD_RET_USAGE;
+
+       /*
+        * check the keywords and if a match is found,
+        * get the function to call
+        */
+       cmd_keywords_check(parsed_cmd, &argc_val);
+
+       /* error if not all commands' parameters were matched */
+       if (argc_val == parsed_cmd->cmd_keywords_nr) {
+               if (!parsed_cmd->cmd_function) {
+                       printf("Command not available for: %s\n", ethsw_name);
+                       rc = CMD_RET_FAILURE;
+               }
+       } else {
+               rc = CMD_RET_USAGE;
+       }
+
+       return rc;
+}
+
+static void command_def_init(struct ethsw_command_def *parsed_cmd)
+{
+       int i;
+
+       for (i = 0; i < ETHSW_MAX_CMD_PARAMS; i++)
+               parsed_cmd->cmd_to_keywords[i] = ethsw_id_key_end;
+
+       parsed_cmd->port = ETHSW_CMD_PORT_ALL;
+       parsed_cmd->cmd_function = NULL;
+}
+
+/* function to interpret commands starting with "ethsw " */
+static int do_ethsw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+       struct ethsw_command_def parsed_cmd;
+       int rc = CMD_RET_SUCCESS;
+
+       if (argc == 1 || argc >= ETHSW_MAX_CMD_PARAMS)
+               return CMD_RET_USAGE;
+
+       command_def_init(&parsed_cmd);
+
+       rc = keywords_find(argc, argv, &parsed_cmd);
+
+       if (rc == CMD_RET_SUCCESS)
+               rc = parsed_cmd.cmd_function(&parsed_cmd);
+
+       return rc;
+}
+
+#define ETHSW_PORT_CONF_HELP "[port <port_no>] { enable | disable | show } " \
+"- enable/disable a port; show shows a port's configuration"
+
+U_BOOT_CMD(ethsw, ETHSW_MAX_CMD_PARAMS, 0, do_ethsw,
+          "Ethernet l2 switch commands",
+          ETHSW_PORT_CONF_HELP"\n"
+);

include/ethsw.h new file +48 -1

new file mode 100644
index 0000000..9e80095
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2015 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:      GPL-2.0+
+ *
+ * Ethernet Switch commands
+ */
+
+#ifndef _CMD_ETHSW_H_
+#define _CMD_ETHSW_H_
+
+#define ETHSW_MAX_CMD_PARAMS 20
+#define ETHSW_CMD_PORT_ALL -1
+
+/* IDs used to track keywords in a command */
+enum ethsw_keyword_id {
+       ethsw_id_key_end = -1,
+       ethsw_id_help,
+       ethsw_id_show,
+       ethsw_id_port,
+       ethsw_id_enable,
+       ethsw_id_disable,
+       ethsw_id_count, /* keep last */
+};
+
+enum ethsw_keyword_opt_id {
+       ethsw_id_port_no = ethsw_id_count + 1,
+       ethsw_id_count_all,     /* keep last */
+};
+
+struct ethsw_command_def {
+       int cmd_to_keywords[ETHSW_MAX_CMD_PARAMS];
+       int cmd_keywords_nr;
+       int port;
+       int (*cmd_function)(struct ethsw_command_def *parsed_cmd);
+};
+
+/* Structure to be created and initialized by an Ethernet Switch driver */
+struct ethsw_command_func {
+       const char *ethsw_name;
+       int (*port_enable)(struct ethsw_command_def *parsed_cmd);
+       int (*port_disable)(struct ethsw_command_def *parsed_cmd);
+       int (*port_show)(struct ethsw_command_def *parsed_cmd);
+};
+
+int ethsw_define_functions(const struct ethsw_command_func *cmd_func);
+
+#endif /* _CMD_ETHSW_H_ */
--

1.9.3

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

common/cmd_ethsw.c line 201

Quoted 7 lines

common/cmd_ethsw.c new file +7 -0

+               }
+
+               /* if all our optional command's keywords perfectly match an
+                * optional pattern, then we can move to the next defined
+                * keywords in our command; remember the one that matched the
+                * greatest number of keywords
+                */

Improper comment format. Please make sure you always run your patches through checkpatch.pl. I recommend using patman!

On 08/07/2015 01:18 PM, Joe Hershberger wrote:
<snip>

Quoted 11 lines

common/cmd_ethsw.c line 201

Quoted 7 lines

common/cmd_ethsw.c new file +7 -0

+               }
+
+               /* if all our optional command's keywords perfectly match an
+                * optional pattern, then we can move to the next defined
+                * keywords in our command; remember the one that matched the
+                * greatest number of keywords
+                */

Improper comment format. Please make sure you always run your patches through checkpatch.pl. I recommend using patman!

Joe,

Do you mean the multiple-line comment should start from the second line? I can change it when I merge the patch.

Checkpatch/patman doesn't complain about this format.

This patch replaces the parser used by VSC9953 L2 Switch driver with the generic one. Also, the config macro that enables the VSC9953 commands has been replaced in all the platforms that use this driver with the config macro that corresponds to the generic parser.

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- this patch was extracted from previous patch:
"drivers/net/vsc9953: Refractor the parser for VSC9953 commands";
- used CMD_RET_* macros instead of magic numbers;
- each variable is declared on a new line, with one space only;

 drivers/net/vsc9953.c      | 349 +++++++++++++++++++++------------------------
 include/configs/T104xRDB.h |   2 +-
 2 files changed, 166 insertions(+), 185 deletions(-)

The new added command:
ethsw [port <port_no>] statistics { [help] | [clear] }

will print counters like the number of Rx/Tx frames, number of Rx/Tx bytes, number of Rx/Tx unicast frames, etc. This patch also adds this commnd in the genereric ethsw parser from cmd_ethsw.c

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v2:
- removed Change-id field;

Changes for v3:
- moved each variable declaration on a separate line, with one space only;
- replaced magic numbers on functions called by the parser with CMD_RET_* macros;

 common/cmd_ethsw.c    |  42 +++++++++
 drivers/net/vsc9953.c | 256 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/ethsw.h       |   4 +
 include/vsc9953.h     | 116 ++++++++++++++++++++++-
 4 files changed, 415 insertions(+), 3 deletions(-)

The command:
ethsw [port <port_no>] learning { [help] | show | auto | disable }

can be used to enable/disable HW learning on a port. This patch also adds this command to the generic ethsw parser from cmd_ethsw.

Signed-off-by Johnson Leung <[email protected]>

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v2:
- removed Change-id field;

Changes for v3:
- replaced values returned by functions called by the parser with CMD_RET_* macros;
- removed "CONFIG_" from macros added in vsc9953.h;
- each variabled is declared on a separate line, with one space instead of tab(s);

 common/cmd_ethsw.c    |  60 +++++++++++++++++++++
 drivers/net/vsc9953.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/ethsw.h       |   4 ++
 include/vsc9953.h     |   6 +++
 4 files changed, 211 insertions(+)

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c +2 -13

index 5189f5b..3e39fd1 100644
@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
 		}
 		break;
 	case env_flags_vartype_macaddr:
-		cur = value;
-		for (i = 0; i < 6; i++) {
-			skip_num(1, cur, &end, 2);
-			if (cur == end)
-				return -1;
-			if (cur + 2 == end && is_hex_prefix(cur))
-				return -1;
-			if (i != 5 && *end != ':')
-				return -1;
-			if (i == 5 && *end != '\0')
-				return -1;
-			cur = end + 1;
-		}
+		if (eth_validate_ethaddr_str(value))
+			return -1;
 		break;
 #endif
 	case env_flags_vartype_end:

include/net.h +1 -0

index d17173d..c487aa7 100644
@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);	/* Change the device */
 void eth_set_current(void);		/* set nterface to ethcur var */
 
 int eth_get_dev_index(void);		/* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);
 int eth_getenv_enetaddr(char *name, uchar *enetaddr);
 int eth_setenv_enetaddr(char *name, const uchar *enetaddr);

net/eth.c +23 -0

index 953b731..a6fdf1b 100644
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr)
+{
+	unsigned long val;
+	int i;
+	const char *cur;
+	char *end;
+
+	if (!addr)
+		return -1;
+
+	cur = addr;
+	for (i = 0; i < 6; i++) {
+		val = simple_strtoul(cur, &end, 16);
+		if (cur + 1 != end && cur + 2 != end)
+			return -1;
+		if (val > 0xff)
+			return -1;
+		if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+			return -1;
+		if (i != 5 && *end != ':')
+			return -1;
+		if (i == 5 && *end != '\0')

-----Original Message-----
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de
Cc: joe.hershberger at ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

include/net.h void eth_try_another(int first_restart); /* Change the

Quoted 46 lines · Codrin Ciubotariu

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c index renamed from common/env_flags.c +0 -0

5189f5b..3e39fd1 100644

common/env_flags.c +2 -13

@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
 		}
 		break;
 	case env_flags_vartype_macaddr:
-		cur = value;
-		for (i = 0; i < 6; i++) {
-			skip_num(1, cur, &end, 2);
-			if (cur == end)
-				return -1;
-			if (cur + 2 == end && is_hex_prefix(cur))
-				return -1;
-			if (i != 5 && *end != ':')
-				return -1;
-			if (i == 5 && *end != '\0')
-				return -1;
-			cur = end + 1;
-		}
+		if (eth_validate_ethaddr_str(value))
+			return -1;
 		break;
 #endif
 	case env_flags_vartype_end:

include/net.h index d17173d..c487aa7 renamed from include/net.h +0 -0

100644

include/net.h +0 -0

@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);	/* Change the

device */

Quoted 43 lines

include/net.h +1 -0

 void eth_set_current(void);		/* set nterface to ethcur var */
 
 int eth_get_dev_index(void);		/* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);  int

eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644

net/eth.c +22 -0

@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr) {
+	unsigned long val;
+	int i;
+	const char *cur;
+	char *end;
+
+	if (!addr)
+		return -1;
+
+	cur = addr;
+	for (i = 0; i < 6; i++) {
+		val = simple_strtoul(cur, &end, 16);
+		if (cur + 1 != end && cur + 2 != end)
+			return -1;
+		if (val > 0xff)
+			return -1;
+		if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+			return -1;
+		if (i != 5 && *end != ':')
+			return -1;
+		if (i == 5 && *end != '\0')

net/eth.c line 53

Quoted 10 lines

net/eth.c +7 -0

+			return -1;
+		cur = end + 1;
+	}
+
+	return 0;
+}
+
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr)  {
 	char *end;
 

Codrin,

This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str().

York

Hi York,

I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()?

Thanks and best regards,
Codrin

Quoted 115 lines

-----Original Message-----
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de
Cc: joe.hershberger at ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

include/net.h void eth_try_another(int first_restart); /* Change the

Quoted 46 lines · Codrin Ciubotariu

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c index renamed from common/env_flags.c +0 -0

5189f5b..3e39fd1 100644

common/env_flags.c +2 -13

@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
 		}
 		break;
 	case env_flags_vartype_macaddr:
-		cur = value;
-		for (i = 0; i < 6; i++) {
-			skip_num(1, cur, &end, 2);
-			if (cur == end)
-				return -1;
-			if (cur + 2 == end && is_hex_prefix(cur))
-				return -1;
-			if (i != 5 && *end != ':')
-				return -1;
-			if (i == 5 && *end != '\0')
-				return -1;
-			cur = end + 1;
-		}
+		if (eth_validate_ethaddr_str(value))
+			return -1;
 		break;
 #endif
 	case env_flags_vartype_end:

include/net.h index d17173d..c487aa7 renamed from include/net.h +0 -0

100644

include/net.h +0 -0

@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);	/* Change the

device */

Quoted 43 lines

include/net.h +1 -0

 void eth_set_current(void);		/* set nterface to ethcur var */
 
 int eth_get_dev_index(void);		/* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);  int

eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644

net/eth.c +22 -0

@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr) {
+	unsigned long val;
+	int i;
+	const char *cur;
+	char *end;
+
+	if (!addr)
+		return -1;
+
+	cur = addr;
+	for (i = 0; i < 6; i++) {
+		val = simple_strtoul(cur, &end, 16);
+		if (cur + 1 != end && cur + 2 != end)
+			return -1;
+		if (val > 0xff)
+			return -1;
+		if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+			return -1;
+		if (i != 5 && *end != ':')
+			return -1;
+		if (i == 5 && *end != '\0')

net/eth.c line 53

Quoted 10 lines

net/eth.c +7 -0

+			return -1;
+		cur = end + 1;
+	}
+
+	return 0;
+}
+
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr)  {
 	char *end;
 

Codrin,

This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str().

York

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

Quoted 122 lines

Hi York,

I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()?

Thanks and best regards,
Codrin

Quoted 115 lines

-----Original Message-----
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de
Cc: joe.hershberger at ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

include/net.h void eth_try_another(int first_restart); /* Change the

Quoted 46 lines · Codrin Ciubotariu

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c index renamed from common/env_flags.c +0 -0

5189f5b..3e39fd1 100644

common/env_flags.c +2 -13

@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
            }
            break;
    case env_flags_vartype_macaddr:
-           cur = value;
-           for (i = 0; i < 6; i++) {
-                   skip_num(1, cur, &end, 2);
-                   if (cur == end)
-                           return -1;
-                   if (cur + 2 == end && is_hex_prefix(cur))
-                           return -1;
-                   if (i != 5 && *end != ':')
-                           return -1;
-                   if (i == 5 && *end != '\0')
-                           return -1;
-                   cur = end + 1;
-           }
+           if (eth_validate_ethaddr_str(value))
+                   return -1;
            break;
 #endif
    case env_flags_vartype_end:

include/net.h index d17173d..c487aa7 renamed from include/net.h +0 -0

100644

include/net.h +0 -0

@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);        /* Change the

device */

Quoted 43 lines

include/net.h +1 -0

 void eth_set_current(void);                /* set nterface to ethcur var */
 
 int eth_get_dev_index(void);               /* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);  int

eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644

net/eth.c +22 -0

@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr) {
+   unsigned long val;
+   int i;
+   const char *cur;
+   char *end;
+
+   if (!addr)
+           return -1;
+
+   cur = addr;
+   for (i = 0; i < 6; i++) {
+           val = simple_strtoul(cur, &end, 16);
+           if (cur + 1 != end && cur + 2 != end)
+                   return -1;
+           if (val > 0xff)
+                   return -1;
+           if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+                   return -1;
+           if (i != 5 && *end != ':')
+                   return -1;
+           if (i == 5 && *end != '\0')

net/eth.c line 53

Quoted 10 lines

net/eth.c +7 -0

+                   return -1;
+           cur = end + 1;
+   }
+
+   return 0;
+}
+
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr)  {
    char *end;
 

Codrin,

This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str().

York

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Quoted 128 lines · On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Quoted 122 lines

Hi York,

I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()?

Thanks and best regards,
Codrin

Quoted 115 lines

-----Original Message-----
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de
Cc: joe.hershberger at ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

include/net.h void eth_try_another(int first_restart); /* Change the

Quoted 46 lines · Codrin Ciubotariu

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c index renamed from common/env_flags.c +0 -0

5189f5b..3e39fd1 100644

common/env_flags.c +2 -13

@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
            }
            break;
    case env_flags_vartype_macaddr:
-           cur = value;
-           for (i = 0; i < 6; i++) {
-                   skip_num(1, cur, &end, 2);
-                   if (cur == end)
-                           return -1;
-                   if (cur + 2 == end && is_hex_prefix(cur))
-                           return -1;
-                   if (i != 5 && *end != ':')
-                           return -1;
-                   if (i == 5 && *end != '\0')
-                           return -1;
-                   cur = end + 1;
-           }
+           if (eth_validate_ethaddr_str(value))
+                   return -1;
            break;
 #endif
    case env_flags_vartype_end:

include/net.h index d17173d..c487aa7 renamed from include/net.h +0 -0

100644

include/net.h +0 -0

@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);        /* Change the

device */

Quoted 43 lines

include/net.h +1 -0

 void eth_set_current(void);                /* set nterface to ethcur var */
 
 int eth_get_dev_index(void);               /* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);  int

eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644

net/eth.c +22 -0

@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr) {
+   unsigned long val;
+   int i;
+   const char *cur;
+   char *end;
+
+   if (!addr)
+           return -1;
+
+   cur = addr;
+   for (i = 0; i < 6; i++) {
+           val = simple_strtoul(cur, &end, 16);
+           if (cur + 1 != end && cur + 2 != end)
+                   return -1;
+           if (val > 0xff)
+                   return -1;
+           if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+                   return -1;
+           if (i != 5 && *end != ':')
+                   return -1;
+           if (i == 5 && *end != '\0')

net/eth.c line 53

Quoted 10 lines

net/eth.c +7 -0

+                   return -1;
+           cur = end + 1;
+   }
+
+   return 0;
+}
+
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr)  {
    char *end;
 

Codrin,

This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str().

York

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 10 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

That is probably fine.

Quoted 5 lines

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

Quoted 128 lines

Quoted 128 lines · On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Quoted 122 lines

Hi York,

I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()?

Thanks and best regards,
Codrin

Quoted 115 lines

-----Original Message-----
From: Sun York-R58495
Sent: Saturday, August 08, 2015 3:31 AM
To: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de
Cc: joe.hershberger at ni.com; Kushwaha Prabhakar-B32579
Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

include/net.h void eth_try_another(int first_restart); /* Change the

Quoted 46 lines · Codrin Ciubotariu

The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str().

Signed-off-by Codrin Ciubotariu <[email protected]>

---

Changes for v3:
- none, new patch;

 common/env_flags.c | 15 ++-------------
 include/net.h      |  1 +
 net/eth.c          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 13 deletions(-)

common/env_flags.c index renamed from common/env_flags.c +0 -0

5189f5b..3e39fd1 100644

common/env_flags.c +2 -13

@@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value,
            }
            break;
    case env_flags_vartype_macaddr:
-           cur = value;
-           for (i = 0; i < 6; i++) {
-                   skip_num(1, cur, &end, 2);
-                   if (cur == end)
-                           return -1;
-                   if (cur + 2 == end && is_hex_prefix(cur))
-                           return -1;
-                   if (i != 5 && *end != ':')
-                           return -1;
-                   if (i == 5 && *end != '\0')
-                           return -1;
-                   cur = end + 1;
-           }
+           if (eth_validate_ethaddr_str(value))
+                   return -1;
            break;
 #endif
    case env_flags_vartype_end:

include/net.h index d17173d..c487aa7 renamed from include/net.h +0 -0

100644

include/net.h +0 -0

@@ -218,6 +218,7 @@ void eth_try_another(int first_restart);        /* Change the

device */

Quoted 43 lines

include/net.h +1 -0

 void eth_set_current(void);                /* set nterface to ethcur var */
 
 int eth_get_dev_index(void);               /* get the device index */
+int eth_validate_ethaddr_str(const char *addr);
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr);  int

eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644

net/eth.c +22 -0

@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <command.h>
 #include <dm.h>
 #include <environment.h>

@@ -19,6 +20,35 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int eth_validate_ethaddr_str(const char *addr) {
+   unsigned long val;
+   int i;
+   const char *cur;
+   char *end;
+
+   if (!addr)
+           return -1;
+
+   cur = addr;
+   for (i = 0; i < 6; i++) {
+           val = simple_strtoul(cur, &end, 16);
+           if (cur + 1 != end && cur + 2 != end)
+                   return -1;
+           if (val > 0xff)
+                   return -1;
+           if (cur + 2 >= end && tolower(*(cur + 1)) == 'x')
+                   return -1;
+           if (i != 5 && *end != ':')
+                   return -1;
+           if (i == 5 && *end != '\0')

net/eth.c line 53

Quoted 10 lines

net/eth.c +7 -0

+                   return -1;
+           cur = end + 1;
+   }
+
+   return 0;
+}
+
 void eth_parse_enetaddr(const char *addr, uchar *enetaddr)  {
    char *end;
 

Codrin,

This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str().

York

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi York,

Quoted 16 lines · York Sun

Quoted 13 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

Quoted 5 lines

Quoted 2 lines

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

OK, sure... but that breaks intended behavior, I think.

Quoted 25 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

Quoted 2 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the original code in env_flag.c.

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 28 lines · York Sun

Quoted 25 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

Quoted 6 lines

Quoted 2 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 28 lines · York Sun

Quoted 25 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

Quoted 6 lines

Quoted 2 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

Hi Joe, York,

Quoted 52 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 28 lines · York Sun

Quoted 25 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way.

Quoted 6 lines

Quoted 2 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Codrin,

Quoted 4 lines

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file?

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is built, but net/net.c or net/eth.c not.

-----Original Message-----
From: Sun York-R58495
Sent: Thursday, August 13, 2015 6:55 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 42 lines · On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi Joe, York,

Quoted 40 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 27 lines · York Sun

Quoted 24 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another

way.

Quoted 11 lines

Quoted 11 lines

Quoted 8 lines

Quoted 3 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Quoted 8 lines

Quoted 6 lines

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on

simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file?

Quoted 2 lines

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is

built, but net/net.c or net/eth.c not.

Quoted 1 line

I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me.

York

Hi York,

Quoted 81 lines

-----Original Message-----
From: Sun York-R58495
Sent: Thursday, August 13, 2015 6:55 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 42 lines · On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi Joe, York,

Quoted 40 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 27 lines · York Sun

Quoted 24 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another

way.

Quoted 11 lines

Quoted 11 lines

Quoted 8 lines

Quoted 3 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Quoted 8 lines

Quoted 6 lines

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on

simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file?

Quoted 2 lines

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is

built, but net/net.c or net/eth.c not.

Quoted 1 line

Quoted 6 lines

I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me.

York

Something like

#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
	case env_flags_vartype_ipaddr:
		cur = value;
		for (i = 0; i < 4; i++) {
			skip_num(0, cur, &end, 3);
			if (cur == end)
				return -1;
			if (i != 3 && *end != '.')
				return -1;
			if (i == 3 && *end != '\0')
				return -1;
			cur = end + 1;
		}
		break;
	case env_flags_vartype_macaddr:
		if (eth_validate_ethaddr_str(value))
			return -1;
		break;

#endif

?

I get two warnings on this:
../common/env_flags.c: In function '_env_flags_validate_type':
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]
switch (type) {
^
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_macaddr' not handled in switch [-Wswitch]

Unless I guard these values in env_flags.h:
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
env_flags_vartype_ipaddr, env_flags_vartype_macaddr,
#endif

Quoted 121 lines · On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi York,

Quoted 81 lines

-----Original Message-----
From: Sun York-R58495
Sent: Thursday, August 13, 2015 6:55 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 42 lines · On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi Joe, York,

Quoted 40 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 27 lines · York Sun

Quoted 24 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another

way.

Quoted 11 lines

Quoted 11 lines

Quoted 8 lines

Quoted 3 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Quoted 8 lines

Quoted 6 lines

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on

simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file?

Quoted 2 lines

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is

built, but net/net.c or net/eth.c not.

Quoted 1 line

Quoted 6 lines

I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me.

York

Something like

#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
      case env_flags_vartype_ipaddr:
              cur = value;
              for (i = 0; i < 4; i++) {
                      skip_num(0, cur, &end, 3);
                      if (cur == end)
                              return -1;
                      if (i != 3 && *end != '.')
                              return -1;
                      if (i == 3 && *end != '\0')
                              return -1;
                      cur = end + 1;
              }
              break;
      case env_flags_vartype_macaddr:
              if (eth_validate_ethaddr_str(value))
                      return -1;
              break;

#endif

?

I get two warnings on this:
../common/env_flags.c: In function '_env_flags_validate_type':
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]
switch (type) {
^
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_macaddr' not handled in switch [-Wswitch]

Quoted 6 lines

Unless I guard these values in env_flags.h:
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
env_flags_vartype_ipaddr, env_flags_vartype_macaddr,
#endif

It makes sense to me to take out these two for SPL build. The whole purpose of SPL is to load the final image. I don't see a chance users would be able to type any command.

Joe, do you agree the CMD_NET shouldn't be used for SPL?

Quoted 135 lines · York Sun

Quoted 121 lines · On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi York,

Quoted 81 lines

-----Original Message-----
From: Sun York-R58495
Sent: Thursday, August 13, 2015 6:55 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 42 lines · On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi Joe, York,

Quoted 40 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 27 lines · York Sun

Quoted 24 lines · Joe Hershberger

Hi York,

Quoted 21 lines · York Sun

Quoted 8 lines · Joe Hershberger

Too much top-posting.

Quoted 4 lines · York Sun

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

Quoted 7 lines

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another

way.

Quoted 11 lines

Quoted 11 lines

Quoted 8 lines

Quoted 3 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Quoted 8 lines

Quoted 6 lines

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on

simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file?

Quoted 2 lines

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is

built, but net/net.c or net/eth.c not.

Quoted 1 line

Quoted 6 lines

I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me.

York

Something like

#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
      case env_flags_vartype_ipaddr:
              cur = value;
              for (i = 0; i < 4; i++) {
                      skip_num(0, cur, &end, 3);
                      if (cur == end)
                              return -1;
                      if (i != 3 && *end != '.')
                              return -1;
                      if (i == 3 && *end != '\0')
                              return -1;
                      cur = end + 1;
              }
              break;
      case env_flags_vartype_macaddr:
              if (eth_validate_ethaddr_str(value))
                      return -1;
              break;

#endif

?

I get two warnings on this:
../common/env_flags.c: In function '_env_flags_validate_type':
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]
switch (type) {
^
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_macaddr' not handled in switch [-Wswitch]

Quoted 6 lines

Unless I guard these values in env_flags.h:
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
env_flags_vartype_ipaddr, env_flags_vartype_macaddr,
#endif

It makes sense to me to take out these two for SPL build. The whole purpose of SPL is to load the final image. I don't see a chance users would be able to type any command.

Quoted 1 line

Joe, do you agree the CMD_NET shouldn't be used for SPL?

I tend to agree, but I have a vague recollection that there is at least one SPL that uses net. Sorry I don't recall what board it was that I'm thinking about.

-----Original Message-----
From: Joe Hershberger [mailto:joe.hershberger at gmail.com]
Sent: Monday, August 17, 2015 5:38 PM
To: Sun York-R58495
Cc: Ciubotariu Codrin Constantin-B43658; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 43 lines · York Sun

Quoted 40 lines · On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi York,

Quoted 38 lines

-----Original Message-----
From: Sun York-R58495
Sent: Thursday, August 13, 2015 6:55 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

Quoted 27 lines · On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote:

Hi Joe, York,

Quoted 25 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, August 12, 2015 10:59 PM
To: Ciubotariu Codrin Constantin-B43658
Cc: Joe Hershberger; u-boot at lists.denx.de; joe.hershberger at ni.com
Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address

+Codrin

Somehow I dropped Codrin in last reply.

Quoted 12 lines · York Sun

Quoted 9 lines · Joe Hershberger

Hi York,

Quoted 6 lines · York Sun

Quoted 3 lines · Joe Hershberger

Too much top-posting.

On Mon, Aug 10, 2015 at 2:41 PM, York Sun <[email protected]>

Quoted 33 lines · wrote:

Quoted 33 lines

Quoted 33 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

SPL doesn't use net/eth.c. You add a call in env_flags.c.

I think you can put it in header file and use static inline, or keep it in the same file where it is called.

That is probably fine.

Quoted 2 lines

Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

I don't think this is the reason. The problem is that net is *not* build for SPL, but env is.

Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another

way.

Quoted 16 lines

Quoted 16 lines

Quoted 8 lines

Quoted 3 lines

OK, sure... but that breaks intended behavior, I think.

I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the

original code in env_flag.c.

Quoted 1 line

Codrin,

Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

York

I can't inline eth_validate_ethaddr_str in eth.h since it depends on

simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used

by SPL targets. Could you please suggest such a file?

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 3 lines

SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is

built, but net/net.c or net/eth.c not.

Quoted 1 line

I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable

to me.

Quoted 25 lines

Quoted 25 lines

Quoted 2 lines

York

Something like

#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
      case env_flags_vartype_ipaddr:
              cur = value;
              for (i = 0; i < 4; i++) {
                      skip_num(0, cur, &end, 3);
                      if (cur == end)
                              return -1;
                      if (i != 3 && *end != '.')
                              return -1;
                      if (i == 3 && *end != '\0')
                              return -1;
                      cur = end + 1;
              }
              break;
      case env_flags_vartype_macaddr:
              if (eth_validate_ethaddr_str(value))
                      return -1;
              break;

#endif

Quoted 5 lines

Quoted 5 lines

?

I get two warnings on this:
../common/env_flags.c: In function '_env_flags_validate_type':
../common/env_flags.c:203:2: warning: enumeration value

'env_flags_vartype_ipaddr' not handled in switch [-Wswitch]

Quoted 12 lines

Quoted 11 lines

switch (type) {
^
../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_macaddr' not handled in switch [-Wswitch]

Unless I guard these values in env_flags.h:
#if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD)
env_flags_vartype_ipaddr, env_flags_vartype_macaddr,
#endif

Quoted 5 lines

It makes sense to me to take out these two for SPL build. The whole purpose of SPL is to load the final image. I don't see a chance users would be able to type any command.

Joe, do you agree the CMD_NET shouldn't be used for SPL?

I tend to agree, but I have a vague recollection that there is at least one SPL that uses net. Sorry I don't recall what board it was that I'm thinking about.

-Joe