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

Simon Glass
2015-04-24T04:34:53Z

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