[U-Boot] [RFC PATCH 1/2] fdt: Add fdtdec_find_aliases() to deal with alias nodes

[Stephen Warren]

On 12/26/2011 03:31 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).
...
> +/**
> + * Find the nodes for a peripheral and return a list of them in the correct
> + * order. This is used to enumerate all the peripherals of a certain type.
> + *
> + * To use this, optionally set up a /aliases node with alias properties for
> + * a peripheral. For example, for usb you could have:
> + *
> + * aliases {
> + *		usb0 = "/ehci at c5008000";
> + *		usb1 = "/ehci at c5000000";
> + * };
> + *
> + * Pass "usb" as the name to this function and will return a list of two
> + * nodes offsets: /ehci at c5008000 and ehci at c5000000.
> + *
> + * All nodes returned will match the compatible ID, as it is assumed that
> + * all peripherals use the same driver.
> + *
> + * If no alias node is found, then the node list will be returned in the
> + * order found in the fdt. If the aliases mention a node which doesn't
> + * exist, then this will be ignored. If nodes are found with no aliases,
> + * they will be added in any order.
> + *
> + * The array returned will not have any gaps.

You can't make that guarantee without incorrectly parsing the device
tree; I don't believe there's any restriction on the IDs in the aliases
being contiguous. Maybe in practice this restriction will be fine, but
it doesn't seem like a great idea.

> + * If there is a gap in the aliases, then this function will only return up
> + * to the number of nodes it found until the gap. It will also print a warning
> + * in this case. As an example, say you define aliases for usb2 and usb3, and
> + * have 3 nodes. Then in this case the node without an alias will become usb0
> + * and the aliases will be use for usb2 and usb3. But since there is no
> + * usb1, this function will only list one node (usb0), and will print a
> + * warning.
> + *
> + * This function does not check node properties - so it is possible that the
> + * node is marked disabled (status = "disabled"). The caller is expected to
> + * deal with this.
> + * TBD: It might be nicer to handle this here since we don't want a
> + * discontiguous list to result in the caller.

Yes, I think handling disabled is a requirement; Tegra has quite a few
instances of each HW module, and in many cases, not all of them are used
by a given board design, so they're marked disabled.

I don't think this has any impact on handling discontiguous device IDs;
I think we need that anyway.

The itself array could always be contiguous if each entry were a pair
(id, node) instead of the ID being implied by the array index.

> + *
> + * Note: the algorithm used is O(maxcount).
> + *
> + * @param blob		FDT blob to use
> + * @param name		Root name of alias to search for
> + * @param id		Compatible ID to look for

That's a little restrictive. Many drivers will handle multiple
compatible values, e.g. N manufactures each making identical chips but
giving each its own marketing name. These need different compatible
flags in case some bug WAR needs to differentiate between them. Equally,
Tegra30's say I2C controllers will be compatible with both
nvidia,tegra30-i2c and nvidia,tegra20-i2c. While missing out the Tegra20
compatible value would probably technically be a bug in the device tree,
it does seem reasonable to expect the driver to still match on the
Tegra30 compatible value.

> + * @param node		Place to put list of found nodes
> + * @param maxcount	Maximum number of nodes to find

It'd be nice not to have maxcount; it seems slightly restrictive for a
helper function. I suppose that most drivers can supply a reasonable
value for this since there's a certain max number of devices possible
given extant HW designs, but when you start talking about e.g. a driver
for an I2C bus multiplexer, where there's one instance per chip on a
board, the number begins to get a bit arbitrary.

> + * @return number of nodes found on success, FTD_ERR_... on error
> + */
> +int fdtdec_find_aliases(const void *blob, const char *name,
> +			enum fdt_compat_id id, int *node_list, int maxcount);

Overall, I was expecting something more like:

Enumeration:
* Enumerate device tree solely based on compatible values

After enumeration is complete:
* Walk aliases node, and for each entry assign that ID value to the
appropriate device.
* Walk all devices, and assign a free ID to each device that doesn't
already have one.

Still, I suppose in practice this current code will probably achieve
equivalent results, and it doesn't have any impact of the device tree
syntax/content (unless people change the way they write .dts files to
WAR the issues above instead of enhancing the code), so can easily be
enhanced to address the issues above as needed. So, I guess it'll do
(with the exception of needing to handled disabled devices).

-- 
nvpublic