devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] of: fix bugs and improve codes
@ 2024-12-17 13:07 Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 1/7] of: Correct child specifier used as input of the 2nd nexus node Zijun Hu
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

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

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v3:
- Drop 2 applied patches and pick up patch 4/7 again
- Fix build error for patch 6/7.
- Include of_private.h instead of function declaration for patch 2/7
- Correct tile and commit messages.
- Link to v2: https://lore.kernel.org/r/20241216-of_core_fix-v2-0-e69b8f60da63@quicinc.com

Changes in v2:
- Drop applied/conflict/TBD patches.
- Correct based on Rob's comments.
- Link to v1: https://lore.kernel.org/r/20241206-of_core_fix-v1-0-dc28ed56bec3@quicinc.com

---
Zijun Hu (7):
      of: Correct child specifier used as input of the 2nd nexus node
      of: Do not expose of_alias_scan() and correct its comments
      of: Make of_property_present() applicable to all kinds of property
      of: property: Use of_property_present() for of_fwnode_property_present()
      of: Fix available buffer size calculating error in API of_device_uevent_modalias()
      of: Fix potential wrong MODALIAS uevent value
      of: Do not expose of_modalias()

 drivers/of/base.c       |   7 ++--
 drivers/of/device.c     |  33 +++++++--------
 drivers/of/module.c     | 109 +++++++++++++++++++++++++++++-------------------
 drivers/of/of_private.h |   4 ++
 drivers/of/pdt.c        |   2 +
 drivers/of/property.c   |   2 +-
 include/linux/of.h      |  31 ++++++--------
 7 files changed, 102 insertions(+), 86 deletions(-)
---
base-commit: 0f7ca6f69354e0c3923bbc28c92d0ecab4d50a3e
change-id: 20241206-of_core_fix-dc3021a06418

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/7] of: Correct child specifier used as input of the 2nd nexus node
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 2/7] of: Do not expose of_alias_scan() and correct its comments Zijun Hu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

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 bf18d5997770eb81e47e749198dd505a35203d10..969b99838655534915882abe358814d505c6f748 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1536,7 +1536,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);
 
@@ -1545,6 +1544,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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/7] of: Do not expose of_alias_scan() and correct its comments
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 1/7] of: Correct child specifier used as input of the 2nd nexus node Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 3/7] of: Make of_property_present() applicable to all kinds of property Zijun Hu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

