[PATCH 0/3] Overlay syntax sugar using <&label>

[PATCH 0/3] Overlay syntax sugar using <&label>

[Pantelis Antoniou]

This patchset makes overlays simpler to write by utilizing
the standard &label { }; syntax instead of the manual
method which is more error prone.

Pantelis Antoniou (3):
  overlay: Documentation for the overlay sugar syntax
  overlay: Add syntactic sugar version of overlays
  tests: Add a test for overlays syntactic sugar

 Documentation/dt-object-internal.txt | 17 +++++++++++++++++
 dtc-parser.y                         | 20 +++++++++++++++++---
 dtc.h                                |  1 +
 livetree.c                           | 22 ++++++++++++++++++++++
 tests/run_tests.sh                   |  6 ++++++
 5 files changed, 63 insertions(+), 3 deletions(-)

-- 
2.1.4

[PATCH 1/3] overlay: Documentation for the overlay sugar syntax

[Pantelis Antoniou]

There exists a syntactic sugar version of overlays which
make them simpler to write for the trivial case of a single target.

Document it in the device tree object internals.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 Documentation/dt-object-internal.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
index 51d68ab..624aa43 100644
--- a/Documentation/dt-object-internal.txt
+++ b/Documentation/dt-object-internal.txt
@@ -308,3 +308,20 @@ the run time loader must apply an offset to each phandle in every dynamic
 DT object loaded. The __local_fixups__ node records the offset relative to the
 start of every local reference within that property so that the loader can apply
 the offset.
+
+There is an alternative syntax to the expanded form for overlays with phandle
+targets which makes the format similar to the one using in .dtsi include files.
+
+So for the &ocp target example above one can simply write:
+
+/dts-v1/;
+/plugin/;
+&ocp {
+	/* bar peripheral */
+	bar {
+		compatible = "corp,bar";
+		... /* various properties and child nodes */
+	}
+};
+
+The resulting dtb object is identical.
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] overlay: Documentation for the overlay sugar syntax

[Frank Rowand]

Hi David, Pantelis,

On 06/14/17 07:53, Pantelis Antoniou wrote:
> There exists a syntactic sugar version of overlays which
> make them simpler to write for the trivial case of a single target.
> 
> Document it in the device tree object internals.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> index 51d68ab..624aa43 100644
> --- a/Documentation/dt-object-internal.txt
> +++ b/Documentation/dt-object-internal.txt
> @@ -308,3 +308,20 @@ the run time loader must apply an offset to each phandle in every dynamic
>  DT object loaded. The __local_fixups__ node records the offset relative to the
>  start of every local reference within that property so that the loader can apply
>  the offset.
> +
> +There is an alternative syntax to the expanded form for overlays with phandle
> +targets which makes the format similar to the one using in .dtsi include files.
> +
> +So for the &ocp target example above one can simply write:
> +
> +/dts-v1/;
> +/plugin/;
> +&ocp {
> +	/* bar peripheral */
> +	bar {
> +		compatible = "corp,bar";
> +		... /* various properties and child nodes */
> +	}
> +};
> +
> +The resulting dtb object is identical.


Ignoring the fact that this file describes the device tree internals
instead of the device tree source format -- did we decide that we can
deprecate specifying nodes __symbols__,  __overlay__, __fixups__, and
__local_fixups__ (any node name beginning with an underscore) in
device tree source.  Then at a future date make node names beginning
with an underscore an error (maybe with a command line flag to override
the error?)?

-Frank

Re: [PATCH 1/3] overlay: Documentation for the overlay sugar syntax

[Pantelis Antoniou]

Hi Frank,

On Thu, 2017-06-22 at 14:30 -0700, Frank Rowand wrote:
> Hi David, Pantelis,
> 
> On 06/14/17 07:53, Pantelis Antoniou wrote:
> > There exists a syntactic sugar version of overlays which
> > make them simpler to write for the trivial case of a single target.
> > 
> > Document it in the device tree object internals.
> > 
> > Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > ---
> >  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> > index 51d68ab..624aa43 100644
> > --- a/Documentation/dt-object-internal.txt
> > +++ b/Documentation/dt-object-internal.txt
> > @@ -308,3 +308,20 @@ the run time loader must apply an offset to each phandle in every dynamic
> >  DT object loaded. The __local_fixups__ node records the offset relative to the
> >  start of every local reference within that property so that the loader can apply
> >  the offset.
> > +
> > +There is an alternative syntax to the expanded form for overlays with phandle
> > +targets which makes the format similar to the one using in .dtsi include files.
> > +
> > +So for the &ocp target example above one can simply write:
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +&ocp {
> > +	/* bar peripheral */
> > +	bar {
> > +		compatible = "corp,bar";
> > +		... /* various properties and child nodes */
> > +	}
> > +};
> > +
> > +The resulting dtb object is identical.
> 
> 
> Ignoring the fact that this file describes the device tree internals
> instead of the device tree source format -- did we decide that we can
> deprecate specifying nodes __symbols__,  __overlay__, __fixups__, and
> __local_fixups__ (any node name beginning with an underscore) in
> device tree source.  Then at a future date make node names beginning
> with an underscore an error (maybe with a command line flag to override
> the error?)?
> 

