diff mbox series

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

Message ID 20201203052545.2387-1-a-govindraju@ti.com
State Accepted
Delegated to: Simon Glass
Headers show
Series [v3] fdt: Use phandle to distinguish DT nodes with same name | expand

Commit Message

Aswath Govindraju Dec. 3, 2020, 5:25 a.m. UTC
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 U-Boot
startup. 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 v2:
 - changed #ifdef to if(IS_ENABLED())
 - corrected errors in commit description

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

Comments

Simon Glass Dec. 12, 2020, 3:35 p.m. UTC | #1
On Wed, 2 Dec 2020 at 22:26, 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 U-Boot
> startup. 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 v2:
>  - changed #ifdef to if(IS_ENABLED())
>  - corrected errors in commit description
>
> 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(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
Simon Glass Dec. 23, 2020, 3:43 p.m. UTC | #2
On Wed, 2 Dec 2020 at 22:26, 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 U-Boot
> startup. 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 v2:
>  - changed #ifdef to if(IS_ENABLED())
>  - corrected errors in commit description
>
> 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(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!
diff mbox series

Patch

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..0ab7105fef0d 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
+		 */
+		if (IS_ENABLED(CONFIG_PHANDLE_CHECK_SEQ)) {
+			if (fdt_get_phandle(blob, offset) !=
+			    fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
+				continue;
+		}
+
 		val = trailing_strtol(name);
 		if (val != -1) {
 			*seqp = val;