For of_alias_scan():
- Do not expose it since it has no external callers.
- Correct its comments shown below:
  1) Replace /* with /** to start comments since it is not a API.
  2) Delete return value descriptions since it is a void function.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/base.c       | 5 ++---
 drivers/of/of_private.h | 2 ++
 drivers/of/pdt.c        | 2 ++
 include/linux/of.h      | 1 -
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 969b99838655534915882abe358814d505c6f748..5485307e2a3a3d3a216d271c60bdfc346dd38460 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1806,14 +1806,13 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
 		 ap->alias, ap->stem, ap->id, np);
 }
 
-/**
+/*
  * of_alias_scan - Scan all properties of the 'aliases' node
  * @dt_alloc:	An allocator that provides a virtual address to memory
  *		for storing the resulting tree
  *
  * The function scans all the properties of the 'aliases' node and populates
- * the global lookup table with the properties.  It returns the
- * number of alias properties found, or an error code in case of failure.
+ * the global lookup table with the properties.
  */
 void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 {
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index ea5a0951ec5e107bab265ab5f6c043e2bfb15ecc..3433ccd330e84fd3a4b54638e0e922069757c8f0 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -119,6 +119,8 @@ extern void *__unflatten_device_tree(const void *blob,
 			      void *(*dt_alloc)(u64 size, u64 align),
 			      bool detached);
 
+void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
+
 /**
  * General utilities for working with live trees.
  *
diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
index 7eda43c66c916198b1c2d8fc5043fcb1edaede7a..cb0cb374b21ff89323e11f34bd767b183e7a401e 100644
--- a/drivers/of/pdt.c
+++ b/drivers/of/pdt.c
@@ -19,6 +19,8 @@
 #include <linux/of.h>
 #include <linux/of_pdt.h>
 
+#include "of_private.h"
+
 static struct of_pdt_ops *of_pdt_prom_ops __initdata;
 
 #if defined(CONFIG_SPARC)
diff --git a/include/linux/of.h b/include/linux/of.h
index f921786cb8ac782286ed5ff4425a35668204d050..d451c46132b01efe6d4e0b6cf83a3e2084bb3ec6 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -397,7 +397,6 @@ extern int of_phandle_iterator_args(struct of_phandle_iterator *it,
 				    uint32_t *args,
 				    int size);
 
-extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
 extern int of_alias_get_id(const struct device_node *np, const char *stem);
 extern int of_alias_get_highest_id(const char *stem);
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/7] of: Make of_property_present() applicable to all kinds of property
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 1/7] of: Correct child specifier used as input of the 2nd nexus node Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 2/7] of: Do not expose of_alias_scan() and correct its comments Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 4/7] of: property: Use of_property_present() for of_fwnode_property_present() Zijun Hu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

API of_property_present() invokes of_property_read_bool() to check if
a property is present or not, and that has 2 shortcomings shown below:

- That narrows down property scope applicable to of_property_present()
  from all kinds of property to only bool type.

- That is less logical since it says a property's presence is decided by
  its bool property value.

Fix by making of_property_read_bool() invoke of_property_present().

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 include/linux/of.h | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index d451c46132b01efe6d4e0b6cf83a3e2084bb3ec6..fe5d7b74c23b9701743f5debc3d030b765bc914f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1242,17 +1242,16 @@ static inline int of_property_read_string_index(const struct device_node *np,
 }
 
 /**
- * of_property_read_bool - Find a property
- * @np:		device node from which the property value is to be read.
+ * of_property_present - Test if a property is present in a node
+ * @np:		device node to search for the property.
  * @propname:	name of the property to be searched.
  *
- * Search for a boolean property in a device node. Usage on non-boolean
- * property types is deprecated.
+ * Test for a property present in a device node.
  *
  * Return: true if the property exists false otherwise.
  */
-static inline bool of_property_read_bool(const struct device_node *np,
-					 const char *propname)
+static inline bool of_property_present(const struct device_node *np,
+				       const char *propname)
 {
 	const struct property *prop = of_find_property(np, propname, NULL);
 
@@ -1260,17 +1259,19 @@ static inline bool of_property_read_bool(const struct device_node *np,
 }
 
 /**
- * of_property_present - Test if a property is present in a node
- * @np:		device node to search for the property.
+ * of_property_read_bool - Find a property
+ * @np:		device node from which the property value is to be read.
  * @propname:	name of the property to be searched.
  *
- * Test for a property present in a device node.
+ * Search for a boolean property in a device node. Usage on non-boolean
+ * property types is deprecated.
  *
  * Return: true if the property exists false otherwise.
  */
-static inline bool of_property_present(const struct device_node *np, const char *propname)
+static inline bool of_property_read_bool(const struct device_node *np,
+					 const char *propname)
 {
-	return of_property_read_bool(np, propname);
+	return of_property_present(np, propname);
 }
 
 /**

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 4/7] of: property: Use of_property_present() for of_fwnode_property_present()
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
                   ` (2 preceding siblings ...)
  2024-12-17 13:07 ` [PATCH v3 3/7] of: Make of_property_present() applicable to all kinds of property Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 5/7] of: Fix available buffer size calculating error in API of_device_uevent_modalias() Zijun Hu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

Use of_property_present() instead of of_property_read_bool() for
of_fwnode_property_present() since the former is more applicable
obviously.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

---
Hi Rob,

i pick up this change again after some considerations as below:

1) of_property_present() is more suitable than of_property_read_bool()
   here, deprecated API is not main reason.

2) it does not conflict with your job which warns when use
   of_property_read_bool() for non-bool property.
---
 drivers/of/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 519bf9229e613906547b57d8c68e7b8558eff327..dca1a3ebccae1093b2b11f51e8e692bca854d0e3 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -966,7 +966,7 @@ of_fwnode_device_get_dma_attr(const struct fwnode_handle *fwnode)
 static bool of_fwnode_property_present(const struct fwnode_handle *fwnode,
 				       const char *propname)
 {
-	return of_property_read_bool(to_of_node(fwnode), propname);
+	return of_property_present(to_of_node(fwnode), propname);
 }
 
 static int of_fwnode_property_read_int_array(const struct fwnode_handle *fwnode,

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 5/7] of: Fix available buffer size calculating error in API of_device_uevent_modalias()
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
                   ` (3 preceding siblings ...)
  2024-12-17 13:07 ` [PATCH v3 4/7] of: property: Use of_property_present() for of_fwnode_property_present() Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 6/7] of: Fix potential wrong MODALIAS uevent value Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 7/7] of: Do not expose of_modalias() Zijun Hu
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

of_device_uevent_modalias() saves MODALIAS value from offset
(@env->buflen - 1), so the available buffer size should be
(sizeof(@env->buf) - @env->buflen + 1), but it uses the wrong
size (sizeof(@env->buf) - @env->buflen).

Fix by using size of space from char '\0' inclusive which ends "MODALIAS=".

Fixes: dd27dcda37f0 ("of/device: merge of_device_uevent")
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/device.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index edf3be1972658f6dc165f577da53b10c7eebc116..f24c19e7aba8e01656f503ae328a4e08aab5a5f3 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -257,6 +257,7 @@ EXPORT_SYMBOL_GPL(of_device_uevent);
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env)
 {
 	int sl;
+	int pos;
 
 	if ((!dev) || (!dev->of_node) || dev->of_node_reused)
 		return -ENODEV;
@@ -265,13 +266,18 @@ int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *
 	if (add_uevent_var(env, "MODALIAS="))
 		return -ENOMEM;
 
-	sl = of_modalias(dev->of_node, &env->buf[env->buflen-1],
-			 sizeof(env->buf) - env->buflen);
+	/*
+	 * @env->buflen is pointing to the char after '\0' now
+	 * override the '\0' to save MODALIAS value.
+	 */
+	pos = env->buflen - 1;
+	sl = of_modalias(dev->of_node, &env->buf[pos],
+			 sizeof(env->buf) - pos);
 	if (sl < 0)
 		return sl;
-	if (sl >= (sizeof(env->buf) - env->buflen))
+	if (sl >= (sizeof(env->buf) - pos))
 		return -ENOMEM;
-	env->buflen += sl;
+	env->buflen = pos + sl + 1;
 
 	return 0;
 }

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 6/7] of: Fix potential wrong MODALIAS uevent value
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
                   ` (4 preceding siblings ...)
  2024-12-17 13:07 ` [PATCH v3 5/7] of: Fix available buffer size calculating error in API of_device_uevent_modalias() Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  2024-12-17 13:07 ` [PATCH v3 7/7] of: Do not expose of_modalias() Zijun Hu
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

API of_device_uevent_modalias() makes uevent "MODALIAS=ITS_VALUE" in two
steps, namely, produces "MODALIAS=" with add_uevent_var() fistly, then
remainning "ITS_VALUE" with of_modalias() finally, and that may result
in various wrong uevents as explained below:

"MODALIAS=\0"                  // @env->buf is full after the 1st step.
"MODALIAS=ITS_\0"              // @env->buf is not enough during 2nd step.
"MODALIAS=ITS_VAR=VAR_VALUE\0" // another uevent "VAR=VAR_VALUE" comes.

The API depends on uevent internal design, so is not good practice as well.

Fix by:
1) Respin the callee of_modalias() with new prototype which is friendly
   with its callers.
2) Invoke add_uevent_var() to make the whole MODALIAS uevent.
3) Adapt new of_modalias() for its other callers.

BTW, there are no external callers of of_modalias() now.

Closes: https://lore.kernel.org/all/CAL_JsqL+CRmCQMzcF4-A-PRBrCsfK8nduJtOO=RrsDtCUUR7og@mail.gmail.com
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/device.c |  39 +++++++-------------
 drivers/of/module.c | 103 +++++++++++++++++++++++++++++++---------------------
 include/linux/of.h  |   7 ++--
 3 files changed, 79 insertions(+), 70 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index f24c19e7aba8e01656f503ae328a4e08aab5a5f3..6355707c200da9ced354132528adbcce24121247 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -195,19 +195,18 @@ EXPORT_SYMBOL(of_device_get_match_data);
 ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len)
 {
 	ssize_t sl;
+	char *ptr __free(kfree) = NULL;
 
 	if (!dev || !dev->of_node || dev->of_node_reused)
 		return -ENODEV;
 
-	sl = of_modalias(dev->of_node, str, len - 2);
-	if (sl < 0)
-		return sl;
-	if (sl > len - 2)
+	ptr = of_modalias(dev->of_node, &sl);
+	if (IS_ERR(ptr))
+		return PTR_ERR(no_free_ptr(ptr));
+	if (sl + 2 > len)
 		return -ENOMEM;
 
-	str[sl++] = '\n';
-	str[sl] = 0;
-	return sl;
+	return snprintf(str, len, "%s\n", ptr);
 }
 EXPORT_SYMBOL_GPL(of_device_modalias);
 
@@ -256,30 +255,20 @@ EXPORT_SYMBOL_GPL(of_device_uevent);
 
 int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env)
 {
-	int sl;
-	int pos;
+	int ret;
+	char *ptr;
 
 	if ((!dev) || (!dev->of_node) || dev->of_node_reused)
 		return -ENODEV;
 
-	/* Devicetree modalias is tricky, we add it in 2 steps */
-	if (add_uevent_var(env, "MODALIAS="))
-		return -ENOMEM;
+	ptr = of_modalias(dev->of_node, NULL);
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
 
-	/*
-	 * @env->buflen is pointing to the char after '\0' now
-	 * override the '\0' to save MODALIAS value.
-	 */
-	pos = env->buflen - 1;
-	sl = of_modalias(dev->of_node, &env->buf[pos],
-			 sizeof(env->buf) - pos);
-	if (sl < 0)
-		return sl;
-	if (sl >= (sizeof(env->buf) - pos))
-		return -ENOMEM;
-	env->buflen = pos + sl + 1;
+	ret = add_uevent_var(env, "MODALIAS=%s", ptr);
 
-	return 0;
+	kfree(ptr);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
 
diff --git a/drivers/of/module.c b/drivers/of/module.c
index 1e735fc130ad3ea9046f08bfab2cc9a07914e633..03a2b1b381e5b353b6699dac183c03186afb0486 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -8,71 +8,92 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
-ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
+/*
+ * of_modalias - get MODALIAS string value for a OF device node
+ * @np: the OF device node
+ * @lenp: MODALIAS string length returned if set, exclude '\0'
+ *
+ * This function gets MODALIAS value for a device node.
+ *
+ * Returns MODALIAS string on success, or ERR_PTR() on error.
+ *
+ * Note: please kfree successful return value afer using it.
+ */
+char *of_modalias(const struct device_node *np, ssize_t *lenp)
 {
 	const char *compat;
 	char *c;
 	struct property *p;
 	ssize_t csize;
 	ssize_t tsize;
+	char *str = NULL;
+	ssize_t len = 0;
+	ssize_t pos = 0;
+	int counting = 1;
+
+	if (lenp)
+		*lenp = 0;
 
 	/*
-	 * Prevent a kernel oops in vsnprintf() -- it only allows passing a
-	 * NULL ptr when the length is also 0. Also filter out the negative
-	 * lengths...
+	 * Two cycles controlled by @counting, the fist cycle counts
+	 * chars, the second saves chars.
 	 */
-	if ((len > 0 && !str) || len < 0)
-		return -EINVAL;
+	do {
+		/* Name & Type */
+		/* %p eats all alphanum characters, so %c must be used here */
+		csize = snprintf(str + pos, len - pos, "of:N%pOFn%c%s", np, 'T',
+				 of_node_get_device_type(np));
+		if (counting)
+			tsize = csize;
+		else
+			pos += csize;
 
-	/* Name & Type */
-	/* %p eats all alphanum characters, so %c must be used here */
-	csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T',
-			 of_node_get_device_type(np));
-	tsize = csize;
-	if (csize >= len)
-		csize = len > 0 ? len - 1 : 0;
-	len -= csize;
-	str += csize;
+		of_property_for_each_string(np, "compatible", p, compat) {
+			csize = snprintf(str + pos, len - pos, "C%s", compat);
+			if (counting) {
+				tsize += csize;
+				continue;
+			}
 
-	of_property_for_each_string(np, "compatible", p, compat) {
-		csize = snprintf(str, len, "C%s", compat);
-		tsize += csize;
-		if (csize >= len)
-			continue;
-		for (c = str; c; ) {
-			c = strchr(c, ' ');
-			if (c)
-				*c++ = '_';
+			for (c = str + pos; c; ) {
+				c = strchr(c, ' ');
+				if (c)
+					*c++ = '_';
+			}
+			pos += csize;
 		}
-		len -= csize;
-		str += csize;
-	}
 
-	return tsize;
+		if (counting) {
+			/* Include '\0' of MODALIAS string. */
+			len = tsize + 1;
+			/* MODALIAS value is too long */
+			if (unlikely(len > 2048))
+				return ERR_PTR(-EINVAL);
+
+			str = kmalloc(len, GFP_KERNEL);
+			if (!str)
+				return ERR_PTR(-ENOMEM);
+		}
+
+	}	while (counting--);
+
+	if (lenp)
+		*lenp = tsize;
+	return str;
 }
 
 int of_request_module(const struct device_node *np)
 {
 	char *str;
-	ssize_t size;
 	int ret;
 
 	if (!np)
 		return -ENODEV;
 
-	size = of_modalias(np, NULL, 0);
-	if (size < 0)
-		return size;
-
-	/* Reserve an additional byte for the trailing '\0' */
-	size++;
-
-	str = kmalloc(size, GFP_KERNEL);
-	if (!str)
-		return -ENOMEM;
+	str = of_modalias(np, NULL);
+	if (IS_ERR(str))
+		return PTR_ERR(str);
 
-	of_modalias(np, str, size);
-	str[size - 1] = '\0';
 	ret = request_module(str);
 	kfree(str);
 
diff --git a/include/linux/of.h b/include/linux/of.h
index fe5d7b74c23b9701743f5debc3d030b765bc914f..f36bab2caa8ccffbd43593d8b6720946e19503e0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -382,7 +382,7 @@ extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
 
 /* module functions */
-extern ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len);
+char *of_modalias(const struct device_node *np, ssize_t *lenp);
 extern int of_request_module(const struct device_node *np);
 
 /* phandle iterator functions */
@@ -761,10 +761,9 @@ static inline int of_count_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
-static inline ssize_t of_modalias(const struct device_node *np, char *str,
-				  ssize_t len)
+static inline char *of_modalias(const struct device_node *np, ssize_t *lenp)
 {
-	return -ENODEV;
+	return ERR_PTR(-ENODEV);
 }
 
 static inline int of_request_module(const struct device_node *np)

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 7/7] of: Do not expose of_modalias()
  2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
                   ` (5 preceding siblings ...)
  2024-12-17 13:07 ` [PATCH v3 6/7] of: Fix potential wrong MODALIAS uevent value Zijun Hu
@ 2024-12-17 13:07 ` Zijun Hu
  6 siblings, 0 replies; 8+ messages in thread
