| Message ID | 1424822552-4366-17-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Joe, On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@ni.com> wrote: > Allow network devices to be referred to as "eth0" instead of > "eth@12345678" when specified in ethact. > > Add tests to verify this behavior. > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > Reviewed-by: Simon Glass <sjg@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@10002000", getenv("ethact")); > + > + setenv("ethact", "eth1"); > + ut_assertok(NetLoop(PING)); > + ut_asserteq_str("eth@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@10002000", getenv("ethact")); > + > + setenv("ethact", "eth5"); > + ut_assertok(NetLoop(PING)); > + ut_asserteq_str("eth@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@5"; > testfdt8 = "/a-test"; > + eth0 = "/eth@10002000"; > + eth5 = ð_5; > }; > > uart0: serial { > @@ -155,7 +157,7 @@ > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; > }; > > - eth@10003000 { > + eth_5: eth@10003000 { > compatible = "sandbox,eth"; > reg = <0x10003000 0x1000>; > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; > -- > 1.7.11.5 > Regards, Simon
On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg@chromium.org> wrote: > > Hi Joe, > > On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@ni.com> wrote: > > Allow network devices to be referred to as "eth0" instead of > > "eth@12345678" when specified in ethact. > > > > Add tests to verify this behavior. > > > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > > > > Reviewed-by: Simon Glass <sjg@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. OK > Also it seems to requite a minimum length of 3 > characters? Good point. This is a bug. I should be checking the size first. It is not an intended requirement. > > 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. This is simply searching. If a device annot be probed, why error out a search for presumably a different device? I can look into adding a unit test to validate this behavior. > > + /* > > + * 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. I'll think about how to make errors be reported when you explicitly ask for a device, but not when you are just scanning through them. > > > > 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@10002000", getenv("ethact")); > > + > > + setenv("ethact", "eth1"); > > + ut_assertok(NetLoop(PING)); > > + ut_asserteq_str("eth@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@10002000", getenv("ethact")); > > + > > + setenv("ethact", "eth5"); > > + ut_assertok(NetLoop(PING)); > > + ut_asserteq_str("eth@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@5"; > > testfdt8 = "/a-test"; > > + eth0 = "/eth@10002000"; > > + eth5 = ð_5; > > }; > > > > uart0: serial { > > @@ -155,7 +157,7 @@ > > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; > > }; > > > > - eth@10003000 { > > + eth_5: eth@10003000 { > > compatible = "sandbox,eth"; > > reg = <0x10003000 0x1000>; > > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; > > -- > > 1.7.11.5 > > > > Regards, > Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe, On 1 March 2015 at 15:04, Joe Hershberger <joe.hershberger@gmail.com> wrote: > > > On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Joe, >> >> On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger@ni.com> >> wrote: >> > Allow network devices to be referred to as "eth0" instead of >> > "eth@12345678" when specified in ethact. >> > >> > Add tests to verify this behavior. >> > >> > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> > >> >> Reviewed-by: Simon Glass <sjg@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. > > OK > >> Also it seems to requite a minimum length of 3 >> characters? > > Good point. This is a bug. I should be checking the size first. It is not an > intended requirement. > >> > 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. > > This is simply searching. If a device annot be probed, why error out a > search for presumably a different device? I can look into adding a unit test > to validate this behavior. Well normal error behaviour is to report the error to the upper layers which may or may not stop. But a failure to probe a device which should be there seems bad. Anyway if you are wanting to not check these errors you should at least add big comments in those places as to why. I'd hate for people not to understand the rationale and just assume that errors don't matter (because they don't understand the special cases here). > >> > + /* >> > + * 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. > > I'll think about how to make errors be reported when you explicitly ask for > a device, but not when you are just scanning through them. It's an interesting question, and fair enough - this is a design decision. But please comment it. [snip] Regards, Simon
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); 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); + /* + * 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)); 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@10002000", getenv("ethact")); + + setenv("ethact", "eth1"); + ut_assertok(NetLoop(PING)); + ut_asserteq_str("eth@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@10002000", getenv("ethact")); + + setenv("ethact", "eth5"); + ut_assertok(NetLoop(PING)); + ut_asserteq_str("eth@10003000", getenv("ethact")); + + 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@5"; testfdt8 = "/a-test"; + eth0 = "/eth@10002000"; + eth5 = ð_5; }; uart0: serial { @@ -155,7 +157,7 @@ fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; }; - eth@10003000 { + eth_5: eth@10003000 { compatible = "sandbox,eth"; reg = <0x10003000 0x1000>; fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>;
Allow network devices to be referred to as "eth0" instead of "eth@12345678" when specified in ethact. Add tests to verify this behavior. Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- 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(-)