[U-Boot] [RFC PATCH v3 12/14] dm: eth: Add support for ethprime env var

[Simon Glass]

Hi Joe,

On 10 February 2015 at 18:30, Joe Hershberger <joe.hershberger at ni.com> wrote:
> The ethprime env var is used to indicate the starting device if none is
> specified in ethact. Also support aliases specified in the ethprime var.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>

Reviewed-by: Simon Glass <sjg at chromium.org>

Two nits below.

>
> ---
>
> Changes in v3:
> -Added support for ethprime
>
> Changes in v2: None
>
>  net/eth.c     | 13 ++++++++++++-
>  test/dm/eth.c | 21 +++++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/net/eth.c b/net/eth.c
> index 762effe..1770662 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -893,8 +893,19 @@ void eth_set_current(void)
>                 act = getenv("ethact");
>                 env_changed_id = env_id;
>         }
> -       if (act != NULL)
> +
> +       if (act == NULL) {
> +               char *ethprime = getenv("ethprime");
> +
> +               if (ethprime && eth_get_dev_by_name(ethprime)) {
> +                       eth_set_dev(eth_get_dev_by_name(ethprime));

Can you store the result of eth_get_dev_by_name() and reuse it? It
seems like an expensive function.

> +               } else {
> +                       eth_set_dev(NULL);
> +                       eth_set_current_to_next();
> +               }
> +       } else {
>                 eth_set_dev(eth_get_dev_by_name(act));
> +       }
>
>         eth_current_changed();
>  }
> diff --git a/test/dm/eth.c b/test/dm/eth.c
> index c0a8ab5..b39a94a 100644
> --- a/test/dm/eth.c
> +++ b/test/dm/eth.c
> @@ -62,3 +62,24 @@ static int dm_test_eth_alias(struct dm_test_state *dms)
>  }
>
>  DM_TEST(dm_test_eth_alias, DM_TESTF_SCAN_FDT);
> +
> +static int dm_test_eth_prime(struct dm_test_state *dms)
> +{
> +       NetPingIP = string_to_ip("1.1.2.2");
> +
> +       /* Expected to be "eth at 10003000" because of ethprime variable */
> +       setenv("ethact", NULL);
> +       setenv("ethprime", "eth5");
> +       ut_assertok(NetLoop(PING));
> +       ut_asserteq_str("eth at 10003000", getenv("ethact"));
> +
> +       /* Expected to be "eth at 10002000" because it is first */
> +       setenv("ethact", NULL);
> +       setenv("ethprime", NULL);
> +       ut_assertok(NetLoop(PING));
> +       ut_asserteq_str("eth at 10002000", getenv("ethact"));
> +
> +       return 0;
> +}
> +

Can you remove this blank line so it is consistent with other test files?

> +DM_TEST(dm_test_eth_prime, DM_TESTF_SCAN_FDT);
> --
> 1.7.11.5
>

Regards,
Simon