[PATCH 0/3] of: base: of_parse_phandle() fixes and new variant

[PATCH 0/3] of: base: of_parse_phandle() fixes and new variant

[Michael Walle]

This series is a result of the discussion in [1]. Rob suggested to convert
the index parameter to unsigned int and drop the check for negative values
and make them static inline.

It will also introduce a new variant of the function, although it is unused
for now. They will be needed when nvmem phandles are modified to take
additional arguments and need to retain backwards compatibility with older
device trees.

[1] https://lore.kernel.org/linux-devicetree/20211228142549.1275412-1-michael@walle.cc/

Michael Walle (3):
  of: base: convert index to unsigned for of_parse_phandle()
  of: property: use unsigned index for of_link_property()
  of: base: add of_parse_phandle_with_optional_args()

 drivers/of/base.c     | 137 +++-----------------------------
 drivers/of/property.c |  27 ++++---
 include/linux/of.h    | 176 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 189 insertions(+), 151 deletions(-)

-- 
2.30.2

[PATCH 1/3] of: base: convert index to unsigned for of_parse_phandle()

[Michael Walle]

Since commit 2021bd01ffcc ("of: Remove counting special case from
__of_parse_phandle_with_args()"), the index is >=0, thus convert the
paramenter to unsigned of the of_parse_phandle() and all its variants.
Make the smaller variants static inline, too.

Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/of/base.c  | 137 +++---------------------------------------
 include/linux/of.h | 147 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 142 insertions(+), 142 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8a24d37153b4..58b1b6ffc105 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1420,14 +1420,15 @@ int of_phandle_iterator_args(struct of_phandle_iterator *it,
 	return count;
 }
 
-static int __of_parse_phandle_with_args(const struct device_node *np,
-					const char *list_name,
-					const char *cells_name,
-					int cell_count, int index,
-					struct of_phandle_args *out_args)
+int __of_parse_phandle_with_args(const struct device_node *np,
+				 const char *list_name,
+				 const char *cells_name,
+				 int cell_count, unsigned int index,
+				 struct of_phandle_args *out_args)
 {
 	struct of_phandle_iterator it;
-	int rc, cur_index = 0;
+	unsigned int cur_index = 0;
+	int rc;
 
 	/* Loop over the phandles until all the requested entry is found */
 	of_for_each_phandle(&it, rc, np, list_name, cells_name, cell_count) {
@@ -1471,82 +1472,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
 	of_node_put(it.node);
 	return rc;
 }
-
-/**
- * of_parse_phandle - Resolve a phandle property to a device_node pointer
- * @np: Pointer to device node holding phandle property
- * @phandle_name: Name of property holding a phandle value
- * @index: For properties holding a table of phandles, this is the index into
- *         the table
- *
- * Return: The device_node pointer with refcount incremented.  Use
- * of_node_put() on it when done.
- */
-struct device_node *of_parse_phandle(const struct device_node *np,
-				     const char *phandle_name, int index)
-{
-	struct of_phandle_args args;
-
-	if (index < 0)
-		return NULL;
-
-	if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
-					 index, &args))
-		return NULL;
-
-	return args.np;
-}
-EXPORT_SYMBOL(of_parse_phandle);
-
-/**
- * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
- * @np:		pointer to a device tree node containing a list
- * @list_name:	property name that contains a list
- * @cells_name:	property name that specifies phandles' arguments count
- * @index:	index of a phandle to parse out
- * @out_args:	optional pointer to output arguments structure (will be filled)
- *
- * This function is useful to parse lists of phandles and their arguments.
- * Returns 0 on success and fills out_args, on error returns appropriate
- * errno value.
- *
- * Caller is responsible to call of_node_put() on the returned out_args->np
- * pointer.
- *
- * Example::
- *
- *  phandle1: node1 {
- *	#list-cells = <2>;
- *  };
- *
- *  phandle2: node2 {
- *	#list-cells = <1>;
- *  };
- *
- *  node3 {
- *	list = <&phandle1 1 2 &phandle2 3>;
- *  };
- *
- * To get a device_node of the ``node2`` node you may call this:
- * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
- */
-int of_parse_phandle_with_args(const struct device_node *np, const char *list_name,
-				const char *cells_name, int index,
-				struct of_phandle_args *out_args)
-{
-	int cell_count = -1;
-
-	if (index < 0)
-		return -EINVAL;
-
-	/* If cells_name is NULL we assume a cell count of 0 */
-	if (!cells_name)
-		cell_count = 0;
-
-	return __of_parse_phandle_with_args(np, list_name, cells_name,
-					    cell_count, index, out_args);
-}
-EXPORT_SYMBOL(of_parse_phandle_with_args);
+EXPORT_SYMBOL(__of_parse_phandle_with_args);
 
 /**
  * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
@@ -1593,7 +1519,8 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
 int of_parse_phandle_with_args_map(const struct device_node *np,
 				   const char *list_name,
 				   const char *stem_name,
-				   int index, struct of_phandle_args *out_args)
+				   unsigned int index,
+				   struct of_phandle_args *out_args)
 {
 	char *cells_name, *map_name = NULL, *mask_name = NULL;
 	char *pass_name = NULL;
@@ -1606,9 +1533,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
 	int i, ret, map_len, match;
 	u32 list_size, new_size;
 
-	if (index < 0)
-		return -EINVAL;
-
 	cells_name = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
 	if (!cells_name)
 		return -ENOMEM;
@@ -1732,47 +1656,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args_map);
 
-/**
- * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
- * @np:		pointer to a device tree node containing a list
- * @list_name:	property name that contains a list
- * @cell_count: number of argument cells following the phandle
- * @index:	index of a phandle to parse out
- * @out_args:	optional pointer to output arguments structure (will be filled)
- *
- * This function is useful to parse lists of phandles and their arguments.
- * Returns 0 on success and fills out_args, on error returns appropriate
- * errno value.
- *
- * Caller is responsible to call of_node_put() on the returned out_args->np
- * pointer.
- *
- * Example::
- *
- *  phandle1: node1 {
- *  };
- *
- *  phandle2: node2 {
- *  };
- *
- *  node3 {
- *  	list = <&phandle1 0 2 &phandle2 2 3>;
- *  };
- *
- * To get a device_node of the ``node2`` node you may call this:
- * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
- */
-int of_parse_phandle_with_fixed_args(const struct device_node *np,
-				const char *list_name, int cell_count,
-				int index, struct of_phandle_args *out_args)
-{
-	if (index < 0)
-		return -EINVAL;
-	return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
-					   index, out_args);
-}
-EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
-
 /**
  * of_count_phandle_with_args() - Find the number of phandles references in a property
  * @np:		pointer to a device tree node containing a list
diff --git a/include/linux/of.h b/include/linux/of.h
index ff143a027abc..df3af6d3cbe3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -364,17 +364,11 @@ extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
 extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args);
-extern struct device_node *of_parse_phandle(const struct device_node *np,
-					    const char *phandle_name,
-					    int index);
-extern int of_parse_phandle_with_args(const struct device_node *np,
-	const char *list_name, const char *cells_name, int index,
-	struct of_phandle_args *out_args);
+extern int __of_parse_phandle_with_args(const struct device_node *np, const
+	char *list_name, const char *cells_name, int cell_count,
+	unsigned int index, struct of_phandle_args *out_args);
 extern int of_parse_phandle_with_args_map(const struct device_node *np,
-	const char *list_name, const char *stem_name, int index,
-	struct of_phandle_args *out_args);
-extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
-	const char *list_name, int cells_count, int index,
+	const char *list_name, const char *stem_name, unsigned int index,
 	struct of_phandle_args *out_args);
 extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
@@ -416,6 +410,117 @@ extern int of_detach_node(struct device_node *);
 
 #define of_match_ptr(_ptr)	(_ptr)
 
+/**
+ * of_parse_phandle - Resolve a phandle property to a device_node pointer
+ * @np: Pointer to device node holding phandle property
+ * @phandle_name: Name of property holding a phandle value
+ * @index: For properties holding a table of phandles, this is the index into
+ *         the table
+ *
+ * Return: The device_node pointer with refcount incremented.  Use
+ * of_node_put() on it when done.
+ */
+static inline struct device_node *of_parse_phandle(const struct device_node *np,
+						   const char *phandle_name,
+						   unsigned int index)
+{
+	struct of_phandle_args args;
+
+	if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
+					 index, &args))
+		return NULL;
+
+	return args.np;
+}
+
+/**
+ * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_args:	optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example::
+ *
+ *  phandle1: node1 {
+ *	#list-cells = <2>;
+ *  };
+ *
+ *  phandle2: node2 {
+ *	#list-cells = <1>;
+ *  };
+ *
+ *  node3 {
+ *	list = <&phandle1 1 2 &phandle2 3>;
+ *  };
+ *
+ * To get a device_node of the ``node2`` node you may call this:
+ * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
+ */
+static inline int of_parse_phandle_with_args(const struct device_node *np,
+					     const char *list_name,
+					     const char *cells_name,
+					     unsigned int index,
+					     struct of_phandle_args *out_args)
+{
+	int cell_count = -1;
+
+	/* If cells_name is NULL we assume a cell count of 0 */
+	if (!cells_name)
+		cell_count = 0;
+
+	return __of_parse_phandle_with_args(np, list_name, cells_name,
+					    cell_count, index, out_args);
+}
+
+/**
+ * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cell_count: number of argument cells following the phandle
+ * @index:	index of a phandle to parse out
+ * @out_args:	optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example::
+ *
+ *  phandle1: node1 {
+ *  };
+ *
+ *  phandle2: node2 {
+ *  };
+ *
+ *  node3 {
+ *	list = <&phandle1 0 2 &phandle2 2 3>;
+ *  };
+ *
+ * To get a device_node of the ``node2`` node you may call this:
+ * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
+ */
+static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
+						   const char *list_name,
+						   int cell_count,
+						   unsigned int index,
+						   struct of_phandle_args *out_args)
+{
+	return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
+					    index, out_args);
+}
+
 /**
  * of_property_read_u8_array - Find and read an array of u8 from a property.
  *
@@ -865,9 +970,19 @@ static inline int of_property_read_string_helper(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int __of_parse_phandle_with_args(const struct device_node *np,
+					       const char *list_name,
+					       const char *cells_name,
+					       int cell_count,
+					       unsigned int index,
+					       struct of_phandle_args *out_args)
+{
+	return -ENOSYS;
+};
+
 static inline struct device_node *of_parse_phandle(const struct device_node *np,
 						   const char *phandle_name,
-						   int index)
+						   unsigned int index)
 {
 	return NULL;
 }
@@ -875,7 +990,7 @@ static inline struct device_node *of_parse_phandle(const struct device_node *np,
 static inline int of_parse_phandle_with_args(const struct device_node *np,
 					     const char *list_name,
 					     const char *cells_name,
-					     int index,
+					     unsigned int index,
 					     struct of_phandle_args *out_args)
 {
 	return -ENOSYS;
@@ -884,15 +999,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
 static inline int of_parse_phandle_with_args_map(const struct device_node *np,
 						 const char *list_name,
 						 const char *stem_name,
-						 int index,
+						 unsigned int index,
 						 struct of_phandle_args *out_args)
 {
 	return -ENOSYS;
 }
 
 static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
-	const char *list_name, int cells_count, int index,
-	struct of_phandle_args *out_args)
+						   const char *list_name,
+						   int cells_count,
+						   unsigned int index,
+						   struct of_phandle_args *out_args)
 {
 	return -ENOSYS;
 }
-- 
2.30.2

[PATCH 2/3] of: property: use unsigned index for of_link_property()

[Michael Walle]

Now that of_parse_handle() and its variants take an unsigned int,
convert the index used in of_link_property() and its called functions
to unsigned too.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/of/property.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 8e90071de6ed..e77fb6cda0b7 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1173,7 +1173,8 @@ static int of_link_to_phandle(struct device_node *con_np,
  * - NULL if no phandle found at index
  */
 static struct device_node *parse_prop_cells(struct device_node *np,
-					    const char *prop_name, int index,
+					    const char *prop_name,
+					    unsigned int index,
 					    const char *list_name,
 					    const char *cells_name)
 {
@@ -1191,7 +1192,8 @@ static struct device_node *parse_prop_cells(struct device_node *np,
 
 #define DEFINE_SIMPLE_PROP(fname, name, cells)				  \
 static struct device_node *parse_##fname(struct device_node *np,	  \
-					const char *prop_name, int index) \
+					 const char *prop_name,		  \
+					 unsigned int index)		  \
 {									  \
 	return parse_prop_cells(np, prop_name, index, name, cells);	  \
 }
@@ -1227,7 +1229,8 @@ static int strcmp_suffix(const char *str, const char *suffix)
  * - NULL if no phandle found at index
  */
 static struct device_node *parse_suffix_prop_cells(struct device_node *np,
-					    const char *prop_name, int index,
+					    const char *prop_name,
+					    unsigned int index,
 					    const char *suffix,
 					    const char *cells_name)
 {
@@ -1245,7 +1248,8 @@ static struct device_node *parse_suffix_prop_cells(struct device_node *np,
 
 #define DEFINE_SUFFIX_PROP(fname, suffix, cells)			     \
 static struct device_node *parse_##fname(struct device_node *np,	     \
-					const char *prop_name, int index)    \
+					 const char *prop_name,		     \
+					 unsigned int index)		     \
 {									     \
 	return parse_suffix_prop_cells(np, prop_name, index, suffix, cells); \
 }
@@ -1272,7 +1276,8 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
  */
 struct supplier_bindings {
 	struct device_node *(*parse_prop)(struct device_node *np,
-					  const char *prop_name, int index);
+					  const char *prop_name,
+					  unsigned int index);
 	bool optional;
 	bool node_not_dev;
 };
@@ -1308,7 +1313,8 @@ DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
 
 static struct device_node *parse_gpios(struct device_node *np,
-				       const char *prop_name, int index)
+				       const char *prop_name,
+				       unsigned int index)
 {
 	if (!strcmp_suffix(prop_name, ",nr-gpios"))
 		return NULL;
@@ -1318,7 +1324,8 @@ static struct device_node *parse_gpios(struct device_node *np,
 }
 
 static struct device_node *parse_iommu_maps(struct device_node *np,
-					    const char *prop_name, int index)
+					    const char *prop_name,
+					    unsigned int index)
 {
 	if (strcmp(prop_name, "iommu-map"))
 		return NULL;
@@ -1327,7 +1334,8 @@ static struct device_node *parse_iommu_maps(struct device_node *np,
 }
 
 static struct device_node *parse_gpio_compat(struct device_node *np,
-					     const char *prop_name, int index)
+					     const char *prop_name,
+					     unsigned int index)
 {
 	struct of_phandle_args sup_args;
 
@@ -1349,7 +1357,8 @@ static struct device_node *parse_gpio_compat(struct device_node *np,
 }
 
 static struct device_node *parse_interrupts(struct device_node *np,
-					    const char *prop_name, int index)
+					    const char *prop_name,
+					    unsigned int index)
 {
 	struct of_phandle_args sup_args;
 
-- 
2.30.2

[PATCH 3/3] of: base: add of_parse_phandle_with_optional_args()

[Michael Walle]

Add a new variant of the of_parse_phandle_with_args() which treats the
cells name as optional. If it's missing, it is assumed that the phandle
has no arguments.

Up until now, a nvmem node didn't have any arguments, so all the device
trees haven't any '#*-cells' property. But there is a need for an
additional argument for the phandle, for which we need a '#*-cells'
property. Therefore, we need to support nvmem nodes with and without
this property.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/linux/of.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index df3af6d3cbe3..f29f18aa95d9 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -521,6 +521,26 @@ static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
 					    index, out_args);
 }
 