There are a few things in mind about this. In my TODO list is a patch
that completely deprecates the manual overlay crafting.

My goal is to have the overlay format be source identical to a .dtsi
file that usually is included when crafting the single dtb blob we're
booting with. That is a format that most are familiar with.

For that to work, we need to handle transformations of the form
/foo -> <&/foo> -> runtime phandle insertion of <&/foo> if it doesn't
work.

This patch is a prerequisite, and we can take it from there. 

> -Frank

Regards

-- Pantelis


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] overlay: Documentation for the overlay sugar syntax

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

On Fri, Jun 23, 2017 at 09:39:27AM +0300, Pantelis Antoniou wrote:
> Hi Frank,
> 
> On Thu, 2017-06-22 at 14:30 -0700, Frank Rowand wrote:
> > Hi David, Pantelis,
> > 
> > On 06/14/17 07:53, Pantelis Antoniou wrote:
> > > There exists a syntactic sugar version of overlays which
> > > make them simpler to write for the trivial case of a single target.
> > > 
> > > Document it in the device tree object internals.
> > > 
> > > Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> > > index 51d68ab..624aa43 100644
> > > --- a/Documentation/dt-object-internal.txt
> > > +++ b/Documentation/dt-object-internal.txt
> > > @@ -308,3 +308,20 @@ the run time loader must apply an offset to each phandle in every dynamic
> > >  DT object loaded. The __local_fixups__ node records the offset relative to the
> > >  start of every local reference within that property so that the loader can apply
> > >  the offset.
> > > +
> > > +There is an alternative syntax to the expanded form for overlays with phandle
> > > +targets which makes the format similar to the one using in .dtsi include files.
> > > +
> > > +So for the &ocp target example above one can simply write:
> > > +
> > > +/dts-v1/;
> > > +/plugin/;
> > > +&ocp {
> > > +	/* bar peripheral */
> > > +	bar {
> > > +		compatible = "corp,bar";
> > > +		... /* various properties and child nodes */
> > > +	}
> > > +};
> > > +
> > > +The resulting dtb object is identical.
> > 
> > 
> > Ignoring the fact that this file describes the device tree internals
> > instead of the device tree source format -- did we decide that we can
> > deprecate specifying nodes __symbols__,  __overlay__, __fixups__, and
> > __local_fixups__ (any node name beginning with an underscore) in
> > device tree source.  Then at a future date make node names beginning
> > with an underscore an error (maybe with a command line flag to override
> > the error?)?
> > 
> 
> There are a few things in mind about this. In my TODO list is a patch
> that completely deprecates the manual overlay crafting.
> 
> My goal is to have the overlay format be source identical to a .dtsi
> file that usually is included when crafting the single dtb blob we're
> booting with. That is a format that most are familiar with.
> 
> For that to work, we need to handle transformations of the form
> /foo -> <&/foo> -> runtime phandle insertion of <&/foo> if it doesn't
> work.
> 
> This patch is a prerequisite, and we can take it from there. 

The information looks useful, but this is definitely the wrong place
for it.  The file describes internals and the source format is exactly
the opposite of that.

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/3] overlay: Add syntactic sugar version of overlays

[Pantelis Antoniou]

For simple overlays that use a single target there exists a
simpler syntax version.

&foo { }; generates an overlay with a single target at foo.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 dtc-parser.y | 20 +++++++++++++++++---
 dtc.h        |  1 +
 livetree.c   | 22 ++++++++++++++++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index ca3f500..affc81a 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -182,10 +182,19 @@ devicetree:
 		{
 			struct node *target = get_node_by_ref($1, $2);
 
-			if (target)
+			if (target) {
 				merge_nodes(target, $3);
-			else
-				ERROR(&@2, "Label or path %s not found", $2);
+			} 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 ';'
@@ -200,6 +209,11 @@ devicetree:
 
 			$$ = $1;
 		}
+	| /* empty */
+		{
+			/* build empty node */
+			$$ = name_node(build_node(NULL, NULL), "");
+		}
 	;
 
 nodedef:
diff --git a/dtc.h b/dtc.h
index fc24e17..1e70526 100644
--- a/dtc.h
+++ b/dtc.h
@@ -202,6 +202,7 @@ struct node *build_node_delete(void);
 struct node *name_node(struct node *node, char *name);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
+void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
 
 void add_property(struct node *node, struct property *prop);
 void delete_property_by_name(struct node *node, char *name);
diff --git a/livetree.c b/livetree.c
index aecd278..a5d5676 100644
--- a/livetree.c
+++ b/livetree.c
@@ -216,6 +216,28 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 	return old_node;
 }
 