From: Zijun Hu @ 2024-12-17 13:07 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Maxime Ripard, Robin Murphy,
	Grant Likely
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

Do not expose of_modalias() since it has no external callers.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/module.c     | 2 ++
 drivers/of/of_private.h | 2 ++
 include/linux/of.h      | 6 ------
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/of/module.c b/drivers/of/module.c
index 03a2b1b381e5b353b6699dac183c03186afb0486..6dd670f7e4fe68b6a8f4b30a81064a1b8e4a0644 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -8,6 +8,8 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
+#include "of_private.h"
+
 /*
  * of_modalias - get MODALIAS string value for a OF device node
  * @np: the OF device node
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 3433ccd330e84fd3a4b54638e0e922069757c8f0..5facb6ff63cce46205fbe3969ae2e9f7858ac9bd 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -150,6 +150,8 @@ extern void __of_detach_node(struct device_node *np);
 extern void __of_sysfs_remove_bin_file(struct device_node *np,
 				       const struct property *prop);
 
+char *of_modalias(const struct device_node *np, ssize_t *lenp);
+
 /* illegal phandle value (set when unresolved) */
 #define OF_PHANDLE_ILLEGAL	0xdeadbeef
 
diff --git a/include/linux/of.h b/include/linux/of.h
index f36bab2caa8ccffbd43593d8b6720946e19503e0..d9062d4fbd143d42b6b217942aced66fa34f1c80 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -382,7 +382,6 @@ extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
 
 /* module functions */
