| Message ID | 1423618233-11397-12-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Joe, On 10 February 2015 at 18:30, 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> > > --- > > Changes in v3: > -Added support for aliases > > Changes in v2: None > > include/configs/sandbox.h | 4 ++-- > include/fdtdec.h | 1 + > include/net.h | 5 +++++ > lib/fdtdec.c | 1 + > net/eth.c | 53 +++++++++++++++++++++++++++++++++++++++-------- > test/dm/eth.c | 25 ++++++++++++++++++++++ > test/dm/test.dts | 10 +++++---- > 7 files changed, 84 insertions(+), 15 deletions(-) > > diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h > index fdba1c8..9df5f74 100644 > --- a/include/configs/sandbox.h > +++ b/include/configs/sandbox.h > @@ -187,7 +187,7 @@ > "stderr=serial,lcd\0" \ > "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" > #else > > @@ -196,7 +196,7 @@ > "stderr=serial,lcd\0" \ > "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" > #endif > > diff --git a/include/fdtdec.h b/include/fdtdec.h > index 231eed7..e945baa 100644 > --- a/include/fdtdec.h > +++ b/include/fdtdec.h > @@ -167,6 +167,7 @@ enum fdt_compat_id { > COMPAT_INTEL_GMA, /* Intel Graphics Media Accelerator */ > COMPAT_AMS_AS3722, /* AMS AS3722 PMIC */ > COMPAT_INTEL_ICH_SPI, /* Intel ICH7/9 SPI controller */ > + COMPAT_ETHERNET, /* Ethernet devices */ SANDBOX_ETHERNET > > COMPAT_COUNT, > }; > diff --git a/include/net.h b/include/net.h > index 11471bd..4e98850 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -38,6 +38,8 @@ > > #define PKTALIGN ARCH_DMA_MINALIGN > > +#define ETH_MAX_DEVS 32 > + > /* IPv4 addresses are always 32 bits in size */ > typedef __be32 IPaddr_t; > > @@ -79,6 +81,8 @@ enum eth_state_t { > }; > > #ifdef CONFIG_DM_ETH > +#define ETH_ALIAS_ROOT "eth" > + > struct eth_pdata { > phys_addr_t iobase; > unsigned char enetaddr[6]; > @@ -96,6 +100,7 @@ struct eth_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 */ > int eth_init_state_only(bd_t *bis); /* Set active state */ > void eth_halt_state_only(void); /* Set passive state */ > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index 5bf8f29..33b0a53 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -75,6 +75,7 @@ static const char * const compat_names[COMPAT_COUNT] = { > COMPAT(INTEL_GMA, "intel,gma"), > COMPAT(AMS_AS3722, "ams,as3722"), > COMPAT(INTEL_ICH_SPI, "intel,ich-spi"), > + COMPAT(ETHERNET, "eth"), sandbox,eth > }; > > const char *fdtdec_get_compatible(enum fdt_compat_id id) > diff --git a/net/eth.c b/net/eth.c > index e84b948..762effe 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -10,11 +10,14 @@ > #include <command.h> > #include <dm.h> > #include <dm/device-internal.h> > +#include <fdtdec.h> > #include <net.h> > #include <miiphy.h> > #include <phy.h> > #include <asm/errno.h> > > +DECLARE_GLOBAL_DATA_PTR; > + > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) > { > char *end; > @@ -121,6 +124,39 @@ static void eth_set_dev(struct udevice *dev) > uc_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 node_list[ETH_MAX_DEVS]; > + int count; > + int seq; > + char *endp = NULL; > + const char *true_name = devname; > + struct udevice *it; > + struct uclass *uc; > + > + count = fdtdec_find_aliases_for_id(gd->fdt_blob, ETH_ALIAS_ROOT, > + COMPAT_ETHERNET, node_list, > + ETH_MAX_DEVS); > + > + seq = simple_strtoul(devname + strlen(ETH_ALIAS_ROOT), &endp, 10); > + > + if (endp > devname + strlen(ETH_ALIAS_ROOT) && count > seq && > + node_list[seq]) > + true_name = fdt_get_name(gd->fdt_blob, node_list[seq], NULL); > + > + uclass_get(UCLASS_ETH, &uc); > + uclass_foreach_dev(it, uc) { > + if (strcmp(it->name, true_name) == 0 || it->seq == seq) > + return it; > + } Is it possible for eth_get_dev_by_name() to just look up the name provided? You already have this: uclass_foreach_dev(it, uc) { if (strcmp(it->name, true_name) == 0 || it->seq == seq) return it; } It feels like you just need to through through the devices in the uclass and strcmp(dev->name, find_name). I guess I am suffering because I don't understand what you are trying to do, so am not sure if there is an easier way with driver model. We really should not go looking in the device tree in eth_get_dev_by_name()...driver is responsible for parsing that. > + > + return NULL; > +} > + > unsigned char *eth_get_ethaddr(void) > { > struct eth_pdata *pdata; > @@ -396,6 +432,7 @@ UCLASS_DRIVER(eth) = { > .init = eth_uclass_init, > .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 > > @@ -428,6 +465,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; > @@ -844,7 +886,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(); > @@ -852,14 +893,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 2b29fa2..c0a8ab5 100644 > --- a/test/dm/eth.c > +++ b/test/dm/eth.c > @@ -37,3 +37,28 @@ static int dm_test_eth(struct dm_test_state *dms) > } > > 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_asserteq(-1, NetLoop(PING)); > + ut_asserteq_ptr(NULL, 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..c5008c3 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 { > @@ -150,19 +152,19 @@ > }; > > eth@10002000 { > - compatible = "sandbox,eth"; > + compatible = "sandbox,eth", "eth"; > reg = <0x10002000 0x1000>; > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; > }; > > - eth@10003000 { > - compatible = "sandbox,eth"; > + eth_5: eth@10003000 { > + compatible = "sandbox,eth", "eth"; > reg = <0x10003000 0x1000>; > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; > }; > > eth@10004000 { > - compatible = "sandbox,eth"; > + compatible = "sandbox,eth", "eth"; > reg = <0x10004000 0x1000>; > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; > }; > -- > 1.7.11.5 > Regards, Simon
On Sun, Feb 15, 2015 at 9:50 AM, Simon Glass <sjg@chromium.org> wrote: > > Hi Joe, > > On 10 February 2015 at 18:30, 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> > > > > --- > > > > Changes in v3: > > -Added support for aliases > > > > Changes in v2: None > > > > include/configs/sandbox.h | 4 ++-- > > include/fdtdec.h | 1 + > > include/net.h | 5 +++++ > > lib/fdtdec.c | 1 + > > net/eth.c | 53 +++++++++++++++++++++++++++++++++++++++-------- > > test/dm/eth.c | 25 ++++++++++++++++++++++ > > test/dm/test.dts | 10 +++++---- > > 7 files changed, 84 insertions(+), 15 deletions(-) > > > > diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h > > index fdba1c8..9df5f74 100644 > > --- a/include/configs/sandbox.h > > +++ b/include/configs/sandbox.h > > @@ -187,7 +187,7 @@ > > "stderr=serial,lcd\0" \ > > "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" > > #else > > > > @@ -196,7 +196,7 @@ > > "stderr=serial,lcd\0" \ > > "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" > > #endif > > > > diff --git a/include/fdtdec.h b/include/fdtdec.h > > index 231eed7..e945baa 100644 > > --- a/include/fdtdec.h > > +++ b/include/fdtdec.h > > @@ -167,6 +167,7 @@ enum fdt_compat_id { > > COMPAT_INTEL_GMA, /* Intel Graphics Media Accelerator */ > > COMPAT_AMS_AS3722, /* AMS AS3722 PMIC */ > > COMPAT_INTEL_ICH_SPI, /* Intel ICH7/9 SPI controller */ > > + COMPAT_ETHERNET, /* Ethernet devices */ > > SANDBOX_ETHERNET This is not limited to sandbox. This is needed for all Ethernet MACs. Is there some other way that I should be identifying with all devices in the device tree of a certain class? > > > > COMPAT_COUNT, > > }; > > diff --git a/include/net.h b/include/net.h > > index 11471bd..4e98850 100644 > > --- a/include/net.h > > +++ b/include/net.h > > @@ -38,6 +38,8 @@ > > > > #define PKTALIGN ARCH_DMA_MINALIGN > > > > +#define ETH_MAX_DEVS 32 > > + > > /* IPv4 addresses are always 32 bits in size */ > > typedef __be32 IPaddr_t; > > > > @@ -79,6 +81,8 @@ enum eth_state_t { > > }; > > > > #ifdef CONFIG_DM_ETH > > +#define ETH_ALIAS_ROOT "eth" > > + > > struct eth_pdata { > > phys_addr_t iobase; > > unsigned char enetaddr[6]; > > @@ -96,6 +100,7 @@ struct eth_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 */ > > int eth_init_state_only(bd_t *bis); /* Set active state */ > > void eth_halt_state_only(void); /* Set passive state */ > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > index 5bf8f29..33b0a53 100644 > > --- a/lib/fdtdec.c > > +++ b/lib/fdtdec.c > > @@ -75,6 +75,7 @@ static const char * const compat_names[COMPAT_COUNT] = { > > COMPAT(INTEL_GMA, "intel,gma"), > > COMPAT(AMS_AS3722, "ams,as3722"), > > COMPAT(INTEL_ICH_SPI, "intel,ich-spi"), > > + COMPAT(ETHERNET, "eth"), > > sandbox,eth Again, this is used to identify all Ethernet controllers. Perhaps fdtdec_find_aliases_for_id() should not be limiting the alias search to those that have a certain "compatible" string? > > }; > > > > const char *fdtdec_get_compatible(enum fdt_compat_id id) > > diff --git a/net/eth.c b/net/eth.c > > index e84b948..762effe 100644 > > --- a/net/eth.c > > +++ b/net/eth.c > > @@ -10,11 +10,14 @@ > > #include <command.h> > > #include <dm.h> > > #include <dm/device-internal.h> > > +#include <fdtdec.h> > > #include <net.h> > > #include <miiphy.h> > > #include <phy.h> > > #include <asm/errno.h> > > > > +DECLARE_GLOBAL_DATA_PTR; > > + > > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) > > { > > char *end; > > @@ -121,6 +124,39 @@ static void eth_set_dev(struct udevice *dev) > > uc_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 node_list[ETH_MAX_DEVS]; > > + int count; > > + int seq; > > + char *endp = NULL; > > + const char *true_name = devname; > > + struct udevice *it; > > + struct uclass *uc; > > + > > + count = fdtdec_find_aliases_for_id(gd->fdt_blob, ETH_ALIAS_ROOT, > > + COMPAT_ETHERNET, node_list, > > + ETH_MAX_DEVS); > > + > > + seq = simple_strtoul(devname + strlen(ETH_ALIAS_ROOT), &endp, 10); > > + > > + if (endp > devname + strlen(ETH_ALIAS_ROOT) && count > seq && > > + node_list[seq]) > > + true_name = fdt_get_name(gd->fdt_blob, node_list[seq], NULL); > > + > > + uclass_get(UCLASS_ETH, &uc); > > + uclass_foreach_dev(it, uc) { > > + if (strcmp(it->name, true_name) == 0 || it->seq == seq) > > + return it; > > + } > > Is it possible for eth_get_dev_by_name() to just look up the name provided? > > You already have this: > > uclass_foreach_dev(it, uc) { > if (strcmp(it->name, true_name) == 0 || it->seq == seq) > return it; > } > > It feels like you just need to through through the devices in the > uclass and strcmp(dev->name, find_name). That works fine for looking up the exact name, but that's not the point of this patch. Such code was already there and you can see it below. > I guess I am suffering because I don't understand what you are trying > to do, so am not sure if there is an easier way with driver model. We > really should not go looking in the device tree in > eth_get_dev_by_name()...driver is responsible for parsing that. Perhaps this is a limitation of fdtdec_find_aliases_for_id(). I need to be able to look up the device based on its alias. It doesn't seem to me that I'm digging around in the device tree at random places that should be done by the driver. I'm simply wanting to ask the device tree what the alias that the user passed in is pointing at. Potentially is it not an alias, but a full name. Either way, I want to return the udevice. > > + > > + return NULL; > > +} > > + > > unsigned char *eth_get_ethaddr(void) > > { > > struct eth_pdata *pdata; > > @@ -396,6 +432,7 @@ UCLASS_DRIVER(eth) = { > > .init = eth_uclass_init, > > .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 > > > > @@ -428,6 +465,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; > > @@ -844,7 +886,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(); > > @@ -852,14 +893,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()); > > - } This is the simple compare that is being replaced. > > + 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 2b29fa2..c0a8ab5 100644 > > --- a/test/dm/eth.c > > +++ b/test/dm/eth.c > > @@ -37,3 +37,28 @@ static int dm_test_eth(struct dm_test_state *dms) > > } > > > > 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_asserteq(-1, NetLoop(PING)); > > + ut_asserteq_ptr(NULL, 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..c5008c3 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 { > > @@ -150,19 +152,19 @@ > > }; > > > > eth@10002000 { > > - compatible = "sandbox,eth"; > > + compatible = "sandbox,eth", "eth"; > > reg = <0x10002000 0x1000>; > > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; > > }; > > > > - eth@10003000 { > > - compatible = "sandbox,eth"; > > + eth_5: eth@10003000 { > > + compatible = "sandbox,eth", "eth"; > > reg = <0x10003000 0x1000>; > > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; > > }; > > > > eth@10004000 { > > - compatible = "sandbox,eth"; > > + compatible = "sandbox,eth", "eth"; > > reg = <0x10004000 0x1000>; > > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; > > }; > > -- > > 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 16 February 2015 at 22:04, Joe Hershberger <joe.hershberger@gmail.com> wrote: > > > On Sun, Feb 15, 2015 at 9:50 AM, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Joe, >> >> On 10 February 2015 at 18:30, 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> >> > >> > --- >> > >> > Changes in v3: >> > -Added support for aliases >> > >> > Changes in v2: None >> > >> > include/configs/sandbox.h | 4 ++-- >> > include/fdtdec.h | 1 + >> > include/net.h | 5 +++++ >> > lib/fdtdec.c | 1 + >> > net/eth.c | 53 >> > +++++++++++++++++++++++++++++++++++++++-------- >> > test/dm/eth.c | 25 ++++++++++++++++++++++ >> > test/dm/test.dts | 10 +++++---- >> > 7 files changed, 84 insertions(+), 15 deletions(-) >> > >> > diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h >> > index fdba1c8..9df5f74 100644 >> > --- a/include/configs/sandbox.h >> > +++ b/include/configs/sandbox.h >> > @@ -187,7 +187,7 @@ >> > "stderr=serial,lcd\0" \ >> > "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" >> > #else >> > >> > @@ -196,7 +196,7 @@ >> > "stderr=serial,lcd\0" \ >> > "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" >> > #endif >> > >> > diff --git a/include/fdtdec.h b/include/fdtdec.h >> > index 231eed7..e945baa 100644 >> > --- a/include/fdtdec.h >> > +++ b/include/fdtdec.h >> > @@ -167,6 +167,7 @@ enum fdt_compat_id { >> > COMPAT_INTEL_GMA, /* Intel Graphics Media >> > Accelerator */ >> > COMPAT_AMS_AS3722, /* AMS AS3722 PMIC */ >> > COMPAT_INTEL_ICH_SPI, /* Intel ICH7/9 SPI controller >> > */ >> > + COMPAT_ETHERNET, /* Ethernet devices */ >> >> SANDBOX_ETHERNET > > This is not limited to sandbox. This is needed for all Ethernet MACs. Is > there some other way that I should be identifying with all devices in the > device tree of a certain class? Not that I know of. But each Ethernet driver would need its own compatible string, since otherwise driver model won't use the right driver. > >> > >> > COMPAT_COUNT, >> > }; >> > diff --git a/include/net.h b/include/net.h >> > index 11471bd..4e98850 100644 >> > --- a/include/net.h >> > +++ b/include/net.h >> > @@ -38,6 +38,8 @@ >> > >> > #define PKTALIGN ARCH_DMA_MINALIGN >> > >> > +#define ETH_MAX_DEVS 32 >> > + >> > /* IPv4 addresses are always 32 bits in size */ >> > typedef __be32 IPaddr_t; >> > >> > @@ -79,6 +81,8 @@ enum eth_state_t { >> > }; >> > >> > #ifdef CONFIG_DM_ETH >> > +#define ETH_ALIAS_ROOT "eth" >> > + >> > struct eth_pdata { >> > phys_addr_t iobase; >> > unsigned char enetaddr[6]; >> > @@ -96,6 +100,7 @@ struct eth_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 */ >> > int eth_init_state_only(bd_t *bis); /* Set active state */ >> > void eth_halt_state_only(void); /* Set passive state */ >> > diff --git a/lib/fdtdec.c b/lib/fdtdec.c >> > index 5bf8f29..33b0a53 100644 >> > --- a/lib/fdtdec.c >> > +++ b/lib/fdtdec.c >> > @@ -75,6 +75,7 @@ static const char * const compat_names[COMPAT_COUNT] = >> > { >> > COMPAT(INTEL_GMA, "intel,gma"), >> > COMPAT(AMS_AS3722, "ams,as3722"), >> > COMPAT(INTEL_ICH_SPI, "intel,ich-spi"), >> > + COMPAT(ETHERNET, "eth"), >> >> sandbox,eth > > Again, this is used to identify all Ethernet controllers. Perhaps > fdtdec_find_aliases_for_id() should not be limiting the alias search to > those that have a certain "compatible" string? I think you want 'sandbox,ethernet' here, or similar, but you probably don't want to use that fdtdec function ultimately. Driver model should do it all for you. > > >> > }; >> > >> > const char *fdtdec_get_compatible(enum fdt_compat_id id) >> > diff --git a/net/eth.c b/net/eth.c >> > index e84b948..762effe 100644 >> > --- a/net/eth.c >> > +++ b/net/eth.c >> > @@ -10,11 +10,14 @@ >> > #include <command.h> >> > #include <dm.h> >> > #include <dm/device-internal.h> >> > +#include <fdtdec.h> >> > #include <net.h> >> > #include <miiphy.h> >> > #include <phy.h> >> > #include <asm/errno.h> >> > >> > +DECLARE_GLOBAL_DATA_PTR; >> > + >> > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) >> > { >> > char *end; >> > @@ -121,6 +124,39 @@ static void eth_set_dev(struct udevice *dev) >> > uc_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 node_list[ETH_MAX_DEVS]; >> > + int count; >> > + int seq; >> > + char *endp = NULL; >> > + const char *true_name = devname; >> > + struct udevice *it; >> > + struct uclass *uc; >> > + >> > + count = fdtdec_find_aliases_for_id(gd->fdt_blob, ETH_ALIAS_ROOT, >> > + COMPAT_ETHERNET, node_list, >> > + ETH_MAX_DEVS); >> > + >> > + seq = simple_strtoul(devname + strlen(ETH_ALIAS_ROOT), &endp, >> > 10); >> > + >> > + if (endp > devname + strlen(ETH_ALIAS_ROOT) && count > seq && >> > + node_list[seq]) >> > + true_name = fdt_get_name(gd->fdt_blob, node_list[seq], >> > NULL); >> > + >> > + uclass_get(UCLASS_ETH, &uc); >> > + uclass_foreach_dev(it, uc) { >> > + if (strcmp(it->name, true_name) == 0 || it->seq == seq) >> > + return it; >> > + } >> >> Is it possible for eth_get_dev_by_name() to just look up the name >> provided? >> >> You already have this: >> >> uclass_foreach_dev(it, uc) { >> if (strcmp(it->name, true_name) == 0 || it->seq == seq) >> return it; >> } >> >> It feels like you just need to through through the devices in the >> uclass and strcmp(dev->name, find_name). > > That works fine for looking up the exact name, but that's not the point of > this patch. Such code was already there and you can see it below. OK > >> I guess I am suffering because I don't understand what you are trying >> to do, so am not sure if there is an easier way with driver model. We >> really should not go looking in the device tree in >> eth_get_dev_by_name()...driver is responsible for parsing that. > > Perhaps this is a limitation of fdtdec_find_aliases_for_id(). I need to be > able to look up the device based on its alias. It doesn't seem to me that > I'm digging around in the device tree at random places that should be done > by the driver. I'm simply wanting to ask the device tree what the alias > that the user passed in is pointing at. Potentially is it not an alias, but > a full name. Either way, I want to return the udevice. If you want to know the device's number, use dev->req_seq. This will hold the alias number. If it is -1 then the device is not mentioned in /aliases in the device tree. But please see if you can use the existing driver model features to find devices, their numbers, etc. > > >> > + >> > + return NULL; >> > +} >> > + >> > unsigned char *eth_get_ethaddr(void) >> > { >> > struct eth_pdata *pdata; >> > @@ -396,6 +432,7 @@ UCLASS_DRIVER(eth) = { >> > .init = eth_uclass_init, >> > .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 >> > >> > @@ -428,6 +465,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; >> > @@ -844,7 +886,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(); >> > @@ -852,14 +893,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()); >> > - } > > This is the simple compare that is being replaced. > >> > + 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 2b29fa2..c0a8ab5 100644 >> > --- a/test/dm/eth.c >> > +++ b/test/dm/eth.c >> > @@ -37,3 +37,28 @@ static int dm_test_eth(struct dm_test_state *dms) >> > } >> > >> > 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_asserteq(-1, NetLoop(PING)); >> > + ut_asserteq_ptr(NULL, 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..c5008c3 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 { >> > @@ -150,19 +152,19 @@ >> > }; >> > >> > eth@10002000 { >> > - compatible = "sandbox,eth"; >> > + compatible = "sandbox,eth", "eth"; >> > reg = <0x10002000 0x1000>; >> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; >> > }; >> > >> > - eth@10003000 { >> > - compatible = "sandbox,eth"; >> > + eth_5: eth@10003000 { >> > + compatible = "sandbox,eth", "eth"; >> > reg = <0x10003000 0x1000>; >> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; >> > }; >> > >> > eth@10004000 { >> > - compatible = "sandbox,eth"; >> > + compatible = "sandbox,eth", "eth"; >> > reg = <0x10004000 0x1000>; >> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; >> > }; >> > -- >> > 1.7.11.5 Regards, Simon
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index fdba1c8..9df5f74 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -187,7 +187,7 @@ "stderr=serial,lcd\0" \ "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" #else @@ -196,7 +196,7 @@ "stderr=serial,lcd\0" \ "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" #endif diff --git a/include/fdtdec.h b/include/fdtdec.h index 231eed7..e945baa 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -167,6 +167,7 @@ enum fdt_compat_id { COMPAT_INTEL_GMA, /* Intel Graphics Media Accelerator */ COMPAT_AMS_AS3722, /* AMS AS3722 PMIC */ COMPAT_INTEL_ICH_SPI, /* Intel ICH7/9 SPI controller */ + COMPAT_ETHERNET, /* Ethernet devices */ COMPAT_COUNT, }; diff --git a/include/net.h b/include/net.h index 11471bd..4e98850 100644 --- a/include/net.h +++ b/include/net.h @@ -38,6 +38,8 @@ #define PKTALIGN ARCH_DMA_MINALIGN +#define ETH_MAX_DEVS 32 + /* IPv4 addresses are always 32 bits in size */ typedef __be32 IPaddr_t; @@ -79,6 +81,8 @@ enum eth_state_t { }; #ifdef CONFIG_DM_ETH +#define ETH_ALIAS_ROOT "eth" + struct eth_pdata { phys_addr_t iobase; unsigned char enetaddr[6]; @@ -96,6 +100,7 @@ struct eth_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 */ int eth_init_state_only(bd_t *bis); /* Set active state */ void eth_halt_state_only(void); /* Set passive state */ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 5bf8f29..33b0a53 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -75,6 +75,7 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(INTEL_GMA, "intel,gma"), COMPAT(AMS_AS3722, "ams,as3722"), COMPAT(INTEL_ICH_SPI, "intel,ich-spi"), + COMPAT(ETHERNET, "eth"), }; const char *fdtdec_get_compatible(enum fdt_compat_id id) diff --git a/net/eth.c b/net/eth.c index e84b948..762effe 100644 --- a/net/eth.c +++ b/net/eth.c @@ -10,11 +10,14 @@ #include <command.h> #include <dm.h> #include <dm/device-internal.h> +#include <fdtdec.h> #include <net.h> #include <miiphy.h> #include <phy.h> #include <asm/errno.h> +DECLARE_GLOBAL_DATA_PTR; + void eth_parse_enetaddr(const char *addr, uchar *enetaddr) { char *end; @@ -121,6 +124,39 @@ static void eth_set_dev(struct udevice *dev) uc_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 node_list[ETH_MAX_DEVS]; + int count; + int seq; + char *endp = NULL; + const char *true_name = devname; + struct udevice *it; + struct uclass *uc; + + count = fdtdec_find_aliases_for_id(gd->fdt_blob, ETH_ALIAS_ROOT, + COMPAT_ETHERNET, node_list, + ETH_MAX_DEVS); + + seq = simple_strtoul(devname + strlen(ETH_ALIAS_ROOT), &endp, 10); + + if (endp > devname + strlen(ETH_ALIAS_ROOT) && count > seq && + node_list[seq]) + true_name = fdt_get_name(gd->fdt_blob, node_list[seq], NULL); + + uclass_get(UCLASS_ETH, &uc); + uclass_foreach_dev(it, uc) { + if (strcmp(it->name, true_name) == 0 || it->seq == seq) + return it; + } + + return NULL; +} + unsigned char *eth_get_ethaddr(void) { struct eth_pdata *pdata; @@ -396,6 +432,7 @@ UCLASS_DRIVER(eth) = { .init = eth_uclass_init, .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 @@ -428,6 +465,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; @@ -844,7 +886,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(); @@ -852,14 +893,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 2b29fa2..c0a8ab5 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -37,3 +37,28 @@ static int dm_test_eth(struct dm_test_state *dms) } 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_asserteq(-1, NetLoop(PING)); + ut_asserteq_ptr(NULL, 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..c5008c3 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 { @@ -150,19 +152,19 @@ }; eth@10002000 { - compatible = "sandbox,eth"; + compatible = "sandbox,eth", "eth"; reg = <0x10002000 0x1000>; fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>; }; - eth@10003000 { - compatible = "sandbox,eth"; + eth_5: eth@10003000 { + compatible = "sandbox,eth", "eth"; reg = <0x10003000 0x1000>; fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; }; eth@10004000 { - compatible = "sandbox,eth"; + compatible = "sandbox,eth", "eth"; reg = <0x10004000 0x1000>; fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x22>; };
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 v3: -Added support for aliases Changes in v2: None include/configs/sandbox.h | 4 ++-- include/fdtdec.h | 1 + include/net.h | 5 +++++ lib/fdtdec.c | 1 + net/eth.c | 53 +++++++++++++++++++++++++++++++++++++++-------- test/dm/eth.c | 25 ++++++++++++++++++++++ test/dm/test.dts | 10 +++++---- 7 files changed, 84 insertions(+), 15 deletions(-)