olpc ofw question

olpc ofw question

[Andres Salomon]

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?

Re: olpc ofw question

[Segher Boessenkool]

> 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:

[unit addresses are missing]

> Any insight into the reasoning for this mangling?

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

Re: olpc ofw question

[Andres Salomon]

On Wed, 11 Aug 2010 22:48:43 +0200 (CEST)
"Segher Boessenkool" <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote:

> > 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:
> 
> [unit addresses are missing]
> 
> > Any insight into the reasoning for this mangling?
> 
> 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?

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

Re: olpc ofw question

[Mitch Bradley]

Andres Salomon wrote:
> On Wed, 11 Aug 2010 22:48:43 +0200 (CEST)
> "Segher Boessenkool" <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote:
>
>   
>>> 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:
>>>       
>> [unit addresses are missing]
>>
>>     
>>> Any insight into the reasoning for this mangling?
>>>       
>> 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?
>>     
>
> 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..
>   

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, '/'))

>
> 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
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
>   

Re: olpc ofw question

[Andres Salomon]

On Wed, 11 Aug 2010 15:53:44 -1000
Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> wrote:

[...]
> 
> 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, '/'))


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?

Re: olpc ofw question

[Mitch Bradley]

Andres Salomon wrote:
> On Wed, 11 Aug 2010 15:53:44 -1000
> Mitch Bradley <wmb-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> wrote:
>
> [...]
>   
>> 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, '/'))
>>     
>
>
> 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)?
>   

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.
> 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?
>   

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