It seems we have conflicting syntax for path references and label references.<div><br></div><div>Here's what we have now:</div><div><br></div><div>&label</div><div>&{/path/to/the/node}</div><div><br></div><div>
It's not a problem until we want to combine them. If we want a path rooted at label suddenly there is the question of the curly braces. What should we have?</div><blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;">
<div>&label{/path/to/the/subnode/}</div><div>&{label/path/to/the/subnode/}</div><div>&label/path/to/the/subnode</div></blockquote><div><br></div><div>In looking at the dtc code, it appears that '&' is only used to reference exising nodes. So I don't think the curly braces are necessary.</div>
<div><br></div><div>How about we do the following:</div><div><ol><li>deprecate &{/path/to/the/node/} - if it's not used anywhere, can just remove the code and not worry about deprecating it</li><li>use the following to reference nodes:</li>
</ol></div><blockquote class="webkit-indent-blockquote" style="margin: 0 0 0 40px; border: none; padding: 0px;"><div>&label /* just refers to a labeled node */</div><div>&/path/to/the/node /* refers to a node by it's path */</div>
<div>&label/path/to/the/subnode /* refers to a node rooted at the label and follows the path down */</div><div><br></div></blockquote>Thoughts?<div><br></div><div>- John<br><div><br></div><div><div><div><br><div class="gmail_quote">
On Mon, Oct 18, 2010 at 2:51 PM, Grant Likely <span dir="ltr"><<a href="mailto:grant.likely@secretlab.ca">grant.likely@secretlab.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
On Mon, Oct 18, 2010 at 02:08:33PM -0700, John Bonesio wrote:<br>
> On Mon, Oct 18, 2010 at 1:50 PM, Grant Likely <<a href="mailto:grant.likely@secretlab.ca">grant.likely@secretlab.ca</a>>wrote:<br>
<div><div></div><div class="h5">><br>
> > On Mon, Oct 18, 2010 at 01:25:50PM -0700, John Bonesio wrote:<br>
> > > Changes to allow us to specify a node by it's path. A path can be used in<br>
> > > place of a label.<br>
> > ><br>
> > > This is particularly useful when overriding existing nodes.<br>
> > > This way we don't have to label every possible node in a device tree we<br>
> > know<br>
> > > is a base device tree for a class of systems, and we know the tree will<br>
> > be<br>
> > > modified for the specific systems.<br>
> > ><br>
> > > Signed-off-by: John Bonesio <<a href="mailto:bones@secretlab.ca">bones@secretlab.ca</a>><br>
> > > ---<br>
> > ><br>
> > > dtc-parser.y | 16 ++++++++++++----<br>
> > > 1 files changed, 12 insertions(+), 4 deletions(-)<br>
> ><br>
> > Need to add test cases.<br>
> ><br>
> > ><br>
> > > diff --git a/dtc-parser.y b/dtc-parser.y<br>
> > > index 56d7789..0efd0cc 100644<br>
> > > --- a/dtc-parser.y<br>
> > > +++ b/dtc-parser.y<br>
> > > @@ -134,22 +134,30 @@ devicetree:<br>
> > > {<br>
> > > struct node *target;<br>
> > ><br>
> > > - target = get_node_by_label($1, $2);<br>
> > > + if ($2[0] == '/')<br>
> > > + target = get_node_by_path($1, $2);<br>
> > > + else<br>
> > > + target = get_node_by_label($1, $2);<br>
> > > +<br>
> > > if (target)<br>
> > > merge_nodes(target, $3);<br>
> > > else<br>
> > > - print_error("label, '%s' not found", $2);<br>
> > > + print_error("label or path, '%s', not<br>
> > found", $2);<br>
> > > $$ = $1;<br>
> > > }<br>
> > > | devicetree DT_REMOVENODE DT_REF ';'<br>
> > > {<br>
> > > struct node *target;<br>
> > ><br>
> > > - target = get_node_by_label($1, $3);<br>
> > > + if ($3[0] == '/')<br>
> > > + target = get_node_by_path($1, $3);<br>
> > > + else<br>
> > > + target = get_node_by_label($1, $3);<br>
> > > +<br>
> > > if (target)<br>
> > > remove_nodes(target);<br>
> > > else<br>
> > > - print_error("label, '%s' not found", $3);<br>
> > > + print_error("label or path, '%s', not<br>
> > found", $3);<br>
> ><br>
> > Hmmm, this is effectively the same code block implemented twice. Its<br>
> > gotten complex enough with this patch that the common parts should be<br>
> > factored out inot a common function... In fact, after looking at the<br>
> > code, it looks like it should be calling get_node_by_ref() here.<br>
> ><br>
> > Also, this patch doesn't have any way to use the form<br>
> > &<label>/path/based/on/label which is also a critical feature.<br>
> ><br>
> ><br>
</div></div>> Ok, so now I'm glad I separated this patch out.<br>
><br>
> You're right in that it should be call get_node_by_ref(). I missed that.<br>
><br>
> As for the rest of it, allow me to explain ...<br>
><br>
> The lexer already parses &{/ <node path>} as a single lexical unit. When it<br>
> does this, it returns the token DT_REF, which is the same as for a label.<br>
> All I had to do is call the right routine to go find the right node.<br>
<br>
Right, I understand this.<br>
<br>
But I don't currently see how the code can handle the use case of a<br>
subnode from a label. ie:<br>
<br>
/{<br>
my-label: parent {<br>
my-node {<br>
};<br>
};<br>
};<br>
<br>
&my-label/my-node {<br>
hi-there;<br>
};<br>
<br>
The current code seems to be able to handle a full path, or a single<br>
label, but not a child node of a labelled node.<br>
<br>
<br>
><br>
> I've run this through a sample dts file, and it does indeed work.<br>
<br>
(BTW, I think I gave you bad information earlier about syntax. We<br>
have a syntax for full path, but we're missing a syntax for<br>
label+subpath), which you'll have to define.<br>
<font color="#888888"><br>
g.<br>
<br>
</font></blockquote></div><br></div></div></div></div>