[PATCH] dm: uclass: don't assign aliased seq numbers

[Vladimir Oltean]

Hi Michael,

On Wed, 18 Dec 2019 at 18:42, Michael Walle <michael at walle.cc> 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 <fitzsim at fitzsim.org>
> Cc: Michal Simek <michal.simek at xilinx.com>
>
> Signed-off-by: Michael Walle <michael at walle.cc>
> ---

This sounds so obvious that it makes me wonder why it hasn't been the
behavior thus far.

Acked-by: Vladimir Oltean <olteanv at gmail.com>

>  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
> +                * 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)
> --
> 2.20.1
>