[PATCH 00/10] of: fix bugs and improve codes

[PATCH 00/10] of: fix bugs and improve codes

[Zijun Hu]

This patch series is to fix bugs and improve codes for drivers/of/*.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Zijun Hu (10):
      of: Fix alias name length calculating error in API of_find_node_opts_by_path()
      of: Correct return value for API of_parse_phandle_with_args_map()
      of: Correct child specifier used as input of the 2nd nexus node
      of: Fix refcount leakage for OF node returned by __of_get_dma_parent()
      of: Fix available buffer size calculating error in API of_device_uevent_modalias()
      of/fdt: Dump __be32 array in CPU type order in of_dump_addr()
      of: Correct comments for of_alias_scan()
      of: Swap implementation between of_property_present() and of_property_read_bool()
      of: property: Implement of_fwnode_property_present() by of_property_present()
      of: Simplify API of_find_node_with_property() implementation

 drivers/of/address.c     |  2 +-
 drivers/of/base.c        | 25 +++++++++++--------------
 drivers/of/device.c      |  4 ++--
 drivers/of/fdt_address.c |  2 +-
 drivers/of/property.c    |  2 +-
 include/linux/of.h       | 23 ++++++++++++-----------
 6 files changed, 28 insertions(+), 30 deletions(-)
---
base-commit: 16ef9c9de0c48b836c5996c6e9792cb4f658c8f1
change-id: 20241206-of_core_fix-dc3021a06418

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

[PATCH 01/10] of: Fix alias name length calculating error in API of_find_node_opts_by_path()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

Alias name length calculated by of_find_node_opts_by_path() is wrong as
explained below:

Take "alias/serial@llc500:115200n8" as its @patch argument for an example
      ^    ^             ^
      0	   5		 19

The right length of alias 'alias' is 5, but the API results in 19 which is
obvious wrong.

The wrong length will cause finding device node failure for such paths.
Fix by using index of either '/' or ':' as the length who comes earlier.

Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7dc394255a0a14cd1aed02ec79c2f787a222b44c..9a9313183d1f1b61918fe7e6fa80c2726b099a1c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -893,10 +893,10 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
 	/* The path could begin with an alias */
 	if (*path != '/') {
 		int len;
-		const char *p = separator;
+		const char *p = strchrnul(path, '/');
 
-		if (!p)
-			p = strchrnul(path, '/');
+		if (separator && separator < p)
+			p = separator;
 		len = p - path;
 
 		/* of_aliases must not be NULL */

-- 
2.34.1

[PATCH 02/10] of: Correct return value for API of_parse_phandle_with_args_map()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

@ret is used by of_parse_phandle_with_args_map() to record return value
and it is preseted with -EINVAL before the outer while loop, but it is
changed to 0 by below successful operation within the inner loop:
of_property_read_u32(new, cells_name, &new_size)

So cause 0(success) is returned for all failures which happen after the
operation, that is obviously wrong.

Fix by restoring @ret with preseted -EINVAL after the operation.

Fixes: bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/base.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9a9313183d1f1b61918fe7e6fa80c2726b099a1c..b294924aa31cfed1ec06983f420a445f7fae7394 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1516,6 +1516,8 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
 			ret = of_property_read_u32(new, cells_name, &new_size);
 			if (ret)
 				goto put;
+			/* Restore preseted error code */
+			ret = -EINVAL;
 
 			/* Check for malformed properties */
 			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))

-- 
2.34.1

[PATCH 03/10] of: Correct child specifier used as input of the 2nd nexus node

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

API of_parse_phandle_with_args_map() will use wrong input for nexus node
Nexus_2 as shown below:

    Node_1		Nexus_1                              Nexus_2
&Nexus_1,arg_1 -> arg_1,&Nexus_2,arg_2' -> &Nexus_2,arg_2 -> arg_2,...
		  map-pass-thru=<...>

Nexus_1's output arg_2 should be used as input of Nexus_2, but the API
wrongly uses arg_2' instead which != arg_2 due to Nexus_1's map-pass-thru.

Fix by always making @match_array point to @initial_match_array into
which to store nexus output.

Fixes: bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b294924aa31cfed1ec06983f420a445f7fae7394..1c62cda4ebcd9e3dc5f91d10fa68f975226693dd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1542,7 +1542,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
 		 * specifier into the out_args structure, keeping the
 		 * bits specified in <list>-map-pass-thru.
 		 */
-		match_array = map - new_size;
 		for (i = 0; i < new_size; i++) {
 			__be32 val = *(map - new_size + i);
 
@@ -1551,6 +1550,7 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
 				val |= cpu_to_be32(out_args->args[i]) & pass[i];
 			}
 
+			initial_match_array[i] = val;
 			out_args->args[i] = be32_to_cpu(val);
 		}
 		out_args->args_count = list_size = new_size;

-- 
2.34.1

[PATCH 04/10] of: Fix refcount leakage for OF node returned by __of_get_dma_parent()

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

