Re: [PATCH] fdtdec: fdtdec_get_aliases_highest_id: skip aliases to disabled nodes

[Tim Harvey <tharvey@gateworks.com>]

On Thu, Apr 29, 2021 at 9:48 AM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Apr 29, 2021 at 9:10 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Fri, 16 Apr 2021 at 14:30, Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > When looking for an alias with the highest id skip aliases for nodes
> > > that are disabled.
> > >
> > > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > > ---
> > >  lib/fdtdec.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > > index 864589193b..d47195525a 100644
> > > --- a/lib/fdtdec.c
> > > +++ b/lib/fdtdec.c
> > > @@ -546,6 +546,8 @@ int fdtdec_get_alias_highest_id(const void *blob, const char *base)
> > >                 if (*prop != '/' || prop[len - 1] ||
> > >                     strncmp(name, base, base_len))
> > >                         continue;
> > > +               if (!fdtdec_get_is_enabled(blob, fdt_path_offset(blob, prop)))
> > > +                       continue;
> >
> > We really can't do this here. It is quite an expensive operation to
> > locate the node for a path.
> >
> > Why is this needed? It seems odd to have an alias pointing to a disabled device.
> >
>
> Simon,
>
> The issue I ran into here was with an imx6 based board that does not
> use the FEC ethernet on the SoC. In this case imx6qdl.dtsi delcares an
> alias 'thernet0 = &fec' yet the fec node is not enabled. However
> because fdtdec_get_alias_highest_id does not skip this alias to a
> disabled device any enumerated ethernet devices get an index of 1
> instead of 0 which is incorrect and causes the mac addresses to be
> misaligned.
>
> In general it is just wrong to reserve id's for disabled devices.
>

Simon,

Do you have any additional feedback on this? It has not been picked up
yet and does cause issues on IMX boards using dm that do not use the
internal SoC's fec ethernet.

Perhaps there is a better way to resolve this?

Best regards,

Tim