* [PATCH v2] fdt: Use phandle to distinguish DT nodes with same name
@ 2020-12-02 15:41 Aswath Govindraju
2020-12-02 22:21 ` Simon Glass
0 siblings, 1 reply; 3+ messages in thread
From: Aswath Govindraju @ 2020-12-02 15:41 UTC (permalink / raw)
To: u-boot
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;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2] fdt: Use phandle to distinguish DT nodes with same name
2020-12-02 15:41 [PATCH v2] fdt: Use phandle to distinguish DT nodes with same name Aswath Govindraju
@ 2020-12-02 22:21 ` Simon Glass
2020-12-03 5:27 ` Aswath Govindraju
0 siblings, 1 reply; 3+ messages in thread
From: Simon Glass @ 2020-12-02 22:21 UTC (permalink / raw)
To: u-boot
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 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] fdt: Use phandle to distinguish DT nodes with same name
2020-12-02 22:21 ` Simon Glass
@ 2020-12-03 5:27 ` Aswath Govindraju
0 siblings, 0 replies; 3+ messages in thread
From: Aswath Govindraju @ 2020-12-03 5:27 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-03 5:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 15:41 [PATCH v2] fdt: Use phandle to distinguish DT nodes with same name Aswath Govindraju
2020-12-02 22:21 ` Simon Glass
2020-12-03 5:27 ` Aswath Govindraju
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.