From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 1 Mar 2015 11:07:30 -0700 Subject: [U-Boot] [RFC PATCH v4 16/23] dm: eth: Add support for aliases In-Reply-To: <1424822552-4366-17-git-send-email-joe.hershberger@ni.com> References: <1423618233-11397-1-git-send-email-joe.hershberger@ni.com> <1424822552-4366-1-git-send-email-joe.hershberger@ni.com> <1424822552-4366-17-git-send-email-joe.hershberger@ni.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Joe, On 24 February 2015 at 17:02, Joe Hershberger 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 > Reviewed-by: Simon Glass 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