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

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