[U-Boot] [RFC PATCH v3 11/14] dm: eth: Add support for aliases
Joe Hershberger
joe.hershberger at gmail.com
Tue Feb 17 06:04:45 CET 2015
On Sun, Feb 15, 2015 at 9:50 AM, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Joe,
>
> On 10 February 2015 at 18:30, 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>
> >
> > ---
> >
> > 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 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_asserteq(-1, NetLoop(PING));
> > + ut_asserteq_ptr(NULL, getenv("ethact"));
> > +
> > + setenv("ethact", "eth5");
> > + ut_assertok(NetLoop(PING));
> > + ut_asserteq_str("eth at 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 at 5";
> > testfdt8 = "/a-test";
> > + eth0 = "/eth at 10002000";
> > + eth5 = ð_5;
> > };
> >
> > uart0: serial {
> > @@ -150,19 +152,19 @@
> > };
> >
> > eth at 10002000 {
> > - compatible = "sandbox,eth";
> > + compatible = "sandbox,eth", "eth";
> > reg = <0x10002000 0x1000>;
> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>;
> > };
> >
> > - eth at 10003000 {
> > - compatible = "sandbox,eth";
> > + eth_5: eth at 10003000 {
> > + compatible = "sandbox,eth", "eth";
> > reg = <0x10003000 0x1000>;
> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>;
> > };
> >
> > eth at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list