Hi Simon, On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass <s...@chromium.org> wrote: > Hi Joe, > > On 21 April 2015 at 16:02, Joe Hershberger <joe.hershber...@ni.com> wrote: >> Instead of checking for changes to the env each time we enter the >> net_loop, use the env callbacks to update the values of the variables. >> Don't update the variables when the source was programmatic, since the >> variables were the source of the new value. >> >> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com> >> --- >> >> include/env_callback.h | 22 ++++++++++- >> net/net.c | 105 >> +++++++++++++++++++++++++++++++++++++++++-------- >> 2 files changed, 110 insertions(+), 17 deletions(-) > > Reviewed-by: Simon Glass <s...@chromium.org> > > Q below. > >> >> diff --git a/include/env_callback.h b/include/env_callback.h >> index 3de1093..91f3cc0 100644 >> --- a/include/env_callback.h >> +++ b/include/env_callback.h >> @@ -37,6 +37,26 @@ >> #define ENV_DOT_ESCAPE >> #endif >> >> +#ifdef CONFIG_CMD_DNS >> +#define DNS_CALLBACK "dnsip:dnsip," >> +#else >> +#define DNS_CALLBACK >> +#endif >> + >> +#ifdef CONFIG_NET >> +#define NET_CALLBACKS \ >> + "bootfile:bootfile," \ >> + "ipaddr:ipaddr," \ >> + "gatewayip:gatewayip," \ >> + "netmask:netmask," \ >> + "serverip:serverip," \ >> + "nvlan:nvlan," \ >> + "vlan:vlan," \ >> + DNS_CALLBACK >> +#else >> +#define NET_CALLBACKS >> +#endif >> + >> /* >> * This list of callback bindings is static, but may be overridden by >> defining >> * a new association in the ".callbacks" environment variable. >> @@ -44,7 +64,7 @@ >> #define ENV_CALLBACK_LIST_STATIC ENV_DOT_ESCAPE ENV_CALLBACK_VAR >> ":callbacks," \ >> ENV_DOT_ESCAPE ENV_FLAGS_VAR ":flags," \ >> "baudrate:baudrate," \ >> - "bootfile:bootfile," \ >> + NET_CALLBACKS \ >> "loadaddr:loadaddr," \ >> SILENT_CALLBACK \ >> SPLASHIMAGE_CALLBACK \ >> diff --git a/net/net.c b/net/net.c >> index a365df0..57111ad 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -208,6 +208,9 @@ int __maybe_unused net_busy_flag; >> static int on_bootfile(const char *name, const char *value, enum env_op op, >> int flags) >> { >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> switch (op) { >> case env_op_create: >> case env_op_overwrite: >> @@ -222,6 +225,92 @@ static int on_bootfile(const char *name, const char >> *value, enum env_op op, >> } >> U_BOOT_ENV_CALLBACK(bootfile, on_bootfile); >> >> +static int on_ipaddr(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) > > Can you just do this? > > if (flags & H_PROGRAMMATIC)
Probably so, since it's not likely that we'll make a 4th state that combines these "flags" even though they are all mutually exclusive. >> + return 0; >> + >> + net_ip = string_to_ip(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(ipaddr, on_ipaddr); >> + >> +static int on_gatewayip(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_gateway = string_to_ip(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(gatewayip, on_gatewayip); >> + >> +static int on_netmask(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_netmask = string_to_ip(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(netmask, on_netmask); >> + >> +static int on_serverip(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_server_ip = string_to_ip(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(serverip, on_serverip); >> + >> +static int on_nvlan(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_native_vlan = string_to_vlan(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(nvlan, on_nvlan); >> + >> +static int on_vlan(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_our_vlan = string_to_vlan(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(vlan, on_vlan); >> + >> +#if defined(CONFIG_CMD_DNS) >> +static int on_dnsip(const char *name, const char *value, enum env_op op, >> + int flags) >> +{ >> + if ((flags & H_ORIGIN_FLAGS) == H_PROGRAMMATIC) >> + return 0; >> + >> + net_dns_server = string_to_ip(value); >> + >> + return 0; >> +} >> +U_BOOT_ENV_CALLBACK(dnsip, on_dnsip); >> +#endif >> + >> /* >> * Check if autoload is enabled. If so, use either NFS or TFTP to download >> * the boot file. >> @@ -252,22 +341,6 @@ void net_auto_load(void) >> >> static void net_init_loop(void) >> { >> - static int env_changed_id; >> - int env_id = get_env_id(); >> - >> - /* update only when the environment has changed */ >> - if (env_changed_id != env_id) { >> - net_ip = getenv_ip("ipaddr"); >> - net_gateway = getenv_ip("gatewayip"); >> - net_netmask = getenv_ip("netmask"); >> - net_server_ip = getenv_ip("serverip"); >> - net_native_vlan = getenv_vlan("nvlan"); >> - net_our_vlan = getenv_vlan("vlan"); >> -#if defined(CONFIG_CMD_DNS) >> - net_dns_server = getenv_ip("dnsip"); >> -#endif >> - env_changed_id = env_id; >> - } >> if (eth_get_dev()) >> memcpy(net_ethaddr, eth_get_ethaddr(), 6); > > Regards, > Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot