On Sat, 28 Mar 2020 at 23:00, Simon Glass <s...@chromium.org> wrote: > > Hi Vladimir, > > On Sat, 28 Mar 2020 at 14:25, Vladimir Oltean <olte...@gmail.com> wrote: > > > > Hi Simon, > > > > On Sat, 28 Mar 2020 at 22:06, Simon Glass <s...@chromium.org> wrote: > > > > > > Hi Vladimir, > > > > > > On Fri, 27 Mar 2020 at 11:29, Vladimir Oltean <olte...@gmail.com> wrote: > > > > > > > > From: Vladimir Oltean <vladimir.olt...@nxp.com> > > > > > > > > Currently fdtdec_get_alias_seq() calls fdt_get_name() which returns only > > > > the name of the leaf node. So it needs to also trim the path of the > > > > alias to the leaf name only, leading to imperfect matches. > > > > > > > > This means that the following aliases: > > > > > > > > /aliases { > > > > eth0 = "/dspi@2120000/ethernet-switch@0/ports/port@0"; > > > > eth1 = "/pcie@1f0000000/pci@0,5/ports/port@0"; > > > > }; > > > > > > > > will make fdtdec_get_alias_seq to return a seq of zero for both aliases, > > > > as the match will only take place on the "port@0" portion. > > > > > > > > Fix this by calling fdt_get_path and comparing the full path of the > > > > alias. > > > > > > > > Fixes: 5c33c9fdbb3f ("fdt: Add a function to get the alias sequence of > > > > a node") > > > > Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com> > > > > --- > > > > lib/fdtdec.c | 15 +++++++-------- > > > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > > > index eb11fc898e30..55c1b36e823b 100644 > > > > --- a/lib/fdtdec.c > > > > +++ b/lib/fdtdec.c > > > > @@ -455,13 +455,14 @@ int fdtdec_get_alias_seq(const void *blob, const > > > > char *base, int offset, > > > > int *seqp) > > > > { > > > > int base_len = strlen(base); > > > > - const char *find_name; > > > > - int find_namelen; > > > > int prop_offset; > > > > + char path[64]; > > > > + int path_len; > > > > int aliases; > > > > > > > > - find_name = fdt_get_name(blob, offset, &find_namelen); > > > > - debug("Looking for '%s' at %d, name %s\n", base, offset, > > > > find_name); > > > > + fdt_get_path(blob, offset, path, sizeof(path)); > > > > > > This is really slow. I would prefer not to change this for what seems > > > like an odd case. > > > > > > Can you enable CONFIG_OF_LIVE instead? > > > > > > > No, why would I want to do that? > > Because it uses a different algorithm - see of_alias_get_id(). It > might already work. > > > Why is this an odd case? I thought aliases are supposed to match on > > full path, no? > > Because we've been using the current algorithm for 5+ years without any > comment. > > Note that fdt_get_path() likely takes a long time in SPL and before > relocation. > > Regards, > Simon
So we have no disagreement about the fact that the function doesn't do what's written on the box, and that the implementation for CONFIG_OF_LIVE=y and CONFIG_OF_LIVE=n yields different results, however your argument is that we should leave it alone because nobody noticed thus far? Don't I count? Regards, -Vladimir