-char *of_modalias(const struct device_node *np, ssize_t *lenp);
 extern int of_request_module(const struct device_node *np);
 
 /* phandle iterator functions */
@@ -761,11 +760,6 @@ static inline int of_count_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
-static inline char *of_modalias(const struct device_node *np, ssize_t *lenp)
-{
-	return ERR_PTR(-ENODEV);
-}
-
 static inline int of_request_module(const struct device_node *np)
 {
 	return -ENODEV;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-12-17 13:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-17 13:07 [PATCH v3 0/7] of: fix bugs and improve codes Zijun Hu
2024-12-17 13:07 ` [PATCH v3 1/7] of: Correct child specifier used as input of the 2nd nexus node Zijun Hu
2024-12-17 13:07 ` [PATCH v3 2/7] of: Do not expose of_alias_scan() and correct its comments Zijun Hu
2024-12-17 13:07 ` [PATCH v3 3/7] of: Make of_property_present() applicable to all kinds of property Zijun Hu
2024-12-17 13:07 ` [PATCH v3 4/7] of: property: Use of_property_present() for of_fwnode_property_present() Zijun Hu
2024-12-17 13:07 ` [PATCH v3 5/7] of: Fix available buffer size calculating error in API of_device_uevent_modalias() Zijun Hu
2024-12-17 13:07 ` [PATCH v3 6/7] of: Fix potential wrong MODALIAS uevent value Zijun Hu
2024-12-17 13:07 ` [PATCH v3 7/7] of: Do not expose of_modalias() Zijun Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).