[PATCH v2] fdt: Use phandle to distinguish DT nodes with same name

[Aswath Govindraju]

Hi Simon,

On 03/12/20 3:51 am, Simon Glass wrote:
> Hi Aswath,
> 
> On Wed, 2 Dec 2020 at 08:42, Aswath Govindraju <a-govindraju at ti.com> wrote:
>>
>> While assigning the sequence number to subsystem instances by reading the
>> aliases property, only DT nodes names are compared and not the complete
>> path. This causes a problem when there are two DT nodes with same name but
>> have different paths.
>>
>> In arch/arm/dts/k3-am65-main.dtsi there are two USB controllers with the
>> same device tree node name but different path. When aliases are defined for
>> these USB controllers then fdtdec_get_alias_seq() fails to pick the correct
>> instance for a given index.
>>
>> fdt_path_offset() function is slow and this would effect the uboot startup.
> 
> U-Boot
> 

Corrected this.
>> To avert the time penalty on all boards, apply this extra check only when
>> required by using a config option.
>>
>> Fix it by comparing the phandles of DT nodes after the node names match,
>> under a config option.
>>
>> Signed-off-by: Aswath Govindraju <a-govindraju at ti.com>
>> ---
>>
>> Changes since v1:
>>  - Added a config option as fdt_path_offset() slows down the u-boot start
>>    up and would be better to be enabled only when required
>>  - Added an example case in commit message where the following fix is
>>    required.
>>
>>  lib/Kconfig  |  8 ++++++++
>>  lib/fdtdec.c | 11 +++++++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 8efb154f7344..9813a3a5e0ba 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -685,3 +685,11 @@ config LIB_ELF
>>           This supports fir 32 bit and 64 bit versions.
>>
>>  endmenu
>> +
>> +config PHANDLE_CHECK_SEQ
>> +       bool "Enable phandle check while getting sequence number"
>> +       default n
>> +       help
>> +         When there are multiple device tree nodes with same name,
>> +         enable this config option to distinguish them using
>> +         phandles in fdtdec_get_alias_seq() function.
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index ee1bd41b0818..60aa1bd8cf1b 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -500,6 +500,17 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>>                 slash = strrchr(prop, '/');
>>                 if (strcmp(slash + 1, find_name))
>>                         continue;
>> +
>> +               /*
>> +                * Adding an extra check to distinguish DT nodes with
>> +                * same name
>> +                */
>> +               #ifdef CONFIG_PHANDLE_CHECK_SEQ
> 
> if (IS_ENABLED(CONFIG...
>
Corrected this.

> (did you not get a warning from patman on that?)

No, I am not getting it.

Thank you for the comments. I made the changes that you suggested and I
posted a respin.

Thanks,
Aswath

> 
>> +               if (fdt_get_phandle(blob, offset) !=
>> +                   fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
>> +                       continue;
>> +               #endif
>> +
>>                 val = trailing_strtol(name);
>>                 if (val != -1) {
>>                         *seqp = val;
>> --
>> 2.17.1
>>
> 
> Regards,
> SImon
>