On 5/19/22 3:16 PM, Rasmus Villemoes wrote:
On 19/05/2022 13.50, Aswath Govindraju wrote:
Understood, thanks for the explanation. I am good with this patch.
Acked-by: Aswath Govindraju a-govindraju@ti.com
Thanks.
For completeness, to expand on this:
it's somewhat fragile to rely on (at least one of) the nodes in question to even have a phandle.
One way in which to ensure all nodes (with a label) do get a phandle is to build with the -@ flag to dtc, which is implied by setting CONFIG_OF_LIBFDT_OVERLAY=y. Three of the four _defconfigs that set PHANDLE_CHECK_SEQ also set that option, so for them it was guaranteed to work, but mostly by chance - if I randomly discovered CONFIG_PHANDLE_CHECK_SEQ and found I needed to enable it for my board, I wouldn't know to also enable CONFIG_OF_LIBFDT_OVERLAY. And if I didn't need overlay support as such, it would also bloat the .dtb (and U-Boot itself) needlessly; the difference for sama7g5ek_mmc_defconfig is 34784 bytes for the current .dtb and 26920 bytes if one disables OF_LIBFDT_OVERLAY.
The last _defconfig with PHANDLE_CHECK_SEQ=y didn't actually seem to need it; I've built it and looked at u-boot.dtb, and there are no collisions in basenames in the aliases. Commit ddd778ae doesn't say anything about why it was added.
Rasmus
Hi Rasmus,
For sama7g5 boards, we have the following situation:
some node { i2c1: i2c@600: { some props; }; };
some other node { i2c2: i2c@600: { other props; }; };
and then we want to be able to reference aliases { i2c0 = &i2c1; i2c1 = &i2c2; };
Without CONFIG_PHANDLE_CHECK_SEQ , both i2c0 and i2c1 aliases pointed to the same 'i2c@600' node, because the code was searching through the DT for 'i2c@600' and it was hitting the first in both cases.
With your patch, it can happen that we hit the same problem again ? Or we should be safe ?
Thanks, Eugen