Re: [PATCH 2/9] Add dtget utility to read property values from device tree

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

On Fri, Sep 02, 2011 at 10:54:28AM -0700, Simon Glass wrote:
> Hi David,
> 
> On Thu, Jul 28, 2011 at 5:22 AM, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > Hi David,
> >
> > On Tue, Jul 19, 2011 at 3:34 PM, David Gibson
> > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >> On Tue, Jul 05, 2011 at 12:02:50PM -0700, Simon Glass wrote:
> >>> This simply utility makes it easy for scripts to read values from the device
> >>> tree.
> [snip]
> 
> >>> +static int show_data_for_key(const void *blob, const char *format, int type,
> >>> +                          char *key)
> >>> +{
> >>> +     char *ptr, *prop;
> >>> +     int node, guess_node = 0;
> >>> +     const void *value;
> >>> +     int len;
> >>> +
> >>> +     /* First extract the property from the end */
> >>> +     ptr = strrchr(key, '/');
> >>> +     if (!ptr) {
> >>> +             fprintf(stderr, "No node found in key '%s'\n", key);
> >>> +             return 5;
> >>> +     }
> >>
> >> This is wrong.  properties do not have paths - they exist in a
> >> separate namespace to nodes.  The node path and property name should
> >> be supplied as separate parameters.  As well as being actually right,
> >> it will greatly simplify this function's convoluted logic.
> >
> > Well yes, but I feel that it makes it harder to use also. My thinking
> > with this was to try to make it easy to extract information. In my
> > view:
> >
> > /lcd/width
> >
> > is better than
> >
> > /lcd width
> >
> > But this is the kind of discussion / feedback I was hoping to get as I
> > am new to device trees. My thoughts:
> >
> > 1. / is not generally used in property names so there is no conflict
> > 2. Neither is there any ambiguity

Yes there is.  Real device trees exist where a node has both a
property and a subnode with the same name.

> > 3. The 'complex' logic is there to make life easy for the poor
> > hard-pressed user.
> > 4. Creating the concept of a sort-of unified name space for a device
> > tree property is probably not a bad idea.
> 
> I didn't hear back on this so I going to assume this is reasonable,

No, I'm just busy.

> and submit another patch set with your other comments addressed. The
> intent of the fdt seems clearly hierarchical to me. Hope this is ok.

No, treating node+property as a single path remains cause for an
instant nack.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson