[PATCH v3] dm: uclass: don't assign aliased seq numbers
Michael Walle
michael at walle.cc
Mon Feb 3 18:11:52 CET 2020
If there are aliases for an uclass, set the base for the "dynamically"
allocated numbers next to the highest alias.
Please note, that this might lead to holes in the sequences, depending
on the device tree. For example if there is only an alias "ethernet1",
the next device seq number would be 2.
In particular this fixes a problem with boards which are using ethernet
aliases but also might have network add-in cards like the E1000. If the
board is started with the add-in card and depending on the order of the
drivers, the E1000 might occupy the first ethernet device and mess up
all the hardware addresses, because the devices are now shifted by one.
Also adapt the test cases to the new handling and add test cases
checking the holes in the seq numbers.
Signed-off-by: Michael Walle <michael at walle.cc>
Reviewed-by: Alex Marginean <alexandru.marginean at nxp.com>
Tested-by: Alex Marginean <alexandru.marginean at nxp.com>
Acked-by: Vladimir Oltean <olteanv at gmail.com>
---
Please note that I've kept the R-b, T-b, and A-b tags although they were
for an older version. They only affects the drivers/core/uclass.c not the
test/dm/ part. OTOH none of the actual implementation has changed.
I couldn't split the patch, otherwise the tests would fail.
As a side effect, this should also make the following commits
superfluous:
- 7f3289bf6d ("dm: device: Request next sequence number")
- 61607225d1 ("i2c: Fill req_seq in i2c_post_bind()")
Although I don't understand the root cause of the said problem.
Thomas, Michal, could you please test this and then I'd add a second
patch removing the old code.
changes since v2:
- adapt/new test cases, thanks Simon
changes since v1:
- move notice about superfluous commits from commit message to this
section.
- fix the comment style
arch/sandbox/dts/test.dts | 4 ++--
drivers/core/uclass.c | 21 +++++++++++++++------
include/configs/sandbox.h | 6 +++---
test/dm/eth.c | 14 +++++++-------
test/dm/test-fdt.c | 22 +++++++++++++++++-----
5 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index e529c54d8d..d448892a65 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -19,8 +19,8 @@
pci0 = &pci0;
pci1 = &pci1;
pci2 = &pci2;
- remoteproc1 = &rproc_1;
- remoteproc2 = &rproc_2;
+ remoteproc0 = &rproc_1;
+ remoteproc1 = &rproc_2;
rtc0 = &rtc_0;
rtc1 = &rtc_1;
spi0 = "/spi at 0";
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c520ef113a..3c216221e0 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -675,13 +675,14 @@ int uclass_unbind_device(struct udevice *dev)
int uclass_resolve_seq(struct udevice *dev)
{
+ struct uclass *uc = dev->uclass;
+ struct uclass_driver *uc_drv = uc->uc_drv;
struct udevice *dup;
- int seq;
+ int seq = 0;
int ret;
assert(dev->seq == -1);
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, dev->req_seq,
- false, &dup);
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false, &dup);
if (!ret) {
dm_warn("Device '%s': seq %d is in use by '%s'\n",
dev->name, dev->req_seq, dup->name);
@@ -693,9 +694,17 @@ int uclass_resolve_seq(struct udevice *dev)
return ret;
}
- for (seq = 0; seq < DM_MAX_SEQ; seq++) {
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, seq,
- false, &dup);
+ if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
+ (uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
+ /*
+ * dev_read_alias_highest_id() will return -1 if there no
+ * alias. Thus we can always add one.
+ */
+ seq = dev_read_alias_highest_id(uc_drv->name) + 1;
+ }
+
+ for (; seq < DM_MAX_SEQ; seq++) {
+ ret = uclass_find_device_by_seq(uc_drv->id, seq, false, &dup);
if (ret == -ENODEV)
break;
if (ret)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 1c13055cdc..b02c362fed 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -97,9 +97,9 @@
#endif
#define SANDBOX_ETH_SETTINGS "ethaddr=00:00:11:22:33:44\0" \
- "eth1addr=00:00:11:22:33:45\0" \
- "eth3addr=00:00:11:22:33:46\0" \
- "eth5addr=00:00:11:22:33:47\0" \
+ "eth3addr=00:00:11:22:33:45\0" \
+ "eth5addr=00:00:11:22:33:46\0" \
+ "eth6addr=00:00:11:22:33:47\0" \
"ipaddr=1.2.3.4\0"
#define MEM_LAYOUT_ENV_SETTINGS \
diff --git a/test/dm/eth.c b/test/dm/eth.c
index ad5354b4bf..75315a0c6d 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -47,7 +47,7 @@ static int dm_test_eth_alias(struct unit_test_state *uts)
ut_assertok(net_loop(PING));
ut_asserteq_str("eth at 10002000", env_get("ethact"));
- env_set("ethact", "eth1");
+ env_set("ethact", "eth6");
ut_assertok(net_loop(PING));
ut_asserteq_str("eth at 10004000", env_get("ethact"));
@@ -104,7 +104,7 @@ static int dm_test_eth_act(struct unit_test_state *uts)
const char *ethname[DM_TEST_ETH_NUM] = {"eth at 10002000", "eth at 10003000",
"sbe5", "eth at 10004000"};
const char *addrname[DM_TEST_ETH_NUM] = {"ethaddr", "eth5addr",
- "eth3addr", "eth1addr"};
+ "eth3addr", "eth6addr"};
char ethaddr[DM_TEST_ETH_NUM][18];
int i;
@@ -187,15 +187,15 @@ static int dm_test_eth_rotate(struct unit_test_state *uts)
/* Invalidate eth1's MAC address */
memset(ethaddr, '\0', sizeof(ethaddr));
- strncpy(ethaddr, env_get("eth1addr"), 17);
- /* Must disable access protection for eth1addr before clearing */
- env_set(".flags", "eth1addr");
- env_set("eth1addr", NULL);
+ strncpy(ethaddr, env_get("eth6addr"), 17);
+ /* Must disable access protection for eth6addr before clearing */
+ env_set(".flags", "eth6addr");
+ env_set("eth6addr", NULL);
retval = _dm_test_eth_rotate1(uts);
/* Restore the env */
- env_set("eth1addr", ethaddr);
+ env_set("eth6addr", ethaddr);
env_set("ethrotate", NULL);
if (!retval) {
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index d59c449ce0..a4e5c64a1f 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -359,20 +359,32 @@ static int dm_test_fdt_uclass_seq(struct unit_test_state *uts)
ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 2, &dev));
ut_asserteq_str("d-test", dev->name);
- /* d-test actually gets 0 */
- ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 0, &dev));
+ /*
+ * d-test actually gets 9, because thats the next free one after the
+ * aliases.
+ */
+ ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 9, &dev));
ut_asserteq_str("d-test", dev->name);
- /* initially no one wants seq 1 */
- ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 1,
+ /* initially no one wants seq 10 */
+ ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 10,
&dev));
ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 4, &dev));
/* But now that it is probed, we can find it */
- ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 1, &dev));
+ ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 10, &dev));
ut_asserteq_str("f-test", dev->name);
+ /*
+ * And we should still have holes in our sequence numbers, that is 2
+ * and 4 should not be used.
+ */
+ ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 2,
+ true, &dev));
+ ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 4,
+ true, &dev));
+
return 0;
}
DM_TEST(dm_test_fdt_uclass_seq, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.20.1
More information about the U-Boot
mailing list