Hi Simon,
On 2026-10-09, Simon Glass wrote:
> The checkpatch remark is for reviewers, not the commit log. Ideally
> this would go below the --- line
Will do.
> The two branches handle the stem differently. of_alias_get_id() does a
> strcmp() against the stem split off by of_alias_scan(), so the stem
> must match exactly. fdtdec_get_alias_seq() just does strncmp(name,
> base, base_len) and then trailing_strtol(), so any alias starting with
> the stem matches.
Yes, and that prefix match is also what started this series: with
"usb" as the stem, fdtdec_get_alias_highest_id() counts usbphy0 and
usbphy1 as usb aliases, which is why the two controllers ended up with
seq 2 and 3 and indexed past the two-entry tables (patch 1). So I
agree it is worth fixing, and in both functions: a stem matches an
alias only when what follows it is the number. It is a separate patch
with tests for both lookups, but it has to come ahead of this one in
v5: the negative test you ask for below ("test" on /some-bus) can only
pass on the flat tree once the prefix match is gone. The series still
follows the PHY phandle (patch 1), which does not depend on how the
aliases are numbered.
> I think patch 5 should pass 'usbphy'.
Yes, that was wrong, v5 passes "usbphy". The header comment will say
the stem must match exactly, and the test gets the negative case with
a prefix stem ("test" on /some-bus) so the two implementations are
checked against each other.
> Since the flat path now uses ofnode_to_fdt() rather than gd->fdt_blob,
> it would be good to also check a node in another tree, i.e. a variant
> using oftree_path() with UTF_OTHER_FDT, like the other *_ot tests in
> this file.
Agreed for the flat tree, that is exactly what the ofnode_to_fdt()
change is for. On the live tree there is a gap though: of_alias_scan()
only runs for the control tree, and the other tree of the tests comes
from unflatten_device_tree() alone, so of_alias_get_id() cannot know
its aliases. I see two ways: (a) the live branch of
ofnode_get_alias_seq() handles a node outside the control tree itself,
by walking that tree's /aliases (the control tree keeps the fast list
lookup), so the _ot test runs in both modes like its neighbours; (b)
the _ot test is flat-only (UTF_FLAT_TREE) and the comment says why.
I would do (a) unless you prefer to keep the livetree side as it is.
Regards,
Mehmet
