Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
Hi Michael, On Wed, 5 Feb 2020 at 13:05, Simon Glass wrote: > > On Mon, 3 Feb 2020 at 10:12, 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. > > > > 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 > > Reviewed-by: Alex Marginean > > Tested-by: Alex Marginean > > Acked-by: Vladimir Oltean > > --- > > Reviewed-by: Simon Glass > > (added Joe for review fo network parts) Unfortunately this breaks some ARM and PPC boards: https://gitlab.denx.de/u-boot/custodians/u-boot-dm/pipelines/2308 I've dropped it for now. Please can you take a look? Regards, Simon
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
On 20. 02. 20 21:16, Michael Walle wrote:
>
> Hi Michal,
>
> Am 2020-02-20 11:14, schrieb Michal Simek:
>> On 20. 02. 20 10:52, Michael Walle wrote:
>>> Hi Michal,
>>>
>>> Am 2020-02-20 09:30, schrieb Michal Simek:
On 03. 02. 20 18:11, 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.
>
> 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
> Reviewed-by: Alex Marginean
> Tested-by: Alex Marginean
> Acked-by: Vladimir Oltean
> ---
>
> 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 =
> pci1 =
> pci2 =
> - remoteproc1 = _1;
> - remoteproc2 = _2;
> + remoteproc0 = _1;
> + remoteproc1 = _2;
> rtc0 = _0;
> rtc1 = _1;
> spi0 = "/spi@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, );
> + ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false,
> );
> 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, );
> + 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,
> );
> 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
>
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
Hi Michal,
Am 2020-02-20 11:14, schrieb Michal Simek:
On 20. 02. 20 10:52, Michael Walle wrote:
Hi Michal,
Am 2020-02-20 09:30, schrieb Michal Simek:
On 03. 02. 20 18:11, 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.
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
Reviewed-by: Alex Marginean
Tested-by: Alex Marginean
Acked-by: Vladimir Oltean
---
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 =
pci1 =
pci2 =
- remoteproc1 = _1;
- remoteproc2 = _2;
+ remoteproc0 = _1;
+ remoteproc1 = _2;
rtc0 = _0;
rtc1 = _1;
spi0 = "/spi@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, );
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq,
false,
);
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, );
+ 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,
);
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));
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
On 20. 02. 20 10:52, Michael Walle wrote:
> Hi Michal,
>
> Am 2020-02-20 09:30, schrieb Michal Simek:
>> On 03. 02. 20 18:11, 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.
>>>
>>> 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
>>> Reviewed-by: Alex Marginean
>>> Tested-by: Alex Marginean
>>> Acked-by: Vladimir Oltean
>>> ---
>>>
>>> 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 =
>>> pci1 =
>>> pci2 =
>>> - remoteproc1 = _1;
>>> - remoteproc2 = _2;
>>> + remoteproc0 = _1;
>>> + remoteproc1 = _2;
>>> rtc0 = _0;
>>> rtc1 = _1;
>>> spi0 = "/spi@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, );
>>> + ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false,
>>> );
>>> 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, );
>>> + 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, );
>>> 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" \
>>> +
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
Hi Michal,
Am 2020-02-20 09:30, schrieb Michal Simek:
On 03. 02. 20 18:11, 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.
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
Reviewed-by: Alex Marginean
Tested-by: Alex Marginean
Acked-by: Vladimir Oltean
---
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 =
pci1 =
pci2 =
- remoteproc1 = _1;
- remoteproc2 = _2;
+ remoteproc0 = _1;
+ remoteproc1 = _2;
rtc0 = _0;
rtc1 = _1;
spi0 = "/spi@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, );
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false,
);
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, );
+ 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, );
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"
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
On 03. 02. 20 18:11, 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.
>
> 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
> Reviewed-by: Alex Marginean
> Tested-by: Alex Marginean
> Acked-by: Vladimir Oltean
> ---
>
> 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 =
> pci1 =
> pci2 =
> - remoteproc1 = _1;
> - remoteproc2 = _2;
> + remoteproc0 = _1;
> + remoteproc1 = _2;
> rtc0 = _0;
> rtc1 = _1;
> spi0 = "/spi@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, );
> + ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false, );
> 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, );
> + 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, );
> 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" \
> +
Re: [PATCH v3] dm: uclass: don't assign aliased seq numbers
On Mon, 3 Feb 2020 at 10:12, 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. > > 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 > Reviewed-by: Alex Marginean > Tested-by: Alex Marginean > Acked-by: Vladimir Oltean > --- Reviewed-by: Simon Glass (added Joe for review fo network parts)
[PATCH v3] dm: uclass: don't assign aliased seq numbers
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
Reviewed-by: Alex Marginean
Tested-by: Alex Marginean
Acked-by: Vladimir Oltean
---
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 =
pci1 =
pci2 =
- remoteproc1 = _1;
- remoteproc2 = _2;
+ remoteproc0 = _1;
+ remoteproc1 = _2;
rtc0 = _0;
rtc1 = _1;
spi0 = "/spi@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, );
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false, );
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, );
+ 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, );
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
