On Tue, Nov 08, 2011 at 06:19:38PM -0700, Grant Likely wrote:
[...]
> /**
> - * 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);
Not introduced by this patch, but if I read the code correctly,
the fourth parameter should be 1?
> */
> -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;
We have some return code here ...
> 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;
... should this be "return ret"?
> -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);
>
--
Regards,
Shawn
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
