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

[Simon Glass]

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

> 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...

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

> +               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