+/**
+ * of_parse_phandle_with_optional_args() - Find a node pointed by phandle in a list
+ *
+ * Same as of_parse_phandle_args() except that if the cells_name property is
+ * not found, cell_count of 0 is assumed.
+ *
+ * This is used to useful, if you have a phandle which didn't have arguments
+ * before and thus doesn't have a '#*-cells' property but is now migrated to
+ * having arguments while retaining backwards compatibility.
+ */
+static inline int of_parse_phandle_with_optional_args(const struct device_node *np,
+						      const char *list_name,
+						      const char *cells_name,
+						      unsigned int index,
+						      struct of_phandle_args *out_args)
+{
+	return __of_parse_phandle_with_args(np, list_name, cells_name,
+					    0, index, out_args);
+}
+
 /**
  * of_property_read_u8_array - Find and read an array of u8 from a property.
  *
@@ -1014,6 +1034,15 @@ static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_parse_phandle_with_optional_args(const struct device_node *np,
+						      char *list_name,
+						      int cells_count,
+						      unsigned int index,
+						      struct of_phandle_args *out_args)
+{
+	return -ENOSYS;
+}
+
 static inline int of_count_phandle_with_args(const struct device_node *np,
 					     const char *list_name,
 					     const char *cells_name)
-- 
2.30.2

Re: [PATCH 0/3] of: base: of_parse_phandle() fixes and new variant

[Michael Walle]

Am 2022-01-13 09:52, schrieb Michael Walle:
> This series is a result of the discussion in [1]. Rob suggested to 
> convert
> the index parameter to unsigned int and drop the check for negative 
> values
> and make them static inline.

Oh I haven't thought this through.. If this is going via another tree 
than
the nvmem patches, then I'd need either wait one kernel release, or 
there
need to be an immutable tag, right?

-michael

Re: [PATCH 1/3] of: base: convert index to unsigned for of_parse_phandle()

[Rob Herring]

On Thu, Jan 13, 2022 at 2:52 AM Michael Walle <michael@walle.cc> wrote:
>
> Since commit 2021bd01ffcc ("of: Remove counting special case from
> __of_parse_phandle_with_args()"), the index is >=0, thus convert the

Ah good, that explains why we had signed in the first place.

> paramenter to unsigned of the of_parse_phandle() and all its variants.

typo.

> Make the smaller variants static inline, too.

This should be a separate patch.

> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/of/base.c  | 137 +++---------------------------------------
>  include/linux/of.h | 147 ++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 142 insertions(+), 142 deletions(-)

A lot of moving around going on...

>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 8a24d37153b4..58b1b6ffc105 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1420,14 +1420,15 @@ int of_phandle_iterator_args(struct of_phandle_iterator *it,
>         return count;
>  }
>
> -static int __of_parse_phandle_with_args(const struct device_node *np,
> -                                       const char *list_name,
> -                                       const char *cells_name,
> -                                       int cell_count, int index,
> -                                       struct of_phandle_args *out_args)
> +int __of_parse_phandle_with_args(const struct device_node *np,
> +                                const char *list_name,
> +                                const char *cells_name,
> +                                int cell_count, unsigned int index,
> +                                struct of_phandle_args *out_args)
>  {
>         struct of_phandle_iterator it;
> -       int rc, cur_index = 0;
> +       unsigned int cur_index = 0;
> +       int rc;
>
>         /* Loop over the phandles until all the requested entry is found */
>         of_for_each_phandle(&it, rc, np, list_name, cells_name, cell_count) {
> @@ -1471,82 +1472,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
>         of_node_put(it.node);
>         return rc;
>  }
> -
> -/**
> - * of_parse_phandle - Resolve a phandle property to a device_node pointer
> - * @np: Pointer to device node holding phandle property
> - * @phandle_name: Name of property holding a phandle value
> - * @index: For properties holding a table of phandles, this is the index into
> - *         the table
> - *
> - * Return: The device_node pointer with refcount incremented.  Use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_parse_phandle(const struct device_node *np,
> -                                    const char *phandle_name, int index)
> -{
> -       struct of_phandle_args args;
> -
> -       if (index < 0)
> -               return NULL;
> -
> -       if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
> -                                        index, &args))
> -               return NULL;
> -
> -       return args.np;
> -}
> -EXPORT_SYMBOL(of_parse_phandle);
> -
> -/**
> - * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
> - * @np:                pointer to a device tree node containing a list
> - * @list_name: property name that contains a list
> - * @cells_name:        property name that specifies phandles' arguments count
> - * @index:     index of a phandle to parse out
> - * @out_args:  optional pointer to output arguments structure (will be filled)
> - *
> - * This function is useful to parse lists of phandles and their arguments.
> - * Returns 0 on success and fills out_args, on error returns appropriate
> - * errno value.
> - *
> - * Caller is responsible to call of_node_put() on the returned out_args->np
> - * pointer.
> - *
> - * Example::
> - *
> - *  phandle1: node1 {
> - *     #list-cells = <2>;
> - *  };
> - *
> - *  phandle2: node2 {
> - *     #list-cells = <1>;
> - *  };
> - *
> - *  node3 {
> - *     list = <&phandle1 1 2 &phandle2 3>;
> - *  };
> - *
> - * To get a device_node of the ``node2`` node you may call this:
> - * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
> - */
> -int of_parse_phandle_with_args(const struct device_node *np, const char *list_name,
> -                               const char *cells_name, int index,
> -                               struct of_phandle_args *out_args)
> -{
> -       int cell_count = -1;
> -
> -       if (index < 0)
> -               return -EINVAL;
> -
> -       /* If cells_name is NULL we assume a cell count of 0 */
> -       if (!cells_name)
> -               cell_count = 0;
> -
> -       return __of_parse_phandle_with_args(np, list_name, cells_name,
> -                                           cell_count, index, out_args);
> -}
> -EXPORT_SYMBOL(of_parse_phandle_with_args);
> +EXPORT_SYMBOL(__of_parse_phandle_with_args);
>
>  /**
>   * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> @@ -1593,7 +1519,8 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
>  int of_parse_phandle_with_args_map(const struct device_node *np,
>                                    const char *list_name,
>                                    const char *stem_name,
> -                                  int index, struct of_phandle_args *out_args)
> +                                  unsigned int index,
> +                                  struct of_phandle_args *out_args)
>  {
>         char *cells_name, *map_name = NULL, *mask_name = NULL;
>         char *pass_name = NULL;
> @@ -1606,9 +1533,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
>         int i, ret, map_len, match;
>         u32 list_size, new_size;
>
> -       if (index < 0)
> -               return -EINVAL;
> -
>         cells_name = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
>         if (!cells_name)
>                 return -ENOMEM;
> @@ -1732,47 +1656,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
>  }
>  EXPORT_SYMBOL(of_parse_phandle_with_args_map);
>
> -/**
> - * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
> - * @np:                pointer to a device tree node containing a list
> - * @list_name: property name that contains a list
> - * @cell_count: number of argument cells following the phandle
> - * @index:     index of a phandle to parse out
> - * @out_args:  optional pointer to output arguments structure (will be filled)
> - *
> - * This function is useful to parse lists of phandles and their arguments.
> - * Returns 0 on success and fills out_args, on error returns appropriate
> - * errno value.
> - *
> - * Caller is responsible to call of_node_put() on the returned out_args->np
> - * pointer.
> - *
> - * Example::
> - *
> - *  phandle1: node1 {
> - *  };
> - *
> - *  phandle2: node2 {
> - *  };
> - *
> - *  node3 {
> - *     list = <&phandle1 0 2 &phandle2 2 3>;
> - *  };
> - *
> - * To get a device_node of the ``node2`` node you may call this:
> - * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
> - */
> -int of_parse_phandle_with_fixed_args(const struct device_node *np,
> -                               const char *list_name, int cell_count,
> -                               int index, struct of_phandle_args *out_args)
> -{
> -       if (index < 0)
> -               return -EINVAL;
> -       return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
> -                                          index, out_args);
> -}
> -EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
> -
>  /**
>   * of_count_phandle_with_args() - Find the number of phandles references in a property
>   * @np:                pointer to a device tree node containing a list
> diff --git a/include/linux/of.h b/include/linux/of.h
> index ff143a027abc..df3af6d3cbe3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -364,17 +364,11 @@ extern const struct of_device_id *of_match_node(
>         const struct of_device_id *matches, const struct device_node *node);
>  extern int of_modalias_node(struct device_node *node, char *modalias, int len);
>  extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args);
> -extern struct device_node *of_parse_phandle(const struct device_node *np,
> -                                           const char *phandle_name,
> -                                           int index);
> -extern int of_parse_phandle_with_args(const struct device_node *np,
> -       const char *list_name, const char *cells_name, int index,
> -       struct of_phandle_args *out_args);
> +extern int __of_parse_phandle_with_args(const struct device_node *np, const
> +       char *list_name, const char *cells_name, int cell_count,
> +       unsigned int index, struct of_phandle_args *out_args);
>  extern int of_parse_phandle_with_args_map(const struct device_node *np,
> -       const char *list_name, const char *stem_name, int index,
> -       struct of_phandle_args *out_args);
> -extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
> -       const char *list_name, int cells_count, int index,
> +       const char *list_name, const char *stem_name, unsigned int index,
>         struct of_phandle_args *out_args);
>  extern int of_count_phandle_with_args(const struct device_node *np,
>         const char *list_name, const char *cells_name);
> @@ -416,6 +410,117 @@ extern int of_detach_node(struct device_node *);
>
>  #define of_match_ptr(_ptr)     (_ptr)
>
> +/**
> + * of_parse_phandle - Resolve a phandle property to a device_node pointer
> + * @np: Pointer to device node holding phandle property
> + * @phandle_name: Name of property holding a phandle value
> + * @index: For properties holding a table of phandles, this is the index into
> + *         the table
> + *
> + * Return: The device_node pointer with refcount incremented.  Use
> + * of_node_put() on it when done.
> + */
> +static inline struct device_node *of_parse_phandle(const struct device_node *np,
> +                                                  const char *phandle_name,
> +                                                  unsigned int index)
> +{
> +       struct of_phandle_args args;
> +
> +       if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
> +                                        index, &args))
> +               return NULL;
> +
> +       return args.np;
> +}
> +
> +/**
> + * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name:        property name that specifies phandles' arguments count
> + * @index:     index of a phandle to parse out
> + * @out_args:  optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example::
> + *
> + *  phandle1: node1 {
> + *     #list-cells = <2>;
> + *  };
> + *
> + *  phandle2: node2 {
> + *     #list-cells = <1>;
> + *  };
> + *
> + *  node3 {
> + *     list = <&phandle1 1 2 &phandle2 3>;
> + *  };
> + *
> + * To get a device_node of the ``node2`` node you may call this:
> + * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
> + */
> +static inline int of_parse_phandle_with_args(const struct device_node *np,
> +                                            const char *list_name,
> +                                            const char *cells_name,
> +                                            unsigned int index,
> +                                            struct of_phandle_args *out_args)
> +{
> +       int cell_count = -1;
> +
> +       /* If cells_name is NULL we assume a cell count of 0 */
> +       if (!cells_name)
> +               cell_count = 0;
> +
> +       return __of_parse_phandle_with_args(np, list_name, cells_name,
> +                                           cell_count, index, out_args);
> +}
> +
> +/**
> + * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cell_count: number of argument cells following the phandle
> + * @index:     index of a phandle to parse out
> + * @out_args:  optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example::
> + *
> + *  phandle1: node1 {
> + *  };
> + *
> + *  phandle2: node2 {
> + *  };
> + *
> + *  node3 {
> + *     list = <&phandle1 0 2 &phandle2 2 3>;
> + *  };
> + *
> + * To get a device_node of the ``node2`` node you may call this:
> + * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
> + */
> +static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
> +                                                  const char *list_name,
> +                                                  int cell_count,
> +                                                  unsigned int index,
> +                                                  struct of_phandle_args *out_args)
> +{
> +       return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
> +                                           index, out_args);
> +}
> +
>  /**
>   * of_property_read_u8_array - Find and read an array of u8 from a property.
>   *
> @@ -865,9 +970,19 @@ static inline int of_property_read_string_helper(const struct device_node *np,
>         return -ENOSYS;
>  }
>
> +static inline int __of_parse_phandle_with_args(const struct device_node *np,
> +                                              const char *list_name,
> +                                              const char *cells_name,
> +                                              int cell_count,
> +                                              unsigned int index,
> +                                              struct of_phandle_args *out_args)
> +{
> +       return -ENOSYS;
> +};
> +
>  static inline struct device_node *of_parse_phandle(const struct device_node *np,
>                                                    const char *phandle_name,
> -                                                  int index)
> +                                                  unsigned int index)
>  {
>         return NULL;
>  }
> @@ -875,7 +990,7 @@ static inline struct device_node *of_parse_phandle(const struct device_node *np,
>  static inline int of_parse_phandle_with_args(const struct device_node *np,
>                                              const char *list_name,
>                                              const char *cells_name,
> -                                            int index,
> +                                            unsigned int index,
>                                              struct of_phandle_args *out_args)
>  {
>         return -ENOSYS;
> @@ -884,15 +999,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
>  static inline int of_parse_phandle_with_args_map(const struct device_node *np,

With these as static inlines, you only need them once unconditionally
as long as __of_parse_phandle_with_args() has an empty static inline.

Rob

Re: [PATCH 0/3] of: base: of_parse_phandle() fixes and new variant

[Rob Herring]

On Thu, Jan 13, 2022 at 6:22 AM Michael Walle <michael@walle.cc> wrote:
>
> Am 2022-01-13 09:52, schrieb Michael Walle:
> > This series is a result of the discussion in [1]. Rob suggested to
> > convert
> > the index parameter to unsigned int and drop the check for negative
> > values
> > and make them static inline.
>
> Oh I haven't thought this through.. If this is going via another tree
> than
> the nvmem patches, then I'd need either wait one kernel release, or
> there
> need to be an immutable tag, right?

I can pick this up for 5.17-rc1 if we can get it sorted soon.

Rob