[U-Boot] [PATCH 10/11] net: Use env callbacks for net variables

[Joe Hershberger]

Hi Simon,

On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Joe,
>
> On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger at 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 at ni.com>
>> ---
>>
>>  include/env_callback.h |  22 ++++++++++-
>>  net/net.c              | 105 +++++++++++++++++++++++++++++++++++++++++--------
>>  2 files changed, 110 insertions(+), 17 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot