Messages in this thread | | | Date | Mon, 21 Nov 2011 23:50:42 +0800 | From | Shawn Guo <> | Subject | Re: [RFC 3/8] of: create of_phandle_args to simplify return of phandle parsing data |
| |
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
| |