[PATCH v2] fdt: Use phandle to distinguish DT nodes with same name
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. 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 (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;
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
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 nhelpWhen there are multiple device tree nodes with same name,enable this config option to distinguish them usingphandles 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;#endifval = trailing_strtol(name); if (val != -1) { *seqp = val;-- 2.17.1
Regards, SImon
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 nhelpWhen there are multiple device tree nodes with same name,enable this config option to distinguish them usingphandles 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_SEQif (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;#endifval = trailing_strtol(name); if (val != -1) { *seqp = val;-- 2.17.1
Regards, SImon
participants (2)
-
Aswath Govindraju -
Simon Glass