| Message ID | 1429653771-11676-11-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Joe, On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger@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.hershberger@ni.com> > --- > > include/env_callback.h | 22 ++++++++++- > net/net.c | 105 +++++++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 110 insertions(+), 17 deletions(-) Reviewed-by: Simon Glass <sjg@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) > + 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
Hi Simon, On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Joe, > > On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger@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.hershberger@ni.com> >> --- >> >> include/env_callback.h | 22 ++++++++++- >> net/net.c | 105 +++++++++++++++++++++++++++++++++++++++++-------- >> 2 files changed, 110 insertions(+), 17 deletions(-) > > Reviewed-by: Simon Glass <sjg@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
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) + 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);
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.hershberger@ni.com> --- include/env_callback.h | 22 ++++++++++- net/net.c | 105 +++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 17 deletions(-)