olpc ofw question

Hi,

I've run a comparison between OLPC's old OFW code (which mounts the device-tree at /ofw, and makes use of the sparc code) versus the code which I'm planning to send upstream (which mounts the device-tree at /proc/device-tree, and makes use of PROC_DEVTREE). The results are here:

http://dev.queued.net/~dilinger/dt.diff

That's from running:

find /proc/device-tree -type f | sort | while read f; do echo $f | sed 's#/proc/device-tree##'; sudo od -c $f; done > ~/dt-proc find /ofw -type f | sort | while read f; do echo $f | sed 's#/ofw##'; sudo od -c $f; done > ~/dt-ofw
diff -u dt-ofw dt-proc > dt.diff

Note that this was run on two different OLPC XO-1 machines, hence the difference in serial numbers, mfg-data, and other such things.

There are two things I'm concerned about; the first is that device names are mangled. For some things (ie, "/battery@0/name" => "/battery/name"), I don't particularly care; for others ("/pci/audio@f,3/device-id" => "/pci/audio/device-id"), it's kind of annoying to lose that extra bit of information. What's the purpose being the mangling that happens w/ /proc/device-tree? I the following comment in proc_devtree.c:

* For a node with a name like "gc@10", we make symlinks called "gc"
* and "@10" to it.

However, I'm not seeing *any* symlinks at all in /proc/device-tree.

My second concern is the renaming of /pci/usb@f,4 (the ohci device) to /pci/usb. I'm not sure if that's due to a bug in the devtree code, OFW, or my patch. Note that the kernel spits out the following messages while populating /proc/device-tree:

[    0.130869] device-tree: Duplicate name in /pci, renamed to "usb#1"
[    0.131589] device-tree: Duplicate name in /, renamed to "dropin-fs#1"

I'm assuming that the code first renamed the ohci device from /pci/usb@f,4 to /pci/usb, attempted to then rename the wireless chip from /pci/usb@f,5 to /pci/usb, and spit out that error. That seems.. odd.

Any insight into the reasoning for this mangling?

[unit addresses are missing]

It sounds to me like you're not putting the (textual representation of the) unit address in the device_node->full_name field. How do you fill that field?

Segher

Ah, that could very well be it. Note that the *old* OLPC code used the 'path_component_name' of device_node. The new code uses just 'name' in pdt_build_full_name(), as path_component_name is #ifdef'd out for !SPARC. I guess I'm not entirely sure why sparc used path_component_name in the first place..

The code that fills in full_name:

                dp->full_name = pdt_build_full_name(dp);

static char * __init pdt_build_full_name(struct device_node *dp)

{
        int len, ourlen, plen;
        char *n;

        plen = strlen(dp->parent->full_name);
        ourlen = strlen(fetch_node_name(dp));
        len = ourlen + plen + 2;

        n = prom_early_alloc(len);
        strcpy(n, dp->parent->full_name);
        if (!of_is_root_node(dp->parent)) {
                strcpy(n + plen, "/");
                plen++;
        }
        strcpy(n + plen, fetch_node_name(dp));

        return n;
}

#if defined(CONFIG_SPARC)
static inline const char *fetch_node_name(struct device_node *dp)

{
        return dp->path_component_name;
}
#else
static inline const char *fetch_node_name(struct device_node *dp)
{
        return dp->name;
}

#endif

I think I understand the situation. The guess that I articulated on IRC is essentially correct. The (human-readable) text representation of the unit address - i.e. the stuff after "@" - is parent-bus-specific. The numerical representation of a unit address is easily determined - it is the first "#address-cells" cells of the "reg" property. But the rendering of that into ASCII is not as obvious.

A live OF environment gets the text representation by calling the parent bus's "decode-unit" method with the numerical representation as an argument. (In the other direction, the "encode-unit" method goes from text to numerical.) There's an easy way to get that effect via the client interface - just call "package-to-path" on the phandle, and OF will return the full pathname in canonical form.

If you look in arch/sparc/kernel/prom.c:sparc32_path_component() you'll see what is going on. That routine derives a "name@address" string using a heuristic described in this comment:

/* The following routines deal with the black magic of fully naming a
* node.
*
* Certain well known named nodes are just the simple name string. *
* Actual devices have an address specifier appended to the base name
* string, like this "foo@addr". The "addr" can be in any number of
* formats, and the platform plus the type of the node determine the
* format and how it is constructed.
*
* For children of the ROOT node, the naming convention is fixed and
* determined by whether this is a sun4u or sun4v system.
*
* For children of other nodes, it is bus type specific. So
* we walk up the tree until we discover a "device_type" property
* we recognize and we go from there.
*/

As I described above, it's not really black magic; extract the numerical form of the unit address and pass it to the parent's encode-unit method . Either someone didn't know about that, or perhaps they had some reason for not using it. It's certainly do-able during the phase while OF is still alive.

In the non-SPARC case, I think the code was dealing with a flattened device tree, where the node name had already been pre-digested to include the @addr suffix. In that case, decoding the unit address into the text representation was somebody else's problem, so there was no (non-SPARC) Linux code to handle it.

The "proc_of.c" code that I wrote in Dec 2006 uses the package-to-path method mentioned above, getting the "name@addr" representation (package-to-path returns the full path, but you can easily extract just the tail component with strrchr(path, '/'))

On Wed, 11 Aug 2010 15:53:44 -1000
Mitch Bradley <[email protected]> wrote:

[...]

Thanks for the tip. I changed the code:

-       dp->name = pdt_get_one_property(node, "name");
+//     dp->name = pdt_get_one_property(node, "name");
+       dp->name = pdt_get_fullname(node);

Where pdt_get_fullname() runs package-to-path and returns strrchr(buf, '/')+1; /proc/device-tree looks much better. Here's the diff now between /ofw and /proc/device-tree:

http://dev.queued.net/~dilinger/dt2.diff

Now I'm wondering a few things;

1) I'm setting node->name to the full node name now (including the "@" suffix). Is there any reason why this might be incorrect (ie, that I should only be using the @ suffix in node->full_name)? It looks fine to me, but it's worth asking...

2) At a later point, it's probably worth looking into changing the sparc code to use this as well. Is there a reason why the sparc code doesn't shouldn't use this (ie, old firmware bugs)?

3) I get the following during proc population:

[    0.126687] device-tree: Duplicate name in /, renamed to "dropin-fs#1"

Looking at the diff, I see

-/dropin-fs/.node
-0000000 ` 222 206 377
-0000004
-/dropin-fs/.node
-0000000 ` 222 206 377

Is this a bug in my version of OFW?

package-to-path first appeared in OBP 3, i.e. the major version that conformed to the IEEE 1275 standard. OBP 1.x and OBP 2.x had a different (less portable and somewhat less functional) client interface ABI called "romvec". If I remember correctly, OBP 3 first appeared on Sun 5 - UltraSPARC - systems. OBP 1 and 2 were on Sun 4c and related machines.

I think it has been about 15 years since the last OBP 2.x system was released. There may still be a few working machines from that generation, but if so, they are of more historic interest than economic value. I ditched my last one something like 10 years ago.

Yes. I just fixed it. Thanks for noticing it. Fortunately, it is innocuous.