On Thu, Sep 13, 2018 at 11:32:13AM +0200, Fredrik Markstrom wrote:
> Commit 8f1b3 (Correct overlay syntactic sugar for generating target-path fragments)
> changes what is produced when using label references. Instead of merging the nodes
> it creates a new fragment. This patch reverts that behavior for labels but still
> generates separate fragments for path references.
>
> Previously the dts below would generate two fragments:
>
> /dts-v1/;
> /plugin/;
> &x { a: a@0 {};};
> &a { b {}; };
>
> I've kept DT_REF upper case in this patch to keep the patch small and readable,
> if it is decided to merge this I'll create a v3 where that is fixed
>
> Signed-off-by: Fredrik Markstrom <fredrik.markstrom@xxxxxxxxx>
I think the patch is correct, however there are a couple of style
issues.
More importantly, though, I'm still not sure what the concrete benefit
of this is - which ought to be in the commit message.
At present, the structure of the dts file will be precisely reflected
in the structure of the fragments in the dtbo, which is a nice
property. That's not a *strong* reason to keep it the way it is, but
it needs some kind of counter-argument to revert it.
> ---
> dtc-lexer.l | 4 ++--
> dtc-parser.y | 27 +++++++++++++++++++++++++--
> 2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 615b7ec..06c0409 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -213,14 +213,14 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
> <*>\&{LABEL} { /* label reference */
> DPRINT("Ref: %s\n", yytext+1);
> yylval.labelref = xstrdup(yytext+1);
> - return DT_REF;
> + return DT_LABEL_REF;
> }
>
> <*>"&{/"{PATHCHAR}*\} { /* new-style path reference */
> yytext[yyleng-1] = '\0';
> DPRINT("Ref: %s\n", yytext+2);
> yylval.labelref = xstrdup(yytext+2);
> - return DT_REF;
> + return DT_PATH_REF;
> }
>
> <BYTESTRING>[0-9a-fA-F]{2} {
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 815481a..44748ed 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -70,7 +70,8 @@ extern bool treesource_error;
> %token <byte> DT_BYTE
> %token <data> DT_STRING
> %token <labelref> DT_LABEL
> -%token <labelref> DT_REF
> +%token <labelref> DT_LABEL_REF
> +%token <labelref> DT_PATH_REF
> %token DT_INCBIN
>
> %type <data> propdata
> @@ -83,6 +84,7 @@ extern bool treesource_error;
> %type <data> bytestring
> %type <prop> propdef
> %type <proplist> proplist
> +%type <labelref> DT_REF
Only terminal symbols should be named with capitals, non-terminals
(which this now is) should be lower case.
>
> %type <node> devicetree
> %type <node> nodedef
> @@ -158,6 +160,8 @@ memreserve:
> }
> ;
>
> +DT_REF: DT_LABEL_REF {} | DT_PATH_REF {};
You don't need empty semantic blocks ({}).
> devicetree:
> '/' nodedef
> {
> @@ -189,7 +193,7 @@ devicetree:
> ERROR(&@3, "Label or path %s not found", $3);
> $$ = $1;
> }
> - | devicetree DT_REF nodedef
> + | devicetree DT_PATH_REF nodedef
> {
> /*
> * We rely on the rule being always:
> @@ -208,6 +212,25 @@ devicetree:
> }
> $$ = $1;
> }
> + | devicetree DT_LABEL_REF nodedef
> + {
> + struct node *target = get_node_by_ref($1, $2);
> +
> + if (target) {
> + merge_nodes(target, $3);
> + } else {
> + /*
> + * We rely on the rule being always:
> + * versioninfo plugindecl memreserves devicetree
> + * so $-1 is what we want (plugindecl)
> + */
> + if ($<flags>-1 & DTSF_PLUGIN)
> + add_orphan_node($1, $3, $2);
> + else
> + ERROR(&@2, "Label or path %s not found", $2);
> + }
> + $$ = $1;
> + }
> | devicetree DT_DEL_NODE DT_REF ';'
> {
> struct node *target = get_node_by_ref($1, $3);
--
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
Attachment:
signature.asc
Description: PGP signature
