Am 2019-12-18 21:00, schrieb Alexandru Marginean:
Hi Michael,
On 12/18/2019 5:42 PM, Michael Walle wrote:
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.
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.
Cc: Thomas Fitzsimmons <[email protected]>
Cc: Michal Simek <[email protected]>
Signed-off-by: Michael Walle <[email protected]>
---
drivers/core/uclass.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c520ef113a..c3b325141a 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,16 @@ 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
nits: there /is/ no and multi-line comment starts with just /* on the
1st line
yeah, I'd prefer that style too. But checkpatch.pl would complain
about it.. or so I thought.. well seems only to be the case in linux and
only in "NETWORKING_BLOCK_COMMENT_STYLE" (only for net/ and
drivers/net/).
I'll definitely fix that ;)
+ * 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)
Reviewed-by: Alex Marginean <[email protected]>
Tested-by: Alex Marginean <[email protected]>
This looks nice.
One thing I noticed is that 'dm tree' indexes now don't match dev->seq
which can be a bit confusing. The index is produced by
dev_get_uclass_index, which in effect counts devices in the uclass.
Any issue if 'dm tree' prints dev->seq instead and maybe call the
column Seq rather than Index?
Mhh, did this ever match if req_seq/aliases was used? "dm uclass" dumps
the seq and req_seq. So I don't know if that was ever intended to match.
But I'm open to suggestions.
-michael
