Hi Stephen,
On Fri, Jan 13, 2012 at 1:41 PM, Stephen Warren swarren@nvidia.com wrote:
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 swarren@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?
These and other changes made - but see one comment below.
+/* 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?
I deal with this in a later series which supports multiple compatible IDs - for now I will add an assert().
- 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?
Yes, they are equivalent so I have changed it.
- node_list[i] = nodes[j++];
- if (i >= num_found)
- num_found = i + 1;
- }
- }
- return num_found;
+}
Regards, Simon
-- nvpublic