[U-Boot] [RFC PATCH v4 16/23] dm: eth: Add support for aliases
Simon Glass
sjg at chromium.org
Sun Mar 1 19:07:30 CET 2015
Hi Joe,
On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger at ni.com> wrote:
> Allow network devices to be referred to as "eth0" instead of
> "eth at 12345678" when specified in ethact.
>
> Add tests to verify this behavior.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>
Reviewed-by: Simon Glass <sjg at chromium.org>
Again a few comments on error handling for follow-up.
> ---
>
> Changes in v4:
> -Use only the seq from DM to find aliases
>
> Changes in v3:
> -Added support for aliases
>
> Changes in v2: None
>
> include/configs/sandbox.h | 2 +-
> include/net.h | 1 +
> net/eth.c | 47 ++++++++++++++++++++++++++++++++++++++---------
> test/dm/eth.c | 24 ++++++++++++++++++++++++
> test/dm/test.dts | 4 +++-
> 5 files changed, 67 insertions(+), 11 deletions(-)
>
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 9189f6a..caf9f5a 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -174,7 +174,7 @@
>
> #define SANDBOX_ETH_SETTINGS "ethaddr=00:00:11:22:33:44\0" \
> "eth1addr=00:00:11:22:33:45\0" \
> - "eth2addr=00:00:11:22:33:46\0" \
> + "eth5addr=00:00:11:22:33:46\0" \
> "ipaddr=1.2.3.4\0"
>
> #define CONFIG_EXTRA_ENV_SETTINGS SANDBOX_SERIAL_SETTINGS \
> diff --git a/include/net.h b/include/net.h
> index 508c572..e9cb4a3 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -122,6 +122,7 @@ struct eth_ops {
> #define eth_get_ops(dev) ((struct eth_ops *)(dev)->driver->ops)
>
> struct udevice *eth_get_dev(void); /* get the current device */
> +struct udevice *eth_get_dev_by_name(const char *devname);
This needs a comment to describe what devname is exactly. I thought it
was a device name. Also it seems to requite a minimum length of 3
characters?
> unsigned char *eth_get_ethaddr(void); /* get the current device MAC */
> /* Used only when NetConsole is enabled */
> int eth_init_state_only(void); /* Set active state */
> diff --git a/net/eth.c b/net/eth.c
> index 9c2dfb9..8b853e8 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -132,6 +132,36 @@ static void eth_set_dev(struct udevice *dev)
> eth_get_uclass_priv()->current = dev;
> }
>
> +/*
> + * Find the udevice that either has the name passed in as devname or has an
> + * alias named devname.
> + */
> +struct udevice *eth_get_dev_by_name(const char *devname)
> +{
> + int seq;
> + char *endp = NULL;
> + const char *startp;
> + struct udevice *it;
> + struct uclass *uc;
> +
> + startp = devname + strlen("eth");
> + seq = simple_strtoul(startp, &endp, 10);
> +
> + uclass_get(UCLASS_ETH, &uc);
> + uclass_foreach_dev(it, uc) {
> + /* We need the seq to be valid, so make sure it's probed */
> + device_probe(it);
Error check. I think this function is should return an error.
> + /*
> + * Check for the name or the sequence number to match
> + */
> + if (strcmp(it->name, devname) == 0 ||
> + (endp > startp && it->seq == seq))
> + return it;
> + }
> +
> + return NULL;
> +}
> +
> unsigned char *eth_get_ethaddr(void)
> {
> struct eth_pdata *pdata;
> @@ -405,6 +435,7 @@ UCLASS_DRIVER(eth) = {
> .pre_remove = eth_pre_remove,
> .priv_auto_alloc_size = sizeof(struct eth_uclass_priv),
> .per_device_auto_alloc_size = sizeof(struct eth_device_priv),
> + .flags = DM_UC_FLAG_SEQ_ALIAS,
> };
> #endif
>
> @@ -437,6 +468,11 @@ static void eth_set_current_to_next(void)
> eth_current = eth_current->next;
> }
>
> +static void eth_set_dev(struct eth_device *dev)
> +{
> + eth_current = dev;
> +}
> +
> struct eth_device *eth_get_dev_by_name(const char *devname)
> {
> struct eth_device *dev, *target_dev;
> @@ -853,7 +889,6 @@ void eth_set_current(void)
> {
> static char *act;
> static int env_changed_id;
> - void *old_current;
> int env_id;
>
> env_id = get_env_id();
> @@ -861,14 +896,8 @@ void eth_set_current(void)
> act = getenv("ethact");
> env_changed_id = env_id;
> }
> - if (act != NULL) {
> - old_current = eth_get_dev();
> - do {
> - if (strcmp(eth_get_name(), act) == 0)
> - return;
> - eth_set_current_to_next();
> - } while (old_current != eth_get_dev());
> - }
> + if (act != NULL)
> + eth_set_dev(eth_get_dev_by_name(act));
Again I think the error handling here is dodgy. You may have a device
which fails to probe but it will not be reported here.
>
> eth_current_changed();
> }
> diff --git a/test/dm/eth.c b/test/dm/eth.c
> index 04ccf49..5688b71 100644
> --- a/test/dm/eth.c
> +++ b/test/dm/eth.c
> @@ -36,3 +36,27 @@ static int dm_test_eth(struct dm_test_state *dms)
> return 0;
> }
> DM_TEST(dm_test_eth, DM_TESTF_SCAN_FDT);
> +
> +static int dm_test_eth_alias(struct dm_test_state *dms)
> +{
> + NetPingIP = string_to_ip("1.1.2.2");
> + setenv("ethact", "eth0");
> + ut_assertok(NetLoop(PING));
> + ut_asserteq_str("eth at 10002000", getenv("ethact"));
> +
> + setenv("ethact", "eth1");
> + ut_assertok(NetLoop(PING));
> + ut_asserteq_str("eth at 10004000", getenv("ethact"));
> +
> + /* Expected to fail since eth2 is not defined in the device tree */
> + setenv("ethact", "eth2");
> + ut_assertok(NetLoop(PING));
> + ut_asserteq_str("eth at 10002000", getenv("ethact"));
> +
> + setenv("ethact", "eth5");
> + ut_assertok(NetLoop(PING));
> + ut_asserteq_str("eth at 10003000", getenv("ethact"));
Looks good to me.
> +
> + return 0;
> +}
> +DM_TEST(dm_test_eth_alias, DM_TESTF_SCAN_FDT);
> diff --git a/test/dm/test.dts b/test/dm/test.dts
> index 2f68cdf..bc2409d 100644
> --- a/test/dm/test.dts
> +++ b/test/dm/test.dts
> @@ -17,6 +17,8 @@
> testfdt3 = "/b-test";
> testfdt5 = "/some-bus/c-test at 5";
> testfdt8 = "/a-test";
> + eth0 = "/eth at 10002000";
> + eth5 = ð_5;
> };
>
> uart0: serial {
> @@ -155,7 +157,7 @@
> fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>;
> };
>
> - eth at 10003000 {
> + eth_5: eth at 10003000 {
> compatible = "sandbox,eth";
> reg = <0x10003000 0x1000>;
> fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>;
> --
> 1.7.11.5
>
Regards,
Simon
More information about the U-Boot
mailing list