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
