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

[Sean Anderson <sean.anderson@seco.com>]

On 6/11/21 1:16 PM, Tim Harvey wrote:
 > On Fri, Jun 11, 2021 at 10:09 AM Sean Anderson <sean.anderson@seco.com> wrote:
 >>
 >>
 >>
 >> On 6/11/21 12:32 PM, Tim Harvey wrote:
 >>   > 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.
 >>
 >> Is this due getting mac address from the OTP? As I understand it, the
 >> OTP mac addresses are for fec0/fec1, and not for whatever ethernet0
 >> happens to be.
 >>
 >
 > Sean,
 >
 > No, the issue is that U-Boot thinks there is an fec device simply
 > because an alias points to it even though the fec node pointed to is
 > disabled so when U-Boot finds an ethernet device that is enumerated at
 > runtime (think pcie or usb based mac) those start at index 1 instead
 > of index 0 and end up wanting to be assigned mac's from eth1addr
 > instead of ethaddr.

Ok, and this behavior was different in the past? E.g. it used to be that
these interfaces were assigned to ethernet0, and so their mac address
was stored in ethaddr. And then something changed and now they are
ethernet1 and are trying to get the incorrect mac address?

Do you know whether it would be possible to remove the alias at the same
time you mark the fec as disabled?

--Sean