On 01/11/2012 04:08 PM, Simon Glass wrote:
> Stephen Warren pointed out that we should use nodes whether or not they
> have an alias in the /aliases section. The aliases section specifies the
> order so far as it can, but is not essential. Operating without alisses
> is useful when the enumerated order of nodes does not matter (admittedly
> rare in U-Boot).
>
> This is considerably more complex, and it is important to keep this
> complexity out of driver code. This patch creates a function
> fdtdec_find_aliases() which returns an ordered list of node offsets
> for a particular compatible ID, taking account of alias nodes.
Overall, this looks basically OK. Just a few minor comments below, but
once those are fixed,
Acked-by: Stephen Warren <swar...@nvidia.com>
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> +/**
> + * Find the nodes for a peripheral and return a list of them in the correct
...
> + * This function checks node properties and will not return node which are
s/node/nodes/
> + * marked disabled (status = "disabled").
> + *
> + * @param blob FDT blob to use
> + * @param name Root name of alias to search for
> + * @param id Compatible ID to look for
> + * @param node Place to put list of found nodes
s/node/node_list/
> + * @param maxcount Maximum number of nodes to find
> + * @return number of nodes found on success, FTD_ERR_... on error
> + */
> +int fdtdec_find_aliases_for_id(const void *blob, const char *name,
> + enum fdt_compat_id id, int *node_list, int maxcount);
> +
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> @@ -35,6 +35,11 @@ DECLARE_GLOBAL_DATA_PTR;
> static const char * const compat_names[COMPAT_COUNT] = {
> };
>
> +const char *fdtdec_get_compatible(enum fdt_compat_id id)
> +{
> + return compat_names[id];
Range-check?
> +/* TODO: Can we tighten this code up a little? */
> +int fdtdec_find_aliases_for_id(const void *blob, const char *name,
> + enum fdt_compat_id id, int *node_list, int maxcount)
> +{
> + int name_len = strlen(name);
> + int nodes[maxcount];
> + int num_found = 0;
> + int offset, node;
> + int alias_node;
> + int count;
> + int i, j;
> +
> + /* find the alias node if present */
> + alias_node = fdt_path_offset(blob, "/aliases");
> +
> + /*
> + * start with nothing, and we can assume that the root node can't
> + * match
> + */
> + memset(nodes, '\0', sizeof(nodes));
> +
> + /* First find all the compatible nodes */
> + node = 0;
> + for (node = count = 0; node >= 0 && count < maxcount;) {
node = 0 is duplicated on those two lines.
> + node = fdtdec_next_compatible(blob, node, id);
> + if (node >= 0)
> + nodes[count++] = node;
> + }
> + if (node >= 0)
> + debug("%s: warning: maxcount exceeded with alias '%s'\n",
> + __func__, name);
> +
> + /* Now find all the aliases */
> + memset(node_list, '\0', sizeof(*node_list) * maxcount);
> +
> + for (offset = fdt_first_property_offset(blob, alias_node);
> + offset > 0;
> + offset = fdt_next_property_offset(blob, offset)) {
> + const struct fdt_property *prop;
> + const char *path;
> + int number;
> + int found;
> +
> + node = 0;
> + prop = fdt_get_property_by_offset(blob, offset, NULL);
> + path = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
> + if (prop->len && 0 == strncmp(path, name, name_len))
> + node = fdt_path_offset(blob, prop->data);
> + if (node <= 0)
> + continue;
> +
> + /* Get the alias number */
> + number = simple_strtoul(path + name_len, NULL, 10);
> + if (number < 0 || number >= maxcount) {
> + debug("%s: warning: alias '%s' is out of range\n",
> + __func__, path);
> + continue;
> + }
> +
> + /* Make sure the node we found is actually in our list! */
> + found = -1;
> + for (j = 0; j < count; j++)
> + if (nodes[j] == node) {
> + found = j;
> + break;
> + }
> +
> + if (found == -1) {
> + debug("%s: warning: alias '%s' points to a node "
> + "'%s' that is missing or is not compatible "
> + " with '%s'\n", __func__, path,
> + fdt_get_name(blob, node, NULL),
> + compat_names[id]);
> + continue;
> + }
> +
> + /*
> + * Add this node to our list in the right place, and mark
> + * it as done.
> + */
> + if (fdtdec_get_is_enabled(blob, node)) {
Do you want to warn here if node_list[number] is already set; if the
user creates multiple aliases with the same ID?
> + node_list[number] = node;
> + if (number >= num_found)
> + num_found = number + 1;
> + }
> + nodes[j] = 0;
> + }
> +
> + /* Add any nodes not mentioned by an alias */
> + for (i = j = 0; i < maxcount; i++) {
> + if (!node_list[i]) {
> + for (; j < maxcount; j++)
> + if (nodes[j] &&
> + fdtdec_get_is_enabled(blob, nodes[j]))
> + break;
> +
> + /* Have we run out of nodes to add? */
> + if (j == maxcount)
> + continue;
break?
> +
> + node_list[i] = nodes[j++];
> + if (i >= num_found)
> + num_found = i + 1;
> + }
> + }
> +
> + return num_found;
> +}
> +
--
nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
