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

[Simon Glass]

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)


> +               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