+void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
+{
+	static unsigned int next_orphan_fragment = 0;
+	struct node *node;
+	struct property *p;
+	struct data d = empty_data;
+	char *name;
+
+	d = data_add_marker(d, REF_PHANDLE, ref);
+	d = data_append_integer(d, 0xffffffff, 32);
+
+	p = build_property("target", d);
+
+	xasprintf(&name, "fragment@%u",
+			next_orphan_fragment++);
+	name_node(new_node, "__overlay__");
+	node = build_node(p, new_node);
+	name_node(node, name);
+
+	add_child(dt, node);
+}
+
 struct node *chain_node(struct node *first, struct node *list)
 {
 	assert(first->next_sibling == NULL);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] tests: Add a test for overlays syntactic sugar

[Pantelis Antoniou]

Add a single test makeing sure the &foo { }; syntax works.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 tests/run_tests.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index cecc6d2..c8c5bef 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -205,6 +205,12 @@ libfdt_overlay_tests () {
 
     run_test overlay overlay_base_manual_symbols.test.dtb overlay_overlay_manual_fixups.test.dtb
 
+    # test simplified plugin syntax
+    run_dtc_test -@ -I dts -O dtb -o overlay_overlay_simple.dtb overlay_overlay_simple.dts
+
+    # verify non-generation of local fixups
+    run_test check_path overlay_overlay_simple.dtb not-exists "/__local_fixups__"
+
     # Bad fixup tests
     for test in $BAD_FIXUP_TREES; do
 	tree="overlay_bad_fixup_$test"
-- 
2.1.4

Re: [PATCH 0/3] Overlay syntax sugar using <&label>

[Frank Rowand]

On 06/14/17 07:53, Pantelis Antoniou wrote:
> This patchset makes overlays simpler to write by utilizing
> the standard &label { }; syntax instead of the manual
> method which is more error prone.
> 
> Pantelis Antoniou (3):
>   overlay: Documentation for the overlay sugar syntax
>   overlay: Add syntactic sugar version of overlays
>   tests: Add a test for overlays syntactic sugar
> 
>  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
>  dtc-parser.y                         | 20 +++++++++++++++++---
>  dtc.h                                |  1 +
>  livetree.c                           | 22 ++++++++++++++++++++++
>  tests/run_tests.sh                   |  6 ++++++
>  5 files changed, 63 insertions(+), 3 deletions(-)
> 

Hi David,

Any thoughts on this series?

I have been hoping this new syntax would be accepted.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Overlay syntax sugar using <&label>

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On Thu, Jun 22, 2017 at 02:31:52PM -0700, Frank Rowand wrote:
> On 06/14/17 07:53, Pantelis Antoniou wrote:
> > This patchset makes overlays simpler to write by utilizing
> > the standard &label { }; syntax instead of the manual
> > method which is more error prone.
> > 
> > Pantelis Antoniou (3):
> >   overlay: Documentation for the overlay sugar syntax
> >   overlay: Add syntactic sugar version of overlays
> >   tests: Add a test for overlays syntactic sugar
> > 
> >  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
> >  dtc-parser.y                         | 20 +++++++++++++++++---
> >  dtc.h                                |  1 +
> >  livetree.c                           | 22 ++++++++++++++++++++++
> >  tests/run_tests.sh                   |  6 ++++++
> >  5 files changed, 63 insertions(+), 3 deletions(-)
> > 
> 
> Hi David,
> 
> Any thoughts on this series?
> 
> I have been hoping this new syntax would be accepted.

Sorry, I've been travelling and then sick, so I haven't had a chance
to look at this.


-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/3] Overlay syntax sugar using <&label>

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On Wed, Jun 14, 2017 at 05:53:03PM +0300, Pantelis Antoniou wrote:
> This patchset makes overlays simpler to write by utilizing
> the standard &label { }; syntax instead of the manual
> method which is more error prone.
> 
> Pantelis Antoniou (3):
>   overlay: Documentation for the overlay sugar syntax
>   overlay: Add syntactic sugar version of overlays
>   tests: Add a test for overlays syntactic sugar
> 
>  Documentation/dt-object-internal.txt | 17 +++++++++++++++++
>  dtc-parser.y                         | 20 +++++++++++++++++---
>  dtc.h                                |  1 +
>  livetree.c                           | 22 ++++++++++++++++++++++
>  tests/run_tests.sh                   |  6 ++++++
>  5 files changed, 63 insertions(+), 3 deletions(-)

Sorry I took so long to look at patches 2 & 3.  I've had travel and
sickness, and business at day job and, and, and...

Long term I'd still prefer to handle overlay construction as a
separate processing pass, rather than during parse.  However, that
won't change the interface / syntax, so it can be fixed up later
without breaking things.  Therefore, I've merged patches 2 & 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]