Re: [RFC 3/8] of: create of_phandle_args to simplify return of phandle parsing data

 /**
- * of_parse_phandles_with_args - Find a node pointed by phandle in a list
+ * 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_node:	optional pointer to device_node struct pointer (will be filled)
- * @out_args:	optional pointer to arguments pointer (will be filled)
+ * @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_node and out_args, on error returns
- * appropriate errno value.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
  *
  * Example:
  *

@@ -851,94 +850,105 @@ EXPORT_SYMBOL(of_parse_phandle);
  * }
  *
  * To get a device_node of the `node2' node you may call this:
- * of_parse_phandles_with_args(node3, "list", "#list-cells", 2, &node2, &args);
+ * of_parse_phandle_with_args(node3, "list", "#list-cells", 2, &args);

  */
-int of_parse_phandles_with_args(struct device_node *np, const char *list_name,
+int of_parse_phandle_with_args(struct device_node *np, const char *list_name,
 				const char *cells_name, int index,
-				struct device_node **out_node,
-				const void **out_args)
+				struct of_phandle_args *out_args)
 {
 	int ret = -EINVAL;
-	const __be32 *list;
-	const __be32 *list_end;
-	int size;
-	int cur_index = 0;
+	const __be32 *list, *list_end;
+	int size, cur_index = 0;
+	uint32_t count = 0;
 	struct device_node *node = NULL;
-	const void *args = NULL;
+	phandle phandle;
 
+	/* Retrieve the phandle list property */
 	list = of_get_property(np, list_name, &size);
-	if (!list) {
-		ret = -ENOENT;
+	if (!list)
 		goto err0;
-	}
 	list_end = list + size / sizeof(*list);
 
+	/*
+	 * Loop over all the entries in the list, parsing the #cells property
+	 * for each one to determine how long each argument list is
+	 */
 	while (list < list_end) {
-		const __be32 *cells;
-		phandle phandle;
+		count = 0;
 
+		/*
+		 * If phandle is null, then it is an empty entry with no args,
+		 * skip forward to the next entry
+		 */
 		phandle = be32_to_cpup(list++);
-		args = list;
-
-		/* one cell hole in the list = <>; */
-		if (!phandle)
-			goto next;
-
-		node = of_find_node_by_phandle(phandle);
-		if (!node) {
-			pr_debug("%s: could not find phandle\n",
-				 np->full_name);
-			goto err0;
-		}
+		if (phandle) {
+			/*
+			 * Find the provider node and parse the #*-cells
+			 * property to determine how long the arguements are
+			 */
+			node = of_find_node_by_phandle(phandle);
+			if (!node) {
+				pr_debug("%s: could not find phandle\n",
+					 np->full_name);
+				ret = -EINVAL;
+				goto err0;
+			}
+			if (of_property_read_u32(node, cells_name, &count)) {
+				pr_debug("%s: could not get %s for %s\n",
+					 np->full_name, cells_name,
+					 node->full_name);
+				ret = -EINVAL;
+				goto err1;
+			}
 
-		cells = of_get_property(node, cells_name, &size);
-		if (!cells || size != sizeof(*cells)) {
-			pr_debug("%s: could not get %s for %s\n",
-				 np->full_name, cells_name, node->full_name);
-			goto err1;
+			/*
+			 * Make sure that the arguments actually fit in the
+			 * remaining property data length
+			 */
+			if (list + count > list_end) {
+				pr_debug("%s: insufficient arguments length\n",
+					 np->full_name);
+				goto err1;
+			}
 		}
 
-		list += be32_to_cpup(cells);
-		if (list > list_end) {
-			pr_debug("%s: insufficient arguments length\n",
-				 np->full_name);
-			goto err1;
-		}
-next:
-		if (cur_index == index)
+		/*
+		 * All of the error cases in the loop bail out to the err
+		 * labels, so at this point, the parsing was successful, but
+		 * it might be an empty entry
+		 */
+		if (cur_index == index) {
+			ret = phandle ? 0 : -ENOENT;

 			break;
+		}
 
 		of_node_put(node);
 		node = NULL;
-		args = NULL;
+		list += count;
 		cur_index++;
 	}
 
-	if (!node) {
-		/*
-		 * args w/o node indicates that the loop above has stopped at
-		 * the 'hole' cell. Report this differently.
-		 */
-		if (args)
-			ret = -EEXIST;
-		else
-			ret = -ENOENT;
+	if (!node)
 		goto err0;
-	}
-
-	if (out_node)
-		*out_node = node;
-	if (out_args)
-		*out_args = args;
 
+	if (out_args) {
+		int i;
+		if (WARN_ON(count > MAX_PHANDLE_ARGS))
+			count = MAX_PHANDLE_ARGS;
+		out_args->np = node;
+		out_args->args_count = count;
+		for (i = 0; i < count; i++)
+			out_args->args[i] = be32_to_cpup(list++);
+	}
 	return 0;

-err1:
+
+ err1:
 	of_node_put(node);
-err0:
+ err0:
 	pr_debug("%s failed with status %d\n", __func__, ret);
 	return ret;
 }
-EXPORT_SYMBOL(of_parse_phandles_with_args);
+EXPORT_SYMBOL(of_parse_phandle_with_args);