__of_get_dma_parent() returns OF device node @args.np, but the node's
refcount is increased twice, by both of_parse_phandle_with_args() and
of_node_get(), so causes refcount leakage for the node.

Fix by directly returning the node got by of_parse_phandle_with_args().

Fixes: f83a6e5dea6c ("of: address: Add support for the parent DMA bus")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/address.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index c5b925ac469f16b8ae4b8275b60210a2d583ff83..cda1cbb82eabdcd39d32446023fbb105b69bc99d 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,7 +619,7 @@ struct device_node *__of_get_dma_parent(const struct device_node *np)
 	if (ret < 0)
 		return of_get_parent(np);
 
-	return of_node_get(args.np);
+	return args.np;
 }
 #endif
 

-- 
2.34.1

Re: [PATCH 01/10] of: Fix alias name length calculating error in API of_find_node_opts_by_path()

[Rob Herring]

On Thu, Dec 5, 2024 at 6:53 PM Zijun Hu <zijun_hu@icloud.com> wrote:
>
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> Alias name length calculated by of_find_node_opts_by_path() is wrong as
> explained below:
>
> Take "alias/serial@llc500:115200n8" as its @patch argument for an example
>       ^    ^             ^
>       0    5             19
>
> The right length of alias 'alias' is 5, but the API results in 19 which is
> obvious wrong.
>
> The wrong length will cause finding device node failure for such paths.
> Fix by using index of either '/' or ':' as the length who comes earlier.

Can you add a test case in the unittest for this.

>
> Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/of/base.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 7dc394255a0a14cd1aed02ec79c2f787a222b44c..9a9313183d1f1b61918fe7e6fa80c2726b099a1c 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -893,10 +893,10 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>         /* The path could begin with an alias */
>         if (*path != '/') {
>                 int len;
> -               const char *p = separator;
> +               const char *p = strchrnul(path, '/');
>
> -               if (!p)
> -                       p = strchrnul(path, '/');
> +               if (separator && separator < p)
> +                       p = separator;
>                 len = p - path;
>
>                 /* of_aliases must not be NULL */
>
> --
> 2.34.1
>

Re: [PATCH 02/10] of: Correct return value for API of_parse_phandle_with_args_map()

[Rob Herring]

On Thu, Dec 5, 2024 at 6:53 PM Zijun Hu <zijun_hu@icloud.com> wrote:
>
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> @ret is used by of_parse_phandle_with_args_map() to record return value
> and it is preseted with -EINVAL before the outer while loop, but it is
> changed to 0 by below successful operation within the inner loop:
> of_property_read_u32(new, cells_name, &new_size)
>
> So cause 0(success) is returned for all failures which happen after the
> operation, that is obviously wrong.
>
> Fix by restoring @ret with preseted -EINVAL after the operation.

Already have a similar fix queued up.

Rob

Re: [PATCH 01/10] of: Fix alias name length calculating error in API of_find_node_opts_by_path()

[Zijun Hu]

On 2024/12/9 21:24, Rob Herring wrote:
> On Thu, Dec 5, 2024 at 6:53 PM Zijun Hu <zijun_hu@icloud.com> wrote:
>>
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> Alias name length calculated by of_find_node_opts_by_path() is wrong as
>> explained below:
>>
>> Take "alias/serial@llc500:115200n8" as its @patch argument for an example
>>       ^    ^             ^
>>       0    5             19
>>
>> The right length of alias 'alias' is 5, but the API results in 19 which is
>> obvious wrong.
>>
>> The wrong length will cause finding device node failure for such paths.
>> Fix by using index of either '/' or ':' as the length who comes earlier.
> 
> Can you add a test case in the unittest for this.

sure. let me try to do it.

> 
>>
>> Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>>  drivers/of/base.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 7dc394255a0a14cd1aed02ec79c2f787a222b44c..9a9313183d1f1b61918fe7e6fa80c2726b099a1c 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -893,10 +893,10 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>>         /* The path could begin with an alias */
>>         if (*path != '/') {
>>                 int len;
>> -               const char *p = separator;
>> +               const char *p = strchrnul(path, '/');
>>
>> -               if (!p)
>> -                       p = strchrnul(path, '/');
>> +               if (separator && separator < p)
>> +                       p = separator;
>>                 len = p - path;
>>
>>                 /* of_aliases must not be NULL */
>>
>> --
>> 2.34.1
>>

Re: [PATCH 04/10] of: Fix refcount leakage for OF node returned by __of_get_dma_parent()

[Rob Herring]

On Fri, Dec 06, 2024 at 08:52:30AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> __of_get_dma_parent() returns OF device node @args.np, but the node's
> refcount is increased twice, by both of_parse_phandle_with_args() and
> of_node_get(), so causes refcount leakage for the node.
> 
> Fix by directly returning the node got by of_parse_phandle_with_args().
> 
> Fixes: f83a6e5dea6c ("of: address: Add support for the parent DMA bus")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/of/address